[clang] [clang-format] Fix more bugs in isStartOfName() (PR #72336)

kadir çetinkaya via cfe-commits cfe-commits at lists.llvm.org
Tue Jan 2 22:56:42 PST 2024


kadircet wrote:

hi @owenca looks like this is still regressing certain patterns, e.g:

```cpp
int foo BAR;
```

annotations before your modifications to `isStartOfName`:
```
AnnotatedTokens(L=0):
 M=0 C=0 T=Unknown S=1 F=0 B=0 BK=0 P=0 Name=int L=3 PPK=2 FakeLParens= FakeRParens=0 II=0x216ebe0 Text='int'
 M=0 C=1 T=StartOfName S=1 F=0 B=0 BK=0 P=220 Name=identifier L=7 PPK=2 FakeLParens= FakeRParens=0 II=0x21a3268 Text='foo'
 M=0 C=1 T=StartOfName S=1 F=0 B=0 BK=0 P=130 Name=identifier L=11 PPK=2 FakeLParens= FakeRParens=0 II=0x21a3298 Text='BAR'
 M=0 C=0 T=Unknown S=0 F=0 B=0 BK=0 P=23 Name=semi L=12 PPK=2 FakeLParens= FakeRParens=0 II=0x0 Text=';'
```

new annotations:
```
AnnotatedTokens(L=0, P=0, T=5, C=0):
 M=0 C=0 T=Unknown S=1 F=0 B=0 BK=0 P=0 Name=int L=3 PPK=2 FakeLParens= FakeRParens=0 II=0x558b37655290 Text='int'
 M=0 C=0 T=Unknown S=1 F=0 B=0 BK=0 P=23 Name=identifier L=7 PPK=2 FakeLParens= FakeRParens=0 II=0x558b3769d410 Text='foo'
 M=0 C=1 T=StartOfName S=1 F=0 B=0 BK=0 P=220 Name=identifier L=11 PPK=2 FakeLParens= FakeRParens=0 II=0x558b3769d440 Text='BAR'
 M=0 C=0 T=Unknown S=0 F=0 B=0 BK=0 P=23 Name=semi L=12 PPK=2 FakeLParens= FakeRParens=0 II=0x0 Text=';'
```


as you can see `foo` is no longer recognized as `StartOfName`, resulting in drastically different penalties for breaking around the token and changing formatting for a lot of existing code. 

clang-format previously recognized these type of trailing attribute macros without any extra user configuration. it's quite obvious in https://github.com/llvm/llvm-project/commit/199fc973ced20016b04ba540cf63a1d4914fa513#diff-3f6f57cda9809a57c5b79e22b4181b3f3aaac7216262d0ef44108f39b0443e9bR8484, you're explicitly adding an attribute macro that wasn't explicitly told before (google style didn't have any pre-configured macros up until efeb546865c233dfa7706ee0316c676de9f69897).

not sure if this was deliberate, but it's resulting in a lot of changes for our codebase and even independent of that not recognizing identifier `foo` as `StartOfName` seem clearly wrong to me. since we're close to release cut can you quickly remedy this regression that has started with efeb546865c233dfa7706ee0316c676de9f69897 and accumulated more commits on top?

https://github.com/llvm/llvm-project/pull/72336


More information about the cfe-commits mailing list