[libcxx-commits] [libcxx] f836549 - [libc++] Migrate the additional_features parameter to the DSL

Louis Dionne via libcxx-commits libcxx-commits at lists.llvm.org
Thu Jul 1 10:39:42 PDT 2021


Author: Louis Dionne
Date: 2021-07-01T13:38:09-04:00
New Revision: f83654982be65567d41c513b27ef76c3c64946f5

URL: https://github.com/llvm/llvm-project/commit/f83654982be65567d41c513b27ef76c3c64946f5
DIFF: https://github.com/llvm/llvm-project/commit/f83654982be65567d41c513b27ef76c3c64946f5.diff

LOG: [libc++] Migrate the additional_features parameter to the DSL

This is required to run the tests under any configuration that uses
additional_features using a from-scratch config. That is the case of
e.g. the Debug mode (which uses LIBCXX-DEBUG-FIXME) and the tests on
Windows.

Added: 
    

Modified: 
    libcxx/test/libcxx/selftest/dsl/dsl.sh.py
    libcxx/utils/libcxx/test/config.py
    libcxx/utils/libcxx/test/dsl.py
    libcxx/utils/libcxx/test/params.py

Removed: 
    


################################################################################
diff  --git a/libcxx/test/libcxx/selftest/dsl/dsl.sh.py b/libcxx/test/libcxx/selftest/dsl/dsl.sh.py
index e27c13a9d2056..1901ddb9964d0 100644
--- a/libcxx/test/libcxx/selftest/dsl/dsl.sh.py
+++ b/libcxx/test/libcxx/selftest/dsl/dsl.sh.py
@@ -435,6 +435,26 @@ def test_boolean_value_from_false_boolean_parameter(self):
             a.applyTo(self.config)
         self.assertIn('-fno-exceptions', self.config.available_features)
 
+    def test_list_parsed_from_comma_delimited_string_empty(self):
+        self.litConfig.params['additional_features'] = ""
+        param = dsl.Parameter(name='additional_features', type=list, help='', actions=lambda f: f)
+        self.assertEqual(param.getActions(self.config, self.litConfig.params), [])
+
+    def test_list_parsed_from_comma_delimited_string_1(self):
+        self.litConfig.params['additional_features'] = "feature1"
+        param = dsl.Parameter(name='additional_features', type=list, help='', actions=lambda f: f)
+        self.assertEqual(param.getActions(self.config, self.litConfig.params), ['feature1'])
+
+    def test_list_parsed_from_comma_delimited_string_2(self):
+        self.litConfig.params['additional_features'] = "feature1,feature2"
+        param = dsl.Parameter(name='additional_features', type=list, help='', actions=lambda f: f)
+        self.assertEqual(param.getActions(self.config, self.litConfig.params), ['feature1', 'feature2'])
+
+    def test_list_parsed_from_comma_delimited_string_3(self):
+        self.litConfig.params['additional_features'] = "feature1,feature2, feature3"
+        param = dsl.Parameter(name='additional_features', type=list, help='', actions=lambda f: f)
+        self.assertEqual(param.getActions(self.config, self.litConfig.params), ['feature1', 'feature2', 'feature3'])
+
 
 if __name__ == '__main__':
     unittest.main(verbosity=2)

diff  --git a/libcxx/utils/libcxx/test/config.py b/libcxx/utils/libcxx/test/config.py
index 4978482006c67..9538ecc6db798 100644
--- a/libcxx/utils/libcxx/test/config.py
+++ b/libcxx/utils/libcxx/test/config.py
@@ -232,11 +232,6 @@ def configure_obj_root(self):
                 self.libcxx_obj_root = self.project_obj_root
 
     def configure_features(self):
-        additional_features = self.get_lit_conf('additional_features')
-        if additional_features:
-            for f in additional_features.split(','):
-                self.config.available_features.add(f.strip())
-
         if self.target_info.is_windows():
             if self.cxx_stdlib_under_test == 'libc++':
                 # LIBCXX-WINDOWS-FIXME is the feature name used to XFAIL the

diff  --git a/libcxx/utils/libcxx/test/dsl.py b/libcxx/utils/libcxx/test/dsl.py
index 1071636702478..64988e1609b31 100644
--- a/libcxx/utils/libcxx/test/dsl.py
+++ b/libcxx/utils/libcxx/test/dsl.py
@@ -511,6 +511,13 @@ def _str_to_bool(s):
   else:
     raise ValueError("Got string '{}', which isn't a valid boolean".format(s))
 
