Update from https://crrev.com/327068

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 52 matching lines @@
     return UnwrapType(elaborated->getNamedType().getTypePtr());
   if (const TypedefType* typedefed = dyn_cast<TypedefType>(type))
     return UnwrapType(typedefed->desugar().getTypePtr());
   return type;
 }
 
 bool IsGtestTestFixture(const CXXRecordDecl* decl) {
   return decl->getQualifiedNameAsString() == "testing::Test";
 }
 
+// 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 CXXMethodDecl* method) {
-  // Unfortunately, there doesn't seem to be a good way to determine the
-  // 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 ;-)
   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());
   if (text.trim() != "virtual")
     return FixItHint();
   return FixItHint::CreateRemoval(range);
 }
@@ skipping 315 matching lines @@
   OverrideAttr* override_attr = method->getAttr<OverrideAttr>();
   FinalAttr* final_attr = method->getAttr<FinalAttr>();
 
   if (IsMethodInBannedOrTestingNamespace(method))
     return;
 
   SourceManager& manager = instance().getSourceManager();
 
   // Complain if a method is annotated virtual && (override || final).
   if (has_virtual && (override_attr || final_attr)) {
-    diagnostic().Report(method->getLocStart(),
-                        diag_redundant_virtual_specifier_)
-        << "'virtual'"
-        << (override_attr ? static_cast<Attr*>(override_attr) : final_attr)
-        << FixItRemovalForVirtual(manager, method);
+    // ... 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);
+    }
   }
 
   // Complain if a method is an override and is not annotated with override or
   // final.
   if (is_override && !override_attr && !final_attr) {
-    SourceRange type_info_range =
-        method->getTypeSourceInfo()->getTypeLoc().getSourceRange();
-    FullSourceLoc loc(type_info_range.getBegin(), manager);
-
-    // Build the FixIt insertion point after the end of the method definition,
-    // including any const-qualifiers and attributes, and before the opening
-    // of the l-curly-brace (if inline) or the semi-color (if a declaration).
-    SourceLocation spelling_end =
-        manager.getSpellingLoc(type_info_range.getEnd());
-    if (spelling_end.isValid()) {
-      SourceLocation token_end =
-          Lexer::getLocForEndOfToken(spelling_end, 0, manager, LangOptions());
-      diagnostic().Report(token_end, diag_method_requires_override_)
-          << FixItHint::CreateInsertion(token_end, " override");
+    SourceRange range = method->getSourceRange();
+    SourceLocation loc;
+    if (method->hasInlineBody()) {
+      loc = method->getBody()->getSourceRange().getBegin();
     } else {
-      diagnostic().Report(loc, diag_method_requires_override_);
+      // TODO(dcheng): We should probably use ASTContext's LangOptions here.
+      LangOptions lang_options;
+      loc = Lexer::getLocForEndOfToken(
+          manager.getSpellingLoc(range.getEnd()), 0,
+          manager, lang_options);
+      // 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);
+        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)) {
+          loc = SourceLocation();
+          break;
+        }
+        if (token.is(tok::r_paren)) {
+          break;
+        } else if (token.is(tok::equal)) {
+          loc = l;
+          break;
+        }
+      }
+    }
+    if (loc.isValid()) {
+      diagnostic().Report(loc, diag_method_requires_override_)
+          << FixItHint::CreateInsertion(loc, " override");
+    } else {
+      diagnostic().Report(range.getBegin(), diag_method_requires_override_);
     }
   }
 
   if (final_attr && override_attr) {
     diagnostic().Report(override_attr->getLocation(),
                         diag_redundant_virtual_specifier_)
         << override_attr << final_attr
         << FixItHint::CreateRemoval(override_attr->getRange());
   }
 
@@ skipping 100 matching lines @@
       return PublicDestructor;
     }
   }
 
   return None;
 }
 
 // Adds either a warning or error, based on the current handling of
 // -Werror.
 DiagnosticsEngine::Level FindBadConstructsConsumer::getErrorLevel() {
+#if defined(LLVM_ON_WIN32)
+  // TODO(dcheng): Re-enable -Werror for these diagnostics on Windows once all
+  // the pre-existing warnings are cleaned up. https://crbug.com/467287
+  return DiagnosticsEngine::Warning;
+#else
   return diagnostic().getWarningsAsErrors() ? DiagnosticsEngine::Error
                                             : DiagnosticsEngine::Warning;
+#endif
 }
 
 // Returns true if |base| specifies one of the Chromium reference counted
 // classes (base::RefCounted / base::RefCountedThreadSafe).
 bool FindBadConstructsConsumer::IsRefCountedCallback(
     const CXXBaseSpecifier* base,
     CXXBasePath& path,
     void* user_data) {
   FindBadConstructsConsumer* self =
       static_cast<FindBadConstructsConsumer*>(user_data);
@@ skipping 213 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