congliu created this revision.
congliu added a reviewer: alexfh.
congliu added a subscriber: cfe-commits.
Virtual function override near miss detection. Function complete. Test
complete. Do not conduct Fix for now.
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,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
+};
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: 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,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 <map>
+#include <string>
+
+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<std::string, bool> possible_map_;
+
+ /// 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> 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<std::vector<int>> f(len_source + 1,
+ std::vector<int>(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<OverrideAttr>();
+}
+
+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<CXXConstructorDecl>(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<CXXMethodDecl>("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: 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
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
