[libcxx-commits] [PATCH] D127606: [libc++] Simplify __config a bit

[Nikolas Klauser via Phabricator via libcxx-commits]

philnik added inline comments.


================
Comment at: libcxx/include/__config:209
+#if !defined(_LIBCPP_COMPILER_CLANG_BASED) && __cplusplus < 201103L
+#  error "libc++ does not support compilers other than Clang in C++03. Please enable C++11."
 #endif
----------------
Mordante wrote:
> One of the things bothering about this file is the lack-of overview what indention level parts have.
> After processing this patch would it make sense to format this file?
> Then all `# foo` parts have the proper indention.
I'm not sure. clang-format indents everything an unneeded level because we have `#ifdef __cplusplus` around the whole file. What I've formatted here was done mostly by hand. Although I think it still would be a major improvement over the status quo. I don't think anybody (other than Arthur maybe) would actually care if the default indentation is one to far in. So yeah, it's probably a good idea.


================
Comment at: libcxx/include/__config:414
 
-#if __cplusplus < 201103L
 typedef __char16_t char16_t;
----------------
Mordante wrote:
> Can this be really removed?
It looks weird in the diff, but the `typedef`s are part of the C++03 block above.


================
Comment at: libcxx/include/__config:908
 
-// Try to find out if RTTI is disabled.
-#if defined(_LIBCPP_COMPILER_CLANG_BASED) && !__has_feature(cxx_rtti)
-#  define _LIBCPP_NO_RTTI
-#elif defined(__GNUC__) && !defined(__GXX_RTTI)
-#  define _LIBCPP_NO_RTTI
-#elif defined(_LIBCPP_COMPILER_MSVC) && !defined(_CPPRTTI)
+#if !defined(__cpp_rtti) || __cpp_rtti < 199711L
 #  define _LIBCPP_NO_RTTI
----------------
Mordante wrote:
> This looks odd, `199711L` refers to C++98. When RTTI is disable is it a "random" value or always `0L`?
Neither Clang nor GCC define it actually if RTTI is disabled. The `|| __cpp_rtti < 199711L` is more a technically-correct than anything else. I've been bitten by that before with other FTMs.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D127606