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
--- 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