phosek added a comment. In D152762#4421702 <https://reviews.llvm.org/D152762#4421702>, @ellis wrote:
> In D152762#4421630 <https://reviews.llvm.org/D152762#4421630>, @phosek wrote: > >> This is unrelated to this change but related to the issue this change is >> addressing. The use of regular expressions for special case list with the >> special handling of `*` is error-prone, I've seen many people having issues >> it. Furthermore, regular expression for matching file names requires >> excessive escaping (see for example >> https://fuchsia-review.git.corp.google.com/c/fuchsia/+/763162) and most >> patterns used for function names only utilize `*`, not requiring the full >> strength of regular expression. I think we should consider replacing regular >> expressions with glob patterns. These are already implemented in >> https://github.com/llvm/llvm-project/blob/3391bdc255f1a75c59d71c7305959e84d8d5f468/llvm/include/llvm/Support/GlobPattern.h >> so the implementation should be straightforward, but it's going to be a >> breaking change. I don't know how widespread the use of special case lists >> is and whether that's going to be an issue? > > Oh neat, I wasn't aware of `GlobPattern.h`. Personally, I would be happy with > this change, but it would require changing some blocklists on my end (which > is fine). I'm not sure what the processes is for a breaking change like this, > but since clang 16 has just been released, it seems like a good time to > change this. Thoughts? I think that making this change now and highlighting it as a breaking change for LLVM 17 is probably the right strategy. We should also consider extending the existing `GlobPattern` implementation to support brace expansion (i.e. `{foo,bar,baz}`) to better match the existing functionality. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D152762/new/ https://reviews.llvm.org/D152762 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
