kadircet added inline comments.
================
Comment at: clang-tools-extra/clangd/refactor/tweaks/DefineOutline.cpp:232
+ }
+ CharSourceRange DelRange = CharSourceRange::getTokenRange(
+ AttrTokens->front().location(), AttrTokens->back().location());
----------------
let's use `syntax::Token::range(SM, AttrTokens->front(),
AttrTokens->back())...` instead
================
Comment at: clang-tools-extra/clangd/refactor/tweaks/DefineOutline.cpp:254
+ if (!Spelling) {
+ Errors =
+ llvm::joinErrors(std::move(Errors),
----------------
what about setting `Any` to true below this check and just breaking here
instead? e.g.
```
bool Any = false;
for(...) {
....
if (!Spelling) break;
Any = true;
}
if (!Any) appendToErrors
```
================
Comment at: clang-tools-extra/clangd/refactor/tweaks/DefineOutline.cpp:262
+ }
+ assert(Spelling->size() == 1);
+ if (auto Err = DeclarationCleanups.add(tooling::Replacement(
----------------
why assert on this and not delete the whole Spelling that produced the virtual
keyword ?
e.g.
```
#define STUPID_MACRO(X) virtual
struct F {
STUPID_MACRO(BLA BLA) void foo();
}
```
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D75429/new/
https://reviews.llvm.org/D75429
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
