diff --git a/clang-tools-extra/clangd/AST.h b/clang-tools-extra/clangd/AST.h --- a/clang-tools-extra/clangd/AST.h +++ b/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. diff --git a/clang-tools-extra/clangd/AST.cpp b/clang-tools-extra/clangd/AST.cpp --- a/clang-tools-extra/clangd/AST.cpp +++ b/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; diff --git a/clang-tools-extra/clangd/Protocol.h b/clang-tools-extra/clangd/Protocol.h --- a/clang-tools-extra/clangd/Protocol.h +++ b/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> diagnostics; diff --git a/clang-tools-extra/clangd/Protocol.cpp b/clang-tools-extra/clangd/Protocol.cpp --- a/clang-tools-extra/clangd/Protocol.cpp +++ b/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}}; diff --git a/clang-tools-extra/clangd/refactor/tweaks/CMakeLists.txt b/clang-tools-extra/clangd/refactor/tweaks/CMakeLists.txt --- a/clang-tools-extra/clangd/refactor/tweaks/CMakeLists.txt +++ b/clang-tools-extra/clangd/refactor/tweaks/CMakeLists.txt @@ -29,6 +29,7 @@ RemoveUsingNamespace.cpp SpecialMembers.cpp SwapIfBranches.cpp + DeclarePureVirtuals.cpp LINK_LIBS clangAST diff --git a/clang-tools-extra/clangd/refactor/tweaks/DeclarePureVirtuals.cpp b/clang-tools-extra/clangd/refactor/tweaks/DeclarePureVirtuals.cpp new file mode 100644 --- /dev/null +++ b/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 +#include + +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(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 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 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(); + if (BaseSpec) + return BaseSpec; + + const SelectionTree::Node *const Parent = Node->Parent; + if (Parent) { + if (const CXXBaseSpecifier *BaseSpec = + Parent->ASTNode.get()) { + 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(); + } + } + + return nullptr; +} + +// TODO: doc +llvm::Expected 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 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(); + 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(&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 diff --git a/clang-tools-extra/clangd/unittests/CMakeLists.txt b/clang-tools-extra/clangd/unittests/CMakeLists.txt --- a/clang-tools-extra/clangd/unittests/CMakeLists.txt +++ b/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 diff --git a/clang-tools-extra/clangd/unittests/TestTU.h b/clang-tools-extra/clangd/unittests/TestTU.h --- a/clang-tools-extra/clangd/unittests/TestTU.h +++ b/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 preamble(PreambleParsedCallback PreambleCallback = nullptr) const; ParseInputs inputs(MockFS &FS) const; diff --git a/clang-tools-extra/clangd/unittests/TestTU.cpp b/clang-tools-extra/clangd/unittests/TestTU.cpp --- a/clang-tools-extra/clangd/unittests/TestTU.cpp +++ b/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); diff --git a/clang-tools-extra/clangd/unittests/tweaks/DeclarePureVirtualsTests.cpp b/clang-tools-extra/clangd/unittests/tweaks/DeclarePureVirtualsTests.cpp new file mode 100644 --- /dev/null +++ b/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 topLevelDecls) { + for (const auto *delc : topLevelDecls) { + if (const CXXRecordDecl *rc = dyn_cast(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> + &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 diff --git a/clang-tools-extra/clangd/unittests/tweaks/TweakTesting.h b/clang-tools-extra/clangd/unittests/tweaks/TweakTesting.h --- a/clang-tools-extra/clangd/unittests/tweaks/TweakTesting.h +++ b/clang-tools-extra/clangd/unittests/tweaks/TweakTesting.h @@ -91,9 +91,15 @@ std::string apply(llvm::StringRef MarkedCode, llvm::StringMap *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; 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); diff --git a/clang-tools-extra/clangd/unittests/tweaks/TweakTesting.cpp b/clang-tools-extra/clangd/unittests/tweaks/TweakTesting.cpp --- a/clang-tools-extra/clangd/unittests/tweaks/TweakTesting.cpp +++ b/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 @@ -80,6 +83,26 @@ return Result; } +// TODO: doc +std::optional +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(); } diff --git a/clang/include/clang/AST/CXXInheritance.h b/clang/include/clang/AST/CXXInheritance.h --- a/clang/include/clang/AST/CXXInheritance.h +++ b/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 {}; diff --git a/clang/include/clang/AST/PrettyPrinter.h b/clang/include/clang/AST/PrettyPrinter.h --- a/clang/include/clang/AST/PrettyPrinter.h +++ b/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; }; diff --git a/clang/lib/AST/TypePrinter.cpp b/clang/lib/AST/TypePrinter.cpp --- a/clang/lib/AST/TypePrinter.cpp +++ b/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);