[libcxx-commits] [PATCH] D127644: [libc++][NFC] clang-format <__config>

[Louis Dionne via Phabricator via libcxx-commits]

ldionne requested changes to this revision.
ldionne added inline comments.
This revision now requires changes to proceed.


================
Comment at: libcxx/include/__config:713-735
+#  define _LIBCPP_BEGIN_NAMESPACE_STD                                                                                  \
+    namespace std {                                                                                                    \
+    inline namespace _LIBCPP_ABI_NAMESPACE {
+#  define _LIBCPP_END_NAMESPACE_STD                                                                                    \
+    }                                                                                                                  \
+    }
+#  define _VSTD std
----------------
I wouldn't make this part of the change, I think it makes things strictly worse. Perhaps `// clang-format off`?


================
Comment at: libcxx/include/__config:798-799
+      _LIBCPP_INLINE_VISIBILITY operator int() const { return __v_; }                                                  \
+      }                                                                                                                \
+      ;
+#  else // _LIBCPP_CXX03_LANG
----------------
The semi-colon alone on its own line is crazy, let's put it on the same line as the `}`.


================
Comment at: libcxx/include/__config:973-977
+#  if !defined(_LIBCPP_HAS_NO_THREADS) && !defined(_LIBCPP_HAS_THREAD_API_PTHREAD) &&                                  \
+      !defined(_LIBCPP_HAS_THREAD_API_WIN32) && !defined(_LIBCPP_HAS_THREAD_API_EXTERNAL)
+#    if defined(__FreeBSD__) || defined(__wasi__) || defined(__NetBSD__) || defined(__OpenBSD__) ||                    \
+        defined(__NuttX__) || defined(__linux__) || defined(__GNU__) || defined(__APPLE__) || defined(__sun__) ||      \
+        defined(__MVS__) || defined(_AIX) || defined(__EMSCRIPTEN__)
----------------
These changes make it harder to read the conditions, and quite importantly, they greatly increase the risk of conflicts between upstream and downstream. Having one platform per line is actually quite useful because it's more like a list and it reduces the likelihood of having merge conflicts downstream. We should keep it as-is.


================
Comment at: libcxx/include/__config:1046
 // Some systems do not provide gets() in their C library, for security reasons.
-#if defined(_LIBCPP_MSVCRT) || \
-    (defined(__FreeBSD_version) && __FreeBSD_version >= 1300043) || \
-    defined(__OpenBSD__)
-#   define _LIBCPP_C_HAS_NO_GETS
-#endif
-
-#if defined(__BIONIC__) || defined(__NuttX__) ||           \
-    defined(__Fuchsia__) || defined(__wasi__) ||           \
-    defined(_LIBCPP_HAS_MUSL_LIBC) || defined(__OpenBSD__)
-#define _LIBCPP_PROVIDES_DEFAULT_RUNE_TABLE
-#endif
+#  if defined(_LIBCPP_MSVCRT) || (defined(__FreeBSD_version) && __FreeBSD_version >= 1300043) || defined(__OpenBSD__)
+#    define _LIBCPP_C_HAS_NO_GETS
----------------
Same here, keep on individual lines.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D127644