Backed out changeset 3e78fb4512a6 (bug 1187486) for static analysis failures.
authorRyan VanderMeulen <ryanvm@gmail.com>
Thu, 30 Jul 2015 14:47:52 -0400
changeset 287190 1fe5e8dc9bab72edfbbcba4b879bee78582578e2
parent 287189 2bdaed564656451cc9de7f47f1120548b4cfe228
child 287191 0bd29e2ee5fafe487b99bd308639d3cd942ec4ad
push id5067
push userraliiev@mozilla.com
push dateMon, 21 Sep 2015 14:04:52 +0000
treeherdermozilla-beta@14221ffe5b2f [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
bugs1187486
milestone42.0a1
backs out3e78fb4512a68ea7eedfe0102b56c63ada8b9235
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Backed out changeset 3e78fb4512a6 (bug 1187486) for static analysis failures.
build/clang-plugin/clang-plugin.cpp
build/clang-plugin/tests/TestBadImplicitConversionCtor.cpp
--- a/build/clang-plugin/clang-plugin.cpp
+++ b/build/clang-plugin/clang-plugin.cpp
@@ -106,34 +106,28 @@ private:
     virtual void run(const MatchFinder::MatchResult &Result);
   };
 
   class NonMemMovableChecker : public MatchFinder::MatchCallback {
   public:
     virtual void run(const MatchFinder::MatchResult &Result);
   };
 
-  class ExplicitImplicitChecker : public MatchFinder::MatchCallback {
-  public:
-    virtual void run(const MatchFinder::MatchResult &Result);
-  };
-
   ScopeChecker stackClassChecker;
   ScopeChecker globalClassChecker;
   NonHeapClassChecker nonheapClassChecker;
   ArithmeticArgChecker arithmeticArgChecker;
   TrivialCtorDtorChecker trivialCtorDtorChecker;
   NaNExprChecker nanExprChecker;
   NoAddRefReleaseOnReturnChecker noAddRefReleaseOnReturnChecker;
   RefCountedInsideLambdaChecker refCountedInsideLambdaChecker;
   ExplicitOperatorBoolChecker explicitOperatorBoolChecker;
   NoDuplicateRefCntMemberChecker noDuplicateRefCntMemberChecker;
   NeedsNoVTableTypeChecker needsNoVTableTypeChecker;
   NonMemMovableChecker nonMemMovableChecker;
