[clang-tidy] Detect and fix calls to grand-...parent virtual methods instead of calls to parent's virtual methods

[zinovy.nis]

Summary

Warns if one calls grand-..parent's virtual method in child's virtual method instead of overridden parent's. Can automatically fix such cases by retargeting calls to correct class.

class A {
...
int virtual foo() {...}
}

class B: public A {
int foo() override {...}
}

class C: public B {
...
int foo() override {... A::foo()...} // warning: qualified name A::foo refers to a member overridden in subclass; did you mean 'B'? [bugprone-parent-virtual-call]
}

[malcolm.parsons]

clang-tidy/bugprone/ParentVirtualCallCheck.cpp

153

What does this do for templated parent classes?

docs/clang-tidy/checks/bugprone-parent-virtual-call.rst

7

FIXME.

test/clang-tidy/bugprone-parent-virtual-call.cpp

103

typo: choice

[zinovy.nis]

clang-tidy/bugprone/ParentVirtualCallCheck.cpp

153

Please see my last test case in updated revision.

docs/clang-tidy/checks/bugprone-parent-virtual-call.rst

7

Oops. Thanks!

test/clang-tidy/bugprone-parent-virtual-call.cpp

103

Thanks!

[Eugene.Zelenko]

clang-tidy/bugprone/ParentVirtualCallCheck.cpp

27

Please make this function static and remove anonymous namespace.

41

Please make this function static and remove anonymous namespace.

49

Please make this function static and remove anonymous namespace.

67

Please make this function static and remove anonymous namespace.

92

Please remove this line.

docs/ReleaseNotes.rst

88

Please make description here and first statement in documentation same.

docs/clang-tidy/checks/bugprone-parent-virtual-call.rst

10

Please add .. code-block:: c++

[malcolm.parsons]

clang-tidy/bugprone/ParentVirtualCallCheck.cpp

153

Also, a templated parent class of a templated class.

template <class T>
class DF : public BF<T> {
public:
  int virt_1() override { return A::virt_1(); }
  // CHECK-MESSAGES: :[[@LINE-1]]:34: warning: 'A::virt_1' is a grand-parent's method, not parent's. Did you mean 'BF<T>'?
  // CHECK-FIXES:  int virt_1() override { return BF<T>::virt_1(); }
};

[Eugene.Zelenko]

clang-tidy/bugprone/ParentVirtualCallCheck.cpp

13

Please run Clang-format and remove empty line between headers.

[malcolm.parsons]

test/clang-tidy/bugprone-parent-virtual-call.cpp

126

Does this fixit compile?

[zinovy.nis]

test/clang-tidy/bugprone-parent-virtual-call.cpp

126

Thanks for pointing. Hope I fixed this case in the latest patch. Please have a look at it.

[Eugene.Zelenko]

clang-tidy/bugprone/ParentVirtualCallCheck.cpp

48

Please remove empty line.

117

No need for brackets.

[malcolm.parsons]

test/clang-tidy/bugprone-parent-virtual-call.cpp

116

There isn't an F in scope.
The base class isn't a dependent type here, so no template params are needed.

[zinovy.nis]

test/clang-tidy/bugprone-parent-virtual-call.cpp

116

Finally I decided not to propose a fix for templated parent.

[aaron.ballman]

clang-tidy/bugprone/ParentVirtualCallCheck.cpp

25–26

You can drop these asserts.

29–30

You can use llvm::find_if() for the range-based API instead.

31–33

Factor out Base.getType()->getAs() and use the pointer in both the assert and the comparison.

40–41

And these asserts as well.

I don't think this function adds much value given that it's a one-liner; I'd hoist its functionality out to the caller.

57–61

This should be getNameAsString()

102–104

Convert this to an else if and elide the braces.

113

Do not use auto here as the type is not spelled out in the intialization.

129

The diagnostic should not contain punctuation aside from the question mark.

Also, the diagnostic uses some novel terminology in "grandparent's method". How about: "qualified function name %0 refers to a function that is not defined by a direct base class; did you mean %1?"

test/clang-tidy/bugprone-parent-virtual-call.cpp

128

What should happen in this case?

struct Base {
  virtual void f() {}
};

struct Intermediary : Base {
};

struct Derived : Intermediary {
  void f() override {
    Base::f(); // I would expect this not to diagnose
  }
};

This would be a reasonable test case to add.

[alexfh]

