[libcxx-commits] [libcxxabi] 8a42bf2 - [lit] Move the recursiveExpansionLimit setting to TestingConfig

Louis Dionne via libcxx-commits libcxx-commits at lists.llvm.org
Mon Apr 6 10:58:10 PDT 2020


Author: Louis Dionne
Date: 2020-04-06T13:58:00-04:00
New Revision: 8a42bf24ae99d9d207ec140dadadc7d992bd2fa1

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

LOG: [lit] Move the recursiveExpansionLimit setting to TestingConfig

The LitConfig is shared across the whole test suite. However, since
enabling recursive expansion can be a breaking change for some test
suites, it's important to confine the setting to test suites that
enable it explicitly.

Note that other issues were raised with the way recursiveExpansionLimit
operates. However, this commit simply moves the setting to the right
place -- the mechanism by which it works can be improved independently.

Differential Revision: https://reviews.llvm.org/D77415

Added: 
    

Modified: 
    libcxx/test/lit.cfg
    libcxx/utils/libcxx/test/format.py
    libcxx/utils/libcxx/test/newformat.py
    libcxxabi/test/lit.cfg
    llvm/docs/CommandGuide/lit.rst
    llvm/utils/lit/lit/LitConfig.py
    llvm/utils/lit/lit/TestRunner.py
    llvm/utils/lit/lit/TestingConfig.py
    llvm/utils/lit/tests/Inputs/shtest-recursive-substitution/does-not-substitute-within-limit/lit.cfg
    llvm/utils/lit/tests/Inputs/shtest-recursive-substitution/negative-integer/lit.cfg
    llvm/utils/lit/tests/Inputs/shtest-recursive-substitution/not-an-integer/lit.cfg
    llvm/utils/lit/tests/Inputs/shtest-recursive-substitution/set-to-none/lit.cfg
    llvm/utils/lit/tests/Inputs/shtest-recursive-substitution/substitutes-within-limit/lit.cfg

Removed: 
    


################################################################################
diff  --git a/libcxx/test/lit.cfg b/libcxx/test/lit.cfg
index 8f42dd7c468b..89c465cfe86d 100644
--- a/libcxx/test/lit.cfg
+++ b/libcxx/test/lit.cfg
@@ -20,6 +20,9 @@ config.suffixes = ['.pass.cpp', '.fail.cpp', '.sh.cpp', '.pass.mm']
 # test_source_root: The root path where tests are located.
 config.test_source_root = os.path.dirname(__file__)
 
+# Allow expanding substitutions that are based on other substitutions
+config.recursiveExpansionLimit = 10
+
 loaded_site_cfg = getattr(config, 'loaded_site_config', False)
 if not loaded_site_cfg:
     import libcxx.test.config

diff  --git a/libcxx/utils/libcxx/test/format.py b/libcxx/utils/libcxx/test/format.py
index 3ea045ff0e7d..a4b6d669848e 100644
--- a/libcxx/utils/libcxx/test/format.py
+++ b/libcxx/utils/libcxx/test/format.py
@@ -132,13 +132,13 @@ def _execute(self, test, lit_config):
 
         # Apply substitutions in FILE_DEPENDENCIES markup
         data_files = lit.TestRunner.applySubstitutions(test.file_dependencies, substitutions,
-                                                       recursion_limit=10)
+                                                       recursion_limit=test.config.recursiveExpansionLimit)
         local_cwd = os.path.dirname(test.getSourcePath())
         data_files = [f if os.path.isabs(f) else os.path.join(local_cwd, f) for f in data_files]
         substitutions.append(('%{file_dependencies}', ' '.join(data_files)))
 
         script = lit.TestRunner.applySubstitutions(script, substitutions,
-                                                   recursion_limit=10)
+                                                   recursion_limit=test.config.recursiveExpansionLimit)
 
         test_cxx = copy.deepcopy(self.cxx)
         if is_fail_test:

diff  --git a/libcxx/utils/libcxx/test/newformat.py b/libcxx/utils/libcxx/test/newformat.py
index d75dc6a64915..f2e8c0028c86 100644
--- a/libcxx/utils/libcxx/test/newformat.py
+++ b/libcxx/utils/libcxx/test/newformat.py
@@ -191,7 +191,6 @@ def addCompileFlags(self, config, *flags):
 
     # Modified version of lit.TestRunner.executeShTest to handle custom parsers correctly.
     def _executeShTest(self, test, litConfig, steps, fileDependencies=None):
-        recursiveExpansionLimit = 10 # TODO: Use the value in litConfig once we set it.
         if test.config.unsupported:
             return lit.Test.Result(lit.Test.UNSUPPORTED, 'Test is unsupported')
 
@@ -229,7 +228,7 @@ def _executeShTest(self, test, litConfig, steps, fileDependencies=None):
         # need to resolve %{file_dependencies} now, because otherwise we won't be able to
         # make all paths absolute below.
         fileDependencies = lit.TestRunner.applySubstitutions(fileDependencies, substitutions,
-                                                             recursion_limit=recursiveExpansionLimit)
+                                                             recursion_limit=test.config.recursiveExpansionLimit)
 
         # Add the %{file_dependencies} substitution before we perform substitutions
         # inside the script.
@@ -239,6 +238,6 @@ def _executeShTest(self, test, litConfig, steps, fileDependencies=None):
 
         # Perform substitution in the script itself.
         script = lit.TestRunner.applySubstitutions(script, substitutions,
-                                                   recursion_limit=recursiveExpansionLimit)
+                                                   recursion_limit=test.config.recursiveExpansionLimit)
 
         return lit.TestRunner._runShTest(test, litConfig, useExternalSh, script, tmpBase)

diff  --git a/libcxxabi/test/lit.cfg b/libcxxabi/test/lit.cfg
index 5d79f169de78..d9caab7b8258 100644
--- a/libcxxabi/test/lit.cfg
+++ b/libcxxabi/test/lit.cfg
@@ -20,9 +20,15 @@ config.name = 'libc++abi'
 # suffixes: A list of file extensions to treat as test files.
 config.suffixes = ['.cpp', '.s']
 
+# Allow expanding substitutions that are based on other substitutions
+config.recursiveExpansionLimit = 10
+
 # test_source_root: The root path where tests are located.
 config.test_source_root = os.path.dirname(__file__)
 
+# Allow expanding substitutions that are based on other substitutions
+config.recursiveExpansionLimit = 10
+
 # Infer the libcxx_test_source_root for configuration import.
 # If libcxx_source_root isn't specified in the config, assume that the libcxx
 # and libcxxabi source directories are sibling directories.

diff  --git a/llvm/docs/CommandGuide/lit.rst b/llvm/docs/CommandGuide/lit.rst
index ebb89d2f853d..bbd739f28d42 100644
--- a/llvm/docs/CommandGuide/lit.rst
+++ b/llvm/docs/CommandGuide/lit.rst
@@ -457,10 +457,10 @@ modules :ref:`local-configuration-files`.
 By default, substitutions are expanded exactly once, so that if e.g. a
 substitution ``%build`` is defined in top of another substitution ``%cxx``,
 ``%build`` will expand to ``%cxx`` textually, not to what ``%cxx`` expands to.
-However, if the ``recursiveExpansionLimit`` property of the ``LitConfig`` is
-set to a non-negative integer, substitutions will be expanded recursively until
-that limit is reached. It is an error if the limit is reached and expanding
-substitutions again would yield a 
diff erent result.
+However, if the ``recursiveExpansionLimit`` property of the ``TestingConfig``
+is set to a non-negative integer, substitutions will be expanded recursively
+until that limit is reached. It is an error if the limit is reached and
+expanding substitutions again would yield a 
diff erent result.
 
 More detailed information on substitutions can be found in the
 :doc:`../TestingGuide`.

diff  --git a/llvm/utils/lit/lit/LitConfig.py b/llvm/utils/lit/lit/LitConfig.py
index 9ec634388cc1..58011b5986bf 100644
--- a/llvm/utils/lit/lit/LitConfig.py
+++ b/llvm/utils/lit/lit/LitConfig.py
@@ -66,19 +66,6 @@ def __init__(self, progname, path, quiet,
         self.maxIndividualTestTime = maxIndividualTestTime
         self.parallelism_groups = parallelism_groups
         self.echo_all_commands = echo_all_commands
-        self._recursiveExpansionLimit = None
-
-    @property
-    def recursiveExpansionLimit(self):
-        return self._recursiveExpansionLimit
-
-    @recursiveExpansionLimit.setter
-    def recursiveExpansionLimit(self, value):
-        if value is not None and not isinstance(value, int):
-            self.fatal('recursiveExpansionLimit must be either None or an integer (got <{}>)'.format(value))
-        if isinstance(value, int) and value < 0:
-            self.fatal('recursiveExpansionLimit must be a non-negative integer (got <{}>)'.format(value))
-        self._recursiveExpansionLimit = value
 
     @property
     def maxIndividualTestTime(self):

diff  --git a/llvm/utils/lit/lit/TestRunner.py b/llvm/utils/lit/lit/TestRunner.py
index e227dc8fd4de..6c92a2bc26c4 100644
--- a/llvm/utils/lit/lit/TestRunner.py
+++ b/llvm/utils/lit/lit/TestRunner.py
@@ -1552,6 +1552,6 @@ def executeShTest(test, litConfig, useExternalSh,
     substitutions += getDefaultSubstitutions(test, tmpDir, tmpBase,
                                              normalize_slashes=useExternalSh)
     script = applySubstitutions(script, substitutions,
-                                recursion_limit=litConfig.recursiveExpansionLimit)
+                                recursion_limit=test.config.recursiveExpansionLimit)
 
     return _runShTest(test, litConfig, useExternalSh, script, tmpBase)

diff  --git a/llvm/utils/lit/lit/TestingConfig.py b/llvm/utils/lit/lit/TestingConfig.py
index 738aafe5b415..dd2d3f52f89c 100644
--- a/llvm/utils/lit/lit/TestingConfig.py
+++ b/llvm/utils/lit/lit/TestingConfig.py
@@ -2,7 +2,7 @@
 import sys
 
 
-class TestingConfig:
+class TestingConfig(object):
     """"
     TestingConfig - Information on the tests inside a suite.
     """
@@ -126,6 +126,19 @@ def __init__(self, parent, name, suffixes, test_format,
         # Whether the suite should be tested early in a given run.
         self.is_early = bool(is_early)
         self.parallelism_group = parallelism_group
+        self._recursiveExpansionLimit = None
+
+    @property
+    def recursiveExpansionLimit(self):
+        return self._recursiveExpansionLimit
+
+    @recursiveExpansionLimit.setter
+    def recursiveExpansionLimit(self, value):
+        if value is not None and not isinstance(value, int):
+            raise ValueError('recursiveExpansionLimit must be either None or an integer (got <{}>)'.format(value))
+        if isinstance(value, int) and value < 0:
+            raise ValueError('recursiveExpansionLimit must be a non-negative integer (got <{}>)'.format(value))
+        self._recursiveExpansionLimit = value
 
     def finish(self, litConfig):
         """finish() - Finish this config object, after loading is complete."""

diff  --git a/llvm/utils/lit/tests/Inputs/shtest-recursive-substitution/does-not-substitute-within-limit/lit.cfg b/llvm/utils/lit/tests/Inputs/shtest-recursive-substitution/does-not-substitute-within-limit/lit.cfg
index 8fce89741b66..69e20ceef583 100644
--- a/llvm/utils/lit/tests/Inputs/shtest-recursive-substitution/does-not-substitute-within-limit/lit.cfg
+++ b/llvm/utils/lit/tests/Inputs/shtest-recursive-substitution/does-not-substitute-within-limit/lit.cfg
@@ -9,4 +9,4 @@ config.substitutions = [("%rec1", "STOP"), ("%rec2", "%rec1"),
                         ("%rec3", "%rec2"), ("%rec4", "%rec3"),
                         ("%rec5", "%rec4")]
 
-lit_config.recursiveExpansionLimit = 2
+config.recursiveExpansionLimit = 2

diff  --git a/llvm/utils/lit/tests/Inputs/shtest-recursive-substitution/negative-integer/lit.cfg b/llvm/utils/lit/tests/Inputs/shtest-recursive-substitution/negative-integer/lit.cfg
index 0037b34bb54f..164e4a162fec 100644
--- a/llvm/utils/lit/tests/Inputs/shtest-recursive-substitution/negative-integer/lit.cfg
+++ b/llvm/utils/lit/tests/Inputs/shtest-recursive-substitution/negative-integer/lit.cfg
@@ -5,4 +5,4 @@ config.test_format = lit.formats.ShTest()
 config.test_source_root = None
 config.test_exec_root = None
 
-lit_config.recursiveExpansionLimit = -4
+config.recursiveExpansionLimit = -4

diff  --git a/llvm/utils/lit/tests/Inputs/shtest-recursive-substitution/not-an-integer/lit.cfg b/llvm/utils/lit/tests/Inputs/shtest-recursive-substitution/not-an-integer/lit.cfg
index 3eb36d8663dd..6407d23b6b4e 100644
--- a/llvm/utils/lit/tests/Inputs/shtest-recursive-substitution/not-an-integer/lit.cfg
+++ b/llvm/utils/lit/tests/Inputs/shtest-recursive-substitution/not-an-integer/lit.cfg
@@ -5,4 +5,4 @@ config.test_format = lit.formats.ShTest()
 config.test_source_root = None
 config.test_exec_root = None
 
-lit_config.recursiveExpansionLimit = "not-an-integer"
+config.recursiveExpansionLimit = "not-an-integer"

diff  --git a/llvm/utils/lit/tests/Inputs/shtest-recursive-substitution/set-to-none/lit.cfg b/llvm/utils/lit/tests/Inputs/shtest-recursive-substitution/set-to-none/lit.cfg
index 1dfc57a80944..9c2583383fb6 100644
--- a/llvm/utils/lit/tests/Inputs/shtest-recursive-substitution/set-to-none/lit.cfg
+++ b/llvm/utils/lit/tests/Inputs/shtest-recursive-substitution/set-to-none/lit.cfg
@@ -5,4 +5,4 @@ config.test_format = lit.formats.ShTest()
 config.test_source_root = None
 config.test_exec_root = None
 
-lit_config.recursiveExpansionLimit = None
+config.recursiveExpansionLimit = None

diff  --git a/llvm/utils/lit/tests/Inputs/shtest-recursive-substitution/substitutes-within-limit/lit.cfg b/llvm/utils/lit/tests/Inputs/shtest-recursive-substitution/substitutes-within-limit/lit.cfg
index aa66c0e856eb..6ff0ee5ba1f4 100644
--- a/llvm/utils/lit/tests/Inputs/shtest-recursive-substitution/substitutes-within-limit/lit.cfg
+++ b/llvm/utils/lit/tests/Inputs/shtest-recursive-substitution/substitutes-within-limit/lit.cfg
@@ -9,4 +9,4 @@ config.substitutions = [("%rec1", "STOP"), ("%rec2", "%rec1"),
                         ("%rec3", "%rec2"), ("%rec4", "%rec3"),
                         ("%rec5", "%rec4")]
 
-lit_config.recursiveExpansionLimit = 5
+config.recursiveExpansionLimit = 5


        


More information about the libcxx-commits mailing list