-  ExplicitImplicitChecker explicitImplicitChecker;
   MatchFinder astMatcher;
 };
 
 namespace {
 
 std::string getDeclarationNamespace(const Decl *decl) {
   const DeclContext *DC = decl->getDeclContext()->getEnclosingNamespaceContext();
   const NamespaceDecl *ND = dyn_cast<NamespaceDecl>(DC);
@@ -182,16 +176,17 @@ bool isInIgnoredNamespaceForImplicitConv
 
   return name == "std" ||              // standard C++ lib
          name == "__gnu_cxx" ||        // gnu C++ lib
          name == "google_breakpad" ||  // breakpad
          name == "testing";            // gtest
 }
 
 bool isIgnoredPathForImplicitCtor(const Decl *decl) {
+  decl = decl->getCanonicalDecl();
   SourceLocation Loc = decl->getLocation();
   const SourceManager &SM = decl->getASTContext().getSourceManager();
   SmallString<1024> FileName = SM.getFilename(Loc);
   llvm::sys::fs::make_absolute(FileName);
   llvm::sys::path::reverse_iterator begin = llvm::sys::path::rbegin(FileName),
                                     end   = llvm::sys::path::rend(FileName);
   for (; begin != end; ++begin) {
     if (begin->compare_lower(StringRef("skia")) == 0 ||
@@ -222,16 +217,21 @@ bool isIgnoredPathForImplicitConversion(
   for (; begin != end; ++begin) {
     if (begin->compare_lower(StringRef("graphite2")) == 0) {
       return true;
     }
   }
   return false;
 }
 
+bool isInterestingDeclForImplicitCtor(const Decl *decl) {
+  return !isInIgnoredNamespaceForImplicitCtor(decl) &&
+         !isIgnoredPathForImplicitCtor(decl);
+}
+
 bool isInterestingDeclForImplicitConversion(const Decl *decl) {
   return !isInIgnoredNamespaceForImplicitConversion(decl) &&
          !isIgnoredPathForImplicitConversion(decl);
 }
 
 }
 
 class CustomTypeAnnotation {
@@ -368,16 +368,44 @@ public:
         unsigned overrideNote = Diag.getDiagnosticIDs()->getCustomDiagID(
             DiagnosticIDs::Note, "function to override is here");
         Diag.Report(d->getLocation(), overrideID) << d->getDeclName() <<
           (*it)->getDeclName();
         Diag.Report((*it)->getLocation(), overrideNote);
       }
     }
 
+    if (!d->isAbstract() && isInterestingDeclForImplicitCtor(d)) {
+      for (CXXRecordDecl::ctor_iterator ctor = d->ctor_begin(),
+           e = d->ctor_end(); ctor != e; ++ctor) {
+        // Ignore non-converting ctors
+        if (!ctor->isConvertingConstructor(false)) {
+          continue;
+        }
+        // Ignore copy or move constructors
+        if (ctor->isCopyOrMoveConstructor()) {
+          continue;
+        }
+        // Ignore deleted constructors
+        if (ctor->isDeleted()) {
+          continue;
+        }
+        // Ignore whitelisted constructors
+        if (MozChecker::hasCustomAnnotation(*ctor, "moz_implicit")) {
+          continue;
+        }
+        unsigned ctorID = Diag.getDiagnosticIDs()->getCustomDiagID(
+          DiagnosticIDs::Error, "bad implicit conversion constructor for %0");
+        unsigned noteID = Diag.getDiagnosticIDs()->getCustomDiagID(
+          DiagnosticIDs::Note, "consider adding the explicit keyword to the constructor");
+        Diag.Report(ctor->getLocation(), ctorID) << d->getDeclName();
+        Diag.Report(ctor->getLocation(), noteID);
+      }
+    }
+
     return true;
   }
 
   bool VisitSwitchCase(SwitchCase* stmt) {
     HandleUnusedExprResult(stmt->getSubStmt());
     return true;
   }
   bool VisitCompoundStmt(CompoundStmt* stmt) {
@@ -779,39 +807,16 @@ AST_MATCHER(QualType, isNonMemMovable) {
   return isClassNonMemMovable(Node);
 }
 
 /// This matcher will select classes which require a memmovable template arg
 AST_MATCHER(CXXRecordDecl, needsMemMovable) {
   return MozChecker::hasCustomAnnotation(&Node, "moz_needs_memmovable_type");
 }
 
-AST_MATCHER(CXXConstructorDecl, isInterestingImplicitCtor) {
-  const CXXConstructorDecl *decl = Node.getCanonicalDecl();
-  return
-    // Skip ignored namespaces and paths
-    !isInIgnoredNamespaceForImplicitCtor(decl) &&
-    !isIgnoredPathForImplicitCtor(decl) &&
-    // We only want Converting constructors
-    decl->isConvertingConstructor(false) &&
-    // We don't want copy of move constructors, as those are allowed to be implicit
-    !decl->isCopyOrMoveConstructor() &&
-    // We don't want deleted constructors.
-    !decl->isDeleted();
-}
-
-// We can't call this "isImplicit" since it clashes with an existing matcher in clang.
-AST_MATCHER(CXXConstructorDecl, isMarkedImplicit) {
-  return MozChecker::hasCustomAnnotation(&Node, "moz_implicit");
-}
-
-AST_MATCHER(CXXRecordDecl, isConcreteClass) {
-  return !Node.isAbstract();
-}
-
 }
 }
 
 namespace {
 
 void CustomTypeAnnotation::dumpAnnotationReason(DiagnosticsEngine &Diag, QualType T, SourceLocation Loc) {
   unsigned InheritsID = Diag.getDiagnosticIDs()->getCustomDiagID(
     DiagnosticIDs::Note, "%1 is a %0 type because it inherits from a %0 type %2");
@@ -1041,22 +1046,16 @@ DiagnosticsMatcher::DiagnosticsMatcher()
      &needsNoVTableTypeChecker);
 
   // Handle non-mem-movable template specializations
   astMatcher.addMatcher(classTemplateSpecializationDecl(
       allOf(needsMemMovable(),
             hasAnyTemplateArgument(refersToType(isNonMemMovable())))
       ).bind("specialization"),
       &nonMemMovableChecker);
-
-  astMatcher.addMatcher(
-      constructorDecl(isInterestingImplicitCtor(),
-                      ofClass(allOf(isConcreteClass(), decl().bind("class"))),
-                      unless(isMarkedImplicit())).bind("ctor"),
-      &explicitImplicitChecker);
 }
 
 void DiagnosticsMatcher::ScopeChecker::run(
     const MatchFinder::MatchResult &Result) {
   DiagnosticsEngine &Diag = Result.Context->getDiagnostics();
   unsigned stackID = Diag.getDiagnosticIDs()->getCustomDiagID(
     DiagnosticIDs::Error, "variable of type %0 only valid on the stack");
   unsigned globalID = Diag.getDiagnosticIDs()->getCustomDiagID(
@@ -1321,33 +1320,16 @@ void DiagnosticsMatcher::NonMemMovableCh
       Diag.Report(requestLoc, note1ID)
         << specialization;
       Diag.Report(reason->getLocation(), note2ID)
         << argType << reason;
     }
   }
 }
 
