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

[Sergej Jaskiewicz via Phabricator via llvm-commits]

broadwaylamb created this revision.
broadwaylamb added reviewers: ldionne, yln, dexonsmith, delcypher.
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

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.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D83894

Files:
  llvm/utils/lit/lit/TestRunner.py
  llvm/utils/lit/tests/Inputs/shtest-recursive-substitution/does-not-substitute-no-limit/test.py
  llvm/utils/lit/tests/Inputs/shtest-recursive-substitution/substitutes-within-limit/test.py
  llvm/utils/lit/tests/shtest-recursive-substitution.py


Index: llvm/utils/lit/tests/shtest-recursive-substitution.py
===================================================================
--- llvm/utils/lit/tests/shtest-recursive-substitution.py
+++ llvm/utils/lit/tests/shtest-recursive-substitution.py
@@ -3,7 +3,7 @@
 
 # RUN: %{lit} -j 1 %{inputs}/shtest-recursive-substitution/substitutes-within-limit --show-all | FileCheck --check-prefix=CHECK-TEST1 %s
 # CHECK-TEST1: PASS: substitutes-within-limit :: test.py
-# CHECK-TEST1: $ "echo" "STOP"
+# CHECK-TEST1: $ "echo" "STOP" "%s" "%%s"
 
 # RUN: not %{lit} -j 1 %{inputs}/shtest-recursive-substitution/does-not-substitute-within-limit --show-all | FileCheck --check-prefix=CHECK-TEST2 %s
 # CHECK-TEST2: UNRESOLVED: does-not-substitute-within-limit :: test.py
@@ -11,7 +11,7 @@
 
 # RUN: %{lit} -j 1 %{inputs}/shtest-recursive-substitution/does-not-substitute-no-limit --show-all | FileCheck --check-prefix=CHECK-TEST3 %s
 # CHECK-TEST3: PASS: does-not-substitute-no-limit :: test.py
-# CHECK-TEST3: $ "echo" "%rec4"
+# CHECK-TEST3: $ "echo" "%rec4" "%s" "%%s"
 
 # RUN: not %{lit} -j 1 %{inputs}/shtest-recursive-substitution/not-an-integer --show-all 2>&1 | FileCheck --check-prefix=CHECK-TEST4 %s
 # CHECK-TEST4: recursiveExpansionLimit must be either None or an integer
Index: llvm/utils/lit/tests/Inputs/shtest-recursive-substitution/substitutes-within-limit/test.py
===================================================================
--- llvm/utils/lit/tests/Inputs/shtest-recursive-substitution/substitutes-within-limit/test.py
+++ llvm/utils/lit/tests/Inputs/shtest-recursive-substitution/substitutes-within-limit/test.py
@@ -1 +1 @@
-# RUN: echo %rec5
+# RUN: echo %rec5 %%s %%%%s
Index: llvm/utils/lit/tests/Inputs/shtest-recursive-substitution/does-not-substitute-no-limit/test.py
===================================================================
--- llvm/utils/lit/tests/Inputs/shtest-recursive-substitution/does-not-substitute-no-limit/test.py
+++ llvm/utils/lit/tests/Inputs/shtest-recursive-substitution/does-not-substitute-no-limit/test.py
@@ -1 +1 @@
-# RUN: echo %rec5
+# RUN: echo %rec5 %%s %%%%s
Index: llvm/utils/lit/lit/TestRunner.py
===================================================================
--- llvm/utils/lit/lit/TestRunner.py
+++ llvm/utils/lit/lit/TestRunner.py
@@ -1159,11 +1159,17 @@
     `recursion_limit` times, it is an error. If the `recursion_limit` is
     `None` (the default), no recursive substitution is performed at all.
     """
-    def processLine(ln):
+    def processLine(ln, ignore_double_percent_substitution=False):
         # Apply substitutions
         for a,b in substitutions:
             if kIsWindows:
                 b = b.replace("\\","\\\\")
+
+            if ignore_double_percent_substitution and a == '#_MARKER_#':
+                # Defer the %% substitution until the last pass when
+                # expanding recursively.
+                continue
+
             # re.compile() has a built-in LRU cache with 512 entries. In some
             # test suites lit ends up thrashing that cache, which made e.g.
             # check-llvm run 50% slower.  Use an explicit, unbounded cache
@@ -1180,16 +1186,20 @@
         assert isinstance(recursion_limit, int) and recursion_limit >= 0
         origLine = ln
         steps = 0
-        processed = processLine(ln)
+        processed = processLine(ln, ignore_double_percent_substitution=True)
         while processed != ln and steps < recursion_limit:
             ln = processed
-            processed = processLine(ln)
+            processed = processLine(ln, ignore_double_percent_substitution=True)
             steps += 1
 
         if processed != ln:
             raise ValueError("Recursive substitution of '%s' did not complete "
                              "in the provided recursion limit (%s)" % \
                              (origLine, recursion_limit))
+        
+        # Finally, if everything's alright, process the line one last time.
+        # This time we don't ignore the %% substitution.
+        processed = processLine(ln, ignore_double_percent_substitution=False)
 
         return processed
 


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D83894.278255.patch
Type: text/x-patch
Size: 4147 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20200715/baf0fef4/attachment.bin>