Adds tweak to add declarations for pure virtuals

[robot]

Summary

This tweak searches the current or a selected base class for functions
which are still pure virtual in the current class, and adds function
declarations (overriders) for them.

[robot]

This change is not done, I need some advice:

• I don't understand why there are two levels of data structures in an entry of the final overriders map, hence I can't properly name the variables nor do I fully understand what I'm doing there.
• It's not clear if the tweak should be available if there are no pure virtual functions.
• It's not clear if we need to handle the case that there are multiple final overriders, or what we should do in that case.
• There are some test cases at the end of the file which I don't know how to implement.
• It's not clear if this tweak can apply inside a header.
• It's not clear how to prepare the tweak when the cursor is inside the class body (see tests).

[robot]

Remove left-over file of other patch from CMakeLists.txt

[robot]

Remove left-over test file of other ptch from CMakeLists.txt

[sammccall]

Thanks for working on this!

FWIW I had an old draft of this feature that may have interesting ideas: https://reviews.llvm.org/D122827

clang-tools-extra/clangd/refactor/tweaks/DeclarePureVirtuals.cpp

45

see the long comment on CXXFinalOverriderMap
TL;DR: a derived class can inherit multiple times from the same base, each such subobject could potentially have a separate override

59

copying the tokens directly seems brittle, for example we shouldn't be emitting "virtual" but its's hard to avoid doing so.
(Also, surprising that getEndLoc() doesn't cover the = 0!)

D122827 has a reprint() function that seems to work

76

this probably needs to be restructured somewhat as need to enumerate these in prepare() to be sure there are any, but don't want to pay to produce the strings

87

FWIW, I'm not actually sure that the case of multiple bases classes each providing pure-virtual-methods of which you want to override only one set is common enough to be worth this extra complexity.

140

please use ^^ markers underneath the characters where you expect the tweak to trigger

144

no "virtual"

150

what do you think about listing the functions we'll define?

Especially in the case where there's only one.

154

I don't think this is a quick-fix, which should address a diagnostic or so.

Really there doesn't seem to be a standard kind that fits here. Maybe we should add a new constant "generate"?

157

it's important that we only offer the code action (i.e. prepare returns true) if there are actually methods to override.

This means actually enumerating the functions needs to happen in prepare rather than apply. (Which means it needs to be fast...)

164

this should only trigger on the class definition, not forward declarations

171

this seems confusing: if you're going to walk up looking for a CXXBaseSpecifier, why not just do that in the first place?

197

this doesn't seem like a condition we should be surfacing to the user.
How do we get here? if it's e.g. because the base is dependent, then we should be returning false from prepare() in the appropriate dependent-code cases.

213

inserting at the end of the class isn't the most appropriate place.

we have InsertionPoint.h for this, see usage in D122827

clang-tools-extra/clangd/unittests/tweaks/DeclarePureVirtualsTests.cpp

69

it should be unavailable
(this is important, availability triggers the "lightbulb" icon in vscode and is interpreted by users as "something to do here")

592

EXPECT_UNAVAILABLE, we shouldn't be offering the tweak

[robot]

I'm working on the comments, but it will take a while.

clang-tools-extra/clangd/refactor/tweaks/DeclarePureVirtuals.cpp

59

My original idea was to take the AST node, clone it, remove the pure-specifier, and print that. To me, that seemed like the obvious (object-oriented) way how a tweak would manipulate or generate text.
Alas, I was rather stumped that there didn't seem to be support for this already, and it was rather hard to do. I gave up when default arguments came into play.

I've got the idea of manipulating the tokens from the "extract function" tweak (DefineOutline.cpp / getFunctionSourceCode).

I've tried the reprint function and it does not copy noexcept. That is simple to fix but my guess is there are more issues. The idea of token manipulation was to rather copy too much than too little. I have no intuition how many issues could arise from that :)

When I try to naively remove the virtual I currently print (virtual void foo() override;) in the token manipulation approach by skipping the first tokens, I end up skipping any attributes that precede the virtual as well. So I wrote a test for that, and reprint also doesn't copy attributes at the moment. It is not obvious to me if attributes should be copied, e.g. [[noreturn]].

I have a slight preference for my original approach but it's probably too complicated if this is the only use case. Therefore I'll just implement whatever looks most promising to you.

154

For reference:
rust-analyser shows a "Quick Fix" in VSCode for implementing the functions in a trait.
The go extension shows a "Rewrite" (which is an entirely different action?) in VSCode for filling a structure object (inserting code which explicitly initializes each member with its null value).

[sammccall]

clang-tools-extra/clangd/refactor/tweaks/DeclarePureVirtuals.cpp

59

Yeah, I think extract function copies tokens because there could be anything in the selected code, and there are certain constructs clang doesn't print well, it's unclear what to do in the presence of macros etc. Those concerns don't apply here I think.

My original idea was to take the AST node, clone it, remove the pure-specifier, and print that. To me, that seemed like the obvious (object-oriented) way how a tweak would manipulate or generate text.

Yes, unfortunately this approach doesn't work because the AST isn't designed to be mutated, and carries not just syntactic information but also semantic invariants.
(There was an effort to produce a separate representation of the syntax, but it was never finished)

I've tried the reprint function and it does not copy noexcept

True. It looks like the noexcept is part of the functionprototype, so you could replace the logic that prints the return type + arg list with printType(functype, class, methodname) (from AST.h). That should pick up noexcept etc.

It is not obvious to me if attributes should be copied, e.g. [[noreturn]].

No, 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.

[robot]

clang-tools-extra/clangd/refactor/tweaks/DeclarePureVirtuals.cpp

59

Yeah, I think extract function copies tokens because there could be anything in the selected code, and there are certain constructs clang doesn't print well, it's unclear what to do in the presence of macros etc. Those concerns don't apply here I think.

Sorry, wrong tweak name (but right file/function). I meant "define outline". It also needs to massage the declaration since it needs to remove those parts which are not allowed in an outlined function definition, like virtual and static.

Anyway, I'll try adjusting reprint() next week.

[robot]

so you could replace the logic that prints the return type + arg list with printType(functype, class, methodname) (from AST.h). That should pick up noexcept etc.

I've tried it and it's not obvious to me how to do it properly: Since printType wants to print the _function type_, it will parenthesize the placeholder (methodname):

void (foo)()

I've tried adding a flag to PrintingPolicy to avoid parenthesizing the name, but I wonder if adjusting a "type printer" to print a function declaration is a clean design. Next problem I ran into is parameters: A type printer does not need to concern itself with parameter names nor default arguments:

void foo(int x = 0) = 0;
void foo(int) override;

Shall I continue to pursue that path? Shall I adjust the type printer to be able to print parameter names and default arguments?

[sammccall]

In D153152#4617391, @robot wrote:

so you could replace the logic that prints the return type + arg list with printType(functype, class, methodname) (from AST.h). That should pick up noexcept etc.

I've tried it and it's not obvious to me how to do it properly: Since printType wants to print the _function type_, it will parenthesize the placeholder (methodname):

void (foo)()

I've tried adding a flag to PrintingPolicy to avoid parenthesizing the name, but I wonder if adjusting a "type printer" to print a function declaration is a clean design. Next problem I ran into is parameters: A type printer does not need to concern itself with parameter names nor default arguments:

void foo(int x = 0) = 0;
void foo(int) override;

Shall I continue to pursue that path? Shall I adjust the type printer to be able to print parameter names and default arguments?

Sorry, it does sound like I sent you down a bad path.
You *could* (no need for an option for parens, here we can detect that the placeholder isValidAsciiIdentifier), but entirely possible more things are broken still.

If you prefer the approach with copying the tokens, adding heuristics to patch them up as needed is maybe no worse than the alternatives.

(BTW, I don't think we want to copy default arguments - it's certainly not necessary, and not helpful in common cases where the function is only called through the base)

[robot]

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

[robot]

I've pushed some WIP change to inform that I'm still working on this topic :) Not sure if I also need to submit a comment to publish what I've marked as "done" via checkboxes.

