This revision was automatically updated to reflect the committed changes.
Closed by commit rGb2666ccca027: [clangd] DefineOutline won't copy virtual
specifiers on methods (authored by njames93).
Changed prior to commit:
https://reviews.llvm.org/D75429?vs=247779&id=247828#toc
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D75429/new/
https://reviews.llvm.org/D75429
Files:
clang-tools-extra/clangd/refactor/tweaks/DefineOutline.cpp
clang-tools-extra/clangd/unittests/TweakTests.cpp
Index: clang-tools-extra/clangd/unittests/TweakTests.cpp
===================================================================
--- clang-tools-extra/clangd/unittests/TweakTests.cpp
+++ clang-tools-extra/clangd/unittests/TweakTests.cpp
@@ -2068,6 +2068,80 @@
};)cpp",
"Foo::Foo(int z) __attribute__((weak)) : bar(2){}\n",
},
+ // Virt specifiers.
+ {
+ R"cpp(
+ struct A {
+ virtual void f^oo() {}
+ };)cpp",
+ R"cpp(
+ struct A {
+ virtual void foo() ;
+ };)cpp",
+ " void A::foo() {}\n",
+ },
+ {
+ R"cpp(
+ struct A {
+ virtual virtual void virtual f^oo() {}
+ };)cpp",
+ R"cpp(
+ struct A {
+ virtual virtual void virtual foo() ;
+ };)cpp",
+ " void A::foo() {}\n",
+ },
+ {
+ R"cpp(
+ struct A {
+ virtual void foo() = 0;
+ };
+ struct B : A {
+ void fo^o() override {}
+ };)cpp",
+ R"cpp(
+ struct A {
+ virtual void foo() = 0;
+ };
+ struct B : A {
+ void foo() override ;
+ };)cpp",
+ "void B::foo() {}\n",
+ },
+ {
+ R"cpp(
+ struct A {
+ virtual void foo() = 0;
+ };
+ struct B : A {
+ void fo^o() final {}
+ };)cpp",
+ R"cpp(
+ struct A {
+ virtual void foo() = 0;
+ };
+ struct B : A {
+ void foo() final ;
+ };)cpp",
+ "void B::foo() {}\n",
+ },
+ {
+ R"cpp(
+ struct A {
+ virtual void foo() = 0;
+ };
+ struct B : A {
+ void fo^o() final override {}
+ };)cpp",
+ R"cpp(
+ struct A {
+ virtual void foo() = 0;
+ };
+ struct B : A {
+ void foo() final override ;
+ };)cpp",
+ "void B::foo() {}\n",
+ },
};
for (const auto &Case : Cases) {
SCOPED_TRACE(Case.Test);
@@ -2081,6 +2155,8 @@
llvm::StringMap<std::string> EditedFiles;
ExtraFiles["Test.cpp"] = "";
FileName = "Test.hpp";
+ ExtraArgs.push_back("-DVIRTUAL=virtual");
+ ExtraArgs.push_back("-DOVER=override");
struct {
llvm::StringRef Test;
@@ -2118,6 +2194,48 @@
#define TARGET foo
void TARGET();)cpp",
"void TARGET(){ return; }"},
+ {R"cpp(#define VIRT virtual
+ struct A {
+ VIRT void f^oo() {}
+ };)cpp",
+ R"cpp(#define VIRT virtual
+ struct A {
+ VIRT void foo() ;
+ };)cpp",
+ " void A::foo() {}\n"},
+ {R"cpp(
+ struct A {
+ VIRTUAL void f^oo() {}
+ };)cpp",
+ R"cpp(
+ struct A {
+ VIRTUAL void foo() ;
+ };)cpp",
+ " void A::foo() {}\n"},
+ {R"cpp(
+ struct A {
+ virtual void foo() = 0;
+ };
+ struct B : A {
+ void fo^o() OVER {}
+ };)cpp",
+ R"cpp(
+ struct A {
+ virtual void foo() = 0;
+ };
+ struct B : A {
+ void foo() OVER ;
+ };)cpp",
+ "void B::foo() {}\n"},
+ {R"cpp(#define STUPID_MACRO(X) virtual
+ struct A {
+ STUPID_MACRO(sizeof sizeof int) void f^oo() {}
+ };)cpp",
+ R"cpp(#define STUPID_MACRO(X) virtual
+ struct A {
+ STUPID_MACRO(sizeof sizeof int) void foo() ;
+ };)cpp",
+ " void A::foo() {}\n"},
};
for (const auto &Case : Cases) {
SCOPED_TRACE(Case.Test);
@@ -2229,6 +2347,49 @@
<< Case.TestHeader;
}
}
+
+TEST_F(DefineOutlineTest, FailsMacroSpecifier) {
+ FileName = "Test.hpp";
+ ExtraFiles["Test.cpp"] = "";
+ ExtraArgs.push_back("-DFINALOVER=final override");
+
+ std::pair<StringRef, StringRef> Cases[] = {
+ {
+ R"cpp(
+ #define VIRT virtual void
+ struct A {
+ VIRT fo^o() {}
+ };)cpp",
+ "fail: define outline: Can't move out of line as function has a "
+ "macro `virtual` specifier."},
+ {
+ R"cpp(
+ #define OVERFINAL final override
+ struct A {
+ virtual void foo() {}
+ };
+ struct B : A {
+ void fo^o() OVERFINAL {}
+ };)cpp",
+ "fail: define outline: Can't move out of line as function has a "
+ "macro `override` specifier.\ndefine outline: Can't move out of line "
+ "as function has a macro `final` specifier."},
+ {
+ R"cpp(
+ struct A {
+ virtual void foo() {}
+ };
+ struct B : A {
+ void fo^o() FINALOVER {}
+ };)cpp",
+ "fail: define outline: Can't move out of line as function has a "
+ "macro `override` specifier.\ndefine outline: Can't move out of line "
+ "as function has a macro `final` specifier."},
+ };
+ for (const auto &Case : Cases) {
+ EXPECT_EQ(apply(Case.first), Case.second);
+ }
+}
} // namespace
} // namespace clangd
} // namespace clang
Index: clang-tools-extra/clangd/refactor/tweaks/DefineOutline.cpp
===================================================================
--- clang-tools-extra/clangd/refactor/tweaks/DefineOutline.cpp
+++ clang-tools-extra/clangd/refactor/tweaks/DefineOutline.cpp
@@ -16,6 +16,7 @@
#include "SourceCode.h"
#include "refactor/Tweak.h"
#include "clang/AST/ASTTypeTraits.h"
+#include "clang/AST/Attr.h"
#include "clang/AST/Decl.h"
#include "clang/AST/DeclBase.h"
#include "clang/AST/DeclCXX.h"
@@ -156,7 +157,7 @@
"define outline: couldn't find a context for target");
llvm::Error Errors = llvm::Error::success();
- tooling::Replacements QualifierInsertions;
+ tooling::Replacements DeclarationCleanups;
// Finds the first unqualified name in function return type and name, then
// qualifies those to be valid in TargetContext.
@@ -181,7 +182,7 @@
const NamedDecl *ND = Ref.Targets.front();
const std::string Qualifier = getQualification(
AST, *TargetContext, SM.getLocForStartOfFile(SM.getMainFileID()), ND);
- if (auto Err = QualifierInsertions.add(
+ if (auto Err = DeclarationCleanups.add(
tooling::Replacement(SM, Ref.NameLoc, 0, Qualifier)))
Errors = llvm::joinErrors(std::move(Errors), std::move(Err));
});
@@ -206,14 +207,72 @@
assert(Tok != Tokens.rend());
DelRange.setBegin(Tok->location());
if (auto Err =
- QualifierInsertions.add(tooling::Replacement(SM, DelRange, "")))
+ DeclarationCleanups.add(tooling::Replacement(SM, DelRange, "")))
Errors = llvm::joinErrors(std::move(Errors), std::move(Err));
}
}
+ auto DelAttr = [&](const Attr *A) {
+ if (!A)
+ return;
+ auto AttrTokens =
+ TokBuf.spelledForExpanded(TokBuf.expandedTokens(A->getRange()));
+ assert(A->getLocation().isValid());
+ if (!AttrTokens || AttrTokens->empty()) {
+ Errors = llvm::joinErrors(
+ std::move(Errors),
+ llvm::createStringError(
+ llvm::inconvertibleErrorCode(),
+ llvm::StringRef("define outline: Can't move out of line as "
+ "function has a macro `") +
+ A->getSpelling() + "` specifier."));
+ return;
+ }
+ CharSourceRange DelRange =
+ syntax::Token::range(SM, AttrTokens->front(), AttrTokens->back())
+ .toCharRange(SM);
+ if (auto Err =
+ DeclarationCleanups.add(tooling::Replacement(SM, DelRange, "")))
+ Errors = llvm::joinErrors(std::move(Errors), std::move(Err));
+ };
+
+ DelAttr(FD->getAttr<OverrideAttr>());
+ DelAttr(FD->getAttr<FinalAttr>());
+
+ 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)
+ continue;
+ auto Spelling = TokBuf.spelledForExpanded(llvm::makeArrayRef(Tok));
+ if (!Spelling) {
+ HasErrors = true;
+ break;
+ }
+ HasErrors = false;
+ CharSourceRange DelRange =
+ syntax::Token::range(SM, Spelling->front(), Spelling->back())
+ .toCharRange(SM);
+ if (auto Err =
+ DeclarationCleanups.add(tooling::Replacement(SM, DelRange, "")))
+ Errors = llvm::joinErrors(std::move(Errors), std::move(Err));
+ }
+ if (HasErrors) {
+ 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."));
+ }
+ }
+
if (Errors)
return std::move(Errors);
- return getFunctionSourceAfterReplacements(FD, QualifierInsertions);
+ return getFunctionSourceAfterReplacements(FD, DeclarationCleanups);
}
struct InsertionPoint {
_______________________________________________
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
