This is an archive of the discontinued LLVM Phabricator instance.

[libcxx][ranges] Move `namespace views` into `namespace ranges` and add an alias.
ClosedPublic

Authored by zoecarver on Aug 13 2021, 11:37 AM.

Diff Detail

Event Timeline

zoecarver created this revision.Aug 13 2021, 11:37 AM
zoecarver requested review of this revision.Aug 13 2021, 11:37 AM
Herald added a project: Restricted Project. · View Herald TranscriptAug 13 2021, 11:37 AM
Herald added a reviewer: Restricted Project. · View Herald Transcript
zoecarver added inline comments.Aug 13 2021, 11:39 AM
libcxx/test/std/ranges/range.adaptors/range.all/all.pass.cpp
90

Intentionally testing this inconsistently so that we cover both std::ranges::views and std::views.

cjdb accepted this revision.Aug 13 2021, 11:39 AM

LGTM! Please add a test to make sure that std::views::X is the same as std::ranges::views::X before committing.

Quuxplusone accepted this revision.Aug 13 2021, 12:53 PM
Quuxplusone added inline comments.
libcxx/include/__ranges/all.h
35–36

Any appetite for namespace ranges::views { here? I have a slight preference for it, but won't push hard for it right now.

libcxx/test/std/ranges/range.adaptors/range.all/all.pass.cpp
90

I don't particularly object, because I probably wouldn't have noticed if the test had been committed this way to begin with. But if you want to test that std::views is the same as std::ranges::views, the right place for that test is not semi-accidentally inside all.pass.cpp; it may deserve a dedicated test file.
(However, IMHO it also doesn't need a test of its own, because it's super obvious.)

This revision is now accepted and ready to land.Aug 13 2021, 12:53 PM
zoecarver marked 2 inline comments as done.Aug 13 2021, 4:11 PM
zoecarver added inline comments.
libcxx/include/__ranges/all.h
35–36

Good idea. Done.

libcxx/test/std/ranges/range.adaptors/range.all/all.pass.cpp
90

I updated these to have a specific test in the same file.