Update from https://crrev.com/329939

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 60 matching lines @@
 }
 
 // Generates a fixit hint to remove the 'virtual' keyword.
 // Unfortunately, there doesn't seem to be a good way to determine the source
 // location of the 'virtual' keyword. It's available in Declarator, but that
 // isn't accessible from the AST. So instead, make an educated guess that the
 // first token is probably the virtual keyword. Strictly speaking, this doesn't
 // have to be true, but it probably will be.
 // TODO(dcheng): Add a warning to force virtual to always appear first ;-)
 FixItHint FixItRemovalForVirtual(const SourceManager& manager,
+                                 const LangOptions& lang_opts,
                                  const CXXMethodDecl* method) {
   SourceRange range(method->getLocStart());
   // Get the spelling loc just in case it was expanded from a macro.
   SourceRange spelling_range(manager.getSpellingLoc(range.getBegin()));
   // Sanity check that the text looks like virtual.
   StringRef text = clang::Lexer::getSourceText(
-      CharSourceRange::getTokenRange(spelling_range), manager, LangOptions());
+      CharSourceRange::getTokenRange(spelling_range), manager, lang_opts);
   if (text.trim() != "virtual")
     return FixItHint();
   return FixItHint::CreateRemoval(range);
 }
 
 bool IsPodOrTemplateType(const CXXRecordDecl& record) {
   return record.isPOD() ||
          record.getDescribedClassTemplate() ||
          record.getTemplateSpecializationKind() ||
          record.isDependentType();
@@ skipping 185 matching lines @@
       // find one that tries to be inline.
       for (CXXRecordDecl::ctor_iterator it = record->ctor_begin();
            it != record->ctor_end();
            ++it) {
         // The current check is buggy. An implicit copy constructor does not
         // have an inline body, so this check never fires for classes with a
         // user-declared out-of-line constructor.
         if (it->hasInlineBody()) {
           if (it->isCopyConstructor() &&
               !record->hasUserDeclaredCopyConstructor()) {
-            emitWarning(record_location,
-                        "Complex class/struct needs an explicit out-of-line "
-                        "copy constructor.");
+            // In general, implicit constructors are generated on demand.  But
+            // in the Windows component build, dllexport causes instantiation of
+            // the copy constructor which means that this fires on many more
+            // classes. For now, suppress this on dllexported classes.
+            // (This does mean that windows component builds will not emit this
+            // warning in some cases where it is emitted in other configs, but
+            // that's the better tradeoff at this point).
+            // TODO(dcheng): With the RecursiveASTVisitor, these warnings might
+            // be emitted on other platforms too, reevaluate if we want to keep
+            // surpressing this then http://crbug.com/467288
+            if (!record->hasAttr<DLLExportAttr>())
+              emitWarning(record_location,
+                          "Complex class/struct needs an explicit out-of-line "
+                          "copy constructor.");
           } else {
-            emitWarning(it->getInnerLocStart(),
-                        "Complex constructor has an inlined body.");
+            // See the comment in the previous branch about copy constructors.
+            // This does the same for implicit move constructors.
+            bool is_likely_compiler_generated_dllexport_move_ctor =
+                it->isMoveConstructor() &&
+                !record->hasUserDeclaredMoveConstructor() &&
+                record->hasAttr<DLLExportAttr>();
+            if (!is_likely_compiler_generated_dllexport_move_ctor)
+              emitWarning(it->getInnerLocStart(),
+                          "Complex constructor has an inlined body.");
           }
         } else if (it->isInlined() && !it->isInlineSpecified() &&
                    !it->isDeleted() && (!it->isCopyOrMoveConstructor() ||
                                         it->isExplicitlyDefaulted())) {
           // isInlined() is a more reliable check than hasInlineBody(), but
           // unfortunately, it results in warnings for implicit copy/move
           // constructors in the previously mentioned situation. To preserve
           // compatibility with existing Chromium code, only warn if it's an
           // explicitly defaulted copy or move constructor.
           emitWarning(it->getInnerLocStart(),
@@ skipping 38 matching lines @@
         // incorrectly, but having the plugin enforce override is also nice.
         (InTestingNamespace(overridden) &&
          !IsGtestTestFixture(overridden->getParent()))) {
       return true;
     }
   }
 
   return false;
 }
 
+SuppressibleDiagnosticBuilder
+FindBadConstructsConsumer::ReportIfSpellingLocNotIgnored(
+    SourceLocation loc,
+    unsigned diagnostic_id) {
+  return SuppressibleDiagnosticBuilder(
+      &diagnostic(), loc, diagnostic_id,
+      InBannedDirectory(instance().getSourceManager().getSpellingLoc(loc)));
+}
+
 // Checks that virtual methods are correctly annotated, and have no body in a
 // header file.
 void FindBadConstructsConsumer::CheckVirtualMethods(
     SourceLocation record_location,
     CXXRecordDecl* record,
     bool warn_on_inline_bodies) {
   // Gmock objects trigger these for each MOCK_BLAH() macro used. So we have a
   // trick to get around that. If a class has member variables whose types are
   // in the "testing" namespace (which is how gmock works behind the scenes),
   // there's a really high chance we won't care about these errors
@@ skipping 37 matching lines @@
     const CXXMethodDecl* method) {
   bool is_override = method->size_overridden_methods() > 0;
   bool has_virtual = method->isVirtualAsWritten();
   OverrideAttr* override_attr = method->getAttr<OverrideAttr>();
   FinalAttr* final_attr = method->getAttr<FinalAttr>();
 
   if (IsMethodInBannedOrTestingNamespace(method))
     return;
 
   SourceManager& manager = instance().getSourceManager();
+  const LangOptions& lang_opts = instance().getLangOpts();
 
   // 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.
-    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);
-    }
+    ReportIfSpellingLocNotIgnored(method->getLocStart(),
+                                  diag_redundant_virtual_specifier_)
+        << "'virtual'"
+        << (override_attr ? static_cast<Attr*>(override_attr) : final_attr)
+        << FixItRemovalForVirtual(manager, lang_opts, method);
   }
 
   // Complain if a method is an override and is not annotated with override or
   // final.
   if (is_override && !override_attr && !final_attr) {
     SourceRange range = method->getSourceRange();
     SourceLocation loc;
     if (method->hasInlineBody()) {
       loc = method->getBody()->getSourceRange().getBegin();
     } else {
-      // TODO(dcheng): We should probably use ASTContext's LangOptions here.
-      LangOptions lang_options;
-      loc = Lexer::getLocForEndOfToken(
-          manager.getSpellingLoc(range.getEnd()), 0,
-          manager, lang_options);
+      loc = Lexer::getLocForEndOfToken(manager.getSpellingLoc(range.getEnd()),
+                                       0, manager, lang_opts);
       // The original code used the ending source loc of TypeSourceInfo's
       // TypeLoc. Unfortunately, this breaks down in the presence of attributes.
       // Attributes often appear at the end of a TypeLoc, e.g.
       //   virtual ULONG __stdcall AddRef()
       // has a TypeSourceInfo that looks something like:
       //   ULONG AddRef() __attribute(stdcall)
       // so a fix-it insertion would be generated to insert 'override' after
       // __stdcall in the code as written.
       // While using the spelling loc of the CXXMethodDecl fixes attribute
       // handling, it breaks handling of "= 0" and similar constructs.. To work
       // around this, scan backwards in the source text for a '=' or ')' token
       // and adjust the location as needed...
       for (SourceLocation l = loc.getLocWithOffset(-1);
            l != manager.getLocForStartOfFile(manager.getFileID(loc));
            l = l.getLocWithOffset(-1)) {
-        l = Lexer::GetBeginningOfToken(l, manager, lang_options);
+        l = Lexer::GetBeginningOfToken(l, manager, lang_opts);
         Token token;
         // getRawToken() returns *true* on failure. In that case, just give up
         // and don't bother generating a possibly incorrect fix-it.
-        if (Lexer::getRawToken(l, token, manager, lang_options, true)) {
+        if (Lexer::getRawToken(l, token, manager, lang_opts, true)) {
           loc = SourceLocation();
           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
+    // a system header.
     if (loc.isValid()) {
-      diagnostic().Report(loc, diag_method_requires_override_)
+      ReportIfSpellingLocNotIgnored(loc, diag_method_requires_override_)
           << FixItHint::CreateInsertion(loc, " override");
     } else {
-      diagnostic().Report(range.getBegin(), diag_method_requires_override_);
+      ReportIfSpellingLocNotIgnored(range.getBegin(),
+                                    diag_method_requires_override_);
     }
   }
 
   if (final_attr && override_attr) {
-    diagnostic().Report(override_attr->getLocation(),
-                        diag_redundant_virtual_specifier_)
+    ReportIfSpellingLocNotIgnored(override_attr->getLocation(),
+                                  diag_redundant_virtual_specifier_)
         << override_attr << final_attr
         << FixItHint::CreateRemoval(override_attr->getRange());
   }
 
   if (final_attr && !is_override) {
-    diagnostic().Report(method->getLocStart(),
-                        diag_base_method_virtual_and_final_)
-        << FixItRemovalForVirtual(manager, method)
+    ReportIfSpellingLocNotIgnored(method->getLocStart(),
+                                  diag_base_method_virtual_and_final_)
+        << FixItRemovalForVirtual(manager, lang_opts, method)
         << FixItHint::CreateRemoval(final_attr->getRange());
   }
 }
 
 void FindBadConstructsConsumer::CheckVirtualBodies(
     const CXXMethodDecl* method) {
   // Virtual methods should not have inline definitions beyond "{}". This
   // only matters for header files.
   if (method->hasBody() && method->hasInlineBody()) {
     if (CompoundStmt* cs = dyn_cast<CompoundStmt>(method->getBody())) {
       if (cs->size()) {
-        emitWarning(cs->getLBracLoc(),
-                    "virtual methods with non-empty bodies shouldn't be "
-                    "declared inline.");
+        SourceLocation loc = cs->getLBracLoc();
+        // CR_BEGIN_MSG_MAP_EX and BEGIN_SAFE_MSG_MAP_EX try to be compatible
+        // to BEGIN_MSG_MAP(_EX).  So even though they are in chrome code,
+        // we can't easily fix them, so explicitly whitelist them here.
+        bool emit = true;
+        if (loc.isMacroID()) {
+          SourceManager& manager = instance().getSourceManager();
+          if (InBannedDirectory(manager.getSpellingLoc(loc)))
+            emit = false;
+          else {
+            StringRef name = Lexer::getImmediateMacroName(
+                loc, manager, instance().getLangOpts());
+            if (name == "CR_BEGIN_MSG_MAP_EX" ||
+                name == "BEGIN_SAFE_MSG_MAP_EX")
+              emit = false;
+          }
+        }
+        if (emit)
+          emitWarning(loc,
+                      "virtual methods with non-empty bodies shouldn't be "
+                      "declared inline.");
       }
     }
   }
 }
 
 void FindBadConstructsConsumer::CountType(const Type* type,
                                           int* trivial_member,
                                           int* non_trivial_member,
                                           int* templated_non_trivial_member) {
   switch (type->getTypeClass()) {
     case Type::Record: {
       // Simplifying; the whole class isn't trivial if the dtor is, but
       // we use this as a signal about complexity.
       if (TypeHasNonTrivialDtor(type))
         (*trivial_member)++;
       else
         (*non_trivial_member)++;
       break;
     }
     case Type::TemplateSpecialization: {
       TemplateName name =
           dyn_cast<TemplateSpecializationType>(type)->getTemplateName();
       bool whitelisted_template = false;
 
       // HACK: I'm at a loss about how to get the syntax checker to get
-      // whether a template is exterened or not. For the first pass here,
+      // whether a template is externed or not. For the first pass here,
       // just do retarded string comparisons.
       if (TemplateDecl* decl = name.getAsTemplateDecl()) {
         std::string base_name = decl->getNameAsString();
         if (base_name == "basic_string")
           whitelisted_template = true;
       }
 
       if (whitelisted_template)
         (*non_trivial_member)++;
       else
@@ skipping 274 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