Bug 1156802 - Part 1: Add an analysis which prohibits explicit move constructors, r=ehsan
authorMichael Layzell <michael@thelayzells.com>
Thu, 03 Sep 2015 10:31:57 -0400
changeset 262173 63f3a49b15cbce9c99a4ba7f8fce2602dfc0b3ed
parent 262172 3de175bd4e83775e35077d71167967fa457d0e0a
child 262174 c1cdde5ccb2ce94790fd7f6f7dfd0f21d69cf72d
push id29362
push userphilringnalda@gmail.com
push dateSat, 12 Sep 2015 22:44:56 +0000
treeherdermozilla-central@0bdadeda2489 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersehsan
bugs1156802
milestone43.0a1
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
Bug 1156802 - Part 1: Add an analysis which prohibits explicit move constructors, r=ehsan
build/clang-plugin/clang-plugin.cpp
build/clang-plugin/tests/TestNoExplicitMoveConstructor.cpp
build/clang-plugin/tests/moz.build
--- a/build/clang-plugin/clang-plugin.cpp
+++ b/build/clang-plugin/clang-plugin.cpp
@@ -101,28 +101,34 @@ private:
     virtual void run(const MatchFinder::MatchResult &Result);
   };
 
   class NoAutoTypeChecker : public MatchFinder::MatchCallback {
   public:
     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;
   NaNExprChecker nanExprChecker;
   NoAddRefReleaseOnReturnChecker noAddRefReleaseOnReturnChecker;
   RefCountedInsideLambdaChecker refCountedInsideLambdaChecker;
   ExplicitOperatorBoolChecker explicitOperatorBoolChecker;
   NoDuplicateRefCntMemberChecker noDuplicateRefCntMemberChecker;
   NeedsNoVTableTypeChecker needsNoVTableTypeChecker;
   NonMemMovableChecker nonMemMovableChecker;
   ExplicitImplicitChecker explicitImplicitChecker;
   NoAutoTypeChecker noAutoTypeChecker;
+  NoExplicitMoveConstructorChecker noExplicitMoveConstructorChecker;
   MatchFinder astMatcher;
 };
 
 namespace {
 
 std::string getDeclarationNamespace(const Decl *decl) {
   const DeclContext *DC =
       decl->getDeclContext()->getEnclosingNamespaceContext();
@@ -670,16 +676,20 @@ AST_MATCHER(CXXRecordDecl, isConcreteCla
 AST_MATCHER(QualType, autoNonAutoableType) {
   if (const AutoType *T = Node->getContainedAutoType()) {
     if (const CXXRecordDecl *Rec = T->getAsCXXRecordDecl()) {
       return MozChecker::hasCustomAnnotation(Rec, "moz_non_autoable");
     }
   }
   return false;
 }
+
+AST_MATCHER(CXXConstructorDecl, isExplicitMoveConstructor) {
+  return Node.isExplicit() && Node.isMoveConstructor();
+}
 }
 }
 
 namespace {
 
 void CustomTypeAnnotation::dumpAnnotationReason(DiagnosticsEngine &Diag,
                                                 QualType T,
                                                 SourceLocation Loc) {
@@ -944,16 +954,19 @@ DiagnosticsMatcher::DiagnosticsMatcher()
       constructorDecl(isInterestingImplicitCtor(),
                       ofClass(allOf(isConcreteClass(), decl().bind("class"))),
                       unless(isMarkedImplicit()))
           .bind("ctor"),
       &explicitImplicitChecker);
 
   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.
 enum AllocationVariety {
   AV_None,
   AV_Global,
   AV_Automatic,
   AV_Temporary,
@@ -1302,16 +1315,30 @@ void DiagnosticsMatcher::NoAutoTypeCheck
       DiagnosticIDs::Note, "Please write out this type explicitly");
 
   const VarDecl *D = Result.Nodes.getNodeAs<VarDecl>("node");
 
   Diag.Report(D->getLocation(), ErrorID) << D->getType();
   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,
                                    StringRef fileName) override {
 #if CLANG_VERSION_FULL >= 306
     std::unique_ptr<MozChecker> checker(llvm::make_unique<MozChecker>(CI));
     ASTConsumerPtr other(checker->getOtherConsumer());
 
new file mode 100644
--- /dev/null
+++ b/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());
+}
--- a/build/clang-plugin/tests/moz.build
+++ b/build/clang-plugin/tests/moz.build
@@ -16,16 +16,17 @@ SOURCES += [
     'TestMustUse.cpp',
     'TestNANTestingExpr.cpp',
     'TestNANTestingExprC.c',
     'TestNeedsNoVTableType.cpp',
     'TestNoAddRefReleaseOnReturn.cpp',
     'TestNoArithmeticExprInArgument.cpp',
     'TestNoAutoType.cpp',
     'TestNoDuplicateRefCntMember.cpp',
+    'TestNoExplicitMoveConstructor.cpp',
     'TestNonHeapClass.cpp',
     'TestNonMemMovable.cpp',
     'TestNoRefcountedInsideLambdas.cpp',
     'TestStackClass.cpp',
     'TestTrivialCtorDtor.cpp',
 ]
 
 DISABLE_STL_WRAPPING = True