owenpan added a comment. @kwk D134733 <https://reviews.llvm.org/D134733> reminded me of https://reviews.llvm.org/D121370#3401173, which I realized didn't go far enough. This NFC patch is what I should have suggested to you then. If you want to do the cleanup differently, you can either make suggestions here or create another NFC patch.
================ Comment at: clang/lib/Format/Format.cpp:2774 - -const char CppIncludeRegexPattern[] = - R"(^[\t\ ]*#[\t\ ]*(import|include)[^"<]*(["<][^">]*[">]))"; ---------------- kwk wrote: > 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. > > > > > 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. It should be addressed in a separate NFC patch such as this one. > 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. Making `IncludeRegex` a public static const member is one of the better solutions when `IncludeRegexPattern` is fixed as it has been. If and when we decide to support user specified patterns, we will make any necessary changes then. 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
