[libc++] Unconditionally enable module tests when they're supported

[ldionne]

Summary

I don't see the point in allowing the modules tests to be disabled when
the compiler supports them, and in enabling the modules tests when the
compiler doesn't support them (which is currently an error in lit anyway).

This commit solves the problem that if the compiler supports modules but
enable_modules=True is not passed explicitly to lit, the command-line
defines from __config_site won't be populated properly and the tests
will fail.

[EricWF]

libcxx/utils/libcxx/test/config.py

1014

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?

[ldionne]

libcxx/utils/libcxx/test/config.py

1014

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.

libcxx/utils/libcxx/test/config.py

def get_lit_bool(self, name, default=None, env_var=None):
             val = os.environ[env_var]
             if conf_val is not None:
                 self.lit_config.warning(
                     'Environment variable %s=%s is overriding explicit '
                     '--param=%s=%s' % (env_var, val, name, conf_val))
             return check_value(val, env_var)
         return check_value(conf_val, name)
 
-    def get_modules_enabled(self):
-        return self.get_lit_bool('enable_modules',
-                                default=False,
-                                env_var='LIBCXX_ENABLE_MODULES')
-
     def make_static_lib_name(self, name):
         """Return the full filename for the specified library name"""
         if self.is_windows:
             assert name == 'c++'  # Only allow libc++ to use this function for now.
             return 'lib' + name + '.lib'
         else:
             return 'lib' + name + '.a'
 
def parse_config_site_and_add_features(self, header):
         assert '_LIBCPP_CONFIG_SITE' in feature_macros
         del feature_macros['_LIBCPP_CONFIG_SITE']
         # The __config_site header should be non-empty. Otherwise it should
         # have never been emitted by CMake.
         assert len(feature_macros) > 0
         # FIXME: This is a hack that should be fixed using module maps (or something)
         # If modules are enabled then we have to lift all of the definitions
         # in __config_site onto the command line.
-        modules_enabled = self.get_modules_enabled()
         self.cxx.compile_flags += ['-Wno-macro-redefined']
         # Transform each macro name into the feature name used in the tests.
         # Ex. _LIBCPP_HAS_NO_THREADS -> libcpp-has-no-threads
         for m in feature_macros:
-            if modules_enabled:
-                define = '-D%s' % m
-                if feature_macros[m]:
-                    define += '=%s' % (feature_macros[m])
-                self.cxx.compile_flags += [define]
             if m == '_LIBCPP_DISABLE_VISIBILITY_ANNOTATIONS' or \
                m == '_LIBCPP_HIDE_FROM_ABI_PER_TU_BY_DEFAULT':
                 continue
             if m == '_LIBCPP_ABI_VERSION':
                 self.config.available_features.add('libcpp-abi-version-v%s'
                     % feature_macros[m])
                 continue
             if m == '_LIBCPP_NO_VCRUNTIME':
def configure_coroutines(self):
             if intMacroValue(macros['__cpp_coroutines']) >= 201703:
                 self.config.available_features.add('fcoroutines-ts')
 
     def configure_modules(self):
         modules_flags = ['-fmodules']
         if platform.system() != 'Darwin':
             modules_flags += ['-Xclang', '-fmodules-local-submodule-visibility']
         supports_modules = self.cxx.hasCompileFlag(modules_flags)
-        enable_modules = self.get_modules_enabled()
-        if enable_modules and not supports_modules:
-            self.lit_config.fatal(
-                '-fmodules is enabled but not supported by the compiler')
         if not supports_modules:
             return
         self.config.available_features.add('modules-support')
         module_cache = os.path.join(self.config.test_exec_root,
                                    'modules.cache')
         module_cache = os.path.realpath(module_cache)
         if os.path.isdir(module_cache):
             shutil.rmtree(module_cache)
         os.makedirs(module_cache)
         self.cxx.modules_flags = modules_flags + \
             ['-fmodules-cache-path=' + module_cache]
-        if enable_modules:
-            self.config.available_features.add('-fmodules')
-            self.cxx.useModules()
 
     def configure_substitutions(self):
         sub = self.config.substitutions
         cxx_path = pipes.quote(self.cxx.path)
         # Configure compiler substitutions
         sub.append(('%cxx', cxx_path))
         sub.append(('%libcxx_src_root', self.libcxx_src_root))
         # Configure flags substitutions