This is an archive of the discontinued LLVM Phabricator instance.

[libc++] Adjust tests using ext/* headers that undefine __DEPRECATED
ClosedPublic

Authored by john.brawn on Mar 9 2023, 7:34 AM.

Details

Summary

Several tests undefined DEPRECATED to avoid warnings as they're testing the deprecated ext/hash_map. A better way to do this is to use -Wno-deprecated so it isn't defined in the first place. This prevents these tests from failing when we give a warning when undefining the DEPRECATED macro, as D144654 will do.

For the generated tests however just remove the testing of these header files, so we don't disable the warning when testing the other header files.

Diff Detail

Event Timeline

john.brawn created this revision.Mar 9 2023, 7:34 AM
Herald added a project: Restricted Project. · View Herald TranscriptMar 9 2023, 7:34 AM
john.brawn requested review of this revision.Mar 9 2023, 7:34 AM
Herald added a project: Restricted Project. · View Herald TranscriptMar 9 2023, 7:34 AM
Herald added 1 blocking reviewer(s): Restricted Project. · View Herald Transcript
philnik requested changes to this revision.Mar 9 2023, 7:38 AM
philnik added a subscriber: philnik.

This also disables other warnings though. I think it would be better to just not include the <ext/*> stuff in these tests anymore. They're pretty much unmaintained anyways.

This revision now requires changes to proceed.Mar 9 2023, 7:38 AM
john.brawn retitled this revision from [libc++][NFC] Use -Wno-deprecated instead of undefining __DEPRECATED to [libc++] Remove ext/* headers from tests.
john.brawn edited the summary of this revision. (Show Details)

Changed to removing testing of the ext/* headers.

I really would like a better rationale for removing these test. Since the are not very actively maintained I really dislike the idea to remove regression tests.

libcxx/test/libcxx/assertions/headers_declare_verbose_abort.sh.cpp
44

Why not silence this warning for this test instead of removing the tests.
The ext headers may not get a lot of attention, but removing tests may cause accidental regressions not being detected.

I really would like a better rationale for removing these test. Since the are not very actively maintained I really dislike the idea to remove regression tests.

I only meant removing them from the generated tests, since they are mostly just annoying there. The other tests can just get an // ADDITIONAL_COMPILE_FLAGS: -Wno-deprecated.

john.brawn retitled this revision from [libc++] Remove ext/* headers from tests to [libc++] Adjust tests using ext/* headers that undefine __DEPRECATED.
john.brawn edited the summary of this revision. (Show Details)

Remove only the generated tests, use -Wno-deprecated elsewhere.

philnik accepted this revision as: philnik.Apr 6 2023, 6:02 AM

LGTM. I'll leave final approval to @Mordante.

Rebased, as some other commits have modified the same tests.

ldionne accepted this revision.May 10 2023, 1:08 PM
ldionne added a subscriber: ldionne.

LGTM. I have no love for these headers and I appreciate the cleanup. I have a small suggestion though.

libcxx/test/libcxx/extensions/hash/specializations.pass.cpp
10–11

This comment is not desirable anymore.

This revision is now accepted and ready to land.May 10 2023, 1:08 PM