Bug 1569681: Part 1 - Add support for moz_static_local_class and moz_trivial_dtor to clang-plugin; r=Ehsan
☠☠ backed out by cc011478e0d0 ☠ ☠
authorAaron Klotz <aklotz@mozilla.com>
Mon, 29 Jul 2019 19:40:19 +0000
changeset 485183 21ea6fea046eb197b2ae29b76ed6de08221c59e5
parent 485182 55be6c59850ad8c8497eefa1799826a633f60798
child 485184 2c543b23980887779f8097ac0a0b4e45f603ea3d
push id36361
push userapavel@mozilla.com
push dateTue, 30 Jul 2019 09:49:32 +0000
treeherdermozilla-central@639f502ded6b [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersEhsan
bugs1569681
milestone70.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 1569681: Part 1 - Add support for moz_static_local_class and moz_trivial_dtor to clang-plugin; r=Ehsan This patch is in support of adding a variant of Static{Auto,Ref}Ptr for use as static locals, taking advantage of C++11 "magic statics" such that we can lazily initialize those variables in a thread-safe way. In support of those classes, this patch adds two new attributes: * `moz_static_local_class` to ensure that any instantiations of that class only occur as static local variables; * `moz_trivial_dtor` to ensure that these classes do not implicitly call `atexit` and add a whole bunch of shutdown crap. `moz_static_local_class` works similarly to `moz_global_class`, except that its object must only instantiate as static locals. `TrivialDtorChecker` is based on `TrivialCtorDtorChecker`, with the ctor-specific bits removed. Differential Revision: https://phabricator.services.mozilla.com/D39717
build/clang-plugin/CustomAttributes.inc
build/clang-plugin/CustomMatchers.h
build/clang-plugin/CustomTypeAnnotation.cpp
build/clang-plugin/CustomTypeAnnotation.h
build/clang-plugin/ScopeChecker.cpp
build/clang-plugin/TrivialDtorChecker.cpp
build/clang-plugin/TrivialDtorChecker.h
build/clang-plugin/moz.build
--- a/build/clang-plugin/CustomAttributes.inc
+++ b/build/clang-plugin/CustomAttributes.inc
@@ -19,10 +19,12 @@ ATTR(moz_no_arith_expr_in_arg)
 ATTR(moz_no_dangling_on_temporaries)
 ATTR(moz_non_autoable)
 ATTR(moz_non_memmovable)
 ATTR(moz_non_param)
 ATTR(moz_non_temporary_class)
 ATTR(moz_nonheap_class)
 ATTR(moz_required_base_method)
 ATTR(moz_stack_class)
+ATTR(moz_static_local_class)
 ATTR(moz_temporary_class)
 ATTR(moz_trivial_ctor_dtor)
+ATTR(moz_trivial_dtor)
--- a/build/clang-plugin/CustomMatchers.h
+++ b/build/clang-plugin/CustomMatchers.h
@@ -24,16 +24,22 @@ AST_MATCHER(Decl, noArithmeticExprInArgs
 }
 
 /// This matcher will match any C++ class that is marked as having a trivial
 /// constructor and destructor.
 AST_MATCHER(CXXRecordDecl, hasTrivialCtorDtor) {
   return hasCustomAttribute<moz_trivial_ctor_dtor>(&Node);
 }
 
+/// This matcher will match any C++ class that is marked as having a trivial
+/// destructor.
+AST_MATCHER(CXXRecordDecl, hasTrivialDtor) {
+  return hasCustomAttribute<moz_trivial_dtor>(&Node);
+}
+
 AST_MATCHER(CXXConstructExpr, allowsTemporary) {
   return hasCustomAttribute<moz_allow_temporary>(Node.getConstructor());
 }
 
 /// This matcher will match lvalue-ref-qualified methods.
 AST_MATCHER(CXXMethodDecl, isLValueRefQualified) {
   return Node.getRefQualifier() == RQ_LValue;
 }
--- a/build/clang-plugin/CustomTypeAnnotation.cpp
+++ b/build/clang-plugin/CustomTypeAnnotation.cpp
@@ -11,16 +11,18 @@ CustomTypeAnnotation GlobalClass =
     CustomTypeAnnotation(moz_global_class, "global");
 CustomTypeAnnotation NonHeapClass =
     CustomTypeAnnotation(moz_nonheap_class, "non-heap");
 CustomTypeAnnotation HeapClass = CustomTypeAnnotation(moz_heap_class, "heap");
 CustomTypeAnnotation NonTemporaryClass =
     CustomTypeAnnotation(moz_non_temporary_class, "non-temporary");
 CustomTypeAnnotation TemporaryClass =
     CustomTypeAnnotation(moz_temporary_class, "temporary");
+CustomTypeAnnotation StaticLocalClass =
+    CustomTypeAnnotation(moz_static_local_class, "static-local");
 
 void CustomTypeAnnotation::dumpAnnotationReason(BaseCheck &Check, QualType T,
                                                 SourceLocation Loc) {
   const char *Inherits =
       "%1 is a %0 type because it inherits from a %0 type %2";
   const char *Member = "%1 is a %0 type because member %2 is a %0 type %3";
   const char *Array = "%1 is a %0 type because it is an array of %0 type %2";
   const char *Templ =
--- a/build/clang-plugin/CustomTypeAnnotation.h
+++ b/build/clang-plugin/CustomTypeAnnotation.h
@@ -65,10 +65,11 @@ protected:
 };
 
 extern CustomTypeAnnotation StackClass;
 extern CustomTypeAnnotation GlobalClass;
 extern CustomTypeAnnotation NonHeapClass;
 extern CustomTypeAnnotation HeapClass;
 extern CustomTypeAnnotation NonTemporaryClass;
 extern CustomTypeAnnotation TemporaryClass;
+extern CustomTypeAnnotation StaticLocalClass;
 
 #endif
--- a/build/clang-plugin/ScopeChecker.cpp
+++ b/build/clang-plugin/ScopeChecker.cpp
@@ -32,16 +32,17 @@ typedef DenseMap<const MaterializeTempor
     AutomaticTemporaryMap;
 AutomaticTemporaryMap AutomaticTemporaries;
 
 void ScopeChecker::check(const MatchFinder::MatchResult &Result) {
   // There are a variety of different reasons why something could be allocated
   AllocationVariety Variety = AV_None;
   SourceLocation Loc;
   QualType T;
+  bool isStaticLocal = false;
 
   if (const ParmVarDecl *D =
           Result.Nodes.getNodeAs<ParmVarDecl>("node")) {
     if (D->hasUnparsedDefaultArg() || D->hasUninstantiatedDefaultArg()) {
       return;
     }
     if (const Expr *Default = D->getDefaultArg()) {
       if (const MaterializeTemporaryExpr *E =
@@ -61,16 +62,17 @@ void ScopeChecker::check(const MatchFind
   if (const VarDecl *D = Result.Nodes.getNodeAs<VarDecl>("node")) {
     if (D->hasGlobalStorage()) {
       Variety = AV_Global;
     } else {
       Variety = AV_Automatic;
     }
     T = D->getType();
     Loc = D->getBeginLoc();
+    isStaticLocal = D->isStaticLocal();
   } else if (const CXXNewExpr *E = Result.Nodes.getNodeAs<CXXNewExpr>("node")) {
     // New allocates things on the heap.
     // We don't consider placement new to do anything, as it doesn't actually
     // allocate the storage, and thus gives us no useful information.
     if (!isPlacementNew(E)) {
       Variety = AV_Heap;
       T = E->getAllocatedType();
       Loc = E->getBeginLoc();
@@ -121,47 +123,58 @@ void ScopeChecker::check(const MatchFind
 
   // Error messages for incorrect allocations.
   const char *Stack = "variable of type %0 only valid on the stack";
   const char *Global = "variable of type %0 only valid as global";
   const char *Heap = "variable of type %0 only valid on the heap";
   const char *NonHeap = "variable of type %0 is not valid on the heap";
   const char *NonTemporary = "variable of type %0 is not valid in a temporary";
   const char *Temporary = "variable of type %0 is only valid as a temporary";
+  const char *StaticLocal = "variable of type %0 is only valid as a static "
+                            "local";
 
   const char *StackNote =
       "value incorrectly allocated in an automatic variable";
   const char *GlobalNote = "value incorrectly allocated in a global variable";
   const char *HeapNote = "value incorrectly allocated on the heap";
   const char *TemporaryNote = "value incorrectly allocated in a temporary";
 
   // Report errors depending on the annotations on the input types.
   switch (Variety) {
   case AV_None:
     return;
 
   case AV_Global:
     StackClass.reportErrorIfPresent(*this, T, Loc, Stack, GlobalNote);
     HeapClass.reportErrorIfPresent(*this, T, Loc, Heap, GlobalNote);
     TemporaryClass.reportErrorIfPresent(*this, T, Loc, Temporary, GlobalNote);
+    if (!isStaticLocal) {
+      StaticLocalClass.reportErrorIfPresent(*this, T, Loc, StaticLocal,
+                                            GlobalNote);
+    }
     break;
 
   case AV_Automatic:
     GlobalClass.reportErrorIfPresent(*this, T, Loc, Global, StackNote);
     HeapClass.reportErrorIfPresent(*this, T, Loc, Heap, StackNote);
     TemporaryClass.reportErrorIfPresent(*this, T, Loc, Temporary, StackNote);
+    StaticLocalClass.reportErrorIfPresent(*this, T, Loc, StaticLocal,
+                                          StackNote);
     break;
 
   case AV_Temporary:
     GlobalClass.reportErrorIfPresent(*this, T, Loc, Global, TemporaryNote);
     HeapClass.reportErrorIfPresent(*this, T, Loc, Heap, TemporaryNote);
     NonTemporaryClass.reportErrorIfPresent(*this, T, Loc, NonTemporary,
                                            TemporaryNote);
+    StaticLocalClass.reportErrorIfPresent(*this, T, Loc, StaticLocal,
+                                          TemporaryNote);
     break;
 
   case AV_Heap:
     GlobalClass.reportErrorIfPresent(*this, T, Loc, Global, HeapNote);
     StackClass.reportErrorIfPresent(*this, T, Loc, Stack, HeapNote);
     NonHeapClass.reportErrorIfPresent(*this, T, Loc, NonHeap, HeapNote);
     TemporaryClass.reportErrorIfPresent(*this, T, Loc, Temporary, HeapNote);
+    StaticLocalClass.reportErrorIfPresent(*this, T, Loc, StaticLocal, HeapNote);
     break;
   }
 }
copy from build/clang-plugin/TrivialCtorDtorChecker.cpp
copy to build/clang-plugin/TrivialDtorChecker.cpp
--- a/build/clang-plugin/TrivialCtorDtorChecker.cpp
+++ b/build/clang-plugin/TrivialDtorChecker.cpp
@@ -1,29 +1,24 @@
 /* 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 "TrivialCtorDtorChecker.h"
+#include "TrivialDtorChecker.h"
 #include "CustomMatchers.h"
 
-void TrivialCtorDtorChecker::registerMatchers(MatchFinder *AstMatcher) {
-  AstMatcher->addMatcher(cxxRecordDecl(hasTrivialCtorDtor()).bind("node"),
+void TrivialDtorChecker::registerMatchers(MatchFinder *AstMatcher) {
+  AstMatcher->addMatcher(cxxRecordDecl(hasTrivialDtor()).bind("node"),
                          this);
 }
 
-void TrivialCtorDtorChecker::check(const MatchFinder::MatchResult &Result) {
-  const char *Error = "class %0 must have trivial constructors and destructors";
+void TrivialDtorChecker::check(const MatchFinder::MatchResult &Result) {
+  const char *Error = "class %0 must have a trivial destructor";
   const CXXRecordDecl *Node = Result.Nodes.getNodeAs<CXXRecordDecl>("node");
 
   if (!Node->hasDefinition()) {
     return;
   }
 
-  // We need to accept non-constexpr trivial constructors as well. This occurs
-  // when a struct contains pod members, which will not be initialized. As
-  // constexpr values are initialized, the constructor is non-constexpr.
-  bool BadCtor = !(Node->hasConstexprDefaultConstructor() ||
-                   Node->hasTrivialDefaultConstructor());
   bool BadDtor = !Node->hasTrivialDestructor();
-  if (BadCtor || BadDtor)
+  if (BadDtor)
     diag(Node->getBeginLoc(), Error, DiagnosticIDs::Error) << Node;
 }
copy from build/clang-plugin/TrivialCtorDtorChecker.h
copy to build/clang-plugin/TrivialDtorChecker.h
--- a/build/clang-plugin/TrivialCtorDtorChecker.h
+++ b/build/clang-plugin/TrivialDtorChecker.h
@@ -1,18 +1,18 @@
 /* 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 TrivialCtorDtorChecker_h__
-#define TrivialCtorDtorChecker_h__
+#ifndef TrivialDtorChecker_h__
+#define TrivialDtorChecker_h__
 
 #include "plugin.h"
 
-class TrivialCtorDtorChecker : public BaseCheck {
+class TrivialDtorChecker : public BaseCheck {
 public:
-  TrivialCtorDtorChecker(StringRef CheckName, ContextType *Context = nullptr)
+  TrivialDtorChecker(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
@@ -35,16 +35,17 @@ HOST_SOURCES += [
     'OverrideBaseCallChecker.cpp',
     'OverrideBaseCallUsageChecker.cpp',
     'ParamTraitsEnumChecker.cpp',
     'RefCountedCopyConstructorChecker.cpp',
     'RefCountedInsideLambdaChecker.cpp',
     'ScopeChecker.cpp',
     'SprintfLiteralChecker.cpp',
     'TrivialCtorDtorChecker.cpp',
+    'TrivialDtorChecker.cpp',
     'VariableUsageHelpers.cpp',
 ]
 
 if CONFIG['OS_ARCH'] == 'WINNT':
     HOST_SOURCES += [
         'LoadLibraryUsageChecker.cpp',
     ]