Support virtual-near-miss check.

[congliu]

Summary

Virtual function override near miss detection. Function complete. Test complete. Do not conduct Fix for now.

[alexfh]

Thank you for working on this! See the initial set of comments inline.

clang-tidy/misc/VirtualNearMissCheck.cpp

21 ↗

(On Diff #43781)

Parameters should be StringRef instead of const std::string &. StringRef is the most common way to pass a read-only string to a function in LLVM.

Also, there's already StringRef::edit_distance. It should do what you need, I guess.

23 ↗

(On Diff #43781)

Please use LLVM naming conventions (http://llvm.org/docs/CodingStandards.html#name-types-functions-variables-and-enumerators-properly).

59 ↗

(On Diff #43781)

Why is the hasAttr<OverrideAttr>() part needed? Do you have an example of code that results in size_overridden_methods() == 0 and hasAttr<OverrideAttr>() == true?

62 ↗

(On Diff #43781)

Please maintain 80-columns limit. The easiest way to do this is to use clang-format (with -style=LLVM, if needed).

72 ↗

(On Diff #43781)

nit: Extra space before ==. Please use clang-format.

79 ↗

(On Diff #43781)

You can call .getNonReferenceType() unconditionally to make the code shorter.

84 ↗

(On Diff #43781)

clang-format

85 ↗

(On Diff #43781)

What if both return types are not references and are not pointers? Why do you return false in this case?

128 ↗

(On Diff #43781)

clang-format

129 ↗

(On Diff #43781)

No parentheses are needed around method calls here. Please remove.

130 ↗

(On Diff #43781)

Please use isa<T> instead of dyn_cast<T> in boolean context.

139 ↗

(On Diff #43781)

clang-format

144 ↗

(On Diff #43781)

clang-format

173 ↗

(On Diff #43781)

It's not common to use braces around one-line if bodies in LLVM/Clang code. Please remove the braces.

197 ↗

(On Diff #43781)

Comments should use proper Capitalization and punctuation. See http://llvm.org/docs/CodingStandards.html#commenting for details.

200 ↗

(On Diff #43781)

Please add a trailing period.

201 ↗

(On Diff #43781)

Dead/commented-out code should not be submitted. Either fix the code or remove the comment.

clang-tidy/misc/VirtualNearMissCheck.h

21 ↗

(On Diff #43781)

Please address the FIXME.

docs/clang-tidy/checks/misc-virtual-near-miss.rst

4 ↗

(On Diff #43781)

Please address the FIXME.

test/clang-tidy/misc-virtual-near-miss.cpp

3 ↗

(On Diff #43781)

Please use proper formatting for test code as well, unless some unusual formatting is needed for the purpose of testing. clang-format should do a good job on files in the test/ directory as well (the only difference with the "normal" code is that the // RUN: and // CHECK.* lines in tests may violate the column limit).

9 ↗

(On Diff #43781)

This comment is redundant. The CHECK-MESSAGES line below states clearly that a warning is expected.

17 ↗

(On Diff #43781)

To improve readability of the test, please remove the check name from all CHECK-MESSAGES lines except for the first one. When the CHECK-MESSAGES lines exceed 80 columns, it sometimes makes sense to truncate the message to make it fit the column limit.

51 ↗

(On Diff #43781)

s/miss match/mismatch/

Also, I'm not an English native speaker, but I believe a comma should be used before because in this sentence.

[congliu]

clang-tidy/misc/VirtualNearMissCheck.cpp

79 ↗

(On Diff #43781)

The condition is necessary, for the same reason I explained in the comment for line 85.

85 ↗

(On Diff #43781)

This is to deal with an special case of C++ override. Usually the return type of the override and overriden should be the same. The exception is that the return type ban be a reference (or pointer) to the class themselves. For example,

class Base{
  virtual Base* func();
};
class Derived : Base{
  virtual Derived* func();
};

In this case, the Derived::func() does override Base::func(), even though the return type are not the same.
So if the return types are not the same (line 72 assured that), and are not both references (or pointers), we can rule out the possibility of override, and therefore return false.

[alexfh]

clang-tidy/misc/VirtualNearMissCheck.cpp

85 ↗

(On Diff #43781)

It looks like you're talking about the concept of covariance described in [class.virtual] p7. And you seem to be getting it wrong: the return type doesn't need to be a pointer/reference to the class defining the method. Here's the relevant quote from the standard:

The return type of an overriding function shall be either identical to the return type of the overridden
function or covariant with the classes of the functions. If a function D::f overrides a function B::f, the
return types of the functions are covariant if they satisfy the following criteria:
— both are pointers to classes, both are lvalue references to classes, or both are rvalue references to
classes115
— the class in the return type of B::f is the same class as the class in the return type of D::f, or is an
unambiguous and accessible direct or indirect base class of the class in the return type of D::f
— both pointers or references have the same cv-qualification and the class type in the return type of D::f
has the same cv-qualification as or less cv-qualification than the class type in the return type of B::f.

Please change your code to implement this concept correctly.

[congliu]

clang-tidy/misc/VirtualNearMissCheck.cpp

59 ↗

(On Diff #43781)

I tried some cases, but haven't found an example for that case.
I saw the same code in AST_MATCHER(CXXMethodDecl, isOverride).
I'm not sure why there is a "hasAttr".

[alexfh]

Looks like you've forgotten to upload a new patch.

[congliu]

• 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.

[alexfh]

clang-tidy/misc/VirtualNearMissCheck.cpp

71 ↗

(On Diff #44197)

Please use const auto * to avoid repeating the type name.

217 ↗

(On Diff #44197)

This should be moved to the registerMatchers method: the matcher shouldn't be registered, if you don't intend to handle matches.

225 ↗

(On Diff #44197)

Please remove this check. Clang-tidy handles filtering of warnings by source location itself. Sometimes, developers of the standard library, for example, also want to run clang-tidy on it.

232 ↗

(On Diff #44197)

1. When is this going to be nullptr?
2. Why do you return here instead of continuing to check other base classes?

242 ↗

(On Diff #44197)

Doesn't seem like these variables make the code much shorter or easier to read.

248 ↗

(On Diff #44197)

The message doesn't explain what's wrong with the code, which makes it difficult for the user to understand the issue. Maybe something like this:
"method 'aab' has the same signature as a virtual method 'aaa' of the base class 'X' and a similar name; did you intend to override 'aaa'?"?

clang-tidy/misc/VirtualNearMissCheck.h

21 ↗

(On Diff #44197)

Thanks for adding the description. However, the "near miss" words don't explain the main thing: the check looks for virtual methods with a very similar name and an identical signature defined in a base class. Please mention this in the description.

You can also leave the example in the user-facing documentation only. BTW, please add back the link to it.

39 ↗

(On Diff #44197)

You can use Doxygen tags in the documentation comments, e.g.:

/// Finds out if the method overrides some other method.
///
/// \returns \c true if the given method overrides some other method.

Please read http://llvm.org/docs/CodingStandards.html#doxygen-use-in-documentation-comments

60 ↗

(On Diff #44197)

nit: typo: s/CheckOveridingFunctionReturnType/CheckOverridingFunctionReturnType/

61 ↗

(On Diff #44197)

What does the function return? Also, can we maybe pull out common functionality to avoid code duplication?

73 ↗

(On Diff #44197)

Please add an empty line after the first line (the first line serves as a brief description and the rest is a body of the documentation).

79 ↗

(On Diff #44197)

nit: typo to ve.

85 ↗

(On Diff #44197)

nit: s/OverridenMap/OverriddenMap/ (note the double 'd')

87 ↗

(On Diff #44197)

I'd call this EditDistanceThreshold to avoid questions on what does the threshold apply to.

docs/clang-tidy/checks/misc-virtual-near-miss.rst

4 ↗

(On Diff #44197)

Please mention that function signatures need to match in order for this check to fire.

test/clang-tidy/misc-virtual-near-miss.cpp

41 ↗

(On Diff #44197)

Is private inheritance intended here?

48 ↗

(On Diff #44197)

"because param type mismatch" is not grammatically correct. Use either "because of parameter type mismatch" or "because parameter types don't match". Same applies to "because return type missmatch" and "because access missmatch" below.

50 ↗

(On Diff #44197)

"const type mismatch" is not a clear description. Maybe "Shouldn't warn: method is not const."

52 ↗

(On Diff #44197)

Not actually done. There are still four "missmatch" occurrences in this file.

64 ↗

(On Diff #44197)

nit: Please insert a space after //.

[alexfh]

A few more comments.

clang-tidy/misc/VirtualNearMissCheck.cpp

22 ↗

(On Diff #44197)

This should be a free-standing function.

48 ↗

(On Diff #44197)

IIUC, this can be a free-standing function instead of a method, since it doesn't use any class members.

110 ↗

(On Diff #44197)

nit: typo: s/IsIteself/IsItself/

112 ↗

(On Diff #44197)

Why not a range-for loop?

118 ↗

(On Diff #44197)

Please propagate the negation inside parentheses: !HasPublicAccess && !IsItself.

129 ↗

(On Diff #44197)

nit: Please add a trailing period.

136 ↗

(On Diff #44197)

This should be checkParamTypes, note the plural form.

136 ↗

(On Diff #44197)

This can be a free-standing function, since it doesn't use any class members.

138 ↗

(On Diff #44197)

I'd slightly prefer unsigned over unsigned int.

163 ↗

(On Diff #44197)

This should be a free-standing function.

164 ↗

(On Diff #44197)

The variable is not needed here, just return the expression.

169 ↗

(On Diff #44197)

s/Overriden/Overridden/

175 ↗

(On Diff #44197)

I'd just return Iter->second; and remove else. Same below in the isOverridenByDerivedClass method.

[congliu]

• Completed [class.virtual] p7 covarient check. Updated test.
• Corrected implement of checkParamType.
• Clean up.
• Corrected typos. Changed some methods to free-standing. Changed warning message. Updated tests.
• Finished correcting cpp. Updated doc.

clang-tidy/misc/VirtualNearMissCheck.cpp

232 ↗

(On Diff #44197)

1. Just to assure it's not nullptr. If the base specification contains a class type that was not declared anywhere, it would be nullptr. I found similar code here NewDeleteOverloadsCheck.cpp, line 121
2. Sorry, it's a mistake, should be 'continue' not 'return'.

test/clang-tidy/misc-virtual-near-miss.cpp

41 ↗

(On Diff #44197)

Yes, it's intended. This is for the case in line 57: although Father is a private base class of Child, since that function itself is in Child class, the conversion is accessible.

[alexfh]

A few minor comments.

clang-tidy/misc/VirtualNearMissCheck.cpp

23 ↗

(On Diff #44328)

Please mark this and other functions static to make them local to the translation unit.

43 ↗

(On Diff #44328)

nit: Missing trailing period.

68 ↗

(On Diff #44328)

nit: No braces around one-line if bodies, please.

90 ↗

(On Diff #44328)

I wonder whether BTy.getCanonicalType().getUnqualifiedType() will work here.

99 ↗

(On Diff #44328)

nit: No braces around one-line if bodies, please.

188 ↗

(On Diff #44328)

nit: No braces around one-line if bodies, please.

203 ↗

(On Diff #44328)

nit: No braces around one-line if bodies, please.

258 ↗

(On Diff #44328)

The diagnostic messages are not complete sentences. Please use a semicolon instead of the period and a lower-case letter after it. Also, s/signature with/signature as/ and s/meant/mean/. Overall it should be:
method '%0' has a similar name and the same signature as method '%1'; did you mean to override it?.

test/clang-tidy/misc-virtual-near-miss.cpp

15 ↗

(On Diff #44328)

Please remove the . Did you mean to override it? part from all CHECK lines except for the first one to make the test file easier to read. Maybe even cut the static test in the middle:

// CHECK-MESSAGES: :[[@LINE-1]]:3: warning: 'Derived::func2' has {{.*}} 'Base::func'

42 ↗

(On Diff #44328)

Please mark the inheritance explicitly private then to avoid questions.

[congliu]

This comment was removed by congliu.

clang-tidy/misc/VirtualNearMissCheck.cpp

90 ↗

(On Diff #44328)

I'm afraid they're different. CXXBasePaths.isAmbiguous receives a CanQualType parameter, while BTy.getCanonicalType().getUnqualifiedType() returns a QualType object.

[congliu]

• Removed braces; corrected comments; updated test.

[alexfh]

Looks good with one nit. I'll update the comment myself and commit the patch for you.

Thank you for the contribution!

clang-tidy/misc/VirtualNearMissCheck.cpp

60 ↗

(On Diff #44661)

Answering my own question: I guess, it may be useful for methods of template classes and for template methods, where the template declaration doesn't make it clear whether the method is overridden or not. However the author may want the compiler to ensure the instantiation actually overrides something.

clang-tidy/misc/VirtualNearMissCheck.h

39 ↗

(On Diff #44661)

nit: It should look up ... sounds like a requirement, however, you exactly know what the implementation does, so It looks up ... or updates ... is better. Maybe even Results are memoized in PossibleMap.

Same below.

clang-tools-extra/trunk/clang-tidy/misc/CMakeLists.txt

add_clang_library(clangTidyMiscModule
   StringIntegerAssignmentCheck.cpp
   SwappedArgumentsCheck.cpp
   ThrowByValueCatchByReferenceCheck.cpp
   UndelegatedConstructor.cpp
   UnusedAliasDeclsCheck.cpp
   UnusedParametersCheck.cpp
   UnusedRAIICheck.cpp
   UniqueptrResetReleaseCheck.cpp
+  VirtualNearMissCheck.cpp
 
   LINK_LIBS
   clangAST
   clangASTMatchers
   clangBasic
   clangLex
   clangTidy
   clangTidyUtils
   )

clang-tools-extra/trunk/clang-tidy/misc/MiscTidyModule.cpp

 #include "StringIntegerAssignmentCheck.h"
 #include "SwappedArgumentsCheck.h"
 #include "ThrowByValueCatchByReferenceCheck.h"
 #include "UndelegatedConstructor.h"
 #include "UniqueptrResetReleaseCheck.h"
 #include "UnusedAliasDeclsCheck.h"
 #include "UnusedParametersCheck.h"
 #include "UnusedRAIICheck.h"
+#include "VirtualNearMissCheck.h"
 
 namespace clang {
 namespace tidy {
 namespace misc {
 
 class MiscModule : public ClangTidyModule {
 public:
   void addCheckFactories(ClangTidyCheckFactories &CheckFactories) override {
CheckFactories.registerCheck<UndelegatedConstructorCheck>(
         "misc-undelegated-constructor");
     CheckFactories.registerCheck<UniqueptrResetReleaseCheck>(
         "misc-uniqueptr-reset-release");
     CheckFactories.registerCheck<UnusedAliasDeclsCheck>(
         "misc-unused-alias-decls");
     CheckFactories.registerCheck<UnusedParametersCheck>(
         "misc-unused-parameters");
     CheckFactories.registerCheck<UnusedRAIICheck>("misc-unused-raii");
+    CheckFactories.registerCheck<VirtualNearMissCheck>(
+        "misc-virtual-near-miss");
   }
 };
 
 } // namespace misc
 
 // Register the MiscTidyModule using this statically initialized variable.
 static ClangTidyModuleRegistry::Add<misc::MiscModule>
 X("misc-module", "Adds miscellaneous lint checks.");
 
 // This anchor is used to force the linker to link in the generated object file
 // and thus register the MiscModule.
 volatile int MiscModuleAnchorSource = 0;
 
 } // namespace tidy
 } // namespace clang

clang-tools-extra/trunk/clang-tidy/misc/VirtualNearMissCheck.h

+//===--- 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 {
+
+/// \brief Checks for near miss of virtual methods.
+///
+/// For a method in a derived class, this check looks for virtual method with a
+/// very similar name and an identical signature defined in a base class.
+///
+/// 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;
+
+private:
+  /// Check if the given method is possible to be overridden by some other
+  /// method.
+  ///
+  /// It should look up the PossibleMap or update it.
+  bool isPossibleToBeOverridden(const CXXMethodDecl *BaseMD);
+
+  /// Check if the given base method is overridden by some methods in the given
+  /// derived class.
+  ///
+  /// It should look up the OverriddenMap or update it.
+  bool isOverriddenByDerivedClass(const CXXMethodDecl *BaseMD,
+                                  const CXXRecordDecl *DerivedRD);
+
+  /// key: the unique ID of a method;
+  /// value: whether the method is possible to be overridden.
+  std::map<std::string, bool> PossibleMap;
+
+  /// key: <unique ID of base method, name of derived class>
+  /// value: whether the base method is overridden by some method in the derived
+  /// class.
+  std::map<std::pair<std::string, std::string>, bool> OverriddenMap;
+
+  const unsigned EditDistanceThreshold = 1;
+};
+
+} // namespace misc
+} // namespace tidy
+} // namespace clang
+
+#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MISC_VIRTUAL_NEAR_MISS_H

clang-tools-extra/trunk/clang-tidy/misc/VirtualNearMissCheck.cpp

+//===--- 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 {
+
+/// Finds out if the given method overrides some method.
+static bool isOverrideMethod(const CXXMethodDecl *MD) {
+  return MD->size_overridden_methods() > 0 || MD->hasAttr<OverrideAttr>();
+}
+
+/// Checks whether the return types are covariant, according to
+/// C++[class.virtual]p7.
+///
+/// Similar with clang::Sema::CheckOverridingFunctionReturnType.
+/// \returns true if the return types of BaseMD and DerivedMD are covariant.
+static bool 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 auto *DerivedPT = DerivedReturnTy->getAs<PointerType>()) {
+    if (const auto *BasePT = BaseReturnTy->getAs<PointerType>()) {
+      DTy = DerivedPT->getPointeeType();
+      BTy = BasePT->getPointeeType();
+    }
+  } else if (const auto *DerivedRT = DerivedReturnTy->getAs<ReferenceType>()) {
+    if (const auto *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 (DRD == BRD)
+    return true;
+
+  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 IsItself = DRD == DerivedMD->getParent();
+    bool HasPublicAccess = false;
+    for (const auto &Path : Paths) {
+      if (Path.Access == AS_public)
+        HasPublicAccess = true;
+    }
+    if (!HasPublicAccess && !IsItself)
+      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;
+}
+
+/// \returns true if the param types are the same.
+static bool checkParamTypes(const CXXMethodDecl *BaseMD,
+                            const CXXMethodDecl *DerivedMD) {
+  unsigned NumParamA = BaseMD->getNumParams();
+  unsigned 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;
+}
+
+/// \returns true if derived method can override base method except for the
+/// name.
+static bool 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 checkParamTypes(BaseMD, DerivedMD);
+}
+
+/// Check whether BaseMD overrides DerivedMD.
+///
+/// Prerequisite: the class which BaseMD is in should be a base class of that
+/// DerivedMD is in.
+static bool checkOverrideByDerivedMethod(const CXXMethodDecl *BaseMD,
+                                         const CXXMethodDecl *DerivedMD) {
+  if (BaseMD->getNameAsString() != DerivedMD->getNameAsString())
+    return false;
+
+  if (!checkParamTypes(BaseMD, DerivedMD))
+    return false;
+
+  return true;
+}
+
+/// Generate unique ID for given MethodDecl.
+///
+/// The Id is used as key for 'PossibleMap'.
+/// Typical Id: "Base::func void (void)"
+static std::string generateMethodId(const CXXMethodDecl *MD) {
+  return MD->getQualifiedNameAsString() + " " + MD->getType().getAsString();
+}
+
+bool VirtualNearMissCheck::isPossibleToBeOverridden(
+    const CXXMethodDecl *BaseMD) {
+  std::string Id = generateMethodId(BaseMD);
+  auto Iter = PossibleMap.find(Id);
+  if (Iter != PossibleMap.end())
+    return Iter->second;
+
+  bool IsPossible = !BaseMD->isImplicit() && !isa<CXXConstructorDecl>(BaseMD) &&
+                    BaseMD->isVirtual();
+  PossibleMap[Id] = IsPossible;
+  return IsPossible;
+}
+
+bool VirtualNearMissCheck::isOverriddenByDerivedClass(
+    const CXXMethodDecl *BaseMD, const CXXRecordDecl *DerivedRD) {
+  auto Key = std::make_pair(generateMethodId(BaseMD),
+                            DerivedRD->getQualifiedNameAsString());
+  auto Iter = OverriddenMap.find(Key);
+  if (Iter != OverriddenMap.end())
+    return Iter->second;
+
+  bool IsOverridden = false;
+  for (const CXXMethodDecl *DerivedMD : DerivedRD->methods()) {
+    if (!isOverrideMethod(DerivedMD))
+      continue;
+
+    if (checkOverrideByDerivedMethod(BaseMD, DerivedMD)) {
+      IsOverridden = true;
+      break;
+    }
+  }
+  OverriddenMap[Key] = IsOverridden;
+  return IsOverridden;
+}
+
+void VirtualNearMissCheck::registerMatchers(MatchFinder *Finder) {
+  if (!getLangOpts().CPlusPlus)
+    return;
+
+  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 (DerivedMD->isStatic())
+    return;
+
+  const ASTContext *Context = Result.Context;
+
+  const auto *DerivedRD = DerivedMD->getParent();
+
+  for (const auto &BaseSpec : DerivedRD->bases()) {
+    if (const auto *BaseRD = BaseSpec.getType()->getAsCXXRecordDecl()) {
+      for (const auto *BaseMD : BaseRD->methods()) {
+        if (!isPossibleToBeOverridden(BaseMD))
+          continue;
+
+        if (isOverriddenByDerivedClass(BaseMD, DerivedRD))
+          continue;
+
+        unsigned EditDistance =
+            BaseMD->getName().edit_distance(DerivedMD->getName());
+        if (EditDistance > 0 && EditDistance <= EditDistanceThreshold) {
+          if (checkOverrideWithoutName(Context, BaseMD, DerivedMD)) {
+            // A "virtual near miss" is found.
+            diag(DerivedMD->getLocStart(),
+                 "method '%0' has a similar name and the same signature as "
+                 "virtual method '%1'; did you mean to override it?")
+                << DerivedMD->getQualifiedNameAsString()
+                << BaseMD->getQualifiedNameAsString();
+          }
+        }
+      }
+    }
+  }
+}
+
+} // namespace misc
+} // namespace tidy
+} // namespace clang

clang-tools-extra/trunk/docs/clang-tidy/checks/list.rst

.. toctree::
    misc-string-integer-assignment
    misc-swapped-arguments
    misc-throw-by-value-catch-by-reference
    misc-undelegated-constructor
    misc-uniqueptr-reset-release
    misc-unused-alias-decls
    misc-unused-parameters
    misc-unused-raii
+   misc-virtual-near-miss
    modernize-loop-convert
    modernize-make-unique
    modernize-pass-by-value
    modernize-redundant-void-arg
    modernize-replace-auto-ptr
    modernize-shrink-to-fit
    modernize-use-auto
    modernize-use-default

clang-tools-extra/trunk/docs/clang-tidy/checks/misc-virtual-near-miss.rst

+misc-virtual-near-miss
+======================
+
+Warn if a function is a near miss (ie. the name is very similar and the function signiture is the same) 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'?
+  };

clang-tools-extra/trunk/test/clang-tidy/misc-virtual-near-miss.cpp

+// 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'?", because gunk is already
+  // overriden by this class.
+  virtual void funk();
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: method 'Derived::funk' has a similar name and the same signature as virtual method 'Base::func'; did you mean to override it? [misc-virtual-near-miss]
+
+  void func2();
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: method 'Derived::func2' has {{.*}} 'Base::func'
+
+  void func22(); // Should not warn.
+
+  void gunk(); // Should not warn: gunk is override.
+
+  void fun();
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: method 'Derived::fun' has {{.*}} 'Base::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 : private Father, private Mother {
+public:
+  Child();
+
+  virtual void func2();
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: method 'Child::func2' has {{.*}} 'Father::func'
+
+  int methoe(int x, char **strs); // Should not warn: parameter types don't match.
+
+  int methoe(int x); // Should not warn: method is not const.
+
+  void methof(int x, const char **strs); // Should not warn: return types don't match.
+
+  int methoh(int x, const char **strs);
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: method 'Child::methoh' has {{.*}} 'Mother::method'
+
+  virtual Child *creat(int i);
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: method 'Child::creat' has {{.*}} 'Father::create'
+
+  virtual Derived &&generat();
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: method 'Child::generat' has {{.*}} 'Father::generate'
+
+private:
+  void funk(); // Should not warn: access qualifers don't match.
+};