This is an archive of the discontinued LLVM Phabricator instance.

[libc++] Robust against C++20-hostile iterators
ClosedPublic

Authored by philnik on Jun 13 2022, 10:08 AM.

Details

Reviewers
ldionne
EricWF
Group Reviewers
Restricted Project
Commits
rGa4c805600ef2: [libc++] Robust against C++20-hostile iterators

Diff Detail

Event Timeline

philnik created this revision.Jun 13 2022, 10:08 AM
Herald added a project: Restricted Project. · View Herald TranscriptJun 13 2022, 10:08 AM
Herald added a subscriber: mgrang. · View Herald Transcript
philnik requested review of this revision.Jun 13 2022, 10:08 AM
Herald added a project: Restricted Project. · View Herald TranscriptJun 13 2022, 10:08 AM
Herald added a reviewer: Restricted Project. · View Herald Transcript
ldionne accepted this revision.Jun 13 2022, 10:16 AM
ldionne added inline comments.
libcxx/test/std/algorithms/robust_against_cpp20_hostile_iterators.compile.pass.cpp
9 ↗(On Diff #436456)
This revision is now accepted and ready to land.Jun 13 2022, 10:16 AM
philnik updated this revision to Diff 436523.Jun 13 2022, 12:40 PM
  • Try to fix CI
EricWF accepted this revision.Jun 13 2022, 1:40 PM
EricWF added a subscriber: EricWF.

I still think it would be nice to produce a minimal test that actually ran (no need to check any output). Since having a runnable test would allow the sanitizers to pick up any weird bugs Cpp20HostileIterator causes at runtime.

But as a CI fix, this LGTM once the comments about undefined symbols have been addressed.

libcxx/test/std/algorithms/robust_against_cpp20_hostile_iterators.compile.pass.cpp
67 ↗(On Diff #436523)

You'll want to define this constructor too.

76 ↗(On Diff #436523)

You might as well run these tests over some meaningful input. Then if our implementations do something else weird there's a chance the sanitizers can catch it.

philnik updated this revision to Diff 436649.Jun 13 2022, 10:12 PM
philnik marked 2 inline comments as done.
  • Try to fix CI
  • move the test to test/libcxx/
libcxx/test/std/algorithms/robust_against_cpp20_hostile_iterators.compile.pass.cpp
76 ↗(On Diff #436523)

This should be catched by the runtime tests. They already run with a bunch of different iterator types. This test is just to make sure that the classic STL algorithms can still be instantiated with not-so-nice iterators.

philnik updated this revision to Diff 437474.Jun 16 2022, 1:50 AM
  • Next try
This revision was landed with ongoing or failed builds.Jun 16 2022, 4:27 AM
This revision was automatically updated to reflect the committed changes.