-void DiagnosticsMatcher::ExplicitImplicitChecker::run(
-    const MatchFinder::MatchResult &Result) {
-  DiagnosticsEngine &Diag = Result.Context->getDiagnostics();
-  unsigned ErrorID = Diag.getDiagnosticIDs()->getCustomDiagID(
-      DiagnosticIDs::Error, "bad implicit conversion constructor for %0");
-  unsigned NoteID = Diag.getDiagnosticIDs()->getCustomDiagID(
-      DiagnosticIDs::Note, "consider adding the explicit keyword to the constructor");
-
-  // We've already checked everything in the matcher, so we just have to report the error.
-
-  const CXXConstructorDecl *Ctor = Result.Nodes.getNodeAs<CXXConstructorDecl>("ctor");
-  const CXXRecordDecl *Decl = Result.Nodes.getNodeAs<CXXRecordDecl>("class");
-
-  Diag.Report(Ctor->getLocation(), ErrorID) << Decl->getDeclName();
-  Diag.Report(Ctor->getLocation(), NoteID);
-}
-
 class MozCheckAction : public PluginASTAction {
 public:
   ASTConsumerPtr CreateASTConsumer(CompilerInstance &CI, StringRef fileName) override {
 #if CLANG_VERSION_FULL >= 306
     std::unique_ptr<MozChecker> checker(llvm::make_unique<MozChecker>(CI));
     ASTConsumerPtr other(checker->getOtherConsumer());
 
     std::vector<ASTConsumerPtr> consumers;
--- a/build/clang-plugin/tests/TestBadImplicitConversionCtor.cpp
+++ b/build/clang-plugin/tests/TestBadImplicitConversionCtor.cpp
@@ -1,16 +1,14 @@
 #define MOZ_IMPLICIT __attribute__((annotate("moz_implicit")))
 
 struct Foo {
   Foo(int); // expected-error {{bad implicit conversion constructor for 'Foo'}} expected-note {{consider adding the explicit keyword to the constructor}}
   Foo(int, char=0); // expected-error {{bad implicit conversion constructor for 'Foo'}} expected-note {{consider adding the explicit keyword to the constructor}}
   Foo(...); // expected-error {{bad implicit conversion constructor for 'Foo'}} expected-note {{consider adding the explicit keyword to the constructor}}
-  template<class T>
-  Foo(float); // expected-error {{bad implicit conversion constructor for 'Foo'}} expected-note {{consider adding the explicit keyword to the constructor}}
   Foo(int, unsigned);
   Foo(Foo&);
   Foo(const Foo&);
   Foo(volatile Foo&);
   Foo(const volatile Foo&);
   Foo(Foo&&);
   Foo(const Foo&&);
   Foo(volatile Foo&&);
@@ -36,15 +34,8 @@ struct Barn {
 };
 
 struct Abstract {
   Abstract(int);
   Abstract(int, char=0);
   Abstract(...);
   virtual void f() = 0;
 };
-
-template<class T>
-struct Template {
-  Template(int); // expected-error {{bad implicit conversion constructor for 'Template'}} expected-note {{consider adding the explicit keyword to the constructor}}
-  template<class U>
-  Template(float); // expected-error {{bad implicit conversion constructor for 'Template'}} expected-note {{consider adding the explicit keyword to the constructor}}
-};