This is an archive of the discontinued LLVM Phabricator instance.

[libc++] Use c++20 instead of c++2a consistently.
ClosedPublic

Authored by curdeius on Dec 16 2020, 1:48 AM.

Details

Summary
  • The only exception is that the flag -std=c++2a is still used not to break compatibility with older compilers (clang <= 9, gcc <= 9).
  • Bump _LIBCPP_STD_VER for C++20 to 20 and use 21 for the future standard (C++2b).

That's a preparation step to add c++2b support to libc++.

Diff Detail

Event Timeline

curdeius created this revision.Dec 16 2020, 1:48 AM
curdeius requested review of this revision.Dec 16 2020, 1:48 AM
Herald added a project: Restricted Project. · View Herald TranscriptDec 16 2020, 1:48 AM
Herald added a reviewer: Restricted Project. · View Herald Transcript
miscco added a subscriber: miscco.Dec 16 2020, 2:07 AM

This looks great, I have some minor concerns regarding the next version and whether we should already prepare for that

libcxx/utils/generate_feature_test_macro_components.py
659–662

Are there already C++2b macros, should we already add this here, as we are effectively bumping the version?

802

I guess technically this should now be == 20?

806

Should we already have something here for c++2b?

You need to bump the TEST_STD_VER too - see test/support/test_macros.h

mclow.lists requested changes to this revision.Dec 16 2020, 7:34 AM
This revision now requires changes to proceed.Dec 16 2020, 7:34 AM
curdeius planned changes to this revision.Dec 16 2020, 7:45 AM
curdeius marked 2 inline comments as done.

You need to bump the TEST_STD_VER too - see test/support/test_macros.h

Will do.

libcxx/utils/generate_feature_test_macro_components.py
659–662

I have another patch waiting that will add C++2b.

802

Right. I didn't want to change TEST_STD_VER before the next patch but that will be actually better. Anyway, it addresses Marshall's comment too.

806

As said above. I'll add things in the next patch.

curdeius marked 2 inline comments as done.Dec 16 2020, 7:47 AM
curdeius updated this revision to Diff 312212.Dec 16 2020, 7:50 AM
  • Bump TEST_STD_VER.
curdeius marked an inline comment as done.Dec 16 2020, 7:50 AM

This looks great, I have some minor concerns regarding the next version and whether we should already prepare for that

I was wondering too, but I can't see how to determine the timing for this change.
clang ToT (so 12) accepts -std=c++2b, so we can already test C++2b additions (that's what I do locally).
BTW, I have already patches waiting for P1048 (is_scoped_enum) and P1679 (std::string[_view]::contains).
I'd love to see libc++ keep up with with new features a bit faster than for 17 and 20.

The biggest blocker is going to be to have a buildbot with ToT clang I suppose.

ldionne requested changes to this revision.Dec 16 2020, 1:10 PM

Thanks for the cleanup! Could you fix the usage in libcxx/utils/libcxx/test/params.py?

This revision now requires changes to proceed.Dec 16 2020, 1:10 PM
curdeius updated this revision to Diff 312387.Dec 16 2020, 11:33 PM
  • Fix params.py.

Thanks for the cleanup! Could you fix the usage in libcxx/utils/libcxx/test/params.py?

Fixed!

curdeius updated this revision to Diff 312391.Dec 17 2020, 12:08 AM

Revert change in params.py. Otherwise, it will check for existence of -std=c++20 and not -std=c++2a, and break usage on older compilers.

curdeius added a comment.EditedDec 17 2020, 12:09 AM

@ldionne, I've changed params.py but then reverted it because otherwise it checks for -std=c++20 which not all compilers have (only clang 10+ and gcc 10+).

Quuxplusone added inline comments.
libcxx/include/__config
56

My understanding is that you're planning to make other changes in a separate PR to finish up adding a c++2b mode for real. And I like this patch and don't want to slow it down. But I echo what @miscco said: shouldn't you just go all the way to c++2b in this patch? Is there really a good reason to land this intermediate state rather than go all the way to c++2b in one hop?

libcxx/utils/generate_feature_test_macro_components.py
334

(Naturally, this massively merge-conflicts with D93830, but I'm happy to deal with the conflict on D93830's side.)

curdeius added inline comments.Dec 30 2020, 5:55 AM
libcxx/include/__config
56

The only reason I have is that this patch doesn't need a compiler with -std=c++2b support, whereas adding C++2b does need one.
I don't mind adding C++2b here, but I'll need to wait for D93520 to land first to do it.

libcxx/include/__config
56

I hope/guess you mean that the new codepaths would be exercised only on compilers with 2b modes, right? (Not that the patch would suddenly require everyone's compiler to have a 2b mode?)

Clang trunk apparently already supports a 2b mode: https://godbolt.org/z/T3EjT3
so by adding C++2b here and landing it, you'd be letting people run libcxx tests locally in 2b mode, even though buildkite won't run them in that mode yet. Right?

Anyway, assuming all my assumptions are correct, my opinions are unchanged: I think it'd be nice to jump from 2a to 20+2b in one hop, but I think it's most important to not hold up this PR. (Which at the moment is waiting for the relevant people to get back from holiday break — or so I'm guessing from patterns of Phab activity lately. :))

ldionne accepted this revision.Jan 6 2021, 3:00 PM
This revision was not accepted when it landed; it landed in state Needs Review.Jan 7 2021, 4:11 AM
This revision was automatically updated to reflect the committed changes.