MaskRay added a comment.

In D154014#4461076 <https://reviews.llvm.org/D154014#4461076>, @rupprecht wrote:

> In D154014#4457883 <https://reviews.llvm.org/D154014#4457883>, @MaskRay wrote:
>
>>> This is a breaking change since some SCLs might use .* or (abc|def) which 
>>> are supported regexes but not valid globs. Since we have just cut clang 
>>> 16.x this is a good time to make this change.
>>
>> My user has some ignore lists, but there is no `^[a-z]+:.*\(` or 
>> `^[a-z]+:\.` occurrence, so this change is likely safe for us.
>
> I think I'm looking at the same lists, and I see plenty of `.*` in the 
> sanitizer exclusion lists. Also a few cases of `(abc|def|...)`. IIUC, those 
> would both be broken by this -- instead of `.*` meaning "any character any 
> number of times" (regex) it would mean "dot followed by any number of 
> characters" (glob), right? And the `(abc|...)` would just be that literal 
> text, not matching the individual parts?

Sorry that my regex use was incorrect. We do have some `src:x/a.pb.*` style 
rules, which are technically misused. `src:x/a.pb.*` can match possibly 
unintended files like `x/axpbx`.
To match just glob `x/a.pb.*` files, the rule should be written as 
`src:x/a\.pb\.*` (`*` instead of `.*`).

> If my understanding of that is correct, I don't think this is a good change 
> -- there's possibly plenty of configs out there that assume this is regex, 
> and there doesn't seem to be sufficient motivation to just break those. But I 
> can see that globs are useful for the examples posted in the patch 
> description. Is it possible to have some middle ground, e.g. default to regex 
> but allow a config at the top of sanitizer lists to interpret future patterns 
> as globs instead?

I think the main breakages are:

- patterns using `(` and `)`. Our users don't have such patterns.
- patterns using `\.` to mean a literal `.`, e.g. `src:a\.c`. Our users don't 
use `\`. Such a pattern needs to changed to `src:a.c` (which used to match 
other files like `axc`)
- patterns like `src:a.pb.*` that match other files like `src:axpbx.c`. This 
pattern can be changed to glob `src:a?pb?*`.

I think all of these are likely uncommon. While `\.` is technically the correct 
way to match a literal `.` before this patch, most people likely just use `.`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D154014

_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to