Skip to content

Commit 870eef2

Browse files
committed
[lit] Fix substitutions containing backslashes
Substitutions can be added in a couple different ways; they can be added via the calling python scripts by adding entries to the config.substitutions dictionary, or via DEFINE lines in the scripts themselves. The substitution strings passed to Python's re classes are interpreted so that backslashes expand to escape sequences, and literal backslashes need to be escaped. On Unix, the script defined substitutions don't (usually, so far) contain backslashes - but on Windows, they often do, due to paths containing backslashes. This lead to a Windows specific escaping of backslashes before doing Python re substitutions - since 7c9eab8. There's nothing inherently Windows specific about this though - any intended literal backslashes in the substitution strings need to be escaped; this is how the Python re API works. The DEFINE lines were added later, and in order to cope with backslashes, escaping of backslashes was added in the SubstDirective class in TestRunner, applying to DEFINE lines in the tests only. The fact that the escaping right before passing to the Python re API was done conditionally on Windows led to two inconsistencies: - DEFINE lines in the tests that contain backslashes got double backslashes on Windows. (This was visible as a FIXME in llvm/utils/lit/tests/Inputs/shtest-define/value-escaped.txt.) - Script provided substitutions containing backslashes did not work on Unix, but they did work on Windows. By removing the escaping from SubstDirective and escaping it unconditionally in the processLine function, before feeding the substitutions to Python's re classes, we should have consistent behaviour across platforms, and get rid of the FIXME in the lit test. This fixes issues with substitutions containing backslashes on Unix platforms, as encountered in PR llvm#86649.
1 parent 2c62d08 commit 870eef2

File tree

2 files changed

+5
-10
lines changed

2 files changed

+5
-10
lines changed

llvm/utils/lit/lit/TestRunner.py

Lines changed: 3 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1587,7 +1587,6 @@ def adjust_substitutions(self, substitutions):
15871587
assert (
15881588
not self.needs_continuation()
15891589
), "expected directive continuations to be parsed before applying"
1590-
value_repl = self.value.replace("\\", "\\\\")
15911590
existing = [i for i, subst in enumerate(substitutions) if self.name in subst[0]]
15921591
existing_res = "".join(
15931592
"\nExisting pattern: " + substitutions[i][0] for i in existing
@@ -1600,7 +1599,7 @@ def adjust_substitutions(self, substitutions):
16001599
f"{self.get_location()}"
16011600
f"{existing_res}"
16021601
)
1603-
substitutions.insert(0, (self.name, value_repl))
1602+
substitutions.insert(0, (self.name, self.value))
16041603
return
16051604
if len(existing) > 1:
16061605
raise ValueError(
@@ -1622,7 +1621,7 @@ def adjust_substitutions(self, substitutions):
16221621
f"Expected pattern: {self.name}"
16231622
f"{existing_res}"
16241623
)
1625-
substitutions[existing[0]] = (self.name, value_repl)
1624+
substitutions[existing[0]] = (self.name, self.value)
16261625

16271626

16281627
def applySubstitutions(script, substitutions, conditions={}, recursion_limit=None):
@@ -1738,8 +1737,7 @@ def processLine(ln):
17381737
# Apply substitutions
17391738
ln = substituteIfElse(escapePercents(ln))
17401739
for a, b in substitutions:
1741-
if kIsWindows:
1742-
b = b.replace("\\", "\\\\")
1740+
b = b.replace("\\", "\\\\")
17431741
# re.compile() has a built-in LRU cache with 512 entries. In some
17441742
# test suites lit ends up thrashing that cache, which made e.g.
17451743
# check-llvm run 50% slower. Use an explicit, unbounded cache
Lines changed: 2 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,16 +1,13 @@
1-
# FIXME: The doubled backslashes occur under windows. That's almost surely a
2-
# lit issue beyond DEFINE/REDEFINE.
3-
41
# Escape sequences that can appear in python re.sub replacement strings have no
52
# special meaning in the value.
63

74
# DEFINE: %{escape} = \g<0>\n
85
# RUN: echo '%{escape}'
9-
# CHECK:# | {{\\?}}\g<0>{{\\?}}\n
6+
# CHECK:# | \g<0>\n
107

118
# REDEFINE: %{escape} = \n \
129
# REDEFINE: \g<param>
1310
# RUN: echo '%{escape}'
14-
# CHECK:# | {{\\?}}\n {{\\?}}\g<param>
11+
# CHECK:# | \n \g<param>
1512

1613
# CHECK: Passed: 1 {{\([0-9]*.[0-9]*%\)}}

0 commit comments

Comments
 (0)