[llvm] 4dd5c2b - [lit] Don't expand escapes until all substitutions have been applied

[Sergej Jaskiewicz via llvm-commits]

Author: Sergej Jaskiewicz
Date: 2020-07-27T18:09:00+03:00
New Revision: 4dd5c2bee366514cbc3fc4e6da46462bc11a0a3d

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

LOG: [lit] Don't expand escapes until all substitutions have been applied

Otherwise, if a Lit script contains escaped substitutions (like %%p in this test https://github.com/llvm/llvm-project/blob/master/compiler-rt/test/asan/TestCases/Darwin/asan-symbolize-partial-report-with-module-map.cpp#L10), they are unescaped during recursive application of substitutions, and the results are unexpected.

We solve it using the fact that double percent signs are first replaced with #_MARKER_#, and only after all the other substitutions have been applied, #_MARKER_# is replaced with a single percent sign. The only change is that instead of replacing #_MARKER_# at each recursion step, we replace it once after the last recursion step.

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

Added: 
    llvm/utils/lit/tests/Inputs/shtest-recursive-substitution/escaping/lit.cfg
    llvm/utils/lit/tests/Inputs/shtest-recursive-substitution/escaping/test.py

Modified: 
    llvm/utils/lit/lit/TestRunner.py
    llvm/utils/lit/tests/shtest-recursive-substitution.py

Removed: 
    


################################################################################
diff  --git a/llvm/utils/lit/lit/TestRunner.py b/llvm/utils/lit/lit/TestRunner.py
index d80f0aeee8ce..643b03fc279a 100644
--- a/llvm/utils/lit/lit/TestRunner.py
+++ b/llvm/utils/lit/lit/TestRunner.py
@@ -1081,9 +1081,7 @@ def getDefaultSubstitutions(test, tmpDir, tmpBase, normalize_slashes=False):
         tmpDir = tmpDir.replace('\\', '/')
         tmpBase = tmpBase.replace('\\', '/')
 
-    # We use #_MARKER_# to hide %% while we do the other substitutions.
     substitutions = []
-    substitutions.extend([('%%', '#_MARKER_#')])
     substitutions.extend(test.config.substitutions)
     tmpName = tmpBase + '.tmp'
     baseName = os.path.basename(tmpBase)
@@ -1093,8 +1091,7 @@ def getDefaultSubstitutions(test, tmpDir, tmpBase, normalize_slashes=False):
                           ('%{pathsep}', os.pathsep),
                           ('%t', tmpName),
                           ('%basename_t', baseName),
-                          ('%T', tmpDir),
-                          ('#_MARKER_#', '%')])
+                          ('%T', tmpDir)])
 
     # "%/[STpst]" should be normalized.
     substitutions.extend([
@@ -1159,6 +1156,14 @@ def applySubstitutions(script, substitutions, recursion_limit=None):
     `recursion_limit` times, it is an error. If the `recursion_limit` is
     `None` (the default), no recursive substitution is performed at all.
     """
+
+    # We use #_MARKER_# to hide %% while we do the other substitutions.
+    def escape(ln):
+        return _caching_re_compile('%%').sub('#_MARKER_#', ln)
+
+    def unescape(ln):
+        return _caching_re_compile('#_MARKER_#').sub('%', ln)
+
     def processLine(ln):
         # Apply substitutions
         for a,b in substitutions:
@@ -1171,7 +1176,7 @@ def processLine(ln):
             # short-lived, since the set of substitutions is fairly small, and
             # since thrashing has such bad consequences, not bounding the cache
             # seems reasonable.
-            ln = _caching_re_compile(a).sub(str(b), ln)
+            ln = _caching_re_compile(a).sub(str(b), escape(ln))
 
         # Strip the trailing newline and any extra whitespace.
         return ln.strip()
@@ -1193,10 +1198,9 @@ def processLineToFixedPoint(ln):
 
         return processed
 
-    # Note Python 3 map() gives an iterator rather than a list so explicitly
-    # convert to list before returning.
     process = processLine if recursion_limit is None else processLineToFixedPoint
-    return list(map(process, script))
+    
+    return [unescape(process(ln)) for ln in script]
 
 
 class ParserKind(object):

diff  --git a/llvm/utils/lit/tests/Inputs/shtest-recursive-substitution/escaping/lit.cfg b/llvm/utils/lit/tests/Inputs/shtest-recursive-substitution/escaping/lit.cfg
new file mode 100644
index 000000000000..97a4faac2387
--- /dev/null
+++ b/llvm/utils/lit/tests/Inputs/shtest-recursive-substitution/escaping/lit.cfg
@@ -0,0 +1,10 @@
+import lit.formats
+config.name = 'escaping'
+config.suffixes = ['.py']
+config.test_format = lit.formats.ShTest()
+config.test_source_root = None
+config.test_exec_root = None
+
+config.substitutions = [("%rec1", "%%s"), ("%rec2", "%rec1")]
+
+config.recursiveExpansionLimit = 5

diff  --git a/llvm/utils/lit/tests/Inputs/shtest-recursive-substitution/escaping/test.py b/llvm/utils/lit/tests/Inputs/shtest-recursive-substitution/escaping/test.py
new file mode 100644
index 000000000000..74056e71caf4
--- /dev/null
+++ b/llvm/utils/lit/tests/Inputs/shtest-recursive-substitution/escaping/test.py
@@ -0,0 +1 @@
+# RUN: echo %rec2 %%s %%%%s

diff  --git a/llvm/utils/lit/tests/shtest-recursive-substitution.py b/llvm/utils/lit/tests/shtest-recursive-substitution.py
index 1b3d5f4c4451..d446c422139b 100644
--- a/llvm/utils/lit/tests/shtest-recursive-substitution.py
+++ b/llvm/utils/lit/tests/shtest-recursive-substitution.py
@@ -21,3 +21,7 @@
 
 # RUN: %{lit} -j 1 %{inputs}/shtest-recursive-substitution/set-to-none --show-all | FileCheck --check-prefix=CHECK-TEST6 %s
 # CHECK-TEST6: PASS: set-to-none :: test.py
+
+# RUN: %{lit} -j 1 %{inputs}/shtest-recursive-substitution/escaping --show-all | FileCheck --check-prefix=CHECK-TEST7 %s
+# CHECK-TEST7: PASS: escaping :: test.py
+# CHECK-TEST7: $ "echo" "%s" "%s" "%%s"