Bug 1201309 - Make MOZ_INHERIT_TYPE_ANNOTATIONS_FROM_TEMPLATE_ARGS work with MOZ_NON_MEMMOVABLE. r=ehsan f=mystor
authorJed Davis <jld@mozilla.com>
Thu, 10 Sep 2015 08:23:53 -0700
changeset 294396 27221624668e0b0d5c7af329caa39e3bbb011eee
parent 294395 27c8b0669d5148373ba04ced6e3e81410849ebdb
child 294397 06d15f86c1d38bb88206007c8a2c2a16d7bae9be
push id5245
push userraliiev@mozilla.com
push dateThu, 29 Oct 2015 11:30:51 +0000
treeherdermozilla-beta@dac831dc1bd0 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersehsan
bugs1201309
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 1201309 - Make MOZ_INHERIT_TYPE_ANNOTATIONS_FROM_TEMPLATE_ARGS work with MOZ_NON_MEMMOVABLE. r=ehsan f=mystor This patch migrates moz_non_memmovable into the CustomTypeAnnotation framework; bonus side-effects are more helpful error messages and less code duplication.
build/clang-plugin/clang-plugin.cpp
build/clang-plugin/tests/TestInheritTypeAnnotationsFromTemplateArgs.cpp
build/clang-plugin/tests/TestNonMemMovable.cpp
--- a/build/clang-plugin/clang-plugin.cpp
+++ b/build/clang-plugin/clang-plugin.cpp
@@ -275,16 +275,18 @@ static CustomTypeAnnotation StackClass =
 static CustomTypeAnnotation GlobalClass =
     CustomTypeAnnotation("moz_global_class", "global");
 static CustomTypeAnnotation NonHeapClass =
     CustomTypeAnnotation("moz_nonheap_class", "non-heap");
 static CustomTypeAnnotation HeapClass =
     CustomTypeAnnotation("moz_heap_class", "heap");
 static CustomTypeAnnotation MustUse =
     CustomTypeAnnotation("moz_must_use", "must-use");
