diff --git a/clang-tools-extra/clangd/refactor/tweaks/DefineOutline.cpp b/clang-tools-extra/clangd/refactor/tweaks/DefineOutline.cpp --- a/clang-tools-extra/clangd/refactor/tweaks/DefineOutline.cpp +++ b/clang-tools-extra/clangd/refactor/tweaks/DefineOutline.cpp @@ -239,21 +239,36 @@ DelAttr(FD->getAttr()); DelAttr(FD->getAttr()); - if (FD->isVirtualAsWritten()) { - SourceRange SpecRange{FD->getBeginLoc(), FD->getLocation()}; - bool HasErrors = true; - - // Clang allows duplicating virtual specifiers so check for multiple - // occurances. - for (const auto &Tok : TokBuf.expandedTokens(SpecRange)) { - if (Tok.kind() != tok::kw_virtual) + auto DeleteKeyword = [&](tok::TokenKind Kind, SourceRange FromRange, + bool AllowMultiple = false) { + bool FoundAny = false; + for (const auto &Tok : TokBuf.expandedTokens(FromRange)) { + if (Tok.kind() != Kind) continue; + if (FoundAny && !AllowMultiple) { + Errors = llvm::joinErrors( + std::move(Errors), + llvm::createStringError( + llvm::inconvertibleErrorCode(), + (llvm::StringRef("define outline: can't move outline as " + "multiple `") + + tok::getKeywordSpelling(Kind) + "` keywords exist.") + .str())); + break; + } + FoundAny = true; auto Spelling = TokBuf.spelledForExpanded(llvm::makeArrayRef(Tok)); if (!Spelling) { - HasErrors = true; + Errors = llvm::joinErrors( + std::move(Errors), + llvm::createStringError( + llvm::inconvertibleErrorCode(), + (StringRef( + "define outline: can't move outline as can't remove `") + + tok::getKeywordSpelling(Kind) + "` keyword.") + .str())); break; } - HasErrors = false; CharSourceRange DelRange = syntax::Token::range(SM, Spelling->front(), Spelling->back()) .toCharRange(SM); @@ -261,14 +276,22 @@ DeclarationCleanups.add(tooling::Replacement(SM, DelRange, ""))) Errors = llvm::joinErrors(std::move(Errors), std::move(Err)); } - if (HasErrors) { + if (!FoundAny) Errors = llvm::joinErrors( std::move(Errors), - llvm::createStringError(llvm::inconvertibleErrorCode(), - "define outline: Can't move out of line as " - "function has a macro `virtual` specifier.")); - } - } + llvm::createStringError( + llvm::inconvertibleErrorCode(), + (StringRef( + "define outline: can't move outline as couldn't find `") + + tok::getKeywordSpelling(Kind) + "` keyword to remove.") + .str())); + }; + + if (FD->isVirtualAsWritten()) + DeleteKeyword(tok::kw_virtual, {FD->getBeginLoc(), FD->getLocation()}, + true); + if (isa(FD) && cast(FD)->isStatic()) + DeleteKeyword(tok::kw_static, {FD->getBeginLoc(), FD->getLocation()}); if (Errors) return std::move(Errors); diff --git a/clang-tools-extra/clangd/unittests/TweakTests.cpp b/clang-tools-extra/clangd/unittests/TweakTests.cpp --- a/clang-tools-extra/clangd/unittests/TweakTests.cpp +++ b/clang-tools-extra/clangd/unittests/TweakTests.cpp @@ -2142,6 +2142,17 @@ };)cpp", "void B::foo() {}\n", }, + { + R"cpp( + struct A { + static void fo^o() {} + };)cpp", + R"cpp( + struct A { + static void foo() ; + };)cpp", + " void A::foo() {}\n", + }, }; for (const auto &Case : Cases) { SCOPED_TRACE(Case.Test); @@ -2236,6 +2247,24 @@ STUPID_MACRO(sizeof sizeof int) void foo() ; };)cpp", " void A::foo() {}\n"}, + {R"cpp(#define STAT static + struct A { + STAT void f^oo() {} + };)cpp", + R"cpp(#define STAT static + struct A { + STAT void foo() ; + };)cpp", + " void A::foo() {}\n"}, + {R"cpp(#define STUPID_MACRO(X) static + struct A { + STUPID_MACRO(sizeof sizeof int) void f^oo() {} + };)cpp", + R"cpp(#define STUPID_MACRO(X) static + struct A { + STUPID_MACRO(sizeof sizeof int) void foo() ; + };)cpp", + " void A::foo() {}\n"}, }; for (const auto &Case : Cases) { SCOPED_TRACE(Case.Test); @@ -2360,8 +2389,8 @@ struct A { VIRT fo^o() {} };)cpp", - "fail: define outline: Can't move out of line as function has a " - "macro `virtual` specifier."}, + "fail: define outline: can't move outline as can't remove `virtual` " + "keyword."}, { R"cpp( #define OVERFINAL final override