This is an archive of the discontinued LLVM Phabricator instance.

[libc++] [test] Add ranges_robust_against_copying_*.pass.cpp
ClosedPublic

Authored by philnik on Mar 8 2022, 4:35 PM.

Details

Summary
This tests the same QoI issue as the existing STL Classic test,
but for the Ranges algorithms. Also, do the same thing for all
the algorithms that take projections.

I found a few missing algorithms and added them to the existing test, too. std::find_first_of currently fails; I should look at why that is (and in particular, what is it doing weird that makes it inconsistent with the entire rest of libc++?).

Diff Detail

Event Timeline

Quuxplusone created this revision.Mar 8 2022, 4:35 PM
Herald added a project: Restricted Project. · View Herald TranscriptMar 8 2022, 4:35 PM
Quuxplusone requested review of this revision.Mar 8 2022, 4:35 PM
Herald added 1 blocking reviewer(s): Restricted Project. · View Herald TranscriptMar 8 2022, 4:35 PM
philnik accepted this revision as: philnik.Mar 8 2022, 4:56 PM

LGTM % nits.

libcxx/test/libcxx/algorithms/ranges_robust_against_copying_comparators.pass.cpp
10–11

Shouldn't there also be // UNSUPPORTED: libcpp-no-concepts?

229

I'd like // (void) a bit more.

Add libcpp-no-concepts. I'm surprised that there are platforms with libcpp-no-concepts yet without libcpp-has-no-incomplete-ranges! But OK.

Quuxplusone added inline comments.Mar 9 2022, 9:03 AM
libcxx/test/libcxx/algorithms/ranges_robust_against_copying_comparators.pass.cpp
229

For a permanent, informative comment, I'd 100% agree (but "don't comment out code").
These comments, though, are meant to be uncommented as we implement stuff, and be kinda ugly until then. Also, for consistency with the existing tests' commented-out stuff (e.g. in niebloid.compile.pass.cpp as well). So I'm leaving as-is. Let's make the question moot by implementing all these algorithms! ;)

philnik commandeered this revision.Mar 14 2022, 12:17 PM
philnik edited reviewers, added: Quuxplusone; removed: philnik.

Commandeering, since Arthur can't work on it currently.

philnik updated this revision to Diff 415185.Mar 14 2022, 12:24 PM
  • Fix CI; enable more tests
ldionne accepted this revision.Mar 16 2022, 1:47 PM

LGTM with comments

libcxx/test/libcxx/algorithms/ranges_robust_against_copying_comparators.pass.cpp
9

Both added tests could use a short comment explaining what they're testing. It's not difficult to figure out by reading the code, but it's always nice to have a comment in prose explaining exactly what this does -- it helps save time.

This revision is now accepted and ready to land.Mar 16 2022, 1:47 PM
This revision was automatically updated to reflect the committed changes.
philnik marked an inline comment as done.