This is an archive of the discontinued LLVM Phabricator instance.

[clang] Use -std=c++23 instead of -std=c++2b
ClosedPublic

Authored by Mordante on Apr 30 2023, 9:21 AM.

Details

Summary

During the ISO C++ Committee meeting plenary session the C++23 Standard
has been voted as technical complete.

This updates the reference to c++2b to c++23 and updates the __cplusplus
macro.

Drive-by fixes c++1z -> c++17 and c++2a -> c++20 when seen.

Diff Detail

Event Timeline

Mordante created this revision.Apr 30 2023, 9:21 AM
Mordante requested review of this revision.Apr 30 2023, 9:21 AM
Herald added projects: Restricted Project, Restricted Project, Restricted Project. · View Herald Transcript
Herald added a reviewer: Restricted Project. · View Herald Transcript
philnik added inline comments.
libcxx/test/tools/clang_tidy_checks/uglify_attributes.cpp
46 ↗(On Diff #518335)

You probably want to #ifdef this based on Clang 16/17.

Mordante added inline comments.Apr 30 2023, 11:32 AM
libcxx/test/tools/clang_tidy_checks/uglify_attributes.cpp
46 ↗(On Diff #518335)

Yeah I noticed that when I uploaded the diff and the CI started the libc++ CI instead of Clang.

For now I'll undo this hunk and will do that in a different patch. That way I can test with the Clang CI.

Mordante updated this revision to Diff 518343.Apr 30 2023, 11:32 AM

Removes libc++ changes.

Thank you for working on this! The Clang changes are mostly all good, but I think we should hold off on changing the value of __cplusplus for the moment as it's not set in stone in the standard yet (I thought it was though, so I'm checking with the editor).

clang/lib/Driver/ToolChains/Clang.cpp
6641–6642

Good catch!

clang/lib/Frontend/InitPreprocessor.cpp
457

I think this might still be premature. I noticed that the draft C++ standard does not have this value set yet: https://github.com/cplusplus/draft/blob/main/source/config.tex#L6

I've emailed the editor to find out if that's a mistake (I am pretty sure we're sending this off to PDTS at some point shortly).

Thank you for working on this! The Clang changes are mostly all good, but I think we should hold off on changing the value of __cplusplus for the moment as it's not set in stone in the standard yet (I thought it was though, so I'm checking with the editor).

I noticed the same and I was wondering whether I should contact the editor. I noticed some approved papers also have not been merged yet. (These papers are still open issues in the paper repository.)

I'll wait a few days to see whether the draft gets updated otherwise I revert the macro change.

I would be very surprised if the macro gets a different value, that's why I already bumped it.

Thank you for working on this! The Clang changes are mostly all good, but I think we should hold off on changing the value of __cplusplus for the moment as it's not set in stone in the standard yet (I thought it was though, so I'm checking with the editor).

I noticed the same and I was wondering whether I should contact the editor. I noticed some approved papers also have not been merged yet. (These papers are still open issues in the paper repository.)

I'll wait a few days to see whether the draft gets updated otherwise I revert the macro change.

I would be very surprised if the macro gets a different value, that's why I already bumped it.

I heard back from the editor yesterday and he said that the macro would be replaced with an appropriate value for the DIS, and that he hopes to ensure that value is published in the next meeting mailing (which would be roughly May 15). So I think we can either wait until that mailing comes out and see if it has a concrete value to land these changes, or we can land everything but the macro value changes and deal with that in a follow up (this might be easier due to rebasing woes given how large this patch is).

ldionne added a subscriber: ldionne.May 2 2023, 2:38 PM

Thanks for doing this! No objection, will let Aaron give the thumbs up.

clang/test/Parser/cxx2b-label.cpp
1

We could also consider renaming those files.

Mordante updated this revision to Diff 519113.May 3 2023, 9:21 AM

Addresses review comments.

Mordante marked 3 inline comments as done.May 3 2023, 9:26 AM

Thank you for working on this! The Clang changes are mostly all good, but I think we should hold off on changing the value of __cplusplus for the moment as it's not set in stone in the standard yet (I thought it was though, so I'm checking with the editor).

I noticed the same and I was wondering whether I should contact the editor. I noticed some approved papers also have not been merged yet. (These papers are still open issues in the paper repository.)

I'll wait a few days to see whether the draft gets updated otherwise I revert the macro change.

I would be very surprised if the macro gets a different value, that's why I already bumped it.

I heard back from the editor yesterday and he said that the macro would be replaced with an appropriate value for the DIS, and that he hopes to ensure that value is published in the next meeting mailing (which would be roughly May 15). So I think we can either wait until that mailing comes out and see if it has a concrete value to land these changes, or we can land everything but the macro value changes and deal with that in a follow up (this might be easier due to rebasing woes given how large this patch is).

Yes I created a new patch for that part. That patch is a lot easier to rebase than this one.

clang/lib/Frontend/InitPreprocessor.cpp
457

I've reverted this and similar parts. They are now in D149761. I will update that patch once editor made a release with the updated __cplusplus macro.

clang/test/Parser/cxx2b-label.cpp
1

I could do that but rather in a follow-up there are still files named like clang/test/Parser/cxx0x-lambda-expressions.cpp too. I'm not sure how much the Clang devs want to remove these code names.

aaron.ballman accepted this revision.May 3 2023, 9:48 AM

LGTM! I did not spot any other concerns beyond the macro value one. Thank you for working on this!

Mordante updated this revision to Diff 519500.May 4 2023, 8:03 AM
Mordante marked 2 inline comments as done.

Rebased and trigger CI.

Mordante updated this revision to Diff 519510.May 4 2023, 8:42 AM

CI fixes.

This revision was not accepted when it landed; it landed in state Needs Review.May 4 2023, 10:20 AM
This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.