clang-tidy/bugprone/ParentVirtualCallCheck.cpp

25–26

... and make arguments const references ;)

45

std::list is almost never a good choice due to large overhead, poor locality of the data, etc. Use std::vector instead.

54

Pull this to a variable to avoid repetition.

[zinovy.nis]

clang-tidy/bugprone/ParentVirtualCallCheck.cpp

129

"is not defined by a direct base class" is not a correct proposal. Issues that this check finds are all about calling a grand-parent's method instead of parent's whether base class defines this method or not.

How about

Qualified function name %0 refers to a function not from a direct base class; did you mean %1?

?

[zinovy.nis]

test/clang-tidy/bugprone-parent-virtual-call.cpp

128

Please see // Test virtual method is diagnosted although not overridden in parent. section in the test file.

[aaron.ballman]

test/clang-tidy/bugprone-parent-virtual-call.cpp

106

Typo: diagnosted -> diagnosed

114

This seems like a false positive to me. Yes, the virtual function is technically exposed in BI, but why is the programmer obligated to call that one rather than the one from A, which is written in the source?

[zinovy.nis]

test/clang-tidy/bugprone-parent-virtual-call.cpp

106

oops, thanks.

114

IMHO it's a matter of safety. Today virt_1() is not overridden in BI, but tomorrow someone will implement BI::virt_1() and it will silently lead to bugs or whatever.

[aaron.ballman]

test/clang-tidy/bugprone-parent-virtual-call.cpp

114

If tomorrow someone implements BI::virt_1(), then the check will start diagnosing at that point.

[zinovy.nis]

test/clang-tidy/bugprone-parent-virtual-call.cpp

114

Correct, but anyway I don't think it's a problem.

[aaron.ballman]

clang-tidy/bugprone/ParentVirtualCallCheck.cpp

108–109

You can get rid of this and just pass in the Decl * directly while using %q in the format string.

129

Not keen on "from", but we could use "qualified function name %q0 refers to a function that is not explicitly declared by a direct base class; did you mean %1?"

test/clang-tidy/bugprone-parent-virtual-call.cpp

114

I'd prefer to see this changed to not diagnose. I don't relish the idea of explaining why that code diagnoses as it stands.

[zinovy.nis]

test/clang-tidy/bugprone-parent-virtual-call.cpp

114

Done. Now the check looks for a most recent class with the overridden method. Please review this change.

[zinovy.nis]

clang-tidy/bugprone/ParentVirtualCallCheck.cpp

129

Changed to

qualified name %0 refers to a member which was overridden in subclass%1"

[zinovy.nis]

Hope I satisfied all your requests now.

[zinovy.nis]

Gentle ping)

[alexfh]

clang-tidy/bugprone/ParentVirtualCallCheck.cpp

25

http://llvm.org/docs/CodingStandards.html says: "Function names should be verb phrases (as they represent actions), and command-like function should be imperative. The name should be camel case, and start with a lower case letter (e.g. openFile() or isFoo())."

29

Is Parent the only capture? I'd prefer it to be explicit instead of automatic.

[aaron.ballman]

clang-tidy/bugprone/ParentVirtualCallCheck.cpp

40

Local variables should be capitalized.

41–42

Please add the const qualifier to both of these.

43–44

Do not use auto here.

50–54

Do not use auto here as the type is not explicitly spelled out in the initialization. Also, please do not name the local variable Type as 1) it seem to be a declaration, and 2) Type is a type name.

55

Please add a message to your asserts so that if they trigger, the user gets something more helpful as the diagnostic. Same elsewhere.

90

Please put appropriate explicit qualifiers on all of the auto uses (here and elsewhere).

94

Instead of dyn_cast followed by assert(), you should just use cast<>, but only if you're certain you cannot get anything other than a CXXMethodDecl out of the expression.

99

Do not use auto here.

129

Please use %q0 rather than calling your custom qualified name function. The diagnostics engine will handle printing the qualified name appropriately for you.

docs/clang-tidy/checks/bugprone-parent-virtual-call.rst

7

This formulation is a bit odd because of the grand-...parent part. I would reword to be more along the lines of: detects calls to qualified function names that refer to a function that is not explicitly declared by a direct base class.

[zinovy.nis]

Aaron, I applied the changes you suggest.

I also found and fixed a new case when grandparent method is called via 'typedef'ed or 'using' type. There's also a new test (class C2) for it.

[zinovy.nis]