clang-tools-extra/clangd/AST.h

 //===--- AST.h - Utility AST functions  -------------------------*- 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
 //
 //===----------------------------------------------------------------------===//
 //
 // Various code that examines C++ source code using AST.
 //
 //===----------------------------------------------------------------------===//
 
 #ifndef LLVM_CLANG_TOOLS_EXTRA_CLANGD_AST_H
 #define LLVM_CLANG_TOOLS_EXTRA_CLANGD_AST_H
 
 #include "Headers.h"
 #include "index/Symbol.h"
 #include "index/SymbolID.h"
 #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"
 #include "llvm/ADT/StringRef.h"
 #include <optional>
 #include <string>
 #include <vector>
 

 ///   fallback in the absence of compile flags.
 Symbol::IncludeDirective
 preferredIncludeDirective(llvm::StringRef FileName, const LangOptions &LangOpts,
                           ArrayRef<Inclusion> MainFileIncludes,
                           ArrayRef<const Decl *> TopLevelDecls);
 
 /// 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.
 ///     template <class T> struct vector {};
 ///     vector<int> v; // 'vector<int>' is an implicit instantiation
 bool isImplicitTemplateInstantiation(const NamedDecl *D);
 /// Indicates if \p D is an explicit template specialization, e.g.
 ///   template <class T> struct vector {};

