This is an archive of the discontinued LLVM Phabricator instance.

[libc++] Remove a few unneeded _LIBCPP_CXX03_LANG ifdefs
ClosedPublic

Authored by philnik on Feb 15 2022, 2:52 PM.

Diff Detail

Event Timeline

philnik requested review of this revision.Feb 15 2022, 2:52 PM
philnik created this revision.
Herald added a project: Restricted Project. · View Herald TranscriptFeb 15 2022, 2:52 PM
Herald added a reviewer: Restricted Project. · View Herald Transcript
Quuxplusone requested changes to this revision.Feb 15 2022, 4:34 PM

LGTM except for the unique_ptr parts.

libcxx/include/__memory/unique_ptr.h
261–264

This one is different — it's #ifdef, not #ifndef. Either leave this file alone, or dig into it a bit more to see whether we can just remove the whole block.

484–487

Ditto.

libcxx/include/type_traits
1289–1292 ↗(On Diff #409067)

Consider deferring this one to a later PR where you also update all the call-sites to use __uncvref_t<T> instead of __uncvref<T>::type. Not mandatory, but worth considering.

libcxx/test/std/utilities/tuple/tuple.tuple/tuple.elem/tuple.by.type.fail.cpp
31 ↗(On Diff #409067)

(After removing the non-NFC unique_ptr diffs, this test diff should also go away.)

This revision now requires changes to proceed.Feb 15 2022, 4:34 PM
ldionne accepted this revision as: ldionne.Feb 16 2022, 6:23 AM

LGTM once CI passes, but I agree with the std::unique_ptr bits.

libcxx/include/type_traits
1835 ↗(On Diff #409067)

Based on the weirdness of the test failure in https://reviews.llvm.org/harbormaster/unit/view/2687268/ , I don't think you should do this diff either.

Looks to me like maybe Clang's C++03 mode doesn't let you do sizeof(foo()) unless foo()'s return type is destructible. We could probably fix this most simply by just not deleting ~__nat() in any mode, but that's a majorly scary patch that should go in its own PR and be carefully audited for all the users of __nat. (Off the top of my head I don't know why __nat deletes anything — I think it should just be a plain old tag type — but, as always, I assume there is a reason, which means we need to go find out what that reason is.)

philnik updated this revision to Diff 409308.Feb 16 2022, 9:56 AM
philnik marked 5 inline comments as done.
  • Address comments
Quuxplusone accepted this revision.Feb 16 2022, 12:38 PM

LGTM if CI is green. Thanks!

libcxx/include/atomic
2439

Nit: I'd remove this blank line.

This revision is now accepted and ready to land.Feb 16 2022, 12:38 PM
This revision was automatically updated to reflect the committed changes.