[PATCH] D92404: [FileCheck] Enforce --allow-unused-prefixes=false for llvm/test/Transforms
Mircea Trofin via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Wed Dec 2 21:32:00 PST 2020
mtrofin marked an inline comment as done.
mtrofin added inline comments.
================
Comment at: llvm/utils/update_test_prefix.py:22
- s = re.sub('-?-check-prefixes=([^, ]+\n)', '--check-prefix=\\1', s)
- s = re.sub('-?-check-prefixes=([^, ]+) ', '--check-prefix=\\1', s)
t = re.search('-?-check-(?:prefix|prefixes)=([^ ]+)\s+-?-check-(?:prefix|prefixes)=([^ ]+)', s)
----------------
pengfei wrote:
> mtrofin wrote:
> > pengfei wrote:
> > > I think we just need to add a space for it:
> > > ```
> > > s = re.sub('-?-check-prefixes=([^, ]+) ', '--check-prefix=\\1 ', s)
> > > ```
> > > I once worried we may leave space at the end but didn't consider %s case.
> > > `\w+` is not always correct because many prefixes are using `-` in them.
> > how about
> >
> > s = re.sub('-?-check-prefixes=([\w_-]+)', '--check-prefix=\\1', s)
> >
> > So it'd avoid the extra space, too? Or do we allow other characters?
> I might tend to conservative. As far as I can see it, `[\w-]` should be Okay. But you need to check the last character is `'\n'` or `' '`. Otherwise it will turn somthing `-check-prefixes=A,B` into `-check-prefix=A,B`. Maybe you can use below:
> ```
> s = re.sub('-?-check-prefixes=([\w-]+\s)', '--check-prefix=\\1', s)
> ```
sg, I'll send a separate patch with that.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D92404/new/
https://reviews.llvm.org/D92404
More information about the llvm-commits
mailing list