[libcxx-commits] [PATCH] D142311: [libc++][test] Improves locale validation.

[Louis Dionne via Phabricator via libcxx-commits]

ldionne added a comment.

Thanks for looking into this! I am not a huge fan of using the `config` object to keep state around, but I understand why you had to do it. I'll look into alternative solutions, please give me a bit of time -- let's talk about this again after Issaquah.



================
Comment at: libcxx/test/support/test_macros.h:25
 #endif
 
+#ifdef __has_include
----------------
I would call the header `test_config_site.h`.


================
Comment at: libcxx/test/support/test_macros.h:26-30
+#ifdef __has_include
+#  if __has_include("config_site.h")
+#    include "config_site.h"
+#  endif
+#endif
----------------
I would include this unconditionally. If our setup is messed up and there's no `config_site.h`, we want to know about it instead of silently having all these macros be undefined.


================
Comment at: libcxx/utils/libcxx/test/dsl.py:508-514
+class AddDefine(ConfigAction):
+  """
+  This action adds the given define to the test suite's config_site.
+
+  Currently this only defines macros without a way to specify the macro's
+  value, since there is no usecase for that feature.
+
----------------



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D142311