Skip to content

Provide assign operator for TIter #7641

Merged
pcanal merged 3 commits intoroot-project:masterfrom
linev:iter_assign
Mar 22, 2021
Merged

Provide assign operator for TIter #7641
pcanal merged 3 commits intoroot-project:masterfrom
linev:iter_assign

Conversation

@linev
Copy link
Copy Markdown
Member

@linev linev commented Mar 22, 2021

Let assign TIterator* directly without need of creating
temporary TIter object in between.
Solves #7633

linev added 2 commits March 22, 2021 15:44
@linev
Provide assign operator for TIter
e6eaf71 
Let assign TIterator* directly without need of creating
temporary TIter object in between.
Solves root-project#7633
@linev
Make TIterator &operator=(const TIterator &) abstract
58d3b86 
Forces to correctly implement it in all derived classes,
otherwise TIter iter2 = iter1 does not work for such cases
@linev linev requested a review from pcanal March 22, 2021 15:16
@linev linev self-assigned this Mar 22, 2021
@phsft-bot
Copy link
Copy Markdown

phsft-bot commented Mar 22, 2021

Starting build on ROOT-debian10-i386/cxx14, ROOT-performance-centos8-multicore/default, ROOT-fedora30/cxx14, ROOT-fedora31/noimt, ROOT-ubuntu16/nortcxxmod, mac1014/python3, mac11.0/cxx17, windows10/cxx14
How to customize builds

@phsft-bot
Copy link
Copy Markdown

phsft-bot commented Mar 22, 2021

Build failed on ROOT-performance-centos8-multicore/default.
Running on null:/data/sftnight/workspace/root-pullrequests-build
See console output.

Warnings:

  • [2021-03-22T15:22:17.740Z] /data/sftnight/workspace/root-pullrequests-build/root/core/cont/test/testIter.cxx:23:7: warning: by ‘TListIterTest& TListIterTest::operator=(const TListIterTest&)’ [-Woverloaded-virtual]
  • [2021-03-22T15:22:17.741Z] /data/sftnight/workspace/root-pullrequests-build/root/core/cont/test/testIter.cxx:23:7: warning: by ‘TListIterTest& TListIterTest::operator=(const TListIterTest&)’ [-Woverloaded-virtual]

@phsft-bot
Copy link
Copy Markdown

phsft-bot commented Mar 22, 2021

Starting build on ROOT-debian10-i386/cxx14, ROOT-performance-centos8-multicore/default, ROOT-fedora30/cxx14, ROOT-fedora31/noimt, ROOT-ubuntu16/nortcxxmod, mac1014/python3, mac11.0/cxx17, windows10/cxx14
How to customize builds

@linev linev linked an issue Mar 22, 2021 that may be closed by this pull request
Provide TIter &TIter::operator=(TIterator *it) signature #7633
Closed
@phsft-bot
Copy link
Copy Markdown

phsft-bot commented Mar 22, 2021

Build failed on ROOT-fedora31/noimt.
Running on root-fedora-31-1.cern.ch:/build/workspace/root-pullrequests-build
See console output.

Warnings:

  • [2021-03-22T15:47:33.067Z] /build/workspace/root-pullrequests-build/root/core/cont/test/testIter.cxx:36:19: warning: by ‘TListIterTest& TListIterTest::operator=(const TListIterTest&)’ [-Woverloaded-virtual]
  • [2021-03-22T15:47:33.326Z] /build/workspace/root-pullrequests-build/root/core/cont/test/testIter.cxx:36:19: warning: by ‘TListIterTest& TListIterTest::operator=(const TListIterTest&)’ [-Woverloaded-virtual]

@phsft-bot
Copy link
Copy Markdown

phsft-bot commented Mar 22, 2021

Build failed on ROOT-ubuntu16/nortcxxmod.
Running on sft-ubuntu-1604-1.cern.ch:/build/workspace/root-pullrequests-build
See console output.

Warnings:

  • [2021-03-22T15:48:18.660Z] /mnt/build/workspace/root-pullrequests-build/root/core/cont/inc/TList.h:218:15: warning: ‘virtual TIterator& TListIter::operator=(const TIterator&)’ was hidden [-Woverloaded-virtual]
  • [2021-03-22T15:48:18.660Z] /mnt/build/workspace/root-pullrequests-build/root/core/cont/test/testIter.cxx:36:19: warning: by ‘TListIterTest& TListIterTest::operator=(const TListIterTest&)’ [-Woverloaded-virtual]
  • [2021-03-22T15:48:18.660Z] /mnt/build/workspace/root-pullrequests-build/root/core/cont/inc/TList.h:218:15: warning: ‘virtual TIterator& TListIter::operator=(const TIterator&)’ was hidden [-Woverloaded-virtual]
  • [2021-03-22T15:48:18.660Z] /mnt/build/workspace/root-pullrequests-build/root/core/cont/test/testIter.cxx:36:19: warning: by ‘TListIterTest& TListIterTest::operator=(const TListIterTest&)’ [-Woverloaded-virtual]

@linev
Add simple test for new TIter functionality
ff7c019 
Ensure that created TIterator instance kept inside TIter and not copied
@phsft-bot
Copy link
Copy Markdown

phsft-bot commented Mar 22, 2021

Starting build on ROOT-debian10-i386/cxx14, ROOT-performance-centos8-multicore/default, ROOT-fedora30/cxx14, ROOT-fedora31/noimt, ROOT-ubuntu16/nortcxxmod, mac1014/python3, mac11.0/cxx17, windows10/cxx14
How to customize builds

pcanal
pcanal reviewed Mar 22, 2021
View reviewed changes
core/cont/inc/TCollection.h
Comment on lines +266 to +267
if (fIterator)
delete fIterator;
Copy link
Copy Markdown
Member

@pcanal pcanal Mar 22, 2021 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
if (fIterator)
delete fIterator;
SafeDelete(fIterator);

to use the same syntax the other deletion in this class.

Copy link
Copy Markdown
Member Author

@linev linev Mar 22, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Here SafeDelete not necessary - fIterator is overwritten already with the next line

Copy link
Copy Markdown
Member

@pcanal pcanal Mar 22, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fair enough. I mis-read! :)

@pcanal pcanal merged commit aa49b0a into root-project:master Mar 22, 2021
@linev linev deleted the iter_assign branch March 23, 2021 08:20
@guitargeek guitargeek mentioned this pull request Apr 18, 2021
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@pcanal pcanal pcanal approved these changes

@Axel-Naumann Axel-Naumann Awaiting requested review from Axel-Naumann

@oshadura oshadura Awaiting requested review from oshadura

Assignees

@linev linev

Labels

affects:master improvement in:Core Libraries

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Provide TIter &TIter::operator=(TIterator *it) signature

3 participants

@linev @phsft-bot @pcanal