camelCase last minute fix.

[zinovy.nis]

docs/clang-tidy/checks/bugprone-parent-virtual-call.rst

7

that is not explicitly declared by a direct base class.

In fact, it detects calls to qualified function names

that explicitly overridden in subclasses.

[Eugene.Zelenko]

docs/ReleaseNotes.rst

85

Please rebase from trunk and use :doc: for link.

[malcolm.parsons]

Please add a test where the parent class is in a differently named namespace.

[alexfh]

docs/clang-tidy/checks/bugprone-parent-virtual-call.rst

27

The formatting looks weird. Could you move warning: .* to the start of the next line and put the whole message on the same line?

[zinovy.nis]

• Rebased.
• Added 1 more test for namespaced base clases
• Fixed minor issues.

[alexfh]

docs/clang-tidy/checks/bugprone-parent-virtual-call.rst

27

Could you address the "and put the whole message on the same line" part as well?

Please also put // before warning: and ...s to make the example look more like syntactically valid code.

[zinovy.nis]

• Beautify bugprone-parent-virtual-call.rst

[zinovy.nis]

• Updated warning message to

warning: qualified name 'A::foo' refers to a member overridden in subclass; did you mean 'B'? [bugprone-parent-virtual-call]

[alexfh]

A few more nits.

clang-tidy/bugprone/ParentVirtualCallCheck.cpp

114–117

I suggest using 1. range for loop; 2. std::string::append instead of operator+/operator+=. Not that performance is extremely important here, but it's also cleaner, IMO:

std::string ParentsStr = "'";
for (const CXXRecordDecl *Parent : Parents) {
  if (ParentsStr.size() > 1)
    ParentsStr.append("' or '");
  ParentsStr.append(getNameAsString(Parent));
}
ParentsStr.append("'");

126

Please remove the parentheses.

test/clang-tidy/bugprone-parent-virtual-call.cpp

32

Specifying each unique message completely once should be enough to detect typos in it (e.g. missing or extra spaces or quotes around placeholders). In other messages let's omit repeated static parts to make the test a bit more easier to read, e.g.:

// CHECK-MESSAGES: :[[@LINE-1]]:34: warning: qualified name 'A::virt_1' {{.*}}; did you mean 'B'?

[zinovy.nis]

• Cosmetic fixes.

[zinovy.nis]

BTW, I've recently found few dozens of issues in the Chromium code with my check.
For ex.:

browser/src/cc/layers/nine_patch_layer_impl.cc:89:51: warning: qualified name 'cc::LayerImpl::LayerAsJson' refers to a member overridden in subclass; did you mean 'cc::UIResourceLayerImpl'? [bugprone-parent-virtual-call]
  std::unique_ptr<base::DictionaryValue> result = LayerImpl::LayerAsJson();
                                                  ^~~~~~~~~~~~~~~~~~~~~~
                                                  cc::UIResourceLayerImpl::

[zinovy.nis]

• Minor cosmetic fix: use lexical representation instead of semanic for printing callee string. It helps to print class names hidden behind 'typedef' and 'using':

using A1 = A;
typedef A A2;

class C2 : public B {
public:
  int virt_1() override { return A1::virt_1() + A2::virt_1(); }
  // BEFORE: warning: qualified name 'A::virt_1'...
  // BEFORE: warning: qualified name 'A::virt_1'...

  //  AFTER: warning: qualified name 'A1::virt_1'...
  //  AFTER: warning: qualified name 'A2::virt_1'...
};

[alexfh]

clang-tidy/bugprone/ParentVirtualCallCheck.cpp

74

That thread doesn't seem to be particularly useful. Lexer::getSourceText(CharSourceRange::getTokenRange(E.getSourceRange()), SM, CorrectLangOpts, 0); (or clang::tooling::fixit::getText) should work well in macro-free cases. When there are macros, Lexer::makeFileCharRange may help to find a contiguous range that corresponds to a SourceRange (and when it doesn't, this usually means that there's no such text range).

Both of these don't remove whitespace, but why would you do that anyway? If really needed, this can be done outside.

78

Any reason to not use the actual LangOptions? Substituting it with the default may result in incorrect handling of certain tokens.

80

Consider using llvm::remove_if.

81

Text.at(Text.size() - 1) seems to be the same as Text.back(). And what does this condition do, btw?

82–84

That seems to be a bad thing to fall back to - it uses getCharRange which will drop the last token.