clang-tools-extra/clangd/AST.cpp

 //===--- AST.cpp - Utility AST functions  -----------------------*- 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
 //
 //===----------------------------------------------------------------------===//
 
for (const Decl *D : TopLevelDecls)
     if (isa<ObjCContainerDecl, ObjCIvarDecl, ObjCMethodDecl, ObjCPropertyDecl>(
             D))
       return Symbol::IncludeDirective::Import;
 
   return Symbol::IncludeDirective::Include;
 }
 
 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;
 
   class PrintCB : public PrintingCallbacks {
   public:
     PrintCB(const DeclContext *CurContext) : CurContext(CurContext) {}
     virtual ~PrintCB() {}
     bool isScopeVisible(const DeclContext *DC) const override {

clang-tools-extra/clangd/Protocol.h

 //===--- Protocol.h - Language Server Protocol Implementation ---*- 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
 //
 //===----------------------------------------------------------------------===//
 //
struct CodeAction {
   std::string title;
 
   /// The kind of the code action.
   /// Used to filter code actions.
   std::optional<std::string> kind;
   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;
 
   /// Marks this as a preferred action. Preferred actions are used by the
   /// `auto fix` command and can be targeted by keybindings.
   /// A quick fix should be marked preferred if it properly addresses the
   /// underlying error. A refactoring should be marked preferred if it is the

clang-tools-extra/clangd/Protocol.cpp

 //===--- Protocol.cpp - Language Server Protocol Implementation -----------===//
 //
 // 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
 //
 //===----------------------------------------------------------------------===//
 //
llvm::json::Value toJSON(const Command &C) {
   if (!C.argument.getAsNull())
     Cmd["arguments"] = llvm::json::Array{C.argument};
   return std::move(Cmd);
 }
 
 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}};
   if (CA.kind)
     CodeAction["kind"] = *CA.kind;
   if (CA.diagnostics)
     CodeAction["diagnostics"] = llvm::json::Array(*CA.diagnostics);
   if (CA.isPreferred)

clang-tools-extra/clangd/refactor/tweaks/CMakeLists.txt

add_clang_library(clangDaemonTweaks OBJECT
   MemberwiseConstructor.cpp
   ObjCLocalizeStringLiteral.cpp
   ObjCMemberwiseInitializer.cpp
   PopulateSwitch.cpp
   RawStringLiteral.cpp
   RemoveUsingNamespace.cpp
   SpecialMembers.cpp
   SwapIfBranches.cpp
+  DeclarePureVirtuals.cpp
 
   LINK_LIBS
   clangAST
   clangBasic
   clangDaemon
   clangdSupport
   clangFormat
   clangLex
   clangSema
   clangToolingCore
   clangToolingRefactoring
   clangToolingSyntax
 
   DEPENDS
   omp_gen
   )

clang-tools-extra/clangd/refactor/tweaks/DeclarePureVirtuals.cpp

+//===--- 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

clang-tools-extra/clangd/unittests/CMakeLists.txt

add_unittest(ClangdUnitTests ClangdTests
   support/TestTracer.cpp
   support/ThreadingTests.cpp
   support/TraceTests.cpp
 
   tweaks/AddUsingTests.cpp
   tweaks/AnnotateHighlightingsTests.cpp
   tweaks/DefineInlineTests.cpp
   tweaks/DefineOutlineTests.cpp
+  tweaks/DeclarePureVirtualsTests.cpp
   tweaks/DumpASTTests.cpp
   tweaks/DumpRecordLayoutTests.cpp
   tweaks/DumpSymbolTests.cpp
   tweaks/ExpandDeducedTypeTests.cpp
   tweaks/ExpandMacroTests.cpp
   tweaks/ExtractFunctionTests.cpp
   tweaks/ExtractVariableTests.cpp
   tweaks/MemberwiseConstructorTests.cpp

clang-tools-extra/clangd/unittests/TestTU.h

 //===--- TestTU.h - Scratch source files for testing -------------*- 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
 //
 //===----------------------------------------------------------------------===//
 //
struct TestTU {
   bool OverlayRealFileSystemForModules = false;
 
   FeatureModuleSet *FeatureModules = nullptr;
 
   // By default, build() will report Error diagnostics as GTest errors.
   // 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;
   SymbolSlab headerSymbols() const;
   RefSlab headerRefs() const;
   std::unique_ptr<SymbolIndex> index() const;
 };
 

clang-tools-extra/clangd/unittests/TestTU.cpp

 //===--- TestTU.cpp - Scratch source files for testing --------------------===//
 //
 // 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
 //
 //===----------------------------------------------------------------------===//
 
TestTU::preamble(PreambleParsedCallback PreambleCallback) const {
   if (OverlayRealFileSystemForModules)
     initializeModuleCache(*CI);
   auto ModuleCacheDeleter = llvm::make_scope_exit(
       std::bind(deleteModuleCache, CI->getHeaderSearchOpts().ModuleCachePath));
   return clang::clangd::buildPreamble(testPath(Filename), *CI, Inputs,
                                       /*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);
   Inputs.Opts = ParseOpts;
   StoreDiags Diags;
   auto CI = buildCompilerInvocation(Inputs, Diags);
   assert(CI && "Failed to build compilation invocation.");
   if (OverlayRealFileSystemForModules)

clang-tools-extra/clangd/unittests/tweaks/DeclarePureVirtualsTests.cpp

+//===-- 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

clang-tools-extra/clangd/unittests/tweaks/TweakTesting.h

 //===--- TweakTesting.h - Test helpers for refactoring actions ---*- 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
 //
 //===----------------------------------------------------------------------===//
 
protected:
   //    The context added to MarkedCode will be stripped away before returning,
   //    unless the tweak edited it.
   //  - if the tweak produces a message, returns "message:\n<message>"
   //  - if prepare() returns false, returns "unavailable"
   //  - if apply() returns an error, returns "fail: <message>"
   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);
   static std::string decorate(llvm::StringRef, llvm::Annotations::Range);
 };
 
 MATCHER_P2(FileWithContents, FileName, Contents, "") {
   return arg.first() == FileName && arg.second == Contents;

clang-tools-extra/clangd/unittests/tweaks/TweakTesting.cpp

 //===-- TweakTesting.cpp ------------------------------------------------*-===//
 //
 // 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 "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>
 #include <string>
 
 namespace clang {
 namespace clangd {
 namespace {
SelectionTree::createEach(AST.getASTContext(), AST.getTokens(), Range.Begin,
                               } else {
                                 llvm::consumeError(T.takeError());
                                 return false;
                               }
                             });
   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,
                              llvm::StringMap<std::string> *EditedFiles) const {
   std::string WrappedCode = wrap(Context, MarkedCode);
   llvm::Annotations Input(WrappedCode);
   TestTU TU;
   TU.Filename = std::string(FileName);
else {
                          "provided a nullptr for EditedFiles.";
       else
         EditedFiles->insert_or_assign(It.first(), Unwrapped.str());
     }
   }
   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.
   Range.Begin += AST.second;
   Range.End += AST.second;
   auto Result = applyTweak(
       AST.first, Range, TweakID, Index.get(),
       &AST.first.getSourceManager().getFileManager().getVirtualFileSystem());
   // We only care if prepare() succeeded, but must handle Errors.
   if (Result && !*Result)
     consumeError(Result->takeError());
   return Result.has_value();
 }
 
 TweakTest::WrappedAST TweakTest::build(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.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();
 }
 
 std::string TweakTest::decorate(llvm::StringRef Code,
                                 llvm::Annotations::Range Range) {
   return (Code.substr(0, Range.Begin) + "[[" +
           Code.substr(Range.Begin, Range.End - Range.Begin) + "]]" +
           Code.substr(Range.End))
       .str();
 }
 
 } // namespace clangd
 } // namespace clang

clang/include/clang/AST/CXXInheritance.h

 //===- CXXInheritance.h - C++ Inheritance -----------------------*- 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
 //
 //===----------------------------------------------------------------------===//
 //
public:
   // class subobjects.
   void add(const OverridingMethods &Other);
 
   // Replace all overriding virtual methods in all subobjects with the
   // given virtual method.
   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> {};
 
 /// A set of all the primary bases for a class.
 class CXXIndirectPrimaryBaseSet
   : public llvm::SmallSet<const CXXRecordDecl*, 32> {};
 
 inline bool

clang/include/clang/AST/PrettyPrinter.h

 //===--- PrettyPrinter.h - Classes for aiding with AST printing -*- 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
 //
 //===----------------------------------------------------------------------===//
 //
PrintingPolicy(const LangOptions &LO)
         SplitTemplateClosers(!LO.CPlusPlus11), TerseOutput(false),
         PolishForDeclaration(false), Half(LO.Half),
         MSWChar(LO.MicrosoftExt && !LO.WChar), IncludeNewlines(true),
         MSVCFormatting(false), ConstantsAsWritten(false),
         SuppressImplicitBase(false), FullyQualifiedName(false),
         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
   /// construct). This should not be used if a real LangOptions object is
   /// available.
   void adjustForCPlusPlus() {
     SuppressTagKeyword = true;
     Bool = true;
struct PrintingPolicy {
   /// Whether to print the entire array initializers, especially on non-type
   /// template parameters, no matter how many elements there are.
   unsigned EntireContentsOfLargeArray : 1;
 
   /// Whether to print enumerator non-type template parameters with a matching
   /// 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;
 };
 
 } // end namespace clang
 
 #endif

clang/lib/AST/TypePrinter.cpp

 //===- TypePrinter.cpp - Pretty-Print Clang Types -------------------------===//
 //
 // 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
 //
 //===----------------------------------------------------------------------===//
 //
if (hasDynamicExceptionSpec()) {
     }
   }
 }
 
 void TypePrinter::printFunctionProtoBefore(const FunctionProtoType *T,
                                            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 << '(';
   }
 }
 
 StringRef clang::getParameterABISpelling(ParameterABI ABI) {
   switch (ABI) {
   case ParameterABI::Ordinary:
     llvm_unreachable("asking for spelling of ordinary parameter ABI");
   case ParameterABI::SwiftContext:
     return "swift_context";
   case ParameterABI::SwiftAsyncContext:
     return "swift_async_context";
   case ParameterABI::SwiftErrorResult:
     return "swift_error_result";
   case ParameterABI::SwiftIndirectResult:
     return "swift_indirect_result";
   }
   llvm_unreachable("bad parameter ABI kind");
 }
 
 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);
 
   OS << '(';
   {
     ParamPolicyRAII ParamPolicy(Policy);
     for (unsigned i = 0, e = T->getNumParams(); i != e; ++i) {
       if (i) OS << ", ";