[lit] Fix substitutions containing backslashes (#103042)

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
7c9eab8fef0ed79a5911d21eb97b6b0fa9d39f82. 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 #86649.
This commit is contained in:
Martin Storsjö 2024-08-22 12:57:39 +03:00 committed by GitHub
parent 57dc09341e
commit 51ca2354d0
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
4 changed files with 15 additions and 12 deletions

View File

@ -864,8 +864,9 @@ Additional substitutions can be defined as follows:
- Lit configuration files (e.g., ``lit.cfg`` or ``lit.local.cfg``) can define
substitutions for all tests in a test directory. They do so by extending the
substitution list, ``config.substitutions``. Each item in the list is a tuple
consisting of a pattern and its replacement, which lit applies using python's
``re.sub`` function.
consisting of a pattern and its replacement, which lit applies as plain text
(even if it contains sequences that python's ``re.sub`` considers to be
escape sequences).
- To define substitutions within a single test file, lit supports the
``DEFINE:`` and ``REDEFINE:`` directives, described in detail below. So that
they have no effect on other test files, these directives modify a copy of the

View File

@ -1591,7 +1591,6 @@ class SubstDirective(ExpandableScriptDirective):
assert (
not self.needs_continuation()
), "expected directive continuations to be parsed before applying"
value_repl = self.value.replace("\\", "\\\\")
existing = [i for i, subst in enumerate(substitutions) if self.name in subst[0]]
existing_res = "".join(
"\nExisting pattern: " + substitutions[i][0] for i in existing
@ -1604,7 +1603,7 @@ class SubstDirective(ExpandableScriptDirective):
f"{self.get_location()}"
f"{existing_res}"
)
substitutions.insert(0, (self.name, value_repl))
substitutions.insert(0, (self.name, self.value))
return
if len(existing) > 1:
raise ValueError(
@ -1626,7 +1625,7 @@ class SubstDirective(ExpandableScriptDirective):
f"Expected pattern: {self.name}"
f"{existing_res}"
)
substitutions[existing[0]] = (self.name, value_repl)
substitutions[existing[0]] = (self.name, self.value)
def applySubstitutions(script, substitutions, conditions={}, recursion_limit=None):
@ -1742,8 +1741,7 @@ def applySubstitutions(script, substitutions, conditions={}, recursion_limit=Non
# Apply substitutions
ln = substituteIfElse(escapePercents(ln))
for a, b in substitutions:
if kIsWindows:
b = b.replace("\\", "\\\\")
b = b.replace("\\", "\\\\")
# 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

View File

@ -23,6 +23,10 @@ config.substitutions.insert(
0, ("%{global:echo}", "echo GLOBAL: %{global:greeting} %{global:what}")
)
# This substitution includes an re.sub replacement string escape sequence,
# which lit should treat as plain text.
config.substitutions.insert(0, ("%{global:subst-with-escapes}", r"value-with-\g"))
# The following substitution definitions are confusing and should be avoided.
# We define them here so we can test that 'DEFINE:' and 'REDEFINE:' directives
# guard against the confusion they cause.

View File

@ -1,16 +1,16 @@
# FIXME: The doubled backslashes occur under windows. That's almost surely a
# lit issue beyond DEFINE/REDEFINE.
# Escape sequences that can appear in python re.sub replacement strings have no
# special meaning in the value.
# DEFINE: %{escape} = \g<0>\n
# RUN: echo '%{escape}'
# CHECK:# | {{\\?}}\g<0>{{\\?}}\n
# CHECK:# | \g<0>\n
# REDEFINE: %{escape} = \n \
# REDEFINE: \g<param>
# RUN: echo '%{escape}'
# CHECK:# | {{\\?}}\n {{\\?}}\g<param>
# CHECK:# | \n \g<param>
# RUN: echo '%{global:subst-with-escapes}'
# CHECK:# | value-with-\g
# CHECK: Passed: 1 {{\([0-9]*.[0-9]*%\)}}