145–147

Please remove the commented out code.

test/clang-tidy/bugprone-parent-virtual-call.cpp

32

I'd also remove the [bugprone-parent-virtual-call] part from repeated CHECK-patterns.

[zinovy.nis]

• Switched to use 'tooling::fixit::getText' for qualified names.
• Shortened test code with {{.*}}.

[alexfh]

LG

[zinovy.nis]

Alexander, thanks for your patience and comments :-)

clang-tidy/bugprone/BugproneTidyModule.cpp

 #include "IntegerDivisionCheck.h"
 #include "LambdaFunctionNameCheck.h"
 #include "MacroParenthesesCheck.h"
 #include "MacroRepeatedSideEffectsCheck.h"
 #include "MisplacedOperatorInStrlenInAllocCheck.h"
 #include "MisplacedWideningCastCheck.h"
 #include "MoveForwardingReferenceCheck.h"
 #include "MultipleStatementMacroCheck.h"
+#include "ParentVirtualCallCheck.h"
 #include "SizeofContainerCheck.h"
 #include "SizeofExpressionCheck.h"
 #include "StringConstructorCheck.h"
 #include "StringIntegerAssignmentCheck.h"
 #include "StringLiteralWithEmbeddedNulCheck.h"
 #include "SuspiciousEnumUsageCheck.h"
 #include "SuspiciousMemsetUsageCheck.h"
 #include "SuspiciousMissingCommaCheck.h"
