Revert "Pull chromium at 2c3ffb2355a27c32f45e508ef861416b820c823b"

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 265 matching lines @@
     if (!record->hasUserDeclaredConstructor()) {
       emitWarning(record_location,
                   "Complex class/struct needs an explicit out-of-line "
                   "constructor.");
     } else {
       // Iterate across all the constructors in this file and yell if we
       // 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.");
           } else {
             emitWarning(it->getInnerLocStart(),
                         "Complex constructor has an inlined body.");
           }
-        } else if (it->isInlined() && (!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(),
-                      "Complex constructor has an inlined body.");
         }
       }
     }
   }
 
   // The destructor side is equivalent except that we don't check for
   // trivial members; 20 ints don't need a destructor.
   if (dtor_score >= 10 && !record->hasTrivialDestructor()) {
     if (!record->hasUserDeclaredDestructor()) {
       emitWarning(record_location,
                   "Complex class/struct needs an explicit out-of-line "
                   "destructor.");
     } else if (CXXDestructorDecl* dtor = record->getDestructor()) {
-      if (dtor->isInlined()) {
+      if (dtor->hasInlineBody()) {
         emitWarning(dtor->getInnerLocStart(),
                     "Complex destructor has an inline body.");
       }
     }
   }
 }
 
 bool FindBadConstructsConsumer::InTestingNamespace(const Decl* record) {
   return GetNamespace(record).find("testing") != std::string::npos;
 }
 
 bool FindBadConstructsConsumer::IsMethodInBannedOrTestingNamespace(
     const CXXMethodDecl* method) {
   if (InBannedNamespace(method))
     return true;
   for (CXXMethodDecl::method_iterator i = method->begin_overridden_methods();
        i != method->end_overridden_methods();
        ++i) {
     const CXXMethodDecl* overridden = *i;
     if (IsMethodInBannedOrTestingNamespace(overridden) ||
         // Provide an exception for ::testing::Test. gtest itself uses some
         // magic to try to make sure SetUp()/TearDown() aren't capitalized
         // incorrectly, but having the plugin enforce override is also nice.
         (InTestingNamespace(overridden) &&
-         !IsGtestTestFixture(overridden->getParent()))) {
+         (!options_.strict_virtual_specifiers ||
+          !IsGtestTestFixture(overridden->getParent())))) {
       return true;
     }
   }
 
   return false;
 }
 
 // Checks that virtual methods are correctly annotated, and have no body in a
 // header file.
 void FindBadConstructsConsumer::CheckVirtualMethods(
@@ skipping 40 matching lines @@
 // virtual method overrides a method from a base class, only the override
 // specifier should be used. If the method should not be overridden by derived
 // classes, only the final specifier should be used.
 void FindBadConstructsConsumer::CheckVirtualSpecifiers(
     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 (method->isPure() && !options_.strict_virtual_specifiers)
+    return;
+
   if (IsMethodInBannedOrTestingNamespace(method))
     return;
 
+  if (isa<CXXDestructorDecl>(method) && !options_.strict_virtual_specifiers)
+    return;
+
   SourceManager& manager = instance().getSourceManager();
 
   // Complain if a method is annotated virtual && (override || final).
-  if (has_virtual && (override_attr || final_attr)) {
+  if (has_virtual && (override_attr || final_attr) &&
+      options_.strict_virtual_specifiers) {
     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");
     } else {
       diagnostic().Report(loc, diag_method_requires_override_);
     }
   }
 
-  if (final_attr && override_attr) {
+  if (final_attr && override_attr && options_.strict_virtual_specifiers) {
     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 && options_.strict_virtual_specifiers) {
     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 324 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