plugin: Do not warn on virtual/override stuff coming from macros in system headers.

tools/clang/plugins/FindBadConstructsConsumer.cpp

 // Copyright (c) 2012 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "FindBadConstructsConsumer.h"
 
 #include "clang/Frontend/CompilerInstance.h"
 #include "clang/AST/Attr.h"
 #include "clang/Lex/Lexer.h"
 #include "llvm/Support/raw_ostream.h"
@@ skipping 399 matching lines @@
   if (IsMethodInBannedOrTestingNamespace(method))
     return;
 
   SourceManager& manager = instance().getSourceManager();
 
   // Complain if a method is annotated virtual && (override || final).
   if (has_virtual && (override_attr || final_attr)) {
     // ... but only if virtual does not originate in a macro from a banned file.
     // Note this is just an educated guess: the assumption here is that any
     // macro for declaring methods will probably be at the start of the method's
     // source range.

[Nico, 2015/05/06 22:49:24]

to this

     if (!InBannedDirectory(manager.getSpellingLoc(method->getLocStart()))) {
       diagnostic().Report(method->getLocStart(),
                           diag_redundant_virtual_specifier_)
           << "'virtual'"
           << (override_attr ? static_cast<Attr*>(override_attr) : final_attr)
           << FixItRemovalForVirtual(manager, method);
     }
   }
 
   // Complain if a method is an override and is not annotated with override or
@@ skipping 33 matching lines @@
           break;
         }
         if (token.is(tok::r_paren)) {
           break;
         } else if (token.is(tok::equal)) {
           loc = l;
           break;
         }
       }
     }
+    // Again, only emit the warning if it doesn't originate from a macro in

[hans, 2015/05/06 22:47:42]

Silly question: what does "Again" refer to here?

+    // a system header.
     if (loc.isValid()) {
-      diagnostic().Report(loc, diag_method_requires_override_)
-          << FixItHint::CreateInsertion(loc, " override");
+      if (!InBannedDirectory(manager.getSpellingLoc(loc))) {

[dcheng, 2015/05/06 22:44:26]

It almost feels like this should be a helper.

reportIfLegit(loc, diagnostic_) << FixItHint::CreateRandom();

[Nico, 2015/05/06 22:49:24]

I considered that, but it's really only this function that needs it from what I
can tell. We already only diagnose records that are not from system headers, and
this is the only diag so far where stuff is added to non-system-header structs
with system-header macros.

+        diagnostic().Report(loc, diag_method_requires_override_)
+            << FixItHint::CreateInsertion(loc, " override");
+      }
     } else {
-      diagnostic().Report(range.getBegin(), diag_method_requires_override_);
+      if (!InBannedDirectory(manager.getSpellingLoc(range.getBegin())))
+        diagnostic().Report(range.getBegin(), diag_method_requires_override_);
     }
   }
 
-  if (final_attr && override_attr) {
+  if (final_attr && override_attr &&
+      !InBannedDirectory(
+          manager.getSpellingLoc(override_attr->getLocation()))) {
     diagnostic().Report(override_attr->getLocation(),
                         diag_redundant_virtual_specifier_)
         << override_attr << final_attr
         << FixItHint::CreateRemoval(override_attr->getRange());
   }
 
-  if (final_attr && !is_override) {
+  if (final_attr && !is_override &&
+      !InBannedDirectory(manager.getSpellingLoc(method->getLocStart()))) {
     diagnostic().Report(method->getLocStart(),
                         diag_base_method_virtual_and_final_)
         << FixItRemovalForVirtual(manager, method)
         << FixItHint::CreateRemoval(final_attr->getRange());
   }
 }
 
 void FindBadConstructsConsumer::CheckVirtualBodies(
     const CXXMethodDecl* method) {
   // Virtual methods should not have inline definitions beyond "{}". This
@@ skipping 317 matching lines @@
     // one of those, it means there is at least one member after a factory.
     if (weak_ptr_factory_location.isValid() &&
         !param_is_weak_ptr_factory_to_self) {
       diagnostic().Report(weak_ptr_factory_location,
                           diag_weak_ptr_factory_order_);
     }
   }
 }
 
 }  // namespace chrome_checker