[PATCH] D134852: [clang-format][NFC] Clean up class HeaderIncludes and Format.cpp

[Konrad Wilhelm Kleine via Phabricator via cfe-commits]

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