Skip to content

Inheriting std::iterator is deprecated in C++17 - #101

Merged
jzmaddock merged 1 commit into
boostorg:developfrom
DanielaE:fix/no-iterator-inheritance
Oct 15, 2018
Merged

Inheriting std::iterator is deprecated in C++17#101
jzmaddock merged 1 commit into
boostorg:developfrom
DanielaE:fix/no-iterator-inheritance

Conversation

@DanielaE

Copy link
Copy Markdown
Contributor

Boost's iterator.hpp is deprecated, too. Therefore get rid of all of that and replace inheritance by lifting std::iterator's members into the derived class.

Signed-off-by: Daniela Engert dani@ngrt.de

@DanielaE
DanielaEforce-pushed the fix/no-iterator-inheritance branch from 463d2c2 to 50709e6CompareMarch 4, 2018 09:43
@davydden

Copy link
Copy Markdown
Contributor

@davyddendavydden mentioned this pull request Mar 31, 2018
@davydden

Copy link
Copy Markdown
Contributor

@DanielaE please rebase and see #104 (comment)

@DanielaE
DanielaEforce-pushed the fix/no-iterator-inheritance branch from 50709e6 to 2e07dadCompareApril 4, 2018 14:45
@DanielaE

Copy link
Copy Markdown
ContributorAuthor

Done 😄

{ // replace with iterator_adaptor implementation -JGS
typedef std::bidirectional_iterator_tag iterator_category;
typedef ArrayBinaryTreeNode value_type;
typedef size_type difference_type;

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

typedef ArrayBinaryTreeNode::difference_type difference_type;
typedef ArrayBinaryTreeNode* pointer;

would be more readable?

Boost's iterator.hpp is deprecated, too. Therefore get rid of all of that and replace inheritance by lifting std::iterator's members into the derived class.
Signed-off-by: Daniela Engert <dani@ngrt.de>
@DanielaE
DanielaEforce-pushed the fix/no-iterator-inheritance branch from 2e07dad to 93cfa18CompareApril 14, 2018 16:16
@anadon

Copy link
Copy Markdown
Contributor

I'm helping out with the PR backlog. Looks like you have a code change. I need to look into how this will affect backwards compatibility with C++ standards. This is to let you know and help me prioritize PR's.

@DanielaE

Copy link
Copy Markdown
ContributorAuthor

It doesn't change it in any respect.

@anadon

Copy link
Copy Markdown
Contributor

It uses tags/traits, which are not something necessarily safe for C++03. I'm going to do some additional testing to satisfy myself that this is either a safe change, or going to be pushed off.

@DanielaE

Copy link
Copy Markdown
ContributorAuthor

Well, no - it does not.

@anadon

Copy link
Copy Markdown
Contributor

OK, I'm mixing up 2a/2z stuff with 03.
Suggested by http://www.open-std.org/jtc1/sc22/wg21/docs/papers/2003/n1424.htm
This will be put into the second batch of merges.

@DanielaE

Copy link
Copy Markdown
ContributorAuthor

Graph is one of the 5 last users of boost::iterator, the empty shell around std::iterator which itself is deprecated in C++17 http://www.open-std.org/jtc1/sc22/wg21/docs/papers/2016/p0174r2.html#2.1 (with all it's consequences) and about to be removed from the language.

jzmaddock added a commit that referenced this pull request Oct 12, 2018
Refer to discussion in #101
Original PR was #80.
@jzmaddock

Copy link
Copy Markdown
Contributor

Ooops, that reference was to the wrong PR :(

@anadon

Copy link
Copy Markdown
Contributor

@jzmaddock Can you trigger a build on this?

@jzmaddock

Copy link
Copy Markdown
Contributor

Closing and re-opening to trigger a CI build.

@jzmaddockjzmaddock reopened this Oct 12, 2018
@jzmaddock

Copy link
Copy Markdown
Contributor

Looks good to me, but I'll wait for Appveyor to catch up before merging...

@anadon

Copy link
Copy Markdown
Contributor

@jzmaddock All clear.

@jzmaddock
jzmaddock merged commit f30f9ca into boostorg:developOct 15, 2018
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants

@DanielaE@davydden@anadon@jzmaddock@morinmorin