+static CustomTypeAnnotation NonMemMovable =
+  CustomTypeAnnotation("moz_non_memmovable", "non-memmove()able");
 
 class MozChecker : public ASTConsumer, public RecursiveASTVisitor<MozChecker> {
   DiagnosticsEngine &Diag;
   const CompilerInstance &CI;
   DiagnosticsMatcher matcher;
 
 public:
   MozChecker(const CompilerInstance &CI) : Diag(CI.getDiagnostics()), CI(CI) {}
@@ -469,102 +471,16 @@ bool isClassRefCounted(const CXXRecordDe
 
 bool isClassRefCounted(QualType T) {
   while (const ArrayType *arrTy = T->getAsArrayTypeUnsafe())
     T = arrTy->getElementType();
   CXXRecordDecl *clazz = T->getAsCXXRecordDecl();
   return clazz ? isClassRefCounted(clazz) : false;
 }
 
-/// A cached data of whether classes are memmovable, and if not, what
-/// declaration
-/// makes them non-movable
-typedef DenseMap<const CXXRecordDecl *, const CXXRecordDecl *>
-    InferredMovability;
-InferredMovability inferredMovability;
-
-bool isClassNonMemMovable(QualType T);
-const CXXRecordDecl *isClassNonMemMovableWorker(QualType T);
-
-const CXXRecordDecl *isClassNonMemMovableWorker(const CXXRecordDecl *D) {
-  // If we have a definition, then we want to standardize our reference to point
-  // to the definition node. If we don't have a definition, that means that
-  // either
-  // we only have a forward declaration of the type in our file, or we are being
-  // passed a template argument which is not used, and thus never instantiated
-  // by
-  // clang.
-  // As the argument isn't used, we can't memmove it (as we don't know it's
-  // size),
-  // which means not reporting an error is OK.
-  if (!D->hasDefinition()) {
-    return 0;
-  }
-  D = D->getDefinition();
-
-  // Are we explicitly marked as non-memmovable class?
-  if (MozChecker::hasCustomAnnotation(D, "moz_non_memmovable")) {
-    return D;
-  }
-
-  // Look through all base cases to figure out if the parent is a non-memmovable
-  // class.
-  for (CXXRecordDecl::base_class_const_iterator base = D->bases_begin();
-       base != D->bases_end(); ++base) {
-    const CXXRecordDecl *result = isClassNonMemMovableWorker(base->getType());
-    if (result) {
-      return result;
-    }
-  }
-
-  // Look through all members to figure out if a member is a non-memmovable
-  // class.
-  for (RecordDecl::field_iterator field = D->field_begin(), e = D->field_end();
-       field != e; ++field) {
-    const CXXRecordDecl *result = isClassNonMemMovableWorker(field->getType());
-    if (result) {
-      return result;
-    }
-  }
-
-  return 0;
-}
-
-const CXXRecordDecl *isClassNonMemMovableWorker(QualType T) {
-  while (const ArrayType *arrTy = T->getAsArrayTypeUnsafe())
-    T = arrTy->getElementType();
-  const CXXRecordDecl *clazz = T->getAsCXXRecordDecl();
-  return clazz ? isClassNonMemMovableWorker(clazz) : 0;
-}
-
-bool isClassNonMemMovable(const CXXRecordDecl *D) {
-  InferredMovability::iterator it = inferredMovability.find(D);
-  if (it != inferredMovability.end())
-    return !!it->second;
-  const CXXRecordDecl *result = isClassNonMemMovableWorker(D);
-  inferredMovability.insert(std::make_pair(D, result));
-  return !!result;
-}
-
-bool isClassNonMemMovable(QualType T) {
-  while (const ArrayType *arrTy = T->getAsArrayTypeUnsafe())
-    T = arrTy->getElementType();
-  const CXXRecordDecl *clazz = T->getAsCXXRecordDecl();
-  return clazz ? isClassNonMemMovable(clazz) : false;
-}
-
-const CXXRecordDecl *findWhyClassIsNonMemMovable(QualType T) {
-  while (const ArrayType *arrTy = T->getAsArrayTypeUnsafe())
-    T = arrTy->getElementType();
-  CXXRecordDecl *clazz = T->getAsCXXRecordDecl();
-  InferredMovability::iterator it = inferredMovability.find(clazz);
-  assert(it != inferredMovability.end());
-  return it->second;
-}
-
 template <class T> bool IsInSystemHeader(const ASTContext &AC, const T &D) {
   auto &SourceManager = AC.getSourceManager();
   auto ExpansionLoc = SourceManager.getExpansionLoc(D.getLocStart());
   if (ExpansionLoc.isInvalid()) {
     return false;
   }
   return SourceManager.isInSystemHeader(ExpansionLoc);
 }
@@ -714,17 +630,19 @@ AST_MATCHER(CXXRecordDecl, hasRefCntMemb
 
 AST_MATCHER(QualType, hasVTable) { return typeHasVTable(Node); }
 
 AST_MATCHER(CXXRecordDecl, hasNeedsNoVTableTypeAttr) {
   return MozChecker::hasCustomAnnotation(&Node, "moz_needs_no_vtable_type");
 }
 
 /// This matcher will select classes which are non-memmovable
-AST_MATCHER(QualType, isNonMemMovable) { return isClassNonMemMovable(Node); }
+AST_MATCHER(QualType, isNonMemMovable) {
+  return NonMemMovable.hasEffectiveAnnotation(Node);
+}
 
 /// This matcher will select classes which require a memmovable template arg
 AST_MATCHER(CXXRecordDecl, needsMemMovable) {
   return MozChecker::hasCustomAnnotation(&Node, "moz_needs_memmovable_type");
 }
 
 AST_MATCHER(CXXConstructorDecl, isInterestingImplicitCtor) {
   const CXXConstructorDecl *decl = Node.getCanonicalDecl();
@@ -798,16 +716,17 @@ void CustomTypeAnnotation::dumpAnnotatio
     case RK_TemplateInherited: {
       const CXXRecordDecl *Decl = T->getAsCXXRecordDecl();
       assert(Decl && "This type should be a C++ class");
 
       Diag.Report(Decl->getLocation(), TemplID) << Pretty << T << Reason.Type;
       break;
     }
     default:
+      // FIXME (bug 1203263): note the original annotation.
       return;
     }
 
     T = Reason.Type;
     Reason = directAnnotationReason(T);
   }
 }
 
@@ -1318,49 +1237,43 @@ void DiagnosticsMatcher::NeedsNoVTableTy
 void DiagnosticsMatcher::NonMemMovableChecker::run(
     const MatchFinder::MatchResult &Result) {
   DiagnosticsEngine &Diag = Result.Context->getDiagnostics();
   unsigned errorID = Diag.getDiagnosticIDs()->getCustomDiagID(
       DiagnosticIDs::Error,
       "Cannot instantiate %0 with non-memmovable template argument %1");
   unsigned note1ID = Diag.getDiagnosticIDs()->getCustomDiagID(
       DiagnosticIDs::Note, "instantiation of %0 requested here");
-  unsigned note2ID = Diag.getDiagnosticIDs()->getCustomDiagID(
-      DiagnosticIDs::Note, "%0 is non-memmovable because of the "
-                           "MOZ_NON_MEMMOVABLE annotation on %1");
-  unsigned note3ID =
-      Diag.getDiagnosticIDs()->getCustomDiagID(DiagnosticIDs::Note, "%0");
 
   // Get the specialization
   const ClassTemplateSpecializationDecl *specialization =
       Result.Nodes.getNodeAs<ClassTemplateSpecializationDecl>("specialization");
   SourceLocation requestLoc = specialization->getPointOfInstantiation();
   const CXXRecordDecl *templ =
       specialization->getSpecializedTemplate()->getTemplatedDecl();
 
   // Report an error for every template argument which is non-memmovable
   const TemplateArgumentList &args =
       specialization->getTemplateInstantiationArgs();
   for (unsigned i = 0; i < args.size(); ++i) {
     QualType argType = args[i].getAsType();
-    if (isClassNonMemMovable(args[i].getAsType())) {
-      const CXXRecordDecl *reason = findWhyClassIsNonMemMovable(argType);
+    if (NonMemMovable.hasEffectiveAnnotation(args[i].getAsType())) {
       Diag.Report(specialization->getLocation(), errorID) << specialization
                                                           << argType;
       // XXX It would be really nice if we could get the instantiation stack
       // information
       // from Sema such that we could print a full template instantiation stack,
       // however,
       // it seems as though that information is thrown out by the time we get
       // here so we
       // can only report one level of template specialization (which in many
       // cases won't
       // be useful)
       Diag.Report(requestLoc, note1ID) << specialization;
-      Diag.Report(reason->getLocation(), note2ID) << argType << reason;
+      NonMemMovable.dumpAnnotationReason(Diag, argType, requestLoc);
     }
   }
 }
 
 void DiagnosticsMatcher::ExplicitImplicitChecker::run(
     const MatchFinder::MatchResult &Result) {
   DiagnosticsEngine &Diag = Result.Context->getDiagnostics();
   unsigned ErrorID = Diag.getDiagnosticIDs()->getCustomDiagID(
--- a/build/clang-plugin/tests/TestInheritTypeAnnotationsFromTemplateArgs.cpp
+++ b/build/clang-plugin/tests/TestInheritTypeAnnotationsFromTemplateArgs.cpp
@@ -1,17 +1,37 @@
 #define MOZ_INHERIT_TYPE_ANNOTATIONS_FROM_TEMPLATE_ARGS                 \
   __attribute__((annotate("moz_inherit_type_annotations_from_template_args")))
 #define MOZ_STACK_CLASS __attribute__((annotate("moz_stack_class")))
+#define MOZ_NON_MEMMOVABLE __attribute__((annotate("moz_non_memmovable")))
+#define MOZ_NEEDS_MEMMOVABLE_TYPE __attribute__((annotate("moz_needs_memmovable_type")))
 
 class Normal {};
 class MOZ_STACK_CLASS Stack {};
 class IndirectStack : Stack {}; // expected-note {{'IndirectStack' is a stack type because it inherits from a stack type 'Stack'}}
+class ContainsStack { Stack m; }; // expected-note {{'ContainsStack' is a stack type because member 'm' is a stack type 'Stack'}}
+class MOZ_NON_MEMMOVABLE Pointery {};
+class IndirectPointery : Pointery {}; // expected-note {{'IndirectPointery' is a non-memmove()able type because it inherits from a non-memmove()able type 'Pointery'}}
+class ContainsPointery { Pointery m; }; // expected-note {{'ContainsPointery' is a non-memmove()able type because member 'm' is a non-memmove()able type 'Pointery'}}
 
 template<class T>
-class MOZ_INHERIT_TYPE_ANNOTATIONS_FROM_TEMPLATE_ARGS Template {}; // expected-note 2 {{'Template<Stack>' is a stack type because it has a template argument stack type 'Stack'}} expected-note {{'Template<IndirectStack>' is a stack type because it has a template argument stack type 'IndirectStack'}}
+class MOZ_INHERIT_TYPE_ANNOTATIONS_FROM_TEMPLATE_ARGS Template {}; // expected-note-re 5 {{'Template<{{.*}}>' is a stack type because it has a template argument stack type '{{.*}}'}} expected-note-re 5 {{'Template<{{.*}}>' is a non-memmove()able type because it has a template argument non-memmove()able type '{{.*}}'}}
 class IndirectTemplate : Template<Stack> {}; // expected-note {{'IndirectTemplate' is a stack type because it inherits from a stack type 'Template<Stack>'}}
+class ContainsTemplate { Template<Stack> m; }; // expected-note {{'ContainsTemplate' is a stack type because member 'm' is a stack type 'Template<Stack>'}}
 
 static Template<Stack> a; // expected-error {{variable of type 'Template<Stack>' only valid on the stack}} expected-note {{value incorrectly allocated in a global variable}}
 static Template<IndirectStack> b; // expected-error {{variable of type 'Template<IndirectStack>' only valid on the stack}} expected-note {{value incorrectly allocated in a global variable}}
-static Template<Normal> c;
+static Template<ContainsStack> c; // expected-error {{variable of type 'Template<ContainsStack>' only valid on the stack}} expected-note {{value incorrectly allocated in a global variable}}
 static IndirectTemplate d; // expected-error {{variable of type 'IndirectTemplate' only valid on the stack}} expected-note {{value incorrectly allocated in a global variable}}
+static ContainsTemplate e; // expected-error {{variable of type 'ContainsTemplate' only valid on the stack}} expected-note {{value incorrectly allocated in a global variable}}
+static Template<Normal> f;
 
+template<class T>
+class MOZ_NEEDS_MEMMOVABLE_TYPE Mover { char mForceInstantiation[sizeof(T)]; }; // expected-error-re 5 {{Cannot instantiate 'Mover<{{.*}}>' with non-memmovable template argument '{{.*}}'}}
+class IndirectTemplatePointery : Template<Pointery> {}; // expected-note {{'IndirectTemplatePointery' is a non-memmove()able type because it inherits from a non-memmove()able type 'Template<Pointery>'}}
+class ContainsTemplatePointery { Template<Pointery> m; }; // expected-note {{'ContainsTemplatePointery' is a non-memmove()able type because member 'm' is a non-memmove()able type 'Template<Pointery>'}}
+
+static Mover<Template<Pointery>> n; // expected-note {{instantiation of 'Mover<Template<Pointery> >' requested here}}
+static Mover<Template<IndirectPointery>> o; // expected-note {{instantiation of 'Mover<Template<IndirectPointery> >' requested here}}
+static Mover<Template<ContainsPointery>> p; // expected-note {{instantiation of 'Mover<Template<ContainsPointery> >' requested here}}
+static Mover<IndirectTemplatePointery> q; // expected-note {{instantiation of 'Mover<IndirectTemplatePointery>' requested here}}
+static Mover<ContainsTemplatePointery> r; // expected-note {{instantiation of 'Mover<ContainsTemplatePointery>' requested here}}
+static Mover<Template<Normal>> s;
--- a/build/clang-plugin/tests/TestNonMemMovable.cpp
+++ b/build/clang-plugin/tests/TestNonMemMovable.cpp
@@ -1,61 +1,61 @@
 #define MOZ_NON_MEMMOVABLE __attribute__((annotate("moz_non_memmovable")))
 #define MOZ_NEEDS_MEMMOVABLE_TYPE __attribute__((annotate("moz_needs_memmovable_type")))
 
 /*
   These are a bunch of structs with variable levels of memmovability.
   They will be used as template parameters to the various NeedyTemplates
 */
-struct MOZ_NON_MEMMOVABLE NonMovable {}; // expected-note-re + {{'{{.*}}' is non-memmovable because of the MOZ_NON_MEMMOVABLE annotation on 'NonMovable'}}
+struct MOZ_NON_MEMMOVABLE NonMovable {};
 struct Movable {};
 
 // Subclasses
-struct S_NonMovable : NonMovable {};
+struct S_NonMovable : NonMovable {}; // expected-note 48 {{'S_NonMovable' is a non-memmove()able type because it inherits from a non-memmove()able type 'NonMovable'}}
 struct S_Movable : Movable {};
 
 // Members
 struct W_NonMovable {
-  NonMovable m;
+  NonMovable m; // expected-note 32 {{'W_NonMovable' is a non-memmove()able type because member 'm' is a non-memmove()able type 'NonMovable'}}
 };
 struct W_Movable {
   Movable m;
 };
 
 // Wrapped Subclasses
 struct WS_NonMovable {
-  S_NonMovable m;
+  S_NonMovable m; // expected-note 32 {{'WS_NonMovable' is a non-memmove()able type because member 'm' is a non-memmove()able type 'S_NonMovable'}}
 };
 struct WS_Movable {
   S_Movable m;
 };
 
 // Combinations of the above
-struct SW_NonMovable : W_NonMovable {};
+struct SW_NonMovable : W_NonMovable {}; // expected-note 16 {{'SW_NonMovable' is a non-memmove()able type because it inherits from a non-memmove()able type 'W_NonMovable'}}
 struct SW_Movable : W_Movable {};
 
-struct SWS_NonMovable : WS_NonMovable {};
+struct SWS_NonMovable : WS_NonMovable {}; // expected-note 16 {{'SWS_NonMovable' is a non-memmove()able type because it inherits from a non-memmove()able type 'WS_NonMovable'}}
 struct SWS_Movable : WS_Movable {};
 
 // Basic templated wrapper
 template <class T>
 struct Template_Inline {
-  T m;
+  T m; // expected-note-re 56 {{'Template_Inline<{{.*}}>' is a non-memmove()able type because member 'm' is a non-memmove()able type '{{.*}}'}}
 };
 
 template <class T>
 struct Template_Ref {
   T* m;
 };
 
 template <class T>
 struct Template_Unused {};
 
 template <class T>
-struct MOZ_NON_MEMMOVABLE Template_NonMovable {}; // expected-note-re + {{'{{.*}}' is non-memmovable because of the MOZ_NON_MEMMOVABLE annotation on 'Template_NonMovable<{{.*}}>'}}
+struct MOZ_NON_MEMMOVABLE Template_NonMovable {};
 
 /*
   These tests take the following form:
   DECLARATIONS => Declarations of the templates which are either marked with MOZ_NEEDS_MEMMOVABLE_TYPE
                   or which instantiate a MOZ_NEEDS_MEMMOVABLE_TYPE through some mechanism.
   BAD N        => Instantiations of the wrapper template with each of the non-memmovable types.
                   The prefix S_ means subclass, W_ means wrapped. Each of these rows should produce an error
                   on the NeedyTemplate in question, and a note at the instantiation location of that template.
@@ -494,17 +494,17 @@ template <class T>
 struct MOZ_NEEDS_MEMMOVABLE_TYPE NeedyTemplate6 {T m;}; // expected-error-re 27 {{Cannot instantiate 'NeedyTemplate6<{{.*}}>' with non-memmovable template argument '{{.*}}'}}
 template <class T>
 struct W_NeedyTemplate6 {
   NeedyTemplate6<T> m; // expected-note-re 27 {{instantiation of 'NeedyTemplate6<{{.*}}>' requested here}}
 };
 template <class T>
 struct SW_NeedyTemplate6 : W_NeedyTemplate6<T> {};
 // We create a different NonMovable type here, as NeedyTemplate6 will already be instantiated with NonMovable
-struct MOZ_NON_MEMMOVABLE NonMovable2 {}; // expected-note {{'NonMovable2' is non-memmovable because of the MOZ_NON_MEMMOVABLE annotation on 'NonMovable2'}}
+struct MOZ_NON_MEMMOVABLE NonMovable2 {};
 template <class T = NonMovable2>
 struct Defaulted_SW_NeedyTemplate6 {
   SW_NeedyTemplate6<T> m;
 };
 void bad6() {
   Defaulted_SW_NeedyTemplate6<NonMovable> a1;
   Defaulted_SW_NeedyTemplate6<S_NonMovable> a2;
   Defaulted_SW_NeedyTemplate6<W_NonMovable> a3;
@@ -743,18 +743,18 @@ void good8() {
   SpecializedNonMovable is a non-movable class which has an explicit specialization of NeedyTemplate
   for it. Instantiations of NeedyTemplateN<SpecializedNonMovable> should be legal as the explicit
   specialization isn't annotated with MOZ_NEEDS_MEMMOVABLE_TYPE.
 
   However, as it is MOZ_NON_MEMMOVABLE, derived classes and members shouldn't be able to be used to
   instantiate NeedyTemplate.
 */
 
-struct MOZ_NON_MEMMOVABLE SpecializedNonMovable {}; // expected-note 8 {{'S_SpecializedNonMovable' is non-memmovable because of the MOZ_NON_MEMMOVABLE annotation on 'SpecializedNonMovable'}} expected-note 8 {{'Template_Inline<SpecializedNonMovable>' is non-memmovable because of the MOZ_NON_MEMMOVABLE annotation on 'SpecializedNonMovable'}}
-struct S_SpecializedNonMovable : SpecializedNonMovable {};
+struct MOZ_NON_MEMMOVABLE SpecializedNonMovable {};
+struct S_SpecializedNonMovable : SpecializedNonMovable {}; // expected-note 8 {{'S_SpecializedNonMovable' is a non-memmove()able type because it inherits from a non-memmove()able type 'SpecializedNonMovable'}}
 
 // Specialize all of the NeedyTemplates with SpecializedNonMovable.
 template <>
 struct NeedyTemplate1<SpecializedNonMovable> {};
 template <>
 struct NeedyTemplate2<SpecializedNonMovable> {};
 template <>
 struct NeedyTemplate3<SpecializedNonMovable> {};