congliu updated this revision to Diff 44197.
congliu added a comment.
- Corrected naming styles; Used clang-format; Add doc to .h
- Removed useless parentheses, braces around one-line ifs.
- Added doc; Corrected style and typos for test.
- Implemented c++ [class.virtual]p7. But has bug.
- Support ambiguity checking.
- Completed virtual covarient check. Updated test.
http://reviews.llvm.org/D15823
Files:
clang-tidy/misc/CMakeLists.txt
clang-tidy/misc/MiscTidyModule.cpp
clang-tidy/misc/VirtualNearMissCheck.cpp
clang-tidy/misc/VirtualNearMissCheck.h
docs/clang-tidy/checks/list.rst
docs/clang-tidy/checks/misc-virtual-near-miss.rst
test/clang-tidy/misc-virtual-near-miss.cpp
Index: test/clang-tidy/misc-virtual-near-miss.cpp
===================================================================
--- /dev/null
+++ test/clang-tidy/misc-virtual-near-miss.cpp
@@ -0,0 +1,65 @@
+// RUN: %check_clang_tidy %s misc-virtual-near-miss %t
+
+struct Base {
+ virtual void func();
+ virtual void gunk();
+};
+
+struct Derived : Base {
+ // Should not warn "do you want to override 'gunk'?", becuase gunk is already
+ // overriden by this class.
+ virtual void funk();
+ // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: do you want to override 'func'? [misc-virtual-near-miss]
+
+ void func2();
+ // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: do you want to override 'func'?
+
+ void func22(); // Should not warn.
+
+ void gunk(); // Should not warn, because gunk is override.
+
+ void fun();
+ // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: do you want to override 'func'?
+};
+
+class Father {
+public:
+ Father();
+ virtual void func();
+ virtual Father *create(int i);
+ virtual Base &&generate();
+};
+
+class Mother {
+public:
+ Mother();
+ static void method();
+ virtual int method(int argc, const char **argv);
+ virtual int method(int argc) const;
+};
+
+class Child : Father, Mother {
+public:
+ Child();
+
+ virtual void func2();
+ // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: do you want to override 'func'?
+
+ int methoe(int x, char **strs); // Should not warn, because param type missmatch.
+
+ int methoe(int x); // Should not warn, because const type missmatch.
+
+ void methof(int x, const char **strs); // Should not warn, because return type missmatch.
+
+ int methoh(int x, const char **strs);
+ // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: do you want to override 'method'?
+
+ virtual Child *creat(int i);
+ // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: do you want to override 'create'?
+
+ virtual Derived &&generat();
+ // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: do you want to override 'generate'?
+
+private:
+ void funk(); //Should not warn, because access missmatch.
+};
Index: docs/clang-tidy/checks/misc-virtual-near-miss.rst
===================================================================
--- /dev/null
+++ docs/clang-tidy/checks/misc-virtual-near-miss.rst
@@ -0,0 +1,17 @@
+misc-virtual-near-miss
+======================
+
+Warn if a function is a near miss (ie. short edit distance) to a virtual function from a base class.
+
+Example:
+
+.. code-block:: c++
+
+ struct Base {
+ virtual void func();
+ };
+
+ struct Derived : Base {
+ virtual funk();
+ // warning: Do you want to override 'func'?
+ };
Index: docs/clang-tidy/checks/list.rst
===================================================================
--- docs/clang-tidy/checks/list.rst
+++ docs/clang-tidy/checks/list.rst
@@ -56,6 +56,7 @@
misc-unused-alias-decls
misc-unused-parameters
misc-unused-raii
+ misc-virtual-near-miss
modernize-loop-convert
modernize-make-unique
modernize-pass-by-value
Index: clang-tidy/misc/VirtualNearMissCheck.h
===================================================================
--- /dev/null
+++ clang-tidy/misc/VirtualNearMissCheck.h
@@ -0,0 +1,94 @@
+//===--- VirtualNearMissCheck.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_MISC_VIRTUAL_NEAR_MISS_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MISC_VIRTUAL_NEAR_MISS_H
+
+#include "../ClangTidy.h"
+#include <map>
+#include <string>
+
+namespace clang {
+namespace tidy {
+namespace misc {
+
+/// Generate warning if an method in derived class is a near miss to some virtual
+/// to base class:
+/// \code
+/// struct Base{
+/// virtual void func();
+/// };
+/// struct Derived:Base{
+/// virtual void funk(); // warning: do you want to override 'func'?
+/// };
+/// \endcode
+class VirtualNearMissCheck : public ClangTidyCheck {
+public:
+ VirtualNearMissCheck(StringRef Name, ClangTidyContext *Context)
+ : ClangTidyCheck(Name, Context) {}
+ void registerMatchers(ast_matchers::MatchFinder *Finder) override;
+ void check(const ast_matchers::MatchFinder::MatchResult &Result) override;
+
+private:
+ /// Return true if the given method overrides some method.
+ bool isOverrideMethod(const CXXMethodDecl *DerivedMD);
+
+ /// Return true if the given method is possible to be overriden by some other
+ /// method.
+ /// It should look up the PossibleMap or update it.
+ bool isPossibleToBeOverriden(const CXXMethodDecl *BaseMD);
+
+ /// Return true if the given base method is overriden by some methods in the
+ /// given derived class.
+ /// It should look up the OverridenMap or update it.
+ bool isOverridenByDerivedClass(const ASTContext *Context, const CXXMethodDecl *BaseMD,
+ const CXXRecordDecl *DerivedRD);
+
+ /// Return true if derived method can override base method except for the
+ /// name.
+ bool checkOverrideWithoutName(const ASTContext *Context,
+ const CXXMethodDecl *BaseMD,
+ const CXXMethodDecl *DerivedMD);
+
+ /// Check whether the return types are covariant.
+ /// Similar with clang::Sema::CheckOveridingFunctionReturnType.
+ bool checkOverridingFunctionReturnType(const ASTContext *Context,
+ const CXXMethodDecl *BaseMD,
+ const CXXMethodDecl *DerivedMDs);
+
+ /// Check whether the param types are the same.
+ bool checkParamType(const CXXMethodDecl *BaseMD,
+ const CXXMethodDecl *DerivedMD);
+
+ /// Check if derived method overrides base method.
+ bool checkOverride(const ASTContext *Context, const CXXMethodDecl *BaseMD,
+ const CXXMethodDecl *DerivedMD);
+
+ /// Generate unique ID for given MethodDecl.
+ /// The Id is used as key for 'PossibleMap'.
+ /// Typical Id: "Base::func void (void)"
+ std::string generateMethodId(const CXXMethodDecl *MD);
+
+ /// key: the unique ID of a method;
+ /// value: whether the method is possible to ve overriden.
+ std::map<std::string, bool> PossibleMap;
+
+ /// key: <unique ID of base method, name of derived class>
+ /// value: whether the base method is overriden by some method in the derived
+ /// class.
+ std::map<std::pair<std::string, std::string>, bool> OverridenMap;
+
+ const unsigned NearMissThreshold = 1;
+};
+
+} // namespace misc
+} // namespace tidy
+} // namespace clang
+
+#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MISC_VIRTUAL_NEAR_MISS_H
Index: clang-tidy/misc/VirtualNearMissCheck.cpp
===================================================================
--- /dev/null
+++ clang-tidy/misc/VirtualNearMissCheck.cpp
@@ -0,0 +1,258 @@
+//===--- VirtualNearMissCheck.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 "VirtualNearMissCheck.h"
+#include "clang/AST/ASTContext.h"
+#include "clang/AST/CXXInheritance.h"
+#include "clang/ASTMatchers/ASTMatchFinder.h"
+#include "clang/Lex/Lexer.h"
+
+using namespace clang::ast_matchers;
+
+namespace clang {
+namespace tidy {
+namespace misc {
+
+bool VirtualNearMissCheck::isOverrideMethod(const CXXMethodDecl *MD) {
+ return MD->size_overridden_methods() > 0 || MD->hasAttr<OverrideAttr>();
+}
+
+bool VirtualNearMissCheck::checkOverrideWithoutName(
+ const ASTContext *Context, const CXXMethodDecl *BaseMD,
+ const CXXMethodDecl *DerivedMD) {
+ if (BaseMD->getTypeQualifiers() != DerivedMD->getTypeQualifiers())
+ return false;
+
+ if (BaseMD->isStatic() != DerivedMD->isStatic())
+ return false;
+
+ if (BaseMD->getAccess() != DerivedMD->getAccess())
+ return false;
+
+ if (BaseMD->getType() == DerivedMD->getType())
+ return true;
+
+ // Now the function types are not identical. Then check if the return types
+ // are covariant and if the param types are the same.
+ if (!checkOverridingFunctionReturnType(Context, BaseMD, DerivedMD))
+ return false;
+ return checkParamType(BaseMD, DerivedMD);
+}
+
+bool VirtualNearMissCheck::checkOverridingFunctionReturnType(
+ const ASTContext *Context, const CXXMethodDecl *BaseMD,
+ const CXXMethodDecl *DerivedMD) {
+ QualType BaseReturnTy =
+ BaseMD->getType()->getAs<FunctionType>()->getReturnType();
+ QualType DerivedReturnTy =
+ DerivedMD->getType()->getAs<FunctionType>()->getReturnType();
+
+ if (DerivedReturnTy->isDependentType() || BaseReturnTy->isDependentType())
+ return false;
+
+ // Check if return types are identical
+ if (Context->hasSameType(DerivedReturnTy, BaseReturnTy))
+ return true;
+
+ /// Check if the return types are covariant.
+ /// BTy is the class type in return type of BaseMD. For example,
+ /// B* Base::md()
+ /// While BRD is the declaration of B.
+ QualType BTy, DTy;
+ const CXXRecordDecl *BRD, *DRD;
+
+ // Both types must be pointers or references to classes.
+ if (const PointerType *DerivedPT = DerivedReturnTy->getAs<PointerType>()) {
+ if (const PointerType *BasePT = BaseReturnTy->getAs<PointerType>()) {
+ DTy = DerivedPT->getPointeeType();
+ BTy = BasePT->getPointeeType();
+ }
+ } else if (const ReferenceType *DerivedRT =
+ DerivedReturnTy->getAs<ReferenceType>()) {
+ if (const ReferenceType *BaseRT = BaseReturnTy->getAs<ReferenceType>()) {
+ DTy = DerivedRT->getPointeeType();
+ BTy = BaseRT->getPointeeType();
+ }
+ }
+
+ // The return types aren't either both pointers or references to a class type.
+ if (DTy.isNull()) {
+ return false;
+ }
+
+ DRD = DTy->getAsCXXRecordDecl();
+ BRD = BTy->getAsCXXRecordDecl();
+ if (DRD == nullptr || BRD == nullptr)
+ return false;
+
+ if (!Context->hasSameUnqualifiedType(DTy, BTy)) {
+ // Begin checking whether the conversion from D to B is valid.
+ CXXBasePaths Paths(/*FindAmbiguities=*/true, /*RecordPaths=*/true,
+ /*DetectVirtual=*/false);
+
+ // Check whether D is derived from B, and fill in a CXXBasePaths object.
+ if (!DRD->isDerivedFrom(BRD, Paths))
+ return false;
+
+ // Check ambiguity.
+ if (Paths.isAmbiguous(Context->getCanonicalType(BTy).getUnqualifiedType()))
+ return false;
+
+ // Check accessibility.
+ // FIXME: We currently only support checking if B is accessible base class
+ // of D, or D is the same class which DerivedMD is in.
+ bool IsIteself = DRD == DerivedMD->getParent();
+ bool HasPublicAccess = false;
+ for (CXXBasePaths::paths_iterator Path = Paths.begin(); Path != Paths.end();
+ ++Path) {
+ if (Path->Access == AS_public) {
+ HasPublicAccess = true;
+ }
+ }
+ if (!(HasPublicAccess || IsIteself))
+ return false;
+ // End checking conversion from D to B.
+ }
+
+ // Both pointers or references should have the same cv-qualification.
+ if (DerivedReturnTy.getLocalCVRQualifiers() !=
+ BaseReturnTy.getLocalCVRQualifiers())
+ return false;
+
+ // The class type D should have the same cv-qualification as or less
+ // cv-qualification than the class type B
+ if (DTy.isMoreQualifiedThan(BTy))
+ return false;
+
+ return true;
+}
+
+bool VirtualNearMissCheck::checkParamType(const CXXMethodDecl *BaseMD,
+ const CXXMethodDecl *DerivedMD) {
+ unsigned int NumParamA = BaseMD->getNumParams();
+ unsigned int NumParamB = DerivedMD->getNumParams();
+ if (NumParamA != NumParamB)
+ return false;
+
+ for (unsigned I = 0; I < NumParamA; I++) {
+ if (BaseMD->getParamDecl(I)->getType() !=
+ DerivedMD->getParamDecl(I)->getType())
+ return false;
+ }
+ return true;
+}
+
+bool VirtualNearMissCheck::checkOverride(const ASTContext *Context,
+ const CXXMethodDecl *BaseMD,
+ const CXXMethodDecl *DerivedMD) {
+ if (BaseMD->getNameAsString() != DerivedMD->getNameAsString())
+ return false;
+
+ if (!checkOverrideWithoutName(Context, BaseMD, DerivedMD))
+ return false;
+
+ return true;
+}
+
+std::string VirtualNearMissCheck::generateMethodId(const CXXMethodDecl *MD) {
+ std::string Id =
+ MD->getQualifiedNameAsString() + " " + MD->getType().getAsString();
+ return Id;
+}
+
+bool VirtualNearMissCheck::isPossibleToBeOverriden(
+ const CXXMethodDecl *BaseMD) {
+ std::string Id = generateMethodId(BaseMD);
+ auto Iter = PossibleMap.find(Id);
+ bool IsPossible;
+ if (Iter != PossibleMap.end()) {
+ IsPossible = Iter->second;
+ } else {
+ IsPossible = !BaseMD->isImplicit() && !isa<CXXConstructorDecl>(BaseMD) &&
+ BaseMD->isVirtual();
+ PossibleMap[Id] = IsPossible;
+ }
+ return IsPossible;
+}
+
+bool VirtualNearMissCheck::isOverridenByDerivedClass(const ASTContext *Context,
+ const CXXMethodDecl *BaseMD,
+ const CXXRecordDecl *DerivedRD) {
+ auto Key = std::make_pair(generateMethodId(BaseMD),
+ DerivedRD->getQualifiedNameAsString());
+ auto Iter = OverridenMap.find(Key);
+ bool IsOverriden;
+ if (Iter != OverridenMap.end()) {
+ IsOverriden = Iter->second;
+ } else {
+ IsOverriden = false;
+ for (const CXXMethodDecl *DerivedMD : DerivedRD->methods()) {
+ if (!isOverrideMethod(DerivedMD))
+ continue;
+
+ if (checkOverride(Context, BaseMD, DerivedMD)) {
+ IsOverriden = true;
+ break;
+ }
+ }
+ OverridenMap[Key] = IsOverriden;
+ }
+ return IsOverriden;
+}
+
+void VirtualNearMissCheck::registerMatchers(MatchFinder *Finder) {
+ Finder->addMatcher(cxxMethodDecl(unless(anyOf(isOverride(), isImplicit(),
+ cxxConstructorDecl())))
+ .bind("method"),
+ this);
+}
+
+void VirtualNearMissCheck::check(const MatchFinder::MatchResult &Result) {
+ if (!getLangOpts().CPlusPlus)
+ return;
+
+ const auto *DerivedMD = Result.Nodes.getNodeAs<CXXMethodDecl>("method");
+ assert(DerivedMD != nullptr);
+
+ const ASTContext *Context = Result.Context;
+
+ if (Result.SourceManager->isInSystemHeader(DerivedMD->getLocation()))
+ return;
+
+ const auto *DerivedRD = DerivedMD->getParent();
+
+ for (const auto &BaseSpec : DerivedRD->bases()) {
+ const auto *BaseRD = BaseSpec.getType()->getAsCXXRecordDecl();
+ if (BaseRD == nullptr)
+ return;
+
+ for (const auto *BaseMD : BaseRD->methods()) {
+ if (!isPossibleToBeOverriden(BaseMD))
+ continue;
+
+ if (isOverridenByDerivedClass(Context, BaseMD, DerivedRD))
+ continue;
+
+ StringRef BaseMDName = BaseMD->getName();
+ StringRef DerivedMDName = DerivedMD->getName();
+ unsigned EditDistance = BaseMDName.edit_distance(DerivedMDName);
+ if (EditDistance > 0 && EditDistance <= NearMissThreshold) {
+ if (checkOverrideWithoutName(Context, BaseMD, DerivedMD)) {
+ // A "virtual near miss" is found.
+ diag(DerivedMD->getLocStart(), "do you want to override '%0'?")
+ << BaseMD->getName();
+ }
+ }
+ }
+ }
+}
+
+} // namespace misc
+} // namespace tidy
+} // namespace clang
Index: clang-tidy/misc/MiscTidyModule.cpp
===================================================================
--- clang-tidy/misc/MiscTidyModule.cpp
+++ clang-tidy/misc/MiscTidyModule.cpp
@@ -33,6 +33,7 @@
#include "UnusedAliasDeclsCheck.h"
#include "UnusedParametersCheck.h"
#include "UnusedRAIICheck.h"
+#include "VirtualNearMissCheck.h"
namespace clang {
namespace tidy {
@@ -84,6 +85,8 @@
CheckFactories.registerCheck<UnusedParametersCheck>(
"misc-unused-parameters");
CheckFactories.registerCheck<UnusedRAIICheck>("misc-unused-raii");
+ CheckFactories.registerCheck<VirtualNearMissCheck>(
+ "misc-virtual-near-miss");
}
};
Index: clang-tidy/misc/CMakeLists.txt
===================================================================
--- clang-tidy/misc/CMakeLists.txt
+++ clang-tidy/misc/CMakeLists.txt
@@ -25,6 +25,7 @@
UnusedParametersCheck.cpp
UnusedRAIICheck.cpp
UniqueptrResetReleaseCheck.cpp
+ VirtualNearMissCheck.cpp
LINK_LIBS
clangAST
_______________________________________________
cfe-commits mailing list
[email protected]
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