+def _parse_parameter(s, type):
+  if type is bool and isinstance(s, str):
+    return _str_to_bool(s)
+  elif type is list and isinstance(s, str):
+    return [x.strip() for x in s.split(',') if x.strip()]
+  return type(s)
+
 
 class Parameter(object):
   """
@@ -554,7 +561,8 @@ def __init__(self, name, type, help, actions, choices=None, default=None):
     - type
         A callable that can be used to parse the value of the parameter given
         on the command-line. As a special case, using the type `bool` also
-        allows parsing strings with boolean-like contents.
+        allows parsing strings with boolean-like contents, and the type `list`
+        will parse a string delimited by commas into a list of the substrings.
 
     - help
         A string explaining the parameter, for documentation purposes.
@@ -584,8 +592,7 @@ def __init__(self, name, type, help, actions, choices=None, default=None):
     else:
       self._choices = None
 
-    self._parse = lambda x: (_str_to_bool(x) if type is bool and isinstance(x, str)
-                                             else type(x))
+    self._parse = lambda x: _parse_parameter(x, type)
     self._help = help
     self._actions = actions
     self._default = default
@@ -599,10 +606,16 @@ def _getValue(self, config, litParams):
     if param is None and self._default is None:
       raise ValueError("Parameter {} doesn't have a default value, but it was not specified in the Lit parameters or in the Lit config".format(self.name))
     getDefault = lambda: self._default(config) if callable(self._default) else self._default
-    value = self._parse(param) if param is not None else getDefault()
+
+    if param is not None:
+      (pretty, value) = (param, self._parse(param))
+    else:
+      value = getDefault()
+      pretty = '{} (default)'.format(value)
+
     if self._choices and value not in self._choices:
       raise ValueError("Got value '{}' for parameter '{}', which is not in the provided set of possible choices: {}".format(value, self.name, self._choices))
-    return value
+    return (pretty, value)
 
   @property
   def name(self):
@@ -618,10 +631,12 @@ def getActions(self, config, litParams):
     """
     Return the list of actions associated to this value of the parameter.
     """
-    return self._actions(self._getValue(config, litParams))
+    (_, parameterValue) = self._getValue(config, litParams)
+    return self._actions(parameterValue)
 
   def pretty(self, config, litParams):
     """
     Return a pretty representation of the parameter's name and value.
     """
-    return "{}={}".format(self.name, self._getValue(config, litParams))
+    (prettyParameterValue, _) = self._getValue(config, litParams)
+    return "{}={}".format(self.name, prettyParameterValue)

diff  --git a/libcxx/utils/libcxx/test/params.py b/libcxx/utils/libcxx/test/params.py
index 5ab82711c1b40..bdaa573205e80 100644
--- a/libcxx/utils/libcxx/test/params.py
+++ b/libcxx/utils/libcxx/test/params.py
@@ -53,7 +53,6 @@ def getStdFlag(cfg, std):
   return None
 
 DEFAULT_PARAMETERS = [
-  # Core parameters of the test suite
   Parameter(name='target_triple', type=str, default=getHostTriple,
             help="The target triple to compile the test suite for. This must be "
                  "compatible with the target that the tests will be run on.",
@@ -142,7 +141,6 @@ def getStdFlag(cfg, std):
               AddFeature('sanitizer-new-delete') if sanitizer in ['Address', 'Memory', 'MemoryWithOrigins', 'Thread'] else None,
             ])),
 
-  # Parameters to enable or disable parts of the test suite
   Parameter(name='enable_experimental', choices=[True, False], type=bool, default=True,
             help="Whether to enable tests for experimental C++ libraries (typically Library Fundamentals TSes).",
             actions=lambda experimental: [] if not experimental else [
@@ -166,6 +164,12 @@ def getStdFlag(cfg, std):
             actions=lambda enabled: [] if enabled else [
               AddFeature('libcxx-no-debug-mode')
             ]),
+
+  Parameter(name='additional_features', type=list, default=[],
+            help="A comma-delimited list of additional features that will be enabled when running the tests. "
+                 "This should be used sparingly since specifying ad-hoc features manually is error-prone and "
+                 "brittle in the long run as changes are made to the test suite.",
+            actions=lambda features: [AddFeature(f) for f in features]),
 ]
 
 DEFAULT_PARAMETERS += [


        


More information about the libcxx-commits mailing list