xazax.hun added inline comments.

================
Comment at: clang-tidy/misc/VirtualNearMissCheck.cpp:240
@@ -247,2 +239,3 @@
         unsigned EditDistance =
-            BaseMD->getName().edit_distance(DerivedMD->getName());
+            StringRef(BaseMD->getNameAsString())
+                .edit_distance(DerivedMD->getNameAsString());
----------------
congliu wrote:
> NamedDecl::getName() directly returns a StringRef. Why using 
> "getNameAsString()"? 
Unfortunately getName will cause an assertion fail for methods that has a 
non-identifier name, such as destructors and overloaded operators.

================
Comment at: test/clang-tidy/misc-virtual-near-miss.cpp:52
@@ -49,2 +51,3 @@
 
-  int methoe(int x); // Should not warn: method is not const.
+  int methoe(int x);
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: method 'Child::methoe' has 
{{.*}} 'Mother::method'
----------------
congliu wrote:
> If a function in derived class has a same name but different cv-qualifiers as 
> a function in base class, they are not regarded as overriding. For example, 
> 
> ```
> class Mother{ 
>   virtual int method(int argc) const;
> };
> class Child : Mother{
>   int method(int x);
> };
> ```
> In this case, Child::method does not overrides Mother::method, but hides it. 
> So I think we should not warn for "methoe", because even if the programmer 
> changes "methoe" to "method", it's not an overriding. 
Sure, in order to override the method the developer also needs to make the  
method const in the child. But it is a common mistake to forget to add the 
qualifiers. The purpose of the change is to find those cases. I think it is 
more likely that the developer forgot to add the const qualifier than an 
intentional hiding of a virtual method.


http://reviews.llvm.org/D16179



_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to