This is an archive of the discontinued LLVM Phabricator instance.

[libc++] [test] Add a test for proper _Uglification of template parameter names
ClosedPublic

Authored by Quuxplusone on Jan 10 2022, 9:41 AM.

Details

Summary

And, as a separate preliminary commit, fix the pre-existing issues this test found:
[libc++] _Uglify some template parameter names. NFCI.

Diff Detail

Event Timeline

Quuxplusone requested review of this revision.Jan 10 2022, 9:41 AM
Herald added 1 blocking reviewer(s): Restricted Project. · View Herald TranscriptJan 10 2022, 9:41 AM
libcxx/utils/generate_header_tests.py
207–208

Oh, I guess this should be alphabetized above double_include. Will fix.

ldionne requested changes to this revision.Jan 11 2022, 9:15 AM

I think we should somehow merge this with test/support/nasty_macros.h. Either add these macros to nasty_macros.h (but that would probably break a lot of our test suite), or instead remove nasty_macros.h and move the macros defined therein into this test. I think the latter would be better.

This revision now requires changes to proceed.Jan 11 2022, 9:15 AM

I think we should somehow merge this with test/support/nasty_macros.h. Either add these macros to nasty_macros.h (but that would probably break a lot of our test suite), or instead remove nasty_macros.h and move the macros defined therein into this test. I think the latter would be better.

I did try editing nasty_macros.h first, because I assumed that would work; but indeed it broke tons of tests. nasty_macros.h is -included on every single test in the test suite, which means those "nasty" macros are poison to use both in libcxx/include/ and in the tests themselves.

I'll merge nasty_macros.h into this test, and eliminate the mentions of nasty_macros.h from libcxx/test/configs/llvm-libc++-*.cfg.in.

Merge with nasty_macros.h.

Oops, #include "test_macros.h" is no longer needed

ldionne accepted this revision.Jan 12 2022, 1:32 PM
ldionne added inline comments.
libcxx/test/configs/apple-libc++-shared.cfg.in
16

I *love* these simplifications.

This revision is now accepted and ready to land.Jan 12 2022, 1:32 PM

https://buildkite.com/llvm-project/libcxx-ci/builds/7765

c:\llvm-mingw\include\wchar.h:241:31: error: expected ')'
  int __cdecl iswalpha(wint_t _C);
                              ^

@mstorsjo perhaps: Do you think #ifndef _WIN32 is the right way to catch both MinGW and clang-cl, without catching anything else?

Oops, restore the libcxx/include/ bugfixes that are required in order to make CI green here. If this is still green, imma land it in the morning.

https://buildkite.com/llvm-project/libcxx-ci/builds/7765

c:\llvm-mingw\include\wchar.h:241:31: error: expected ')'
  int __cdecl iswalpha(wint_t _C);
                              ^

@mstorsjo perhaps: Do you think #ifndef _WIN32 is the right way to catch both MinGW and clang-cl, without catching anything else?

Yep, that's the most canonical way of identifying Windows as a platform.

mstorsjo added inline comments.Jan 12 2022, 11:38 PM
libcxx/utils/libcxx/test/config.py
292

FWIW it seems like this the nasty headers include file hasn't been in use at all on Windows so far (either in the legacy config or in the new standalone config files) - even if the old config file used to have some annotations for it... So it might be ok to just mark the test as unsupported on Windows for now, if you don't want to try to navigate through and figure out which other defines need to be skipped.

(I guess I could give that a shot too.)

Also skip _P on Windows.

Undef yet more _X macros on Win32. Perhaps I should just not-define them all, but eh, I must be almost done by now, right?...

Okay, I give up, let's just not #define _X for any X on Win32. Expect CI to be green now.

ldionne accepted this revision.Jan 14 2022, 8:04 AM

LGTM when green. Thanks for doing this, I'm really happy about the testing configuration simplifications *and* the added test coverage we're getting.

This revision was landed with ongoing or failed builds.Jan 14 2022, 12:51 PM
This revision was automatically updated to reflect the committed changes.
haowei added a subscriber: haowei.Jan 18 2022, 10:31 AM

Hi, we are seeing test failures from libc++ :: libcxx/nasty_macros.compile.pass.cpp in Fuchsia's linux-x64 and arm64 clang builders after this patch was landed.

Link to the failed build: https://ci.chromium.org/ui/p/fuchsia/builders/toolchain.ci/clang-linux-x64/b8825027967296150705/overview

We confirmed patch "6cc305764f62c9eb32194dcdb3bc6b4209e3175b" is the culprit by locally reverting it. Could you take a look please? If it takes a long time to fix. Could you revert the this change? Thanks!

Hi @haowei , could you please provide the output from the failed test?
The link you gave did lead me to https://logs.chromium.org/logs/fuchsia/buildbucket/cr-buildbucket/8825027967296150705/+/u/clang/test/stdout?format=raw but there's no output there; it just says

PASS: libc++ :: libcxx/min_max_macros.compile.pass.cpp (6610 of 13450)
PASS: libc++ :: libcxx/ranges/range.nonprop.cache/emplace.pass.cpp (6611 of 13450)
FAIL: libc++ :: libcxx/nasty_macros.compile.pass.cpp (6612 of 13450)
PASS: libc++ :: libcxx/ranges/range.nonprop.cache/emplace_from.pass.cpp (6613 of 13450)
PASS: libc++ :: libcxx/no_assert_include.compile.pass.cpp (6614 of 13450)

What I'd need to see is, how did the test fail? What error messages did the compiler produce?
If you have access to a machine with the "Fuchsia" config on it, you can probably run that test yourself, grab the output and paste it here.

Actually @haowei never mind, I bet this is fixed by D117582, right?

Hi @haowei , could you please provide the output from the failed test?
The link you gave did lead me to https://logs.chromium.org/logs/fuchsia/buildbucket/cr-buildbucket/8825027967296150705/+/u/clang/test/stdout?format=raw but there's no output there; it just says

PASS: libc++ :: libcxx/min_max_macros.compile.pass.cpp (6610 of 13450)
PASS: libc++ :: libcxx/ranges/range.nonprop.cache/emplace.pass.cpp (6611 of 13450)
FAIL: libc++ :: libcxx/nasty_macros.compile.pass.cpp (6612 of 13450)
PASS: libc++ :: libcxx/ranges/range.nonprop.cache/emplace_from.pass.cpp (6613 of 13450)
PASS: libc++ :: libcxx/no_assert_include.compile.pass.cpp (6614 of 13450)

What I'd need to see is, how did the test fail? What error messages did the compiler produce?
If you have access to a machine with the "Fuchsia" config on it, you can probably run that test yourself, grab the output and paste it here.

Hi, you can get the test output from https://ci.chromium.org/ui/p/fuchsia/builders/toolchain.ci/clang-linux-x64/b8825027967296150705/test-results from the "Test Result" Tab at the top of the page from the link posted earlier. We are using using ResultDB so the test output was not in the stdout. Sorry I forgot to mention it in the earlier post.

It should be fixed by D117582 now.

libcxx/test/libcxx/nasty_macros.compile.pass.cpp