Bug 1322962 - UniquePtrForCpp11Lambda static checker - r?Ehsan draft
authorGerald Squelart <gsquelart@mozilla.com>
Thu, 05 Jan 2017 16:06:02 -0800
changeset 456854 47953a863f83f423bbcca8e39c3cf19da6f776dc
parent 456851 dbdd84147be52c203cdb33bcc3adc7007ef724de
child 541341 c2ec33887fa2bfa766c5e8ee07d08879a0b16f7e
push id40626
push usergsquelart@mozilla.com
push dateFri, 06 Jan 2017 11:14:25 +0000
reviewersEhsan
bugs1322962
milestone53.0a1
Bug 1322962 - UniquePtrForCpp11Lambda static checker - r?Ehsan Ensures that UniquePtrForCpp11Lambda is never copied, except when captured in a C++11 lambda. This is done by catching all calls to the copy constructor that are not directly under a lambda expression (which is where captures are done.) Note that this is a temporary measure, until we can use C++14, after which UniquePtrForCpp11Lambda and this checker won't be needed anymore. So I didn't try to make it more generalizable (by e.g. having an annotation for this type of classes.) MozReview-Commit-ID: C7kv4KvLf1k
build/clang-plugin/Checks.inc
build/clang-plugin/ChecksIncludes.inc
build/clang-plugin/UniquePtrForCpp11LambdaChecker.cpp
build/clang-plugin/UniquePtrForCpp11LambdaChecker.h
build/clang-plugin/moz.build
build/clang-plugin/tests/TestUniquePtrForCpp11LambdaChecker.cpp
build/clang-plugin/tests/moz.build
--- a/build/clang-plugin/Checks.inc
+++ b/build/clang-plugin/Checks.inc
@@ -22,8 +22,9 @@ CHECK(NonMemMovableTemplateArgChecker, "
 CHECK(NonParamInsideFunctionDeclChecker, "non-memmovable-template-arg")
 CHECK(OverrideBaseCallChecker, "override-base-call")
 CHECK(OverrideBaseCallUsageChecker, "override-base-call-usage")
 CHECK(RefCountedCopyConstructorChecker, "refcounted-copy-constructor")
 CHECK(RefCountedInsideLambdaChecker, "refcounted-inside-lambda")
 CHECK(ScopeChecker, "scope")
 CHECK(SprintfLiteralChecker, "sprintf-literal")
 CHECK(TrivialCtorDtorChecker, "trivial-constructor-destructor")
+CHECK(UniquePtrForCpp11LambdaChecker, "UniquePtrForCpp11LambdaChecker-checker")
--- a/build/clang-plugin/ChecksIncludes.inc
+++ b/build/clang-plugin/ChecksIncludes.inc
@@ -23,9 +23,9 @@
 #include "NonParamInsideFunctionDeclChecker.h"
 #include "OverrideBaseCallChecker.h"
 #include "OverrideBaseCallUsageChecker.h"
 #include "RefCountedCopyConstructorChecker.h"
 #include "RefCountedInsideLambdaChecker.h"
 #include "ScopeChecker.h"
 #include "SprintfLiteralChecker.h"
 #include "TrivialCtorDtorChecker.h"
-
+#include "UniquePtrForCpp11LambdaChecker.h"
new file mode 100644
--- /dev/null
+++ b/build/clang-plugin/UniquePtrForCpp11LambdaChecker.cpp
@@ -0,0 +1,63 @@
+/* This Source Code Form is subject to the terms of the Mozilla Public
+ * License, v. 2.0. If a copy of the MPL was not distributed with this
+ * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
+
+#include "UniquePtrForCpp11LambdaChecker.h"
+#include "CustomMatchers.h"
+
+#include <iostream>
+
+void UniquePtrForCpp11LambdaChecker::registerMatchers(MatchFinder* AstMatcher) {
+  // UniquePtrForCpp11Lambda copy under Decl is always bad.
+  AstMatcher->addMatcher(
+    decl(
+      has(
+        cxxConstructExpr(
+          hasDeclaration(
+            cxxConstructorDecl(
+              isCopyConstructor(),
+              hasParent(
+                classTemplateSpecializationDecl(
+                  hasName("mozilla::UniquePtrForCpp11Lambda")
+                )
+              )
+            )
+          )
+        ).bind("constructExpr")
+      )
+    ),
+    this);
+
+  // UniquePtrForCpp11Lambda copy under non-lambda Expr is always bad.
+  AstMatcher->addMatcher(
+    expr(
+      unless(lambdaExpr()),
+      has(
+        cxxConstructExpr(
+          hasDeclaration(
+            cxxConstructorDecl(
+              isCopyConstructor(),
+              hasParent(
+                classTemplateSpecializationDecl(
+                  hasName("mozilla::UniquePtrForCpp11Lambda")
+                )
+              )
+            )
+          )
+        ).bind("constructExpr")
+      )
+    ),
+    this);
+}
+
+void UniquePtrForCpp11LambdaChecker::check(
+    const MatchFinder::MatchResult &Result) {
+  const char* Error =
+    "Invalid copy of UniquePtrForCpp11Lambda outside of a lambda capture";
+
+  // Everything we needed to know was checked in the matcher - we just report
+  // the error here
+  const CXXConstructExpr* constructExpr =
+    Result.Nodes.getNodeAs<CXXConstructExpr>("constructExpr");
+  diag(constructExpr->getLocation(), Error, DiagnosticIDs::Error);
+}
new file mode 100644
--- /dev/null
+++ b/build/clang-plugin/UniquePtrForCpp11LambdaChecker.h
@@ -0,0 +1,19 @@
+/* This Source Code Form is subject to the terms of the Mozilla Public
+ * License, v. 2.0. If a copy of the MPL was not distributed with this
+ * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
+
+#ifndef UniquePtrForCpp11LambdaChecker_h__
+#define UniquePtrForCpp11LambdaChecker_h__
+
+#include "plugin.h"
+
+class UniquePtrForCpp11LambdaChecker : public BaseCheck {
+public:
+  UniquePtrForCpp11LambdaChecker(StringRef CheckName,
+                                 ContextType *Context = nullptr)
+    : BaseCheck(CheckName, Context) {}
+  void registerMatchers(MatchFinder* AstMatcher) override;
+  void check(const MatchFinder::MatchResult &Result) override;
+};
+
+#endif
--- a/build/clang-plugin/moz.build
+++ b/build/clang-plugin/moz.build
@@ -28,16 +28,17 @@ UNIFIED_SOURCES += [
     'NonParamInsideFunctionDeclChecker.cpp',
     'OverrideBaseCallChecker.cpp',
     'OverrideBaseCallUsageChecker.cpp',
     'RefCountedCopyConstructorChecker.cpp',
     'RefCountedInsideLambdaChecker.cpp',
     'ScopeChecker.cpp',
     'SprintfLiteralChecker.cpp',
     'TrivialCtorDtorChecker.cpp',
+    'UniquePtrForCpp11LambdaChecker.cpp'
 ]
 
 DISABLE_STL_WRAPPING = True
 NO_VISIBILITY_FLAGS = True
 
 # libc++ is required to build plugins against clang on OS X.
 if CONFIG['HOST_OS_ARCH'] == 'Darwin':
     CXXFLAGS += ['-stdlib=libc++']
new file mode 100644
--- /dev/null
+++ b/build/clang-plugin/tests/TestUniquePtrForCpp11LambdaChecker.cpp
@@ -0,0 +1,71 @@
+namespace mozilla {
+
+// Simplified version of UniquePtrForCpp11Lambda.
+template <typename T>
+class UniquePtrForCpp11Lambda
+{
+public:
+  UniquePtrForCpp11Lambda() {}
+  UniquePtrForCpp11Lambda(UniquePtrForCpp11Lambda&& aOther) {}
+  UniquePtrForCpp11Lambda& operator=(UniquePtrForCpp11Lambda&& aOther) { return *this; }
+
+  // The copy-constructor that shouldn't exist, which makes capturing by a
+  // C++11 lambda possible.
+  UniquePtrForCpp11Lambda(
+#ifdef _MSC_VER
+  // For some reason, MSC doesn't see the non-const-ref copy constructor
+  // when lambda-capturing, so as an ugly hack we implement the usual const-ref
+  // copy constructor, but make mUPtr `mutable` to allow moving from it.
+                          const
+#endif
+                                UniquePtrForCpp11Lambda& aOther) {}
+
+  // Disallow copy-assignment.
+  void operator=(const UniquePtrForCpp11Lambda& aOther) = delete;
+};
+
+// Another templated class.
+template <typename T>
+struct Boring {};
+
+} // namespace mozilla;
+
+using namespace mozilla;
+
+// Functions taking UniquePtrForCpp11Lambda by-value, which will induce a copy.
+void foo(UniquePtrForCpp11Lambda<int> aU);
+void foo(const UniquePtrForCpp11Lambda<int> aU);
+template <typename T>
+void foot(UniquePtrForCpp11Lambda<T> aU);
+template <typename T>
+void foot(const UniquePtrForCpp11Lambda<T> aU);
+
+void f()
+{
+  // Making sure we are not breaking other template classes.
+  Boring<int> boring;
+  auto boringCopy(boring); // ok
+
+  // Creation outside lambda.
+  UniquePtrForCpp11Lambda<int> u;
+
+  // Copy outside lambda.
+  auto copyOutsideLambda(u); // expected-error {{Invalid copy of UniquePtrForCpp11Lambda outside of a lambda capture}}
+  foo(u); // expected-error {{Invalid copy of UniquePtrForCpp11Lambda outside of a lambda capture}}
+  foot(u); // expected-error {{Invalid copy of UniquePtrForCpp11Lambda outside of a lambda capture}}
+
+  // Lambda capturing 'u'.
+  [u]() mutable { // ok
+    // Copy inside lambda.
+    auto copyInLambda(u); // expected-error {{Invalid copy of UniquePtrForCpp11Lambda outside of a lambda capture}}
+
+    // Deeper lambda with another capture of u.
+    static_cast<void>([u](){ // ok
+      // Need to cast 'const' away because lambda is not mutable, therefore
+      // captured 'u' is const.
+      auto copyInDeeperLambda(const_cast<UniquePtrForCpp11Lambda<int>&>(u)); // expected-error {{Invalid copy of UniquePtrForCpp11Lambda outside of a lambda capture}}
+      foo(const_cast<UniquePtrForCpp11Lambda<int>&>(u)); // expected-error {{Invalid copy of UniquePtrForCpp11Lambda outside of a lambda capture}}
+      foot(const_cast<UniquePtrForCpp11Lambda<int>&>(u)); // expected-error {{Invalid copy of UniquePtrForCpp11Lambda outside of a lambda capture}}
+    });
+  }();
+}
--- a/build/clang-plugin/tests/moz.build
+++ b/build/clang-plugin/tests/moz.build
@@ -35,12 +35,13 @@ SOURCES += [
     'TestNonTemporaryClass.cpp',
     'TestNoRefcountedInsideLambdas.cpp',
     'TestOverrideBaseCall.cpp',
     'TestOverrideBaseCallAnnotation.cpp',
     'TestRefCountedCopyConstructor.cpp',
     'TestSprintfLiteral.cpp',
     'TestStackClass.cpp',
     'TestTrivialCtorDtor.cpp',
+    'TestUniquePtrForCpp11LambdaChecker.cpp',
 ]
 
 DISABLE_STL_WRAPPING = True
 NO_VISIBILITY_FLAGS = True