[PATCH] D99456: [C++2b] Support size_t literals

[Anton Bikineev via Phabricator via cfe-commits]

AntonBikineev added inline comments.


================
Comment at: clang/lib/Frontend/InitPreprocessor.cpp:593-594
+  // C++2b features.
+  if (LangOpts.CPlusPlus2b)
+    Builder.defineMacro("__cpp_size_t_suffix", "202011L");
   if (LangOpts.Char8)
----------------
Quuxplusone wrote:
> aaron.ballman wrote:
> > AntonBikineev wrote:
> > > aaron.ballman wrote:
> > > > Because we allow this as an extension in all C++ modes, should this be enabled always rather than gated on C++2b?
> > > I was also wondering about this. I've checked that we also do the same for other feature macros, such as __cpp_binary_literals, which is defined for -std>=c++14 while at the same time is allowed as an extension before C++14. Therefore I decided to mimic the behaviour.
> > Thanks for checking on that! I think that seems defensible enough. :-)
> Btw, thus far libc++ has tended to make the opposite choice: for example, libc++ defines `__cpp_lib_variant == 202102` in all modes, because the programmer conceivably might be depending on that macro to make some decision, so we want to make sure it reflects the specific semantics that we implement.  (For `__cpp_binary_literals` specifically, I agree it doesn't really matter because nobody's going to be making decisions based on the value of this macro.)
> 
> See https://reviews.llvm.org/D99290#inline-934563 (D96385, D97394) for previous discussions on the libc++ side.
Thanks for pointing this out, Arthur.

I wish there was some consistency, however, I'm not sure if this is easily feasible. I guess the strategy of defining `__cpp_size_t_literals` on all modes would be problematic, since if the user code depends on `__cpp_size_t_literals`, it could suddenly receive the extension warning (when compiled with -std<2++2b), which is enabled by default.


================
Comment at: clang/lib/Frontend/InitPreprocessor.cpp:593-594
+  // C++2b features.
+  if (LangOpts.CPlusPlus2b)
+    Builder.defineMacro("__cpp_size_t_suffix", "202011L");
   if (LangOpts.Char8)
----------------
AntonBikineev wrote:
> Quuxplusone wrote:
> > aaron.ballman wrote:
> > > AntonBikineev wrote:
> > > > aaron.ballman wrote:
> > > > > Because we allow this as an extension in all C++ modes, should this be enabled always rather than gated on C++2b?
> > > > I was also wondering about this. I've checked that we also do the same for other feature macros, such as __cpp_binary_literals, which is defined for -std>=c++14 while at the same time is allowed as an extension before C++14. Therefore I decided to mimic the behaviour.
> > > Thanks for checking on that! I think that seems defensible enough. :-)
> > Btw, thus far libc++ has tended to make the opposite choice: for example, libc++ defines `__cpp_lib_variant == 202102` in all modes, because the programmer conceivably might be depending on that macro to make some decision, so we want to make sure it reflects the specific semantics that we implement.  (For `__cpp_binary_literals` specifically, I agree it doesn't really matter because nobody's going to be making decisions based on the value of this macro.)
> > 
> > See https://reviews.llvm.org/D99290#inline-934563 (D96385, D97394) for previous discussions on the libc++ side.
> Thanks for pointing this out, Arthur.
> 
> I wish there was some consistency, however, I'm not sure if this is easily feasible. I guess the strategy of defining `__cpp_size_t_literals` on all modes would be problematic, since if the user code depends on `__cpp_size_t_literals`, it could suddenly receive the extension warning (when compiled with -std<2++2b), which is enabled by default.
> Btw, thus far libc++ has tended to make the opposite choice: for example, libc++ defines `__cpp_lib_variant == 202102` in all modes, because the programmer conceivably might be depending on that macro to make some decision, so we want to make sure it reflects the specific semantics that we implement.  (For `__cpp_binary_literals` specifically, I agree it doesn't really matter because nobody's going to be making decisions based on the value of this macro.)
> 
> See https://reviews.llvm.org/D99290#inline-934563 (D96385, D97394) for previous discussions on the libc++ side.




Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D99456