[libcxx-commits] [PATCH] D65004: [libc++] Unconditionally enable module tests when they're supported

[Louis Dionne via Phabricator via libcxx-commits]

ldionne abandoned this revision.
ldionne marked an inline comment as done.
ldionne added inline comments.


================
Comment at: libcxx/utils/libcxx/test/config.py:1014
             ['-fmodules-cache-path=' + module_cache]
-        if enable_modules:
-            self.config.available_features.add('-fmodules')
-            self.cxx.useModules()
+        self.config.available_features.add('-fmodules')
+        self.cxx.useModules()
----------------
EricWF wrote:
> This doesn't enable the "module tests". That's done above when "modules-support" is added. This code enables using modules for *every test in the testsuite*. 
> 
> Although it makes the test suite run a lot faster, I'm not sure we want to enable it by default. We at least need some buildbots that test non-modular configurations.
> 
> Is turning modules on all the time the intent of your patch?
> Is turning modules on all the time the intent of your patch?

Nope, that wasn't my intent. I see you've committed a similar fix here: https://github.com/llvm-project/libcxx/commit/20ea9e39b862b1effdf799293c5ae855203b86f4

I'm closing this.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D65004