#include "MisplacedWideningCastCheck.h"
#include "MoveForwardingReferenceCheck.h"
#include "MultipleStatementMacroCheck.h"
+#include "ParentVirtualCallCheck.h"
#include "SizeofContainerCheck.h"
#include "SizeofExpressionCheck.h"
#include "StringConstructorCheck.h"
"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>(
MisplacedWideningCastCheck.cpp
MoveForwardingReferenceCheck.cpp
MultipleStatementMacroCheck.cpp
+ ParentVirtualCallCheck.cpp
SizeofContainerCheck.cpp
SizeofExpressionCheck.cpp
StringConstructorCheck.cpp
--- /dev/null
+//===--- 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 "clang/Tooling/FixIt.h"
+#include "llvm/ADT/STLExtras.h"
+#include "llvm/ADT/SmallVector.h"
+#include <cctype>
+
+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();
+}
+
+// Returns E as written in the source code. Used to handle 'using' and
+// 'typedef'ed names of grand-parent classes.
+static std::string getExprAsString(const clang::Expr &E,
+ clang::ASTContext &AC) {
+ std::string Text = tooling::fixit::getText(E, AC).str();
+ Text.erase(llvm::remove_if(Text, std::isspace), Text.end());
+ return Text;
+}
+
+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);
+
+ if (!Member->getQualifier())
+ return;
+
+ 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;
+
+ std::string ParentsStr;
+ ParentsStr.reserve(30 * Parents.size());
+ for (const CXXRecordDecl *Parent : Parents) {
+ if (!ParentsStr.empty())
+ ParentsStr.append(" or ");
+ ParentsStr.append("'").append(getNameAsString(Parent)).append("'");
+ }
+
+ assert(Member->getQualifierLoc().getSourceRange().getBegin().isValid());
+ auto Diag = diag(Member->getQualifierLoc().getSourceRange().getBegin(),
+ "qualified name '%0' refers to a member overridden "
+ "in subclass%1; did you mean %2?")
+ << getExprAsString(*Member, *Result.Context)
+ << (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
--- /dev/null
+//===--- 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
- New module ``zircon`` for checks related to Fuchsia's Zircon kernel.
+- 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:`bugprone-throw-keyword-missing
<clang-tidy/checks/bugprone-throw-keyword-missing>` check
--- /dev/null
+.. 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]
+ };
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
--- /dev/null
+// 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' {{.*}}; did you mean 'B'? {{.*}}
+ // 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;
+#define A3 A
+
+class C2 : public B {
+public:
+ int virt_1() override { return A1::virt_1() + A2::virt_1() + A3::virt_1(); }
+ // CHECK-MESSAGES: :[[@LINE-1]]:34: warning: qualified name 'A1::virt_1' {{.*}}; did you mean 'B'? {{.*}}
+ // CHECK-MESSAGES: :[[@LINE-2]]:49: warning: qualified name 'A2::virt_1' {{.*}}; did you mean 'B'? {{.*}}
+ // CHECK-MESSAGES: :[[@LINE-3]]:64: warning: qualified name 'A3::virt_1' {{.*}}; did you mean 'B'? {{.*}}
+ // CHECK-FIXES: int virt_1() override { return B::virt_1() + 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' {{.*}}; did you mean 'C'? {{.*}}
+ // CHECK-MESSAGES: :[[@LINE-2]]:48: warning: qualified name 'B::virt_1' {{.*}}; did you mean 'C'? {{.*}}
+ // 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' {{.*}}; did you mean 'C'? {{.*}}
+ // CHECK-MESSAGES: :[[@LINE-2]]:48: warning: qualified name 'B::virt_1' {{.*}}; did you mean 'C'? {{.*}}
+ // 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' {{.*}}; did you mean 'BN'? {{.*}}
+ // 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' {{.*}}; did you mean 'BN'? {{.*}}
+ // 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' {{.*}}; did you mean 'N2::BN'? {{.*}}
+ // 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' {{.*}}; did you mean 'N2::BN'? {{.*}}
+ // 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'? {{.*}}
+ // 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' {{.*}}; did you mean 'BF'? {{.*}}
+};
+
+// 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' {{.*}}; did you mean 'BF'? {{.*}}
+};
+
+// Just to instantiate DF<F>.
+int bar() { return (new DF<int>())->virt_1(); }