void addCheckFactories(ClangTidyCheckFactories &CheckFactories) override {
     CheckFactories.registerCheck<MisplacedOperatorInStrlenInAllocCheck>(
         "bugprone-misplaced-operator-in-strlen-in-alloc");
     CheckFactories.registerCheck<MisplacedWideningCastCheck>(
         "bugprone-misplaced-widening-cast");
     CheckFactories.registerCheck<MoveForwardingReferenceCheck>(
         "bugprone-move-forwarding-reference");
     CheckFactories.registerCheck<MultipleStatementMacroCheck>(
         "bugprone-multiple-statement-macro");
+    CheckFactories.registerCheck<ParentVirtualCallCheck>(
+        "bugprone-parent-virtual-call");
     CheckFactories.registerCheck<SizeofContainerCheck>(
         "bugprone-sizeof-container");
     CheckFactories.registerCheck<SizeofExpressionCheck>(
         "bugprone-sizeof-expression");
     CheckFactories.registerCheck<StringConstructorCheck>(
         "bugprone-string-constructor");
     CheckFactories.registerCheck<StringIntegerAssignmentCheck>(
         "bugprone-string-integer-assignment");

clang-tidy/bugprone/CMakeLists.txt

add_clang_library(clangTidyBugproneModule
   IntegerDivisionCheck.cpp
   LambdaFunctionNameCheck.cpp
   MacroParenthesesCheck.cpp
   MacroRepeatedSideEffectsCheck.cpp
   MisplacedOperatorInStrlenInAllocCheck.cpp
   MisplacedWideningCastCheck.cpp
   MoveForwardingReferenceCheck.cpp
   MultipleStatementMacroCheck.cpp
+  ParentVirtualCallCheck.cpp
   SizeofContainerCheck.cpp
   SizeofExpressionCheck.cpp
   StringConstructorCheck.cpp
   StringIntegerAssignmentCheck.cpp
   StringLiteralWithEmbeddedNulCheck.cpp
   SuspiciousEnumUsageCheck.cpp
   SuspiciousMemsetUsageCheck.cpp
   SuspiciousMissingCommaCheck.cpp

clang-tidy/bugprone/ParentVirtualCallCheck.h

+//===--- ParentVirtualCallCheck.h - clang-tidy-------------------*- C++ -*-===//
+//
+//                     The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===----------------------------------------------------------------------===//
+
+#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_BUGPRONE_PARENTVIRTUALCALLCHECK_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_BUGPRONE_PARENTVIRTUALCALLCHECK_H
+
+#include "../ClangTidy.h"
+
+namespace clang {
+namespace tidy {
+namespace bugprone {
+
+/// Finds calls to grand..-parent virtual methods instead of parent's.
+///
+/// For the user-facing documentation see:
+/// http://clang.llvm.org/extra/clang-tidy/checks/bugprone-parent-virtual-call.html
+class ParentVirtualCallCheck : public ClangTidyCheck {
+public:
+  ParentVirtualCallCheck(StringRef Name, ClangTidyContext *Context)
+      : ClangTidyCheck(Name, Context) {}
+  void registerMatchers(ast_matchers::MatchFinder *Finder) override;
+  void check(const ast_matchers::MatchFinder::MatchResult &Result) override;
+};
+
+} // namespace bugprone
+} // namespace tidy
+} // namespace clang
+
+#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_BUGPRONE_PARENTVIRTUALCALLCHECK_H

clang-tidy/bugprone/ParentVirtualCallCheck.cpp

+//===--- ParentVirtualCallCheck.cpp - clang-tidy---------------------------===//
+//
+//                     The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===----------------------------------------------------------------------===//
+
+#include "ParentVirtualCallCheck.h"
+#include "clang/AST/ASTContext.h"
+#include "clang/ASTMatchers/ASTMatchFinder.h"
+#include "llvm/ADT/STLExtras.h"
+#include "llvm/ADT/SmallVector.h"
+
+using namespace clang::ast_matchers;
+
+namespace clang {
+namespace tidy {
+namespace bugprone {
+
+using BasesVector = llvm::SmallVector<const CXXRecordDecl *, 5>;
+
+static bool isParentOf(const CXXRecordDecl &Parent,
+                       const CXXRecordDecl &ThisClass) {
+  if (Parent.getCanonicalDecl() == ThisClass.getCanonicalDecl())
+    return true;
+  const auto ClassIter = llvm::find_if(ThisClass.bases(), [=](auto &Base) {
+    auto *BaseDecl = Base.getType()->getAsCXXRecordDecl();
+    assert(BaseDecl);
+    return Parent.getCanonicalDecl() == BaseDecl->getCanonicalDecl();
+  });
+  return ClassIter != ThisClass.bases_end();
+}
+
+static BasesVector getParentsByGrandParent(const CXXRecordDecl &GrandParent,
+                                           const CXXRecordDecl &ThisClass,
+                                           const CXXMethodDecl &MemberDecl) {
+  BasesVector Result;
+  for (const auto &Base : ThisClass.bases()) {
+    const auto *BaseDecl = Base.getType()->getAsCXXRecordDecl();
+    const CXXMethodDecl *ActualMemberDecl =
+        MemberDecl.getCorrespondingMethodInClass(BaseDecl);
+    if (!ActualMemberDecl)
+      continue;
+    // TypePtr is the nearest base class to ThisClass between ThisClass and
+    // GrandParent, where MemberDecl is overridden. TypePtr is the class the
+    // check proposes to fix to.
+    const Type *TypePtr =
+        ActualMemberDecl->getThisType(ActualMemberDecl->getASTContext())
+            .getTypePtr();
+    const CXXRecordDecl *RecordDeclType = TypePtr->getPointeeCXXRecordDecl();
+    assert(RecordDeclType && "TypePtr is not a pointer to CXXRecordDecl!");
+    if (RecordDeclType->getCanonicalDecl()->isDerivedFrom(&GrandParent))
+      Result.emplace_back(RecordDeclType);
+  }
+
+  return Result;
+}
+
+static std::string getNameAsString(const NamedDecl *Decl) {
+  std::string QualName;
+  llvm::raw_string_ostream OS(QualName);
+  PrintingPolicy PP(Decl->getASTContext().getPrintingPolicy());
+  PP.SuppressUnwrittenScope = true;
+  Decl->printQualifiedName(OS, PP);
+  return OS.str();
+}
+
+void ParentVirtualCallCheck::registerMatchers(MatchFinder *Finder) {
+  Finder->addMatcher(
+      cxxMemberCallExpr(
+          callee(memberExpr(hasDescendant(implicitCastExpr(
+                                hasImplicitDestinationType(pointsTo(
+                                    type(anything()).bind("castToType"))),
+                                hasSourceExpression(cxxThisExpr(hasType(
+                                    type(anything()).bind("thisType")))))))
+                     .bind("member")),
+          callee(cxxMethodDecl(isVirtual())))
+          .bind("call"),
+      this);
+}
+
+void ParentVirtualCallCheck::check(const MatchFinder::MatchResult &Result) {
+  const auto *MatchedDecl = Result.Nodes.getNodeAs<CXXMemberCallExpr>("call");
+  assert(MatchedDecl);
+
+  const auto *Member = Result.Nodes.getNodeAs<MemberExpr>("member");
+  assert(Member);
+
+  const auto *MemberDecl = cast<CXXMethodDecl>(Member->getMemberDecl());
+
+  const auto *ThisTypePtr = Result.Nodes.getNodeAs<PointerType>("thisType");
+  assert(ThisTypePtr);
+
+  const auto *ThisType = ThisTypePtr->getPointeeCXXRecordDecl();
+  assert(ThisType);
+
+  const auto *CastToTypePtr = Result.Nodes.getNodeAs<Type>("castToType");
+  assert(CastToTypePtr);
+
+  const auto *CastToType = CastToTypePtr->getAsCXXRecordDecl();
+  assert(CastToType);
+
+  if (isParentOf(*CastToType, *ThisType))
+    return;
+
+  const BasesVector Parents =
+      getParentsByGrandParent(*CastToType, *ThisType, *MemberDecl);
+
+  if (Parents.empty())
+    return;
+
+  auto ParentIter = Parents.begin();
+  std::string ParentsStr = "'" + getNameAsString(*ParentIter) + "'";
+  for (++ParentIter; ParentIter != Parents.end(); ++ParentIter)
+    ParentsStr += " or '" + getNameAsString(*ParentIter) + "'";
+
+  if (!Member->getQualifier())
+    return;
+
+  assert(Member->getQualifierLoc().getSourceRange().getBegin().isValid());
+  auto Diag = diag(Member->getQualifierLoc().getSourceRange().getBegin(),
+                   "qualified name %q0 refers to a member overridden "
+                   "in subclass%1; did you mean %2?")
+              << (MatchedDecl->getCalleeDecl()->getAsFunction())
+              << (Parents.size() > 1 ? "es" : "") << ParentsStr;
+
+  // Propose a fix if there's only one parent class...
+  if (Parents.size() == 1 &&
+      // ...unless parent class is templated
+      !isa<ClassTemplateSpecializationDecl>(Parents.front()))
+    Diag << FixItHint::CreateReplacement(
+        Member->getQualifierLoc().getSourceRange(),
+        getNameAsString(Parents.front()) + "::");
+}
+
+} // namespace bugprone
+} // namespace tidy
+} // namespace clang

docs/ReleaseNotes.rst

 
 - New :doc:`cppcoreguidelines-avoid-goto
   <clang-tidy/checks/cppcoreguidelines-avoid-goto>` check
 
   The usage of ``goto`` for control flow is error prone and should be replaced
   with looping constructs. Every backward jump is rejected. Forward jumps are
   only allowed in nested loops.
 
+- New :doc:`bugprone-parent-virtual-call
+  <clang-tidy/checks/bugprone-parent-virtual-call>` check
+
+  Detects and fixes calls to grand-...parent virtual methods instead of calls
+  to overridden parent's virtual methods.
+
 - New :doc:`fuchsia-multiple-inheritance
   <clang-tidy/checks/fuchsia-multiple-inheritance>` check
 
   Warns if a class inherits from multiple classes that are not pure virtual.
 
 - New :doc:`abseil-string-find-startswith
   <clang-tidy/checks/abseil-string-find-startswith>` check
 

docs/clang-tidy/checks/bugprone-parent-virtual-call.rst

+.. title:: clang-tidy - bugprone-parent-virtual-call
+
+bugprone-parent-virtual-call
+============================
+
+Detects and fixes calls to grand-...parent virtual methods instead of calls
+to overridden parent's virtual methods.
+
+.. code-block:: c++
+
+  class A {
+    int virtual foo() {...}
+  };
+
+  class B: public A {
+    int foo() override {...}
+  };
+
+  class C: public B {
+    int foo() override { A::foo(); }
+  //                     ^^^^^^^^
+  // warning: qualified name A::foo refers to a member overridden in subclass; did you mean 'B'?  [bugprone-parent-virtual-call]
+  };

docs/clang-tidy/checks/list.rst

.. toctree::
    bugprone-integer-division
    bugprone-lambda-function-name
    bugprone-macro-parentheses
    bugprone-macro-repeated-side-effects
    bugprone-misplaced-operator-in-strlen-in-alloc
    bugprone-misplaced-widening-cast
    bugprone-move-forwarding-reference
    bugprone-multiple-statement-macro
+   bugprone-parent-virtual-call
    bugprone-sizeof-container
    bugprone-sizeof-expression
    bugprone-string-constructor
    bugprone-string-integer-assignment
    bugprone-string-literal-with-embedded-nul
    bugprone-suspicious-enum-usage
    bugprone-suspicious-memset-usage
    bugprone-suspicious-missing-comma

test/clang-tidy/bugprone-parent-virtual-call.cpp

+// RUN: %check_clang_tidy %s bugprone-parent-virtual-call %t
+
+extern int foo();
+
+class A {
+public:
+  A() = default;
+  virtual ~A() = default;
+
+  virtual int virt_1() { return foo() + 1; }
+  virtual int virt_2() { return foo() + 2; }
+
+  int non_virt() { return foo() + 3; }
+  static int stat() { return foo() + 4; }
+};
+
+class B : public A {
+public:
+  B() = default;
+
+  // Nothing to fix: calls to direct parent.
+  int virt_1() override { return A::virt_1() + 3; }
+  int virt_2() override { return A::virt_2() + 4; }
+};
+
+class C : public B {
+public:
+  int virt_1() override { return A::virt_1() + B::virt_1(); }
+  // CHECK-MESSAGES: :[[@LINE-1]]:34: warning: qualified name 'A::virt_1' refers to a member overridden in subclass; did you mean 'B'? [bugprone-parent-virtual-call]
+  // CHECK-FIXES:  int virt_1() override { return B::virt_1() + B::virt_1(); }
+  int virt_2() override { return A::virt_1() + B::virt_1(); }
+  // CHECK-MESSAGES: :[[@LINE-1]]:34: warning: qualified name 'A::virt_1' refers to a member overridden in subclass; did you mean 'B'? [bugprone-parent-virtual-call]
+  // CHECK-FIXES:  int virt_2() override { return B::virt_1() + B::virt_1(); }
+
+  // Test that non-virtual and static methods are not affected by this cherker.
+  int method_c() { return A::stat() + A::non_virt(); }
+};
+
+// Check aliased type names
+using A1 = A;
+typedef A A2;
+
+class C2 : public B {
+public:
+  int virt_1() override { return A1::virt_1() + A2::virt_1(); }
+  // CHECK-MESSAGES: :[[@LINE-1]]:34: warning: qualified name 'A::virt_1' refers to a member overridden in subclass; did you mean 'B'? [bugprone-parent-virtual-call]
+  // CHECK-MESSAGES: :[[@LINE-2]]:49: warning: qualified name 'A::virt_1' refers to a member overridden in subclass; did you mean 'B'? [bugprone-parent-virtual-call]
+  // CHECK-FIXES:  int virt_1() override { return B::virt_1() + B::virt_1(); }
+};
+
+// Test that the check affects grand-grand..-parent calls too.
+class D : public C {
+public:
+  int virt_1() override { return A::virt_1() + B::virt_1() + D::virt_1(); }
+  // CHECK-MESSAGES: :[[@LINE-1]]:34: warning: qualified name 'A::virt_1' refers to a member overridden in subclass; did you mean 'C'? [bugprone-parent-virtual-call]
+  // CHECK-MESSAGES: :[[@LINE-2]]:48: warning: qualified name 'B::virt_1' refers to a member overridden in subclass; did you mean 'C'? [bugprone-parent-virtual-call]
+  // CHECK-FIXES:  int virt_1() override { return C::virt_1() + C::virt_1() + D::virt_1(); }
+  int virt_2() override { return A::virt_1() + B::virt_1() + D::virt_1(); }
+  // CHECK-MESSAGES: :[[@LINE-1]]:34: warning: qualified name 'A::virt_1' refers to a member overridden in subclass; did you mean 'C'? [bugprone-parent-virtual-call]
+  // CHECK-MESSAGES: :[[@LINE-2]]:48: warning: qualified name 'B::virt_1' refers to a member overridden in subclass; did you mean 'C'? [bugprone-parent-virtual-call]
+  // CHECK-FIXES:  int virt_2() override { return C::virt_1() + C::virt_1() + D::virt_1(); }
+};
+
+// Test classes in namespaces.
+namespace {
+class BN : public A {
+public:
+  int virt_1() override { return A::virt_1() + 3; }
+  int virt_2() override { return A::virt_2() + 4; }
+};
+} // namespace
+
+namespace N1 {
+class A {
+public:
+  A() = default;
+  virtual int virt_1() { return foo() + 1; }
+  virtual int virt_2() { return foo() + 2; }
+};
+} // namespace N1
+
+namespace N2 {
+class BN : public N1::A {
+public:
+  int virt_1() override { return A::virt_1() + 3; }
+  int virt_2() override { return A::virt_2() + 4; }
+};
+} // namespace N2
+
+class CN : public BN {
+public:
+  int virt_1() override { return A::virt_1() + BN::virt_1(); }
+  // CHECK-MESSAGES: :[[@LINE-1]]:34: warning: qualified name 'A::virt_1' refers to a member overridden in subclass; did you mean 'BN'? [bugprone-parent-virtual-call]
+  // CHECK-FIXES:  int virt_1() override { return BN::virt_1() + BN::virt_1(); }
+  int virt_2() override { return A::virt_1() + BN::virt_1(); }
+  // CHECK-MESSAGES: :[[@LINE-1]]:34: warning: qualified name 'A::virt_1' refers to a member overridden in subclass; did you mean 'BN'? [bugprone-parent-virtual-call]
+  // CHECK-FIXES:  int virt_2() override { return BN::virt_1() + BN::virt_1(); }
+};
+
+class CNN : public N2::BN {
+public:
+  int virt_1() override { return N1::A::virt_1() + N2::BN::virt_1(); }
+  // CHECK-MESSAGES: :[[@LINE-1]]:34: warning: qualified name 'N1::A::virt_1' refers to a member overridden in subclass; did you mean 'N2::BN'? [bugprone-parent-virtual-call]
+  // CHECK-FIXES:  int virt_1() override { return N2::BN::virt_1() + N2::BN::virt_1(); }
+  int virt_2() override { return N1::A::virt_1() + N2::BN::virt_1(); }
+  // CHECK-MESSAGES: :[[@LINE-1]]:34: warning: qualified name 'N1::A::virt_1' refers to a member overridden in subclass; did you mean 'N2::BN'? [bugprone-parent-virtual-call]
+  // CHECK-FIXES:  int virt_2() override { return N2::BN::virt_1() + N2::BN::virt_1(); }
+};
+
+// Test multiple inheritance fixes
+class AA {
+public:
+  AA() = default;
+  virtual ~AA() = default;
+
+  virtual int virt_1() { return foo() + 1; }
+  virtual int virt_2() { return foo() + 2; }
+
+  int non_virt() { return foo() + 3; }
+  static int stat() { return foo() + 4; }
+};
+
+class BB_1 : virtual public AA {
+public:
+  BB_1() = default;
+
+  // Nothing to fix: calls to parent.
+  int virt_1() override { return AA::virt_1() + 3; }
+  int virt_2() override { return AA::virt_2() + 4; }
+};
+
+class BB_2 : virtual public AA {
+public:
+  BB_2() = default;
+  int virt_1() override { return AA::virt_1() + 3; }
+};
+
+class CC : public BB_1, public BB_2 {
+public:
+  int virt_1() override { return AA::virt_1() + 3; }
+  // CHECK-MESSAGES: :[[@LINE-1]]:34: warning: qualified name 'AA::virt_1' refers to a member overridden in subclasses; did you mean 'BB_1' or 'BB_2'? [bugprone-parent-virtual-call]
+  // No fix available due to multiple choice of parent class.
+};
+
+// Test that virtual method is not diagnosed as not overridden in parent.
+class BI : public A {
+public:
+  BI() = default;
+};
+
+class CI : BI {
+  int virt_1() override { return A::virt_1(); }
+};
+
+// Test templated classes.
+template <class F> class BF : public A {
+public:
+  int virt_1() override { return A::virt_1() + 3; }
+};
+
+// Test templated parent class.
+class CF : public BF<int> {
+public:
+  int virt_1() override { return A::virt_1(); }
+  // CHECK-MESSAGES: :[[@LINE-1]]:34: warning: qualified name 'A::virt_1' refers to a member overridden in subclass; did you mean 'BF'? [bugprone-parent-virtual-call]
+};
+
+// Test both templated class and its parent class.
+template <class F> class DF : public BF<F> {
+public:
+  DF() = default;
+  int virt_1() override { return A::virt_1(); }
+  // CHECK-MESSAGES: :[[@LINE-1]]:34: warning: qualified name 'A::virt_1' refers to a member overridden in subclass; did you mean 'BF'? [bugprone-parent-virtual-call]
+};
+
+// Just to instantiate DF<F>.
+int bar() { return (new DF<int>())->virt_1(); }