Bug 1156802 - Part 1: Add an analysis which prohibits explicit move constructors, r=ehsan

build/clang-plugin/clang-plugin.cpp

@@ -106,6 +106,11 @@ private:
     virtual void run(const MatchFinder::MatchResult &Result);
   };
 
+  class NoExplicitMoveConstructorChecker : public MatchFinder::MatchCallback {
+  public:
+    virtual void run(const MatchFinder::MatchResult &Result);
+  };
+
   ScopeChecker scopeChecker;
   ArithmeticArgChecker arithmeticArgChecker;
   TrivialCtorDtorChecker trivialCtorDtorChecker;
@@ -118,6 +123,7 @@ private:
   NonMemMovableChecker nonMemMovableChecker;
   ExplicitImplicitChecker explicitImplicitChecker;
   NoAutoTypeChecker noAutoTypeChecker;
+  NoExplicitMoveConstructorChecker noExplicitMoveConstructorChecker;
   MatchFinder astMatcher;
 };
 
@@ -675,6 +681,10 @@ AST_MATCHER(QualType, autoNonAutoableType) {
   }
   return false;
 }
+
+AST_MATCHER(CXXConstructorDecl, isExplicitMoveConstructor) {
+  return Node.isExplicit() && Node.isMoveConstructor();
+}
 }
 }
 
@@ -949,6 +959,9 @@ DiagnosticsMatcher::DiagnosticsMatcher() {
 
   astMatcher.addMatcher(varDecl(hasType(autoNonAutoableType())).bind("node"),
                         &noAutoTypeChecker);
+
+  astMatcher.addMatcher(constructorDecl(isExplicitMoveConstructor()).bind("node"),
+                        &noExplicitMoveConstructorChecker);
 }
 
 // These enum variants determine whether an allocation has occured in the code.
@@ -1307,6 +1320,20 @@ void DiagnosticsMatcher::NoAutoTypeChecker::run(
   Diag.Report(D->getLocation(), NoteID);
 }
 
+void DiagnosticsMatcher::NoExplicitMoveConstructorChecker::run(
+    const MatchFinder::MatchResult &Result) {
+  DiagnosticsEngine &Diag = Result.Context->getDiagnostics();
+  unsigned ErrorID = Diag.getDiagnosticIDs()->getCustomDiagID(
+      DiagnosticIDs::Error, "Move constructors may not be marked explicit");
+
+  // Everything we needed to know was checked in the matcher - we just report
+  // the error here
+  const CXXConstructorDecl *D =
+    Result.Nodes.getNodeAs<CXXConstructorDecl>("node");
+
+  Diag.Report(D->getLocation(), ErrorID);
+}
+
 class MozCheckAction : public PluginASTAction {
 public:
   ASTConsumerPtr CreateASTConsumer(CompilerInstance &CI,

build/clang-plugin/tests/TestNoExplicitMoveConstructor.cpp

@@ -0,0 +1,25 @@
+class Foo {
+  Foo(Foo&& f);
+};
+
+class Bar {
+  explicit Bar(Bar&& f); // expected-error {{Move constructors may not be marked explicit}}
+};
+
+class Baz {
+  template<typename T>
+  explicit Baz(T&& f) {};
+};
+
+class Quxx {
+  Quxx();
+  Quxx(Quxx& q) = delete;
+  template<typename T>
+  explicit Quxx(T&& f) {};
+};
+
+void f() {
+  // Move a quxx into a quxx! (This speciailizes Quxx's constructor to look like
+  // a move constructor - to make sure it doesn't trigger)
+  Quxx(Quxx());
+}

build/clang-plugin/tests/moz.build

@@ -21,6 +21,7 @@ SOURCES += [
     'TestNoArithmeticExprInArgument.cpp',
     'TestNoAutoType.cpp',
     'TestNoDuplicateRefCntMember.cpp',
+    'TestNoExplicitMoveConstructor.cpp',
     'TestNonHeapClass.cpp',
     'TestNonMemMovable.cpp',
     'TestNoRefcountedInsideLambdas.cpp',