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 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( "misc-unused-parameters"); CheckFactories.registerCheck("misc-unused-raii"); + CheckFactories.registerCheck( + "misc-virtual-near-miss"); } }; Index: clang-tidy/misc/VirtualNearMissCheck.h =================================================================== --- /dev/null +++ clang-tidy/misc/VirtualNearMissCheck.h @@ -0,0 +1,75 @@ +//===--- 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 +#include + +namespace clang { +namespace tidy { +namespace misc { + +/// FIXME: Write a short description. +/// +/// For the user-facing documentation see: +/// http://clang.llvm.org/extra/clang-tidy/checks/misc-virtual-near-miss.html +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; + + static int editDistance(const std::string &SourceStr, const std::string &TargetStr); +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 possible_map 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 overriden_map or update it. + bool isOverriden(const CXXMethodDecl *BaseMD, const CXXRecordDecl *DerivedRD); + + /// Return true if the two methods are the same (in type and qualifier) except for the name. + bool equalWithoutName(const CXXMethodDecl *BaseMD, const CXXMethodDecl *DerivedMD); + + /// Return true if the method A overrides method B. + bool compareOverride(const CXXMethodDecl *BaseMD, const CXXMethodDecl *DerivedMD); + + /// Generate unique ID for given MethodDecl. + /// The ID is used as key for 'possible_map'. + /// e.g. 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 possible_map_; + + /// key: + /// value: whether the base method is overriden by some method in the derived + /// class. + std::map, bool> overriden_map_; + + const int kNearMissThreshold = 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,210 @@ +//===--- 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/ASTMatchers/ASTMatchFinder.h" +#include "clang/Lex/Lexer.h" + +using namespace clang::ast_matchers; + +namespace clang { +namespace tidy { +namespace misc { + +int VirtualNearMissCheck::editDistance(const std::string &SourceStr, + const std::string &TargeStr){ + int len_source = SourceStr.size(); + int len_target = TargeStr.size(); + if (len_source == 0) { + return len_target; + } + if (len_target == 0) { + return len_source; + } + + std::vector> f(len_source + 1, + std::vector(len_target + 1)); + int i, j; + for (i = 1; i <= len_source; i ++){ + f[i][0] = i; + } + for (j = 1; j <= len_target; j ++){ + f[0][j] = j; + } + + for (i = 1; i <= len_source; i ++){ + for (j = 1; j <= len_target; j ++){ + int del = f[i - 1][j] + 1; // delete char SourceStr[i] + int ins = f[i][j - 1] + 1; // insert char TargeStr[j] + int rep = f[i - 1][j - 1]; // replace SourceStr[i] and TargeStr[j] + if (SourceStr[i - 1] != TargeStr[j - 1]){ + rep++; + } + int min_dist = del < ins ? del : ins; + min_dist = rep < min_dist ? rep : min_dist; + f[i][j] = min_dist; + } + } + return f[len_source][len_target]; +} + +bool VirtualNearMissCheck::isOverrideMethod(const CXXMethodDecl *MD){ + return MD->size_overridden_methods() > 0 || MD->hasAttr(); +} + +bool VirtualNearMissCheck::equalWithoutName(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; + + // One exception for return type &Base and &Derived (or *Base and *Derived) + QualType BaseReturnType = BaseMD->getReturnType(); + QualType DerivedReturnType = DerivedMD->getReturnType(); + if (BaseReturnType->isReferenceType() && DerivedReturnType->isReferenceType()){ + BaseReturnType = BaseReturnType.getNonReferenceType(); + DerivedReturnType = DerivedReturnType.getNonReferenceType(); + } else if (BaseReturnType->isPointerType() && DerivedReturnType->isPointerType()){ + BaseReturnType = BaseReturnType->getPointeeType(); + DerivedReturnType = DerivedReturnType->getPointeeType(); + }else { + return false; + } + if (BaseReturnType->getAsCXXRecordDecl() != BaseMD->getParent() || + DerivedReturnType->getAsCXXRecordDecl() != DerivedMD->getParent()){ + return false; + } + + int a_num_param = BaseMD->getNumParams(); + int b_num_param = DerivedMD->getNumParams(); + if (a_num_param != b_num_param) { + return false; + } + for (int i = 0; i < a_num_param; i++){ + if (BaseMD->getParamDecl(i)->getType() != DerivedMD->getParamDecl(i)->getType()){ + return false; + } + } + return true; +} + +bool VirtualNearMissCheck::compareOverride(const CXXMethodDecl *BaseMD, const CXXMethodDecl *DerivedMD){ + if (!equalWithoutName(BaseMD, DerivedMD)){ + return false; + } + if (BaseMD->getNameAsString() != DerivedMD->getNameAsString()){ + 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 it = possible_map_.find(id); + bool is_possible; + if (it != possible_map_.end()){ + is_possible = it->second; + } else{ + is_possible = !(BaseMD->isImplicit()) + && !(dyn_cast(BaseMD)) + && BaseMD->isVirtual(); + possible_map_[id] = is_possible; + } + return is_possible; +} + +bool VirtualNearMissCheck::isOverriden(const CXXMethodDecl *BaseMD, + const CXXRecordDecl *DerivedRD){ + auto key = std::make_pair(generateMethodID(BaseMD), DerivedRD->getQualifiedNameAsString()); + auto it = overriden_map_.find(key); + bool is_overriden; + if (it != overriden_map_.end()){ + is_overriden = it->second; + } else{ + is_overriden = false; + for (const CXXMethodDecl *DerivedMD : DerivedRD->methods()){ + if(!isOverrideMethod(DerivedMD)){ + continue; + } + + if (compareOverride(BaseMD, DerivedMD)){ + is_overriden = true; + break; + } + } + overriden_map_[key] = is_overriden; + } + return is_overriden; +} + +void VirtualNearMissCheck::registerMatchers(MatchFinder *Finder) { + Finder->addMatcher( + cxxMethodDecl( + unless(anyOf(isOverride(), isImplicit(),cxxConstructorDecl()))) + .bind("method"), + this); +} + +void VirtualNearMissCheck::check(const MatchFinder::MatchResult &Result) { + const auto *DerivedMD = Result.Nodes.getNodeAs("method"); + assert(DerivedMD != nullptr); + + if (Result.SourceManager->isInSystemHeader(DerivedMD->getLocation())){ + return; + } + + const auto *DerivedRD = DerivedMD->getParent(); + for (const auto &BasesSpec : DerivedRD->bases()){ + const auto *BaseRD = BasesSpec.getType()->getAsCXXRecordDecl(); + if(BaseRD == nullptr){ + return; + } + + for (const auto *BaseMD : BaseRD->methods()){ + if (!isPossibleToBeOverriden(BaseMD)){ + continue; + } + + if (isOverriden(BaseMD, DerivedRD)){ + continue; + } + + if (equalWithoutName(BaseMD, DerivedMD)){ + const std::string &BaseMDName = BaseMD->getNameAsString(); + const std::string &DerivedMDName = DerivedMD->getNameAsString(); + if (editDistance(BaseMDName, DerivedMDName) <= kNearMissThreshold){ + // virtual-near-miss found + diag(DerivedMD->getLocStart(), "do you want to override '%0'?") + << BaseMD->getName(); + // Auto fix could be risky + // const auto Fixup = FixItHint::CreateReplacement(DerivedMD->getNameInfo().getSourceRange(), BaseMDName); + } + } + } + } +} + +} // namespace misc +} // namespace tidy +} // namespace clang 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: docs/clang-tidy/checks/misc-virtual-near-miss.rst =================================================================== --- /dev/null +++ docs/clang-tidy/checks/misc-virtual-near-miss.rst @@ -0,0 +1,4 @@ +misc-virtual-near-miss +====================== + +FIXME: Describe what patterns does the check detect and why. Give examples. Index: test/clang-tidy/misc-virtual-near-miss.cpp =================================================================== --- /dev/null +++ test/clang-tidy/misc-virtual-near-miss.cpp @@ -0,0 +1,67 @@ +// RUN: %check_clang_tidy %s misc-virtual-near-miss %t + +struct Base{ + virtual void func(); + virtual void gunk(); +}; + +struct Derived:Base{ + // Should warn + // 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] + + // Should warn + void func2(); +// CHECK-MESSAGES: :[[@LINE-1]]:3: warning: do you want to override 'func'? [misc-virtual-near-miss] + + void func22(); // Should not warn + + void gunk(); // Should not warn because gunk is override + + // Should warn + void fun(); +// CHECK-MESSAGES: :[[@LINE-1]]:3: warning: do you want to override 'func'? [misc-virtual-near-miss] +}; + +class Father{ + public: + Father(); + virtual void func(); + virtual Father* create(int i); +}; + +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(); + + // Should warn + virtual void func2(); +// CHECK-MESSAGES: :[[@LINE-1]]:3: warning: do you want to override 'func'? [misc-virtual-near-miss] + + int methoe(int x, char** strs); // Should not warn because param type miss match + + int methoe(int x); // Should not warn because const type miss match + + void methof(int x, const char** strs); // Should not warn because return type miss match + + // Should warn + int methoh(int x, const char** strs); +// CHECK-MESSAGES: :[[@LINE-1]]:3: warning: do you want to override 'method'? [misc-virtual-near-miss] + + // Should warn + virtual Child* creat(int i); +// CHECK-MESSAGES: :[[@LINE-1]]:3: warning: do you want to override 'create'? [misc-virtual-near-miss] + + private: + void funk(); //Should not warn because access miss match +};