[PATCH] D53798: [lit] Add --show-substitutions

Dan Liew via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Thu Jan 24 03:58:53 PST 2019


delcypher added inline comments.
Herald added a reviewer: serge-sans-paille.


================
Comment at: utils/lit/lit/main.py:433
+                    for (pattern, replacement) in ts.config.substitutions:
+                        if re.match(r'[\'"]|.*\s', pattern):
+                            pattern = repr(pattern)
----------------
eush wrote:
> delcypher wrote:
> > Is all this regex really necessary? Couldn't we just always print `repr(pattern), repr(replacement)`?
> My only concern is that otherwise it would be harder to read regexes since they would be (doubly) escaped.
Then why even use `repr(...)`? Just print as a regular string always.

E.g. 
```
>>> print(repr(r'\s\w'))
'\\s\\w'
>>> print(str(r'\s\w'))
\s\w
```

I honestly don't understand what your regex logic is trying to achieve so I really think you should drop it.


Repository:
  rL LLVM

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D53798/new/

https://reviews.llvm.org/D53798





More information about the llvm-commits mailing list