[PATCH] D136449: [Clang] Implement P2513

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added inline comments.


================
Comment at: clang/lib/Frontend/InitPreprocessor.cpp:701
+    Builder.defineMacro("__cpp_char8_t",
+                        LangOpts.CPlusPlus20 ? "202207L" : "201811L");
   Builder.defineMacro("__cpp_impl_destroying_delete", "201806L");
----------------
cor3ntin wrote:
> aaron.ballman wrote:
> > Why do we not want to go with `202207L` regardless of language mode if this is being applied as a DR?
> There is a `-fchar8_t` that is an extension before C++20 and we are not touching that. It is pre-existing that the feature test macro was defined before c++20 if that flag was set.
Ahhhh, I see now that there's a (small) potential for breakage (thank you for that test case btw) and so you want to leave the extension flag behavior alone. We usually don't do that (usually these sort of extension flags enable the feature as it exists in the language mode enabling support for it) unless there's a good reason to do so. My thinking is:

The potential for breakage is small enough we're not even going to call it out as a potentially disruptive change in the release notes. Presuming there are likely more people using `-std=c++20` than there are using `-fchar8_t`, changing `-fchar8_t` seems like less risky behavior than changing `-std=c++20`. So I think it's reasonable for us to enable this functionality for `-fchar8_t` as well because we're already assuming the larger risk.

WDYT?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D136449/new/

https://reviews.llvm.org/D136449