MyDeveloperDay added inline comments.
================
Comment at: clang/lib/Tooling/Inclusions/HeaderIncludes.cpp:407
+ const llvm::SmallVectorImpl<llvm::StringRef> &Matches) {
+ if (Matches.size() >= 3) {
+ return Matches[2];
----------------
kwk wrote:
> MyDeveloperDay wrote:
> > ‘>= 2’
> @MyDeveloperDay correct me if I'm wrong but if an array has size `3` it has
> indexes `0`, `1` and `2`. And an array of size `2` only has `0` and `1`. So
> the case `=2`, which is implied by your suggested`>=2`, is actually an out of
> bounds access when going `Matches[2]`. Because that is effectively accessing
> the third element. The only valid change would be to check for
> `Matches.size() > 2` IMHO and that is the same as `Matches.size() >= 3`.
>
> I must admit that I had to look at the regex a few times only to realize that
> it has two non-optional matching groups `(...)`. The third matching group at
> index `0` is the whole line. So in theory `>=3` isn't possible with the
> current regex. I wanted to give my best to have this logic "survive" a change
> to the regex in which for example something is added after the matching group
> of the include name; let's say a comment or something.
>
> I hope I haven't made myself a complete fool.
No foolery other than my own. I meant to say '> 2', actually the first time I
wrote the comment it disappear because it didn't escape the '>'
So one though I had for the config was something like this (ignore regex itself
I just put anything in it to show the principle)
```
---
Language Cpp
IncludeRegex: ^[\t\ ]*[@#][\t\
]*(import|include)([^"/]*("[^"]+")|[^</]*(<[^>]+>)|[\t/\ ]*([^;]+;)
IncludeRegexGroup: 1
---
Language: ObjectiveC
IncludeRegex: ^[\t\ ]*[@#][\t\
]*(import|include)([^"/]*("[^"]+")|[^</]*(<[^>]+>)|[\t/\ ]*([^;]+;)
IncludeRegexGroup: 2
---
Language: CSharp
IncludeRegex: using ([A-z])*
IncludeRegexGroup: 0
---
Language: Carbon
IncludeRegex: package ([A-z])* api
IncludeRegexGroup: 0
---
```
Wouldn't this allow different languages to have their own Include/Import regex?
Just thinking out loud
It could be more powerful than having 1 regex to rule them all.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D134733/new/
https://reviews.llvm.org/D134733
_______________________________________________
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
