kwk added a comment.
I didn't see much difference in what this patch does compare to mine but I saw
that it removes the need for instantiating multiple `llvm::Regex` objects from
a single static pattern. But that's something I've just done in a new revision
of my own patch: https://reviews.llvm.org/D134733?vs=463205&id=463786#toc .
This fast clean up makes it only harder for me to get my patch in but
essentially both do the same thing except that I also have a single convenience
function for trimming include names from `"` and `<>"`.
================
Comment at: clang/lib/Format/Format.cpp:2774
-
-const char CppIncludeRegexPattern[] =
- R"(^[\t\ ]*#[\t\ ]*(import|include)[^"<]*(["<][^">]*[">]))";
----------------
MyDeveloperDay wrote:
> Did I miss where this comes from now?
@MyDeveloperDay `clang/lib/Tooling/Inclusions/HeaderIncludes.cpp` still has
this:
```lang=c++
const char IncludeRegexPattern[] =
R"(^[\t\ ]*#[\t\ ]*(import|include)[^"<]*(["<][^">]*[">]))";
```
And in `clang/lib/Tooling/Inclusions/HeaderIncludes.cpp` @owenpan uses it to
initialize the final regex
```lang=c++
const llvm::Regex HeaderIncludes::IncludeRegex(IncludeRegexPattern);
```
The fact that we have two regex that are identical is an issue on its own that
I tried to address with [my patch](https://reviews.llvm.org/D134733) as well. I
didn't initialize the regex like @owenpan does here but I had a function to
return it. Eventually a function makes it easier to apply the injection from a
config file as you've suggested
[here](https://reviews.llvm.org/D134733#3821957). So I favor my solution.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D134852/new/
https://reviews.llvm.org/D134852
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
