This is an archive of the discontinued LLVM Phabricator instance.

[libc++] [test] Mechanically qualify calls to std:: functions. NFCI.
ClosedPublic

Authored by Quuxplusone on Feb 15 2022, 10:08 AM.

Details

Reviewers
ldionne
Group Reviewers
Restricted Project
Summary

Following up to D119685, all in one PR to conserve buildkite cycles but to be landed as 9 separate commits just in case:

8b4bf85dedf5 [libc++] [test] Qualify calls to std::get(tuple). NFCI.
ce10eef48c53 [libc++] [test] Qualify calls to std::rethrow_exception. NFCI.
fca25413869d [libc++] [test] Qualify calls to std::atomic_flag_{clear,test} functions. NFCI.
398771c2a181 [libc++] [test] Qualify calls to std::getline. NFCI.
1e3e60cc44e1 [libc++] [test] Qualify iomanip functions in std/input.output/iostream.format/. NFCI.
23cebc56c5d8 [libc++] [test] Qualify calls to iomanip functions in std/localization/. NFCI.
f528c5c4e64c [libc++] [test] Qualify `prev` as `std::prev` in a lot of tests. NFCI.
2986d0a23e2e [libc++] [test] Qualify `next` as `std::next` in a lot of tests. NFCI.
8256cb470412 [libc++] [test] Qualify `move` as `std::move` in a lot of tests. NFCI.

As discussed in D119685, if buildkite is happy with this (which I expect), I'm just going to push it.

Diff Detail

Event Timeline

Quuxplusone requested review of this revision.Feb 15 2022, 10:08 AM
Quuxplusone created this revision.
Herald added 1 blocking reviewer(s): Restricted Project. · View Herald TranscriptFeb 15 2022, 10:08 AM
ldionne accepted this revision.Feb 15 2022, 11:14 AM

LGTM assuming this is all indeed just adding std::. Did you do it 100% mechanically? Did you make sure that none of these tests is trying to check that we can call some of these functions via ADL? Given the list of functions, that would be unlikely because I don't think any of them guarantees being callable via ADL, but you get the point.

This revision is now accepted and ready to land.Feb 15 2022, 11:14 AM

LGTM assuming this is all indeed just adding std::. Did you do it 100% mechanically? Did you make sure that none of these tests is trying to check that we can call some of these functions via ADL? Given the list of functions, that would be unlikely because I don't think any of them guarantees being callable via ADL, but you get the point.

Yep to all. The only one of these functions where ADL seemed potentially relevant was std::get, but the particular user in that case was libcxx/test/std/ranges/range.utility/range.subrange/ctor.pair_like_conv.pass.cpp where clearly the ADL-ness wasn't part of the thing being tested.

(I believe that technically the standard requires all of these ADLs to work, because the standard shows everything in namespace std so it's all one big associated namespace. But I'm sure that testing whether ADL works wasn't the point of any of these test cases.)

Patch application failed. Rebase and try again.

ldionne accepted this revision.Feb 16 2022, 6:26 AM

LGTM despite the backdeployment failures -- clearly this won't have any incidence on backdeployment.

Landed as:

37f7e31015b0 [libc++] [test] Qualify calls to std::get(tuple). NFC.
79dc7551d83b [libc++] [test] Qualify calls to std::rethrow_exception. NFC.
f033bf88b4e2 [libc++] [test] Qualify calls to std::atomic_flag_{clear,test} functions. NFCI.
98bb747c8705 [libc++] [test] Qualify calls to std::getline. NFCI.
eae745c18e2e [libc++] [test] Qualify calls to iomanip functions in std/input.output/iostream.format/. NFCI.
85a92deb590e [libc++] [test] Qualify calls to iomanip functions in std/localization/. NFCI.
3b966c1fe9bf [libc++] [test] Qualify `prev` as `std::prev` in a lot of tests. NFCI.
5ffe11a9fccf [libc++] [test] Qualify `next` as `std::next` in a lot of tests. NFCI.
7853371146d1 [libc++] [test] Qualify `move` as `std::move` in a lot of tests. NFCI.