[PATCH] D66647: [clangd] DefineInline action apply logic with fully qualified names

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov added inline comments.


================
Comment at: clang-tools-extra/clangd/refactor/tweaks/DefineInline.cpp:79
+  Token CurToken;
+  while (!CurToken.is(tok::semi)) {
+    if (RawLexer.LexFromRawLexer(CurToken))
----------------
kadircet wrote:
> ilya-biryukov wrote:
> > What are the tokens we expect to see before the semicolon here?
> > It feels safer to just skip comments and whitespace and check the next token is a semicolon.
> > 
> > Any reason to have an actual loop here?
> it has been a long time since our offline discussions around this one. but the idea was could have any attributes before semicolon, these could be macros that will expand to one thing in some platforms and another in a different platform. Therefore we've decided to skip anything until we've found a semicolon.
> 
> I don't think I follow the second part of the question though? What do you mean by "having an actual loop"?
The loop I mentioned is 
```while (!CurToken.is(tok::semi))```
I believe this could be replaced with
```
auto NextTok = Lexer::findNextToken(...);
if (NextTok && NextTok->is(tok::semi))
  ...
```

There are two common attribute applications we should mostly care about: 
```
// Attribute in declaration specifiers.
[[noreturn]] int foo(); 

// Attribute after declarator name
int foo [[noreturn]]();
```
Both work with a simple if statement, rather than the attributes.

The case we're covering with the loop is super uncommon and I don't think we should even bother to support it:
```
int foo() [[some::attribute]];
```


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D66647/new/

https://reviews.llvm.org/D66647