robot updated this revision to Diff 556283.
robot marked 3 inline comments as done.
robot added a comment.
Herald added a project: clang.
WIP - several improvements
I thought it might be a good idea to give an update even though this is
still work in progress.
- Don't print `virtual` for added declarations
- Some rework of the declaration generation
- Add title with preview
- Make tweak only available if it adds anything
- Mostly fix multiple inheritance
- Disable tweak on forward declarations
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D153152/new/
https://reviews.llvm.org/D153152
Files:
clang-tools-extra/clangd/AST.cpp
clang-tools-extra/clangd/AST.h
clang-tools-extra/clangd/Protocol.cpp
clang-tools-extra/clangd/Protocol.h
clang-tools-extra/clangd/refactor/tweaks/CMakeLists.txt
clang-tools-extra/clangd/refactor/tweaks/DeclarePureVirtuals.cpp
clang-tools-extra/clangd/unittests/CMakeLists.txt
clang-tools-extra/clangd/unittests/TestTU.cpp
clang-tools-extra/clangd/unittests/TestTU.h
clang-tools-extra/clangd/unittests/tweaks/DeclarePureVirtualsTests.cpp
clang-tools-extra/clangd/unittests/tweaks/TweakTesting.cpp
clang-tools-extra/clangd/unittests/tweaks/TweakTesting.h
clang/include/clang/AST/CXXInheritance.h
clang/include/clang/AST/PrettyPrinter.h
clang/lib/AST/TypePrinter.cpp
Index: clang/lib/AST/TypePrinter.cpp
===================================================================
--- clang/lib/AST/TypePrinter.cpp
+++ clang/lib/AST/TypePrinter.cpp
@@ -873,13 +873,13 @@
raw_ostream &OS) {
if (T->hasTrailingReturn()) {
OS << "auto ";
- if (!HasEmptyPlaceHolder)
+ if (!HasEmptyPlaceHolder && Policy.ParenthesizeFunctionName)
OS << '(';
} else {
// If needed for precedence reasons, wrap the inner part in grouping parens.
SaveAndRestore PrevPHIsEmpty(HasEmptyPlaceHolder, false);
printBefore(T->getReturnType(), OS);
- if (!PrevPHIsEmpty.get())
+ if (!PrevPHIsEmpty.get() && Policy.ParenthesizeFunctionName)
OS << '(';
}
}
@@ -903,7 +903,7 @@
void TypePrinter::printFunctionProtoAfter(const FunctionProtoType *T,
raw_ostream &OS) {
// If needed for precedence reasons, wrap the inner part in grouping parens.
- if (!HasEmptyPlaceHolder)
+ if (!HasEmptyPlaceHolder && Policy.ParenthesizeFunctionName)
OS << ')';
SaveAndRestore NonEmptyPH(HasEmptyPlaceHolder, false);
Index: clang/include/clang/AST/PrettyPrinter.h
===================================================================
--- clang/include/clang/AST/PrettyPrinter.h
+++ clang/include/clang/AST/PrettyPrinter.h
@@ -77,7 +77,7 @@
PrintCanonicalTypes(false), PrintInjectedClassNameWithArguments(true),
UsePreferredNames(true), AlwaysIncludeTypeForTemplateArgument(false),
CleanUglifiedParameters(false), EntireContentsOfLargeArray(true),
- UseEnumerators(true) {}
+ UseEnumerators(true), ParenthesizeFunctionName(true) {}
/// Adjust this printing policy for cases where it's known that we're
/// printing C++ code (for instance, if AST dumping reaches a C++-only
@@ -304,6 +304,21 @@
/// enumerator name or via cast of an integer.
unsigned UseEnumerators : 1;
+ /// Whether to wrap function names in function types in parentheses.
+ ///
+ /// This flag determines whether function types will printed with parentheses surrounding the name:
+ ///
+ /// \code
+ /// void (funcName)(int);
+ /// \endcode
+ ///
+ /// or without parentheses:
+ ///
+ /// \code
+ /// void funcName(int);
+ /// \endcode
+ unsigned ParenthesizeFunctionName : 1;
+
/// Callbacks to use to allow the behavior of printing to be customized.
const PrintingCallbacks *Callbacks = nullptr;
};
Index: clang/include/clang/AST/CXXInheritance.h
===================================================================
--- clang/include/clang/AST/CXXInheritance.h
+++ clang/include/clang/AST/CXXInheritance.h
@@ -303,57 +303,78 @@
void replaceAll(UniqueVirtualMethod Overriding);
};
-/// A mapping from each virtual member function to its set of
-/// final overriders.
+/// A mapping from each virtual member function to its set of final overriders.
///
-/// Within a class hierarchy for a given derived class, each virtual
-/// member function in that hierarchy has one or more "final
-/// overriders" (C++ [class.virtual]p2). A final overrider for a
-/// virtual function "f" is the virtual function that will actually be
-/// invoked when dispatching a call to "f" through the
-/// vtable. Well-formed classes have a single final overrider for each
-/// virtual function; in abstract classes, the final overrider for at
-/// least one virtual function is a pure virtual function. Due to
-/// multiple, virtual inheritance, it is possible for a class to have
-/// more than one final overrider. Although this is an error (per C++
-/// [class.virtual]p2), it is not considered an error here: the final
-/// overrider map can represent multiple final overriders for a
-/// method, and it is up to the client to determine whether they are
-/// problem. For example, the following class \c D has two final
-/// overriders for the virtual function \c A::f(), one in \c C and one
-/// in \c D:
+/// Within a class hierarchy for a given derived class, each virtual member
+/// function in that hierarchy has one or more "final overriders" (C++
+/// [class.virtual]p2). A final overrider for a virtual function "f" is the
+/// virtual function that will actually be invoked when dispatching a call to
+/// "f" through the vtable. In abstract classes, the final overrider for at
+/// least one virtual function is a pure virtual function. Well-formed classes
+/// have a single final overrider for each virtual function per base class
+/// subobject. A virtual function can have multiple final overriders in
+/// different base class subobjects. This makes "final overrider" a property of
+/// a base class subobject; in other words, a function "f" is a "final
+/// overrider" for a specific base class subobject:
///
/// \code
/// struct A { virtual void f(); };
-/// struct B : virtual A { virtual void f(); };
-/// struct C : virtual A { virtual void f(); };
+/// struct B : A { virtual void f(); };
+/// struct C : A { virtual void f(); };
/// struct D : B, C { };
+/// // In D, B::f and C::f are final overriders of A::f
/// \endcode
///
-/// This data structure contains a mapping from every virtual
-/// function *that does not override an existing virtual function* and
-/// in every subobject where that virtual function occurs to the set
-/// of virtual functions that override it. Thus, the same virtual
-/// function \c A::f can actually occur in multiple subobjects of type
-/// \c A due to multiple inheritance, and may be overridden by
-/// different virtual functions in each, as in the following example:
+/// Therefore, the first level of this data structure maps from virtual
+/// functions to a list of pairs \c OverridingMethods := (subobject id,
+/// overriders). Subobject id 0 represents the virtual base class subobject of
+/// that type, while subobject numbers greater than 0 refer to non-virtual base
+/// class subobjects of that type.
+/// For the example above, we map \c A::f to the list
+/// [(1, [\c B::f]), (2, [\c C::f])].
+///
+/// The domain of the map is the set of all virtual member functions
+/// declarations of all base class subobjects and the class itself. The order of
+/// the MapVector is the depth-first, post-order traversal of all base classes
+/// and within each base class, the declaration order of the virtual functions.
+/// If a base class appears via multiple subobjects, the order for the methods
+/// it generates for the domain are from the first occurrence in the traversal
+/// order. For the example above:
+/// - \c A::f -> [(1, [\c B::f]), (2, [\c C::f])]
+/// - \c B::f -> [(1, [\c B::f])]
+/// - \c C::f -> [(1, [\c C::f])]
+/// Note that a final overrider like \c C::f can appear multiple times in the
+/// codomain of the map. Further note that the \c A subobject appears via two
+/// different base classes (D->B->A and D->C->A), and its methods are listed
+/// first since D->B->A is before D->C->A in the traversal order, and D->B->A is
+/// the first class we visit post-order at all (D->B->A, then D->B,
+/// then D->C->A, then D->C).
+///
+/// Due to multiple, virtual inheritance, it is possible for a class to have
+/// more than one final overrider per base class suboject. Although this is an
+/// error (per C++ [class.virtual]p2), we will still represent multiple final
+/// overriders per subobject in the final overrider map: it is up to the client
+/// to determine whether they are a problem. For example, the following class \c
+/// D has two final overriders for the virtual function \c A::f() in the same
+/// subobject \c A, one overrider in \c C and one overrider in \c D:
///
/// \code
/// struct A { virtual void f(); };
-/// struct B : A { virtual void f(); };
-/// struct C : A { virtual void f(); };
+/// struct B : virtual A { virtual void f(); };
+/// struct C : virtual A { virtual void f(); };
/// struct D : B, C { };
/// \endcode
///
-/// Unlike in the previous example, where the virtual functions \c
-/// B::f and \c C::f both overrode \c A::f in the same subobject of
-/// type \c A, in this example the two virtual functions both override
-/// \c A::f but in *different* subobjects of type A. This is
-/// represented by numbering the subobjects in which the overridden
-/// and the overriding virtual member functions are located. Subobject
-/// 0 represents the virtual base class subobject of that type, while
-/// subobject numbers greater than 0 refer to non-virtual base class
-/// subobjects of that type.
+/// Note that D->B->A and D->C->A are the same subobject due to the virtual
+/// inheritance.
+///
+/// Thus, the overriders in the \c OverridingMethods are a list of
+/// \c UniqueVirtualMethod, each of which represents a final overrider. For the
+/// example above, we map \c A::f to the list [(0, [\c B::f, \c C::f])].
+///
+/// Combined, we map in general:
+/// virtual function -> [(subobject id, [final overriders in that subobject]),
+/// ...]
class CXXFinalOverriderMap
: public llvm::MapVector<const CXXMethodDecl *, OverridingMethods> {};
Index: clang-tools-extra/clangd/unittests/tweaks/TweakTesting.h
===================================================================
--- clang-tools-extra/clangd/unittests/tweaks/TweakTesting.h
+++ clang-tools-extra/clangd/unittests/tweaks/TweakTesting.h
@@ -91,9 +91,15 @@
std::string apply(llvm::StringRef MarkedCode,
llvm::StringMap<std::string> *EditedFiles = nullptr) const;
+ // TODO: doc
+ // TODO: what if prepare fails?
+ std::string title(llvm::StringRef MarkedCode) const;
+
// Helpers for EXPECT_AVAILABLE/EXPECT_UNAVAILABLE macros.
using WrappedAST = std::pair<ParsedAST, /*WrappingOffset*/ unsigned>;
WrappedAST build(llvm::StringRef) const;
+ // TODO: remove
+ WrappedAST buildErr(llvm::StringRef) const;
bool isAvailable(WrappedAST &, llvm::Annotations::Range) const;
// Return code re-decorated with a single point/range.
static std::string decorate(llvm::StringRef, unsigned);
Index: clang-tools-extra/clangd/unittests/tweaks/TweakTesting.cpp
===================================================================
--- clang-tools-extra/clangd/unittests/tweaks/TweakTesting.cpp
+++ clang-tools-extra/clangd/unittests/tweaks/TweakTesting.cpp
@@ -8,10 +8,13 @@
#include "TweakTesting.h"
+#include "ParsedAST.h"
#include "SourceCode.h"
#include "TestTU.h"
#include "refactor/Tweak.h"
#include "llvm/Support/Error.h"
+#include "llvm/Support/VirtualFileSystem.h"
+#include "llvm/Testing/Annotations/Annotations.h"
#include "gmock/gmock.h"
#include "gtest/gtest.h"
#include <optional>
@@ -80,6 +83,26 @@
return Result;
}
+// TODO: doc
+std::optional<std::string>
+getTweakTitle(ParsedAST &AST, llvm::Annotations::Range Range, StringRef TweakID,
+ const SymbolIndex *Index, llvm::vfs::FileSystem *FS) {
+ std::string Result;
+ SelectionTree::createEach(AST.getASTContext(), AST.getTokens(), Range.Begin,
+ Range.End, [&](SelectionTree ST) {
+ Tweak::Selection S(Index, AST, Range.Begin,
+ Range.End, std::move(ST), FS);
+ if (auto T = prepareTweak(TweakID, S, nullptr)) {
+ Result = (*T)->title();
+ return true;
+ } else {
+ llvm::consumeError(T.takeError());
+ return false;
+ }
+ });
+ return Result;
+}
+
} // namespace
std::string TweakTest::apply(llvm::StringRef MarkedCode,
@@ -126,6 +149,26 @@
return EditedMainFile;
}
+std::string TweakTest::title(llvm::StringRef MarkedCode) const {
+ // TODO: code dup
+ std::string WrappedCode = wrap(Context, MarkedCode);
+ llvm::Annotations Input(WrappedCode);
+ TestTU TU;
+ TU.Filename = std::string(FileName);
+ TU.HeaderCode = Header;
+ TU.AdditionalFiles = std::move(ExtraFiles);
+ TU.Code = std::string(Input.code());
+ TU.ExtraArgs = ExtraArgs;
+ ParsedAST AST = TU.build();
+
+ auto Result = getTweakTitle(
+ AST, rangeOrPoint(Input), TweakID, Index.get(),
+ &AST.getSourceManager().getFileManager().getVirtualFileSystem());
+ if (!Result)
+ return "unavailable";
+ return *Result;
+}
+
bool TweakTest::isAvailable(WrappedAST &AST,
llvm::Annotations::Range Range) const {
// Adjust range for wrapping offset.
@@ -150,6 +193,17 @@
return {TU.build(), wrapping(Context).first.size()};
}
+// TODO: remove
+TweakTest::WrappedAST TweakTest::buildErr(llvm::StringRef Code) const {
+ TestTU TU;
+ TU.Filename = std::string(FileName);
+ TU.HeaderCode = Header;
+ TU.Code = wrap(Context, Code);
+ TU.ExtraArgs = ExtraArgs;
+ TU.AdditionalFiles = std::move(ExtraFiles);
+ return {TU.buildErr(), wrapping(Context).first.size()};
+}
+
std::string TweakTest::decorate(llvm::StringRef Code, unsigned Point) {
return (Code.substr(0, Point) + "^" + Code.substr(Point)).str();
}
Index: clang-tools-extra/clangd/unittests/tweaks/DeclarePureVirtualsTests.cpp
===================================================================
--- /dev/null
+++ clang-tools-extra/clangd/unittests/tweaks/DeclarePureVirtualsTests.cpp
@@ -0,0 +1,978 @@
+//===-- DeclarePureVirtualsTests.cpp ----------------------------*- C++ -*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+#include "ParsedAST.h"
+#include "TestTU.h"
+#include "TweakTesting.h"
+#include "clang/AST/CXXInheritance.h"
+#include "clang/AST/DeclCXX.h"
+#include "llvm/ADT/ArrayRef.h"
+#include "gtest/gtest.h"
+
+namespace clang {
+namespace clangd {
+namespace {
+
+TWEAK_TEST(DeclarePureVirtuals);
+
+// TODO: remove and undo changes
+#if 0
+
+void printer(llvm::ArrayRef<const Decl*> topLevelDecls) {
+ for (const auto *delc : topLevelDecls) {
+ if (const CXXRecordDecl *rc = dyn_cast<CXXRecordDecl>(delc)) {
+ if (rc->getName() == "C") {
+ CXXFinalOverriderMap FinalOverriders;
+ rc->getFinalOverriders(FinalOverriders);
+
+ for (const auto &bar : FinalOverriders) { // TODO: see line 44
+ // const CXXMethodDecl *Method = bar.first;
+ llvm::outs() << bar.first->getQualifiedNameAsString() << ", " << bar.second.size() << "\n";
+ const OverridingMethods &Overrides = bar.second;
+
+ for (const std::pair<unsigned int,
+ llvm::SmallVector<UniqueVirtualMethod, 4>>
+ &meow : // TODO
+ Overrides) {
+ llvm::outs() << " " << meow.first << ", " << meow.second.size() << "\n";
+ const auto &IdontKnowWhatThisIs = meow.second; // TODO
+ for (const UniqueVirtualMethod &Override : IdontKnowWhatThisIs) {
+ llvm::outs() << " "
+ << Override.Method->getQualifiedNameAsString()
+ << ", " << Override.Subobject << ", " << Override.InVirtualSubobject
+ << "\n";
+ }
+ }
+ }
+ }
+ }
+ }
+}
+
+TEST_F(DeclarePureVirtualsTest, Error) {
+ auto foo = [this](const char* txt) {
+ const auto p = buildErr(txt);
+ llvm::outs() << txt << "\n";
+ printer(p.first.getLocalTopLevelDecls());
+ llvm::outs() << "\n======================\n";
+ };
+
+ foo(R"cpp(
+class MyBase { virtual void foo() = 0; };
+class C : MyBase {};
+ )cpp");
+
+ foo(R"cpp(
+class MyBase { virtual void foo() = 0; };
+class C : MyBase { void foo() override; };
+ )cpp");
+
+ foo(R"cpp(
+class MyBase { virtual void foo() = 0; };
+class B : MyBase { void foo() override; };
+class C : MyBase {};
+ )cpp");
+
+ foo(R"cpp(
+class MyBase { virtual void foo() = 0; };
+class B : MyBase {};
+class C : MyBase { void foo() override; };
+ )cpp");
+
+ foo(R"cpp(
+class MyBase { virtual void foo() = 0; };
+class B : MyBase { void foo() override; };
+class C : MyBase { void foo() override; };
+ )cpp");
+
+ foo(R"cpp(
+class MyBase { virtual void foo() = 0; };
+class A : MyBase { void foo() override; };
+class B : MyBase { void foo() override; };
+class C : A, B {};
+ )cpp");
+
+ foo(R"cpp(
+class A : { virtual void foo(); };
+class B : { virtual void bar(); };
+class C : A, B {};
+ )cpp");
+
+ foo(R"cpp(
+class A : { virtual void foo(); };
+class C : virtual A {};
+ )cpp");
+
+ foo(R"cpp(
+class A : { virtual void foo(); };
+class B : { virtual void bar(); };
+class C : virtual A, virtual B {};
+ )cpp");
+
+ foo(R"cpp(
+class A : { virtual void foo(); };
+class B : { virtual void bar(); };
+class C : A, B { void foo() override; void bar() override; };
+ )cpp");
+
+ foo(R"cpp(
+ class MyBase {
+ virtual void myFunction() = 0;
+ };
+
+ class A : virtual MyBase {
+ void myFunction() override;
+ };
+
+ class B : virtual MyBase {
+ void myFunction() override;
+ };
+
+ class C : A, B {};
+ )cpp");
+
+ foo(R"cpp(
+class A : { virtual void foo() = 0; };
+class B : A { virtual void foo() = 0; };
+class C : B { virtual void foo() = 0; };
+ )cpp");
+
+ foo(R"cpp(
+class MyBase {
+ virtual void myFunction() = 0;
+};
+
+class C : MyBase {
+ void myFunction() override;
+};
+ )cpp"
+);
+}
+
+#else
+
+TEST_F(DeclarePureVirtualsTest, AvailabilityTriggerOnBaseSpecifier) {
+ EXPECT_AVAILABLE(R"cpp(
+ class MyBase {
+ virtual void myFunction() = 0;
+ };
+
+ class MyDerived : ^p^u^b^l^i^c^ ^M^y^B^a^s^e {
+ };
+
+ class MyDerived2 : ^M^y^B^a^s^e {
+ };
+ )cpp");
+}
+
+TEST_F(DeclarePureVirtualsTest, AvailabilityTriggerOnClass) {
+ EXPECT_AVAILABLE(R"cpp(
+ class MyBase {
+ virtual void myFunction() = 0;
+ };
+
+ class ^M^y^D^e^r^i^v^e^d: public MyBase {^
+ // but not here, see AvailabilityTriggerInsideClass
+ ^};
+ )cpp");
+}
+
+TEST_F(DeclarePureVirtualsTest, AvailabilityTriggerInsideClass) {
+ // TODO: this should actually be available but I don't know how to implement
+ // it: the common node of the selection returns the TU, so I get no
+ // information about which class we're in.
+ EXPECT_UNAVAILABLE(R"cpp(
+ class MyBase {
+ virtual void myFunction() = 0;
+ };
+
+ class MyDerived : public MyBase {
+ ^
+ };
+ )cpp");
+}
+
+TEST_F(DeclarePureVirtualsTest, UnavailabilityNoBases) {
+ EXPECT_UNAVAILABLE(R"cpp(
+ class ^N^o^D^e^r^i^v^e^d^ ^{^
+ ^};
+ )cpp");
+}
+
+// TODO: should the tweak available if there are no pure virtual functions and
+// do nothing? or should it be unavailable?
+TEST_F(DeclarePureVirtualsTest, UnavailabilityNoVirtuals) {
+ EXPECT_UNAVAILABLE(R"cpp(
+ class MyBase {
+ void myFunction();
+ };
+
+ class MyDerived : public MyBase {
+ ^
+ };
+ )cpp");
+}
+
+TEST_F(DeclarePureVirtualsTest, UnavailabilityNoPureVirtuals) {
+ EXPECT_UNAVAILABLE(R"cpp(
+ class MyBase {
+ virtual void myFunction();
+ };
+
+ class MyDerived : public MyBase {
+ ^
+ };
+ )cpp");
+}
+
+TEST_F(DeclarePureVirtualsTest, UnavailabilityNoVirtualsInSelectedBase) {
+ EXPECT_UNAVAILABLE(R"cpp(
+ class MyBase {
+ void myFunction();
+ };
+
+ class MyOtherBase {
+ virtual void otherFunction() = 0;
+ };
+
+ class MyDerived : ^p^u^b^l^i^c^ ^M^y^B^a^s^e, MyOtherBase {
+ };
+ )cpp");
+}
+
+TEST_F(DeclarePureVirtualsTest, UnavailabilityOnForwardDecls) {
+ EXPECT_UNAVAILABLE(R"cpp(
+ class ^M^y^C^l^a^s^s^;
+ )cpp");
+}
+
+TEST_F(DeclarePureVirtualsTest, SinglePureVirtualFunction) {
+ const char *Test = R"cpp(
+class MyBase {
+ virtual void myFunction() = 0;
+};
+
+class MyDerived : pub^lic MyBase {
+};
+ )cpp";
+
+ const char *Expected = R"cpp(
+class MyBase {
+ virtual void myFunction() = 0;
+};
+
+class MyDerived : public MyBase {
+void myFunction() override;
+};
+ )cpp";
+
+ EXPECT_EQ(apply(Test), Expected);
+}
+
+TEST_F(DeclarePureVirtualsTest, MultipleInheritanceFirstClass) {
+ const char *Test = R"cpp(
+class MyBase1 {
+ virtual void myFunction1() = 0;
+};
+
+class MyBase2 {
+ virtual void myFunction2() = 0;
+};
+
+class MyDerived : pub^lic MyBase1, public MyBase2 {
+};
+ )cpp";
+
+ // TODO: use qualified name to refer to function to show base class; maybe use
+ // original declaration?
+ const char *ExpectedTitle = R"cpp(Override pure virtual function:
+void myFunction1() override;
+)cpp";
+
+ EXPECT_EQ(title(Test), ExpectedTitle);
+
+ const char *Expected = R"cpp(
+class MyBase1 {
+ virtual void myFunction1() = 0;
+};
+
+class MyBase2 {
+ virtual void myFunction2() = 0;
+};
+
+class MyDerived : public MyBase1, public MyBase2 {
+void myFunction1() override;
+};
+ )cpp";
+
+ EXPECT_EQ(apply(Test), Expected);
+}
+
+TEST_F(DeclarePureVirtualsTest, MultipleInheritanceSecondClass) {
+ const char *Test = R"cpp(
+class MyBase1 {
+ virtual void myFunction1() = 0;
+};
+
+class MyBase2 {
+ virtual void myFunction2() = 0;
+};
+
+class MyDerived : public MyBase1, pub^lic MyBase2 {
+};
+ )cpp";
+
+ const char *Expected = R"cpp(
+class MyBase1 {
+ virtual void myFunction1() = 0;
+};
+
+class MyBase2 {
+ virtual void myFunction2() = 0;
+};
+
+class MyDerived : public MyBase1, public MyBase2 {
+void myFunction2() override;
+};
+ )cpp";
+
+ EXPECT_EQ(apply(Test), Expected);
+}
+
+TEST_F(DeclarePureVirtualsTest, SingleInheritanceMultiplePureVirtualFunctions) {
+ const char *Test = R"cpp(
+class MyBase {
+ virtual void myFunction1() = 0;
+ virtual void myFunction2() = 0;
+};
+
+class MyDerived : pub^lic MyBase {
+};
+ )cpp";
+
+ const char *Expected = R"cpp(
+class MyBase {
+ virtual void myFunction1() = 0;
+ virtual void myFunction2() = 0;
+};
+
+class MyDerived : public MyBase {
+void myFunction1() override;
+void myFunction2() override;
+};
+ )cpp";
+
+ EXPECT_EQ(apply(Test), Expected);
+}
+
+TEST_F(DeclarePureVirtualsTest, SingleInheritanceMixedVirtualFunctions) {
+ const char *Test = R"cpp(
+class MyBase {
+ virtual void myFunction1() = 0;
+ virtual void myFunction2();
+};
+
+class MyDerived : pub^lic MyBase {
+};
+ )cpp";
+
+ const char *Expected = R"cpp(
+class MyBase {
+ virtual void myFunction1() = 0;
+ virtual void myFunction2();
+};
+
+class MyDerived : public MyBase {
+void myFunction1() override;
+};
+ )cpp";
+
+ EXPECT_EQ(apply(Test), Expected);
+}
+
+TEST_F(DeclarePureVirtualsTest,
+ TwoLevelsInheritanceOnePureVirtualFunctionInTopBase) {
+ const char *Test = R"cpp(
+class MyBase {
+ virtual void myFunction1() = 0;
+};
+
+class MyIntermediate : public MyBase {
+};
+
+class MyDerived : pub^lic MyIntermediate {
+};
+ )cpp";
+
+ const char *Expected = R"cpp(
+class MyBase {
+ virtual void myFunction1() = 0;
+};
+
+class MyIntermediate : public MyBase {
+};
+
+class MyDerived : public MyIntermediate {
+void myFunction1() override;
+};
+ )cpp";
+
+ EXPECT_EQ(apply(Test), Expected);
+}
+
+TEST_F(DeclarePureVirtualsTest,
+ TwoLevelsInheritancePureVirtualFunctionsInBothBases) {
+ const char *Test = R"cpp(
+class MyBase {
+ virtual void myFunction1() = 0;
+};
+
+class MyIntermediate : public MyBase {
+ virtual void myFunction2() = 0;
+};
+
+class MyDerived : pub^lic MyIntermediate {
+};
+ )cpp";
+
+ const char *Expected = R"cpp(
+class MyBase {
+ virtual void myFunction1() = 0;
+};
+
+class MyIntermediate : public MyBase {
+ virtual void myFunction2() = 0;
+};
+
+class MyDerived : public MyIntermediate {
+void myFunction1() override;
+void myFunction2() override;
+};
+ )cpp";
+
+ EXPECT_EQ(apply(Test), Expected);
+}
+
+TEST_F(DeclarePureVirtualsTest,
+ TwoLevelInheritanceFunctionNoLongerPureVirtual) {
+ const char *Test = R"cpp(
+class MyBase {
+ virtual void myFunction1() = 0;
+};
+
+class MyIntermediate : public MyBase {
+ void myFunction1() override {}
+};
+
+class MyDerived : pub^lic MyIntermediate {
+};
+ )cpp";
+
+ EXPECT_UNAVAILABLE(Test);
+}
+
+TEST_F(DeclarePureVirtualsTest, VirtualFunctionWithDefaultParameters) {
+ const char *Test = R"cpp(
+class MyBase {
+ virtual void myFunction(int x, int y = 42) = 0;
+};
+
+class MyDerived : pub^lic MyBase {
+};
+ )cpp";
+
+ const char *Expected = R"cpp(
+class MyBase {
+ virtual void myFunction(int x, int y = 42) = 0;
+};
+
+class MyDerived : public MyBase {
+void myFunction(int x, int y = 42) override;
+};
+ )cpp";
+
+ EXPECT_EQ(apply(Test), Expected);
+}
+
+TEST_F(DeclarePureVirtualsTest, FunctionOverloading) {
+ const char *Test = R"cpp(
+class MyBase {
+ virtual void myFunction(int x) = 0;
+ virtual void myFunction(float x) = 0;
+};
+
+class MyDerived : pub^lic MyBase {
+};
+ )cpp";
+
+ const char *Expected = R"cpp(
+class MyBase {
+ virtual void myFunction(int x) = 0;
+ virtual void myFunction(float x) = 0;
+};
+
+class MyDerived : public MyBase {
+void myFunction(int x) override;
+void myFunction(float x) override;
+};
+ )cpp";
+
+ EXPECT_EQ(apply(Test), Expected);
+}
+
+TEST_F(DeclarePureVirtualsTest, OverrideAlreadyExisting) {
+ const char *Test = R"cpp(
+class MyBase2 {
+ virtual void foo() = 0;
+};
+
+class C : My^Base2 {
+ void foo() override;
+};
+ )cpp";
+
+ EXPECT_UNAVAILABLE(Test);
+}
+
+TEST_F(DeclarePureVirtualsTest, PureVirtualRedeclarationAlreadyExisting) {
+ const char *Test = R"cpp(
+class MyBase1 {
+ virtual void foo() = 0;
+};
+
+class MyBase2 : MyBase1 {
+ virtual void foo() = 0;
+};
+
+class C : My^Base2 {
+ virtual void foo() = 0;
+};
+ )cpp";
+
+ EXPECT_UNAVAILABLE(Test);
+}
+
+TEST_F(DeclarePureVirtualsTest, TwoOverloadsOnlyOnePureVirtual) {
+ const char *Test = R"cpp(
+class MyBase {
+ virtual void myFunction(int x) = 0;
+ virtual void myFunction(float x);
+};
+
+class MyDerived : pub^lic MyBase {
+};
+ )cpp";
+
+ const char *Expected = R"cpp(
+class MyBase {
+ virtual void myFunction(int x) = 0;
+ virtual void myFunction(float x);
+};
+
+class MyDerived : public MyBase {
+void myFunction(int x) override;
+};
+ )cpp";
+
+ EXPECT_EQ(apply(Test), Expected);
+}
+
+TEST_F(DeclarePureVirtualsTest, DerivedClassHasNonOverridingFunction) {
+ const char *Test = R"cpp(
+class MyBase {
+ virtual void myFunction(int x) = 0;
+};
+
+class MyDerived : pub^lic MyBase {
+ void myFunction(float x);
+};
+ )cpp";
+
+ const char *Expected = R"cpp(
+class MyBase {
+ virtual void myFunction(int x) = 0;
+};
+
+class MyDerived : public MyBase {
+ void myFunction(float x);
+void myFunction(int x) override;
+};
+ )cpp";
+
+ EXPECT_EQ(apply(Test), Expected);
+}
+
+TEST_F(DeclarePureVirtualsTest, PureVirtualFunctionWithNoexcept) {
+ const char *Test = R"cpp(
+class MyBase {
+ virtual void myFunction() noexcept = 0;
+};
+
+class MyDerived : pub^lic MyBase {
+};
+ )cpp";
+
+ const char *Expected = R"cpp(
+class MyBase {
+ virtual void myFunction() noexcept = 0;
+};
+
+class MyDerived : public MyBase {
+void myFunction() noexcept override;
+};
+ )cpp";
+
+ EXPECT_EQ(apply(Test), Expected);
+}
+
+TEST_F(DeclarePureVirtualsTest, MixOfVirtualAndNonVirtualMemberFunctions) {
+ const char *Test = R"cpp(
+class MyBase {
+ virtual void myFunction1() = 0;
+ void myFunction2();
+};
+
+class MyDerived : pub^lic MyBase {
+};
+ )cpp";
+
+ const char *Expected = R"cpp(
+class MyBase {
+ virtual void myFunction1() = 0;
+ void myFunction2();
+};
+
+class MyDerived : public MyBase {
+void myFunction1() override;
+};
+ )cpp";
+
+ EXPECT_EQ(apply(Test), Expected);
+}
+
+TEST_F(DeclarePureVirtualsTest, PureVirtualFunctionWithBody) {
+ const char *Test = R"cpp(
+class MyBase {
+ virtual void myFunction1() = 0;
+};
+
+void MyBase::myFunction1() {
+}
+
+class MyDerived : pub^lic MyBase {
+};
+ )cpp";
+
+ const char *Expected = R"cpp(
+class MyBase {
+ virtual void myFunction1() = 0;
+};
+
+void MyBase::myFunction1() {
+}
+
+class MyDerived : public MyBase {
+void myFunction1() override;
+};
+ )cpp";
+
+ EXPECT_EQ(apply(Test), Expected);
+}
+
+TEST_F(DeclarePureVirtualsTest, TraversalOrder) {
+ const char *Test = R"cpp(
+class MyBase0 {
+ virtual void myFunction0() = 0;
+};
+
+class MyBase1 {
+ virtual void myFunction1() = 0;
+};
+
+class MyBase2 : MyBase0, MyBase1 {
+ virtual void myFunction2() = 0;
+};
+
+class MyBase3 {
+ virtual void myFunction3() = 0;
+};
+
+class MyBase4 : MyBase2, MyBase3 {
+ virtual void myFunction4() = 0;
+};
+
+class MyDerived : pub^lic MyBase4 {
+};
+ )cpp";
+
+ const char *Expected = R"cpp(
+class MyBase0 {
+ virtual void myFunction0() = 0;
+};
+
+class MyBase1 {
+ virtual void myFunction1() = 0;
+};
+
+class MyBase2 : MyBase0, MyBase1 {
+ virtual void myFunction2() = 0;
+};
+
+class MyBase3 {
+ virtual void myFunction3() = 0;
+};
+
+class MyBase4 : MyBase2, MyBase3 {
+ virtual void myFunction4() = 0;
+};
+
+class MyDerived : public MyBase4 {
+void myFunction0() override;
+void myFunction1() override;
+void myFunction2() override;
+void myFunction3() override;
+void myFunction4() override;
+};
+ )cpp";
+
+ EXPECT_EQ(apply(Test), Expected);
+}
+
+TEST_F(DeclarePureVirtualsTest, AllBaseClassSpecifiers) {
+ const char *Test = R"cpp(
+class MyBase0 {
+ virtual void myFunction0() = 0;
+};
+
+class MyBase1 {
+ virtual void myFunction1() = 0;
+};
+
+class My^Derived : MyBase0, MyBase1 {
+};
+ )cpp";
+
+ const char *Expected = R"cpp(
+class MyBase0 {
+ virtual void myFunction0() = 0;
+};
+
+class MyBase1 {
+ virtual void myFunction1() = 0;
+};
+
+class MyDerived : MyBase0, MyBase1 {
+void myFunction0() override;
+void myFunction1() override;
+};
+ )cpp";
+
+ EXPECT_EQ(apply(Test), Expected);
+}
+
+// TODO: I'd like to test the `[[noreturn]]` form as well but there doesn't seem
+// to be a way at the moment to escape the `[[` which is interpreted by
+// `apply()` as selection.
+// TODO: implement attribute removal
+//TEST_F(DeclarePureVirtualsTest, Attributes) {
+// const char *Test = R"cpp(
+//class MyBase {
+// __attribute__((noreturn)) virtual void myFunction() = 0;
+//};
+//
+//class MyDerived : pub^lic MyBase {
+//};
+// )cpp";
+//
+// // I don't think attributes should be copied. They're not required to
+// // override, and whether they semantically belong there depends on the
+// // particular attribute - we can leave this up to the user.
+// const char *Expected = R"cpp(
+//class MyBase {
+// __attribute__((noreturn)) virtual void myFunction() = 0;
+//};
+//
+//class MyDerived : public MyBase {
+//void myFunction() override;
+//};
+// )cpp";
+//
+// EXPECT_EQ(apply(Test), Expected);
+//}
+
+TEST_F(DeclarePureVirtualsTest, NoWhitespaceBeforePureVirtSpecifier) {
+ const char *Test = R"cpp(
+class MyBase {
+ virtual void myFunction()= 0;
+};
+
+class MyDerived : pub^lic MyBase {
+};
+ )cpp";
+
+ const char *Expected = R"cpp(
+class MyBase {
+ virtual void myFunction()= 0;
+};
+
+class MyDerived : public MyBase {
+void myFunction() override;
+};
+ )cpp";
+
+ EXPECT_EQ(apply(Test), Expected);
+}
+
+TEST_F(DeclarePureVirtualsTest, MultiplePureVirtualsInDifferentSubobjects) {
+ const char *Test = R"cpp(
+class MyBase {
+ virtual void myFunction() = 0;
+};
+
+class A : MyBase {
+ virtual void myFunction() = 0;
+};
+
+class B : MyBase {
+ virtual void myFunction() = 0;
+};
+
+class My^Derived : A, B {
+};
+ )cpp";
+
+ const char *Expected = R"cpp(
+class MyBase {
+ virtual void myFunction() = 0;
+};
+
+class A : MyBase {
+ virtual void myFunction() = 0;
+};
+
+class B : MyBase {
+ virtual void myFunction() = 0;
+};
+
+class MyDerived : A, B {
+void myFunction() override;
+};
+ )cpp";
+
+ EXPECT_EQ(apply(Test), Expected);
+}
+
+// TODO: it's not clear how to identify that these two functions A::myFunction, B::myFunction will be overridden by the same function in MyDerived.
+TEST_F(DeclarePureVirtualsTest, MultiplePureVirtualsOfSameNameButSeparateBases) {
+ const char *Test = R"cpp(
+class A {
+ virtual void myFunction() = 0;
+};
+
+class B {
+ virtual void myFunction() = 0;
+};
+
+class My^Derived : A, B {
+};
+ )cpp";
+
+ const char *Expected = R"cpp(
+class A {
+ virtual void myFunction() = 0;
+};
+
+class B {
+ virtual void myFunction() = 0;
+};
+
+class MyDerived : A, B {
+ void myFunction() override;
+};
+ )cpp";
+
+ EXPECT_EQ(apply(Test), Expected);
+}
+
+// This test should fail since MyBase is incomplete. No idea how to test that.
+// TEST_F(DeclarePureVirtualsTest, IncompleteBassClass) {
+// const char *Test = R"cpp(
+// class MyBase;
+//
+// class MyDerived : My^Base {
+// };
+// )cpp";
+//
+// const char *Expected = R"cpp(
+// class MyBase;
+//
+// class MyDerived : MyBase {
+// };
+// )cpp";
+//
+// EXPECT_EQ(apply(Test), Expected);
+// }
+
+// This test should fail since MyBase is incomplete. No idea how to test that.
+// This is a different failure mode since it probably fails to apply but not to
+// prepare.
+// TEST_F(DeclarePureVirtualsTest, IncompleteBassClass) {
+// const char *Test = R"cpp(
+// class MyBase;
+//
+// class My^Derived : MyBase {
+// };
+// )cpp";
+//
+// const char *Expected = R"cpp(
+// class MyBase;
+//
+// class MyDerived : MyBase {
+// };
+// )cpp";
+//
+// EXPECT_EQ(apply(Test), Expected);
+// }
+
+// This test should fail since MyBase is unknown. No idea how to test that.
+// TEST_F(DeclarePureVirtualsTest, UnknownBaseSpecifier) {
+// const char *Test = R"cpp(
+// class MyDerived : My^Base {
+// };
+// )cpp";
+//
+// const char *Expected = R"cpp(
+// class MyDerived : MyBase {
+// };
+// )cpp";
+//
+// EXPECT_EQ(apply(Test), Expected);
+// }
+
+// This test should fail since MyBase is unknown. No idea how to test that.
+// It's a different failure mode since it will probably fail to apply but not to
+// prepare.
+// TEST_F(DeclarePureVirtualsTest, UnknownBaseSpecifier) {
+// const char *Test = R"cpp(
+// class My^Derived : MyBase {
+// };
+// )cpp";
+//
+// const char *Expected = R"cpp(
+// class MyDerived : MyBase {
+// };
+// )cpp";
+//
+// EXPECT_EQ(apply(Test), Expected);
+// }
+
+#endif
+
+} // namespace
+} // namespace clangd
+} // namespace clang
Index: clang-tools-extra/clangd/unittests/TestTU.h
===================================================================
--- clang-tools-extra/clangd/unittests/TestTU.h
+++ clang-tools-extra/clangd/unittests/TestTU.h
@@ -88,6 +88,8 @@
// Suppress this behavior by adding an 'error-ok' comment to the code.
// The result will always have getDiagnostics() populated.
ParsedAST build() const;
+ // TODO: remove
+ ParsedAST buildErr() const;
std::shared_ptr<const PreambleData>
preamble(PreambleParsedCallback PreambleCallback = nullptr) const;
ParseInputs inputs(MockFS &FS) const;
Index: clang-tools-extra/clangd/unittests/TestTU.cpp
===================================================================
--- clang-tools-extra/clangd/unittests/TestTU.cpp
+++ clang-tools-extra/clangd/unittests/TestTU.cpp
@@ -111,6 +111,31 @@
/*StoreInMemory=*/true, PreambleCallback);
}
+// TODO: remove
+ParsedAST TestTU::buildErr() const {
+ MockFS FS;
+ auto Inputs = inputs(FS);
+ Inputs.Opts = ParseOpts;
+ StoreDiags Diags;
+ auto CI = buildCompilerInvocation(Inputs, Diags);
+ assert(CI && "Failed to build compilation invocation.");
+ if (OverlayRealFileSystemForModules)
+ initializeModuleCache(*CI);
+ auto ModuleCacheDeleter = llvm::make_scope_exit(
+ std::bind(deleteModuleCache, CI->getHeaderSearchOpts().ModuleCachePath));
+
+ auto Preamble = clang::clangd::buildPreamble(testPath(Filename), *CI, Inputs,
+ /*StoreInMemory=*/true,
+ /*PreambleCallback=*/nullptr);
+ auto AST = ParsedAST::build(testPath(Filename), Inputs, std::move(CI),
+ Diags.take(), Preamble);
+ if (!AST) {
+ llvm::errs() << "Failed to build code:\n" << Code;
+ std::abort();
+ }
+ return std::move(*AST);
+}
+
ParsedAST TestTU::build() const {
MockFS FS;
auto Inputs = inputs(FS);
Index: clang-tools-extra/clangd/unittests/CMakeLists.txt
===================================================================
--- clang-tools-extra/clangd/unittests/CMakeLists.txt
+++ clang-tools-extra/clangd/unittests/CMakeLists.txt
@@ -118,6 +118,7 @@
tweaks/AnnotateHighlightingsTests.cpp
tweaks/DefineInlineTests.cpp
tweaks/DefineOutlineTests.cpp
+ tweaks/DeclarePureVirtualsTests.cpp
tweaks/DumpASTTests.cpp
tweaks/DumpRecordLayoutTests.cpp
tweaks/DumpSymbolTests.cpp
Index: clang-tools-extra/clangd/refactor/tweaks/DeclarePureVirtuals.cpp
===================================================================
--- /dev/null
+++ clang-tools-extra/clangd/refactor/tweaks/DeclarePureVirtuals.cpp
@@ -0,0 +1,428 @@
+//===--- DeclarePureVirtuals.cpp ---------------------------------*- C++-*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+#include "Selection.h"
+#include "SourceCode.h"
+#include "refactor/Tweak.h"
+#include "clang/AST/ASTContextAllocate.h"
+#include "clang/AST/ASTTypeTraits.h"
+#include "clang/AST/CXXInheritance.h"
+#include "clang/AST/Decl.h"
+#include "clang/AST/DeclCXX.h"
+#include "clang/AST/PrettyPrinter.h"
+#include "clang/AST/TypeLoc.h"
+#include "clang/Basic/AttributeCommonInfo.h"
+#include "clang/Basic/SourceLocation.h"
+#include "clang/Basic/SourceManager.h"
+#include "clang/Basic/TokenKinds.h"
+#include "clang/Tooling/Core/Replacement.h"
+#include "clang/Tooling/Refactoring/ASTSelection.h"
+#include "clang/Tooling/Syntax/Tokens.h"
+#include "llvm/ADT/SmallVector.h"
+#include "llvm/ADT/StringRef.h"
+#include "llvm/Support/Error.h"
+#include "llvm/Support/FormatVariadic.h"
+#include "llvm/Support/raw_ostream.h"
+#include <unordered_map>
+#include <unordered_set>
+
+namespace clang {
+namespace clangd {
+namespace {
+
+// TODO: access control
+
+// TODO: doc
+// void reprint(const CXXMethodDecl &M, llvm::raw_string_ostream &OS,
+// const PrintingPolicy &PP) {
+// std::string Declarator;
+// {
+// llvm::raw_string_ostream OS(Declarator);
+// const char *Sep = "";
+
+// // PrintingPolicy UnparenPP(PP);
+// // UnparenPP.ParenthesizeFunctionName = false;
+// // OS << printType(M.getType(), *M.getDeclContext(), M.getName(),
+// // &UnparenPP) << "(";
+
+// OS << M.getDeclName() << "(";
+// for (const auto &Param : M.parameters()) {
+// OS << Sep;
+// Param->print(OS, PP);
+// Sep = ", ";
+// }
+// OS << ")";
+// }
+// M.getReturnType().print(OS, PP, Declarator);
+// M.getMethodQualifiers().print(OS, PP, /*appendSpaceIfNonEmpty=*/true);
+// switch (M.getRefQualifier()) {
+// case RQ_None:
+// break;
+// case RQ_LValue:
+// OS << " &";
+// break;
+// case RQ_RValue:
+// OS << " &&";
+// break;
+// }
+// OS << " override;\n";
+// }
+
+// void reprintWrapper(std::string &To, const CXXMethodDecl &Method,
+// PrintingPolicy pp, const CXXRecordDecl *Class) {
+// struct HandleScope : public PrintingCallbacks {
+// const CXXRecordDecl *Class;
+// mutable CXXBasePaths Paths;
+// HandleScope(const CXXRecordDecl *Class) : Class(Class), Paths() {}
+// virtual ~HandleScope() = default;
+
+// bool isScopeVisible(const clang::DeclContext *DC) const override {
+// if (DC->Encloses(Class))
+// return true;
+// if (const auto *MaybeBase = llvm::dyn_cast<CXXRecordDecl>(DC))
+// if (Class->isDerivedFrom(MaybeBase, Paths))
+// return true;
+// return false;
+// }
+// } PCallbacks(Class);
+
+// PrintingPolicy PP(pp);
+// PP.PolishForDeclaration = true;
+// PP.TerseOutput = true;
+// PP.Callbacks = &PCallbacks;
+// std::string Code;
+// llvm::raw_string_ostream OS(Code);
+// reprint(Method, OS, PP);
+// To.append(OS.str());
+// }
+
+// Determines if the class Cls which provided the final overrider map contains
+// itself (not inherited) an overrider of Method.
+bool hasOverrideFor(const CXXRecordDecl &Cls, const CXXFinalOverriderMap &Map,
+ const CXXMethodDecl *Method) {
+ const auto *const It = Map.find(Method);
+ if (It == Map.end()) {
+ return false;
+ }
+
+ const auto &SubobjectsWithOverrider = It->second;
+ for (const auto &[SubobjectId, FinalOverridersPerSubobj] :
+ SubobjectsWithOverrider) {
+ for (const UniqueVirtualMethod &FinalOverriderForSubobj :
+ FinalOverridersPerSubobj) {
+ if (FinalOverriderForSubobj.Method->getDeclContext() == &Cls)
+ return true;
+ }
+ }
+
+ return false;
+}
+
+// Appends a declaration to To which overrides Method.
+void appendDeclForOverride(std::string &To, const CXXMethodDecl *Method,
+ const SourceManager &SM,
+ const syntax::TokenBuffer &TokBuf) {
+ const SourceRange MethodDeclRange{Method->getBeginLoc(), Method->getEndLoc()};
+
+ const llvm::ArrayRef<syntax::Token> Tokens =
+ TokBuf.expandedTokens(MethodDeclRange);
+ const auto EqTok =
+ llvm::find_if(llvm::reverse(Tokens), [](const syntax::Token &Tok) {
+ return Tok.kind() == tok::equal;
+ });
+ assert(EqTok != Tokens.rend());
+
+ const syntax::Token *VirtTok =
+ llvm::find_if(Tokens, [](const syntax::Token &Tok) {
+ return Tok.kind() == tok::kw_virtual;
+ });
+ const syntax::Token *PostVirtTok = VirtTok;
+ if (VirtTok == Tokens.end()) {
+ PostVirtTok = Tokens.begin();
+ } else {
+ ++PostVirtTok;
+ }
+
+ assert(!Tokens.empty());
+ // If we copy each token individually, we skip over whitespaces and comments.
+ // Therefore, we copy entire source ranges between the tokens to remove.
+ // TODO: we can't easily skip attributes since they consist of multiple
+ // tokens, including balanced parentheses
+ To.append(SM.getCharacterData(Tokens.begin()->location()),
+ SM.getCharacterData(VirtTok->location()));
+ To.append(SM.getCharacterData(PostVirtTok->location()),
+ SM.getCharacterData(EqTok->location()));
+
+ if (To.back() != ' ')
+ To.push_back(' ');
+ To.append("override;\n");
+}
+
+// TODO: doc
+struct Additions {
+ std::string AllAdditions;
+ size_t Cutoff = 0;
+ int NumAdditions = 0;
+};
+
+// TODO: remove
+void dumpFinalOverriderMap(const CXXFinalOverriderMap &Fom,
+ llvm::raw_ostream &os) {
+ for (const auto &[Method, SubobjectsWithOverrider] : Fom) {
+ os << "Method " << Method->getQualifiedNameAsString() << ":\n";
+ for (const auto &[SubobjectId, FinalOverridersPerSubobj] :
+ SubobjectsWithOverrider) {
+ os << " Subobject " << SubobjectId << ":\n";
+ for (const UniqueVirtualMethod &FinalOverriderForSubobj :
+ FinalOverridersPerSubobj) {
+ os << " Final Overrider "
+ << FinalOverriderForSubobj.Method->getQualifiedNameAsString()
+ << "\n";
+ }
+ }
+ }
+}
+
+// Returns a string with override declarations for all virtual functions in the
+// inheritance hierarchy of Start (including Start itself) which are still pure
+// virtual in Target. Start can be the same class as Target. Target must be the
+// same as or derived from Start.
+Additions collectPureVirtualFuncOverrideDecls(const CXXRecordDecl &Target,
+ const CXXRecordDecl &Start,
+ const SourceManager &SM,
+ const syntax::TokenBuffer &TokBuf,
+ PrintingPolicy PP,
+ int MaxAdditionsToPreview) {
+ Additions Additions;
+ // If the inheritance forms a diamond, we can inherit the same virtual
+ // function from multiple intermediate base classes. Those can themselves
+ // re-declare the function as pure:
+ // struct A { virtual void foo() = 0; };
+ // struct B : A { virtual void foo() = 0; };
+ // struct C : A { virtual void foo() = 0; };
+ // struct D : B, C {};
+ // Since the final overrider map maps subobject -> final overriders, we will
+ // therefore get:
+ // - A::foo -> [(1, [B::foo]), (2, C::foo)]
+ // - B::foo -> [(1, [B::foo])]
+ // - C::foo -> [(2, [C::foo])]
+ // Since both B::foo and C::foo are pure virtual, we need to make sure we only
+ // emit one override for A::foo.
+ std::unordered_set<const CXXMethodDecl *> SeenFinalOverriders;
+
+ assert(&Target == &Start || Target.isDerivedFrom(&Start));
+
+ CXXFinalOverriderMap FinalOverriderMapOfStart;
+ Start.getFinalOverriders(FinalOverriderMapOfStart);
+
+ CXXFinalOverriderMap FinalOverriderMapOfTarget;
+ // TODO: is this true if the class re-declares a pure virtual function?
+ // If &Target == &Start then Target doesn't already have any
+ // overrides for functions that are pure in Start. The map remains empty,
+ // which means hasOverrideFor will return false below, which is OK since the
+ // function is then counted as pure virtual.
+ if (&Target != &Start) {
+ Target.getFinalOverriders(FinalOverriderMapOfTarget);
+ }
+
+ for (const auto &[Method, SubobjectsWithOverrider] :
+ FinalOverriderMapOfStart) {
+ bool AppendedOverriderForThisMethod = false;
+ for (const auto &[SubobjectId, FinalOverridersPerSubobj] :
+ SubobjectsWithOverrider) {
+ // TODO: if FinalOverriders.length() > 1, abort?
+ for (const UniqueVirtualMethod &FinalOverriderForSubobj :
+ FinalOverridersPerSubobj) {
+ if (FinalOverriderForSubobj.Method->isPure() &&
+ !hasOverrideFor(Target, FinalOverriderMapOfTarget,
+ FinalOverriderForSubobj.Method) &&
+ !AppendedOverriderForThisMethod &&
+ !SeenFinalOverriders.count(FinalOverriderForSubobj.Method)) {
+ // reprintWrapper(Additions.AllAdditions, *Override.Method, PP,
+ // &Target);
+ appendDeclForOverride(Additions.AllAdditions,
+ FinalOverriderForSubobj.Method, SM, TokBuf);
+ if (Additions.NumAdditions++ == MaxAdditionsToPreview) {
+ Additions.Cutoff = Additions.AllAdditions.size();
+ }
+
+ // If we inherit the same pure virtual function A::foo from multiple
+ // base classes B, C, it could be pure virtual in B but implemented in
+ // C. In that case, A::foo is still pure virtual in D:
+ // struct A { virtual void foo() = 0; };
+ // struct B : A { virtual void foo() = 0; };
+ // struct C : A { void foo() override; };
+ // struct D : B, C {}; // A::foo is still pure virtual in D for the
+ // // B subobject
+ // Therefore we still have to override A::foo in D to make it
+ // non-abstract. Maybe we should emit some warning in that case, since
+ // it's not obvious that we're overriding A::foo from C as well.
+ AppendedOverriderForThisMethod = true;
+ }
+ // The FinalOverriderMap lists methods declarations from all base
+ // classes, including indirect bases. Therefore we can encounter the
+ // same final overrider multiple times, e.g. via an indirect base and
+ // its derived class.
+ SeenFinalOverriders.emplace(FinalOverriderForSubobj.Method);
+ }
+ }
+ }
+
+ return Additions;
+}
+
+// Finds the CXXBaseSpecifier in/under the selection, if any.
+const CXXBaseSpecifier *findBaseSpecifier(const SelectionTree::Node *Node) {
+ if (!Node)
+ return nullptr;
+
+ const DynTypedNode &ASTNode = Node->ASTNode;
+ const CXXBaseSpecifier *BaseSpec = ASTNode.get<CXXBaseSpecifier>();
+ if (BaseSpec)
+ return BaseSpec;
+
+ const SelectionTree::Node *const Parent = Node->Parent;
+ if (Parent) {
+ if (const CXXBaseSpecifier *BaseSpec =
+ Parent->ASTNode.get<CXXBaseSpecifier>()) {
+ return BaseSpec;
+ }
+
+ // This happens if Node is a RecordTypeLoc, e.g. when selecting the base
+ // name in the base specifier
+ if (auto const *Parent2 = Parent->Parent) {
+ // not sure if this can ever be null
+ return Parent2->ASTNode.get<CXXBaseSpecifier>();
+ }
+ }
+
+ return nullptr;
+}
+
+// TODO: doc
+llvm::Expected<Additions> generateOverrideDeclarations(
+ const ParsedAST &AST, const CXXBaseSpecifier *SelectedBaseSpecifier,
+ const CXXRecordDecl *SelectedDerivedClass, int MaxAdditionsToPreview) {
+ const SourceManager &SM = AST.getSourceManager();
+
+ const clang::PrintingPolicy PP = AST.getASTContext().getPrintingPolicy();
+ if (SelectedBaseSpecifier) {
+ // TODO: can getType return null?
+ auto const *Start = SelectedBaseSpecifier->getType()->getAsCXXRecordDecl();
+ if (!Start) {
+ return llvm::createStringError(
+ llvm::inconvertibleErrorCode(),
+ "selected base class specifier does not refer to a C++ class");
+ }
+ return collectPureVirtualFuncOverrideDecls(*SelectedDerivedClass, *Start,
+ SM, AST.getTokens(), PP,
+ MaxAdditionsToPreview);
+ }
+
+ return collectPureVirtualFuncOverrideDecls(
+ *SelectedDerivedClass, *SelectedDerivedClass, SM, AST.getTokens(), PP,
+ MaxAdditionsToPreview);
+}
+
+/// Declares overrides for all pure virtual function in a class hierarchy,
+/// starting with a base class specifier.
+///
+/// Before:
+/// class Base { virtual void foo() = 0; };
+/// class Derived {};
+/// ^^^^^^^
+///
+/// After:
+/// class Base { virtual void foo() = 0; };
+/// class Derived { void foo() override; };
+class DeclarePureVirtuals : public Tweak {
+public:
+ const char *id() const override;
+
+ std::string title() const override {
+ std::string_view Preview =
+ std::string_view{Additions.AllAdditions}.substr(0, Additions.Cutoff);
+ return llvm::formatv(
+ "Override pure virtual function{0}:\n{1}{2}",
+ Additions.NumAdditions == 1 ? "" : "s", Additions.AllAdditions, Preview,
+ Additions.NumAdditions > MaxAdditionsToPreview ? "\n..." : "");
+ }
+
+ llvm::StringLiteral kind() const override {
+ return CodeAction::GENERATE_KIND;
+ }
+
+ bool prepare(const Selection &Sel) override {
+ if (!select(Sel))
+ return false;
+
+ auto Result = generateOverrideDeclarations(*Sel.AST, SelectedBaseSpecifier,
+ SelectedDerivedClass,
+ MaxAdditionsToPreview);
+ if (Result.takeError()) {
+ // TODO: report error? how?
+ return false;
+ }
+ Additions = std::move(*Result);
+
+ return Additions.NumAdditions > 0;
+ }
+
+ Expected<Effect> apply(const Selection &Sel) override {
+ assert(SelectedDerivedClass); // prepare must have been called and returned
+ // true
+
+ SourceManager &SM = Sel.AST->getSourceManager();
+
+ // TODO: can we apply this tweak in a header if we apply the effect to the
+ // "main file"?
+ return Effect::mainFileEdit(SM, tooling::Replacements(tooling::Replacement(
+ SM, SelectedDerivedClass->getEndLoc(),
+ 0, Additions.AllAdditions)));
+ }
+
+private:
+ // TODO: doc
+ bool select(const Selection &Sel) {
+ const SelectionTree::Node *const CommonAncestor =
+ Sel.ASTSelection.commonAncestor();
+ if (!CommonAncestor)
+ return false;
+
+ // maybe selected a class, in which case override functions of all bases
+ SelectedDerivedClass = CommonAncestor->ASTNode.get<CXXRecordDecl>();
+ if (SelectedDerivedClass) {
+ // if this is a forward-declaration, accessing bases() will crash
+ if (!SelectedDerivedClass->isCompleteDefinition())
+ return false;
+ return !SelectedDerivedClass->bases().empty();
+ }
+
+ // maybe selected a base class specifier, in which case only override those
+ // bases's functions
+ const DeclContext &DC = CommonAncestor->getDeclContext();
+ SelectedDerivedClass = dyn_cast<CXXRecordDecl>(&DC);
+ if (SelectedDerivedClass) {
+ SelectedBaseSpecifier = findBaseSpecifier(CommonAncestor);
+ return SelectedBaseSpecifier != nullptr;
+ }
+
+ return false;
+ }
+
+ const CXXRecordDecl *SelectedDerivedClass = nullptr;
+ const CXXBaseSpecifier *SelectedBaseSpecifier = nullptr;
+ Additions Additions;
+ static constexpr inline int MaxAdditionsToPreview = 5;
+};
+
+REGISTER_TWEAK(DeclarePureVirtuals)
+
+} // namespace
+} // namespace clangd
+} // namespace clang
Index: clang-tools-extra/clangd/refactor/tweaks/CMakeLists.txt
===================================================================
--- clang-tools-extra/clangd/refactor/tweaks/CMakeLists.txt
+++ clang-tools-extra/clangd/refactor/tweaks/CMakeLists.txt
@@ -29,6 +29,7 @@
RemoveUsingNamespace.cpp
SpecialMembers.cpp
SwapIfBranches.cpp
+ DeclarePureVirtuals.cpp
LINK_LIBS
clangAST
Index: clang-tools-extra/clangd/Protocol.h
===================================================================
--- clang-tools-extra/clangd/Protocol.h
+++ clang-tools-extra/clangd/Protocol.h
@@ -1080,6 +1080,7 @@
const static llvm::StringLiteral QUICKFIX_KIND;
const static llvm::StringLiteral REFACTOR_KIND;
const static llvm::StringLiteral INFO_KIND;
+ const static llvm::StringLiteral GENERATE_KIND;
/// The diagnostics that this code action resolves.
std::optional<std::vector<Diagnostic>> diagnostics;
Index: clang-tools-extra/clangd/Protocol.cpp
===================================================================
--- clang-tools-extra/clangd/Protocol.cpp
+++ clang-tools-extra/clangd/Protocol.cpp
@@ -868,6 +868,7 @@
const llvm::StringLiteral CodeAction::QUICKFIX_KIND = "quickfix";
const llvm::StringLiteral CodeAction::REFACTOR_KIND = "refactor";
const llvm::StringLiteral CodeAction::INFO_KIND = "info";
+const llvm::StringLiteral CodeAction::GENERATE_KIND = "generate";
llvm::json::Value toJSON(const CodeAction &CA) {
auto CodeAction = llvm::json::Object{{"title", CA.title}};
Index: clang-tools-extra/clangd/AST.h
===================================================================
--- clang-tools-extra/clangd/AST.h
+++ clang-tools-extra/clangd/AST.h
@@ -19,6 +19,7 @@
#include "clang/AST/Decl.h"
#include "clang/AST/DeclObjC.h"
#include "clang/AST/NestedNameSpecifier.h"
+#include "clang/AST/PrettyPrinter.h"
#include "clang/AST/TypeLoc.h"
#include "clang/Basic/SourceLocation.h"
#include "clang/Lex/MacroInfo.h"
@@ -134,8 +135,9 @@
/// Returns a QualType as string. The result doesn't contain unwritten scopes
/// like anonymous/inline namespace.
+/// If PP is null, uses the printing policy of CurContext.
std::string printType(const QualType QT, const DeclContext &CurContext,
- llvm::StringRef Placeholder = "");
+ llvm::StringRef Placeholder = "", PrintingPolicy* PP = nullptr);
/// Indicates if \p D is a template instantiation implicitly generated by the
/// compiler, e.g.
Index: clang-tools-extra/clangd/AST.cpp
===================================================================
--- clang-tools-extra/clangd/AST.cpp
+++ clang-tools-extra/clangd/AST.cpp
@@ -409,10 +409,10 @@
}
std::string printType(const QualType QT, const DeclContext &CurContext,
- const llvm::StringRef Placeholder) {
+ const llvm::StringRef Placeholder, PrintingPolicy* CustomPP) {
std::string Result;
llvm::raw_string_ostream OS(Result);
- PrintingPolicy PP(CurContext.getParentASTContext().getPrintingPolicy());
+ PrintingPolicy PP(CustomPP ? *CustomPP : CurContext.getParentASTContext().getPrintingPolicy());
PP.SuppressTagKeyword = true;
PP.SuppressUnwrittenScope = true;
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
