Backed out changeset ca8fc58d0cef (bug 1339537)
authorSebastian Hengst <archaeopteryx@coole-files.de>
Fri, 24 Mar 2017 21:23:52 +0100
changeset 349647 609ed9f634a4270237abfa58c121d976d374a7d0
parent 349646 9ffbf8ae7d06dcb05f1385784c1642b66ea54732
child 349648 7b53302c7924134147273bcfc63c60a1491fbdec
push id31556
push userphilringnalda@gmail.com
push dateSun, 26 Mar 2017 01:40:08 +0000
treeherdermozilla-central@f5e214144799 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
bugs1339537
milestone55.0a1
backs outca8fc58d0cef79d38a801e659444c504230cc9b2
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
Backed out changeset ca8fc58d0cef (bug 1339537)
build/clang-plugin/CustomTypeAnnotation.cpp
build/clang-plugin/CustomTypeAnnotation.h
build/clang-plugin/MemMoveAnnotation.h
build/clang-plugin/NonParamInsideFunctionDeclChecker.cpp
build/clang-plugin/tests/TestNonMemMovableStd.cpp
build/clang-plugin/tests/TestNonParameterChecker.cpp
--- a/build/clang-plugin/CustomTypeAnnotation.cpp
+++ b/build/clang-plugin/CustomTypeAnnotation.cpp
@@ -22,18 +22,16 @@ void CustomTypeAnnotation::dumpAnnotatio
   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 =
       "%1 is a %0 type because it has a template argument %0 type %2";
-  const char* Implicit =
-      "%1 is a %0 type because %2";
 
   AnnotationReason Reason = directAnnotationReason(T);
   for (;;) {
     switch (Reason.Kind) {
     case RK_ArrayElement:
       Check.diag(Loc, Array, DiagnosticIDs::Note) << Pretty << T << Reason.Type;
       break;
     case RK_BaseClass: {
@@ -51,47 +49,42 @@ void CustomTypeAnnotation::dumpAnnotatio
     case RK_TemplateInherited: {
       const CXXRecordDecl *Declaration = T->getAsCXXRecordDecl();
       assert(Declaration && "This type should be a C++ class");
 
       Check.diag(Declaration->getLocation(), Templ, DiagnosticIDs::Note)
         << Pretty << T << Reason.Type;
       break;
     }
-    case RK_Implicit: {
-      const TagDecl *Declaration = T->getAsTagDecl();
-      assert(Declaration && "This type should be a TagDecl");
-
-      Check.diag(Declaration->getLocation(), Implicit, DiagnosticIDs::Note)
-        << Pretty << T << Reason.ImplicitReason;
-      return;
-    }
     default:
       // FIXME (bug 1203263): note the original annotation.
       return;
     }
 
     T = Reason.Type;
     Reason = directAnnotationReason(T);
   }
 }
 
+bool CustomTypeAnnotation::hasLiteralAnnotation(QualType T) const {
+#if CLANG_VERSION_FULL >= 306
+  if (const TagDecl *D = T->getAsTagDecl()) {
+#else
+  if (const CXXRecordDecl *D = T->getAsCXXRecordDecl()) {
+#endif
+    return hasFakeAnnotation(D) || hasCustomAnnotation(D, Spelling);
+  }
+  return false;
+}
+
 CustomTypeAnnotation::AnnotationReason
 CustomTypeAnnotation::directAnnotationReason(QualType T) {
-  if (const TagDecl *D = T->getAsTagDecl()) {
-    if (hasCustomAnnotation(D, Spelling)) {
-      AnnotationReason Reason = {T, RK_Direct, nullptr, ""};
-      return Reason;
-    }
-
-    std::string ImplAnnotReason = getImplicitReason(D);
-    if (!ImplAnnotReason.empty()) {
-      AnnotationReason Reason = {T, RK_Implicit, nullptr, ImplAnnotReason};
-      return Reason;
-    }
+  if (hasLiteralAnnotation(T)) {
+    AnnotationReason Reason = {T, RK_Direct, nullptr};
+    return Reason;
   }
 
   // Check if we have a cached answer
   void *Key = T.getAsOpaquePtr();
   ReasonCache::iterator Cached = Cache.find(T.getAsOpaquePtr());
   if (Cached != Cache.end()) {
     return Cached->second;
   }
@@ -108,26 +101,26 @@ CustomTypeAnnotation::directAnnotationRe
 
   // Recurse into Base classes
   if (const CXXRecordDecl *Declaration = T->getAsCXXRecordDecl()) {
     if (Declaration->hasDefinition()) {
       Declaration = Declaration->getDefinition();
 
       for (const CXXBaseSpecifier &Base : Declaration->bases()) {
         if (hasEffectiveAnnotation(Base.getType())) {
-          AnnotationReason Reason = {Base.getType(), RK_BaseClass, nullptr, ""};
+          AnnotationReason Reason = {Base.getType(), RK_BaseClass, nullptr};
           Cache[Key] = Reason;
           return Reason;
         }
       }
 
       // Recurse into members
       for (const FieldDecl *Field : Declaration->fields()) {
         if (hasEffectiveAnnotation(Field->getType())) {
-          AnnotationReason Reason = {Field->getType(), RK_Field, Field, ""};
+          AnnotationReason Reason = {Field->getType(), RK_Field, Field};
           Cache[Key] = Reason;
           return Reason;
         }
       }
 
       // Recurse into template arguments if the annotation
       // MOZ_INHERIT_TYPE_ANNOTATIONS_FROM_TEMPLATE_ARGS is present
       if (hasCustomAnnotation(
@@ -142,17 +135,17 @@ CustomTypeAnnotation::directAnnotationRe
             Cache[Key] = Reason;
             return Reason;
           }
         }
       }
     }
   }
 
-  AnnotationReason Reason = {QualType(), RK_None, nullptr, ""};
+  AnnotationReason Reason = {QualType(), RK_None, nullptr};
   Cache[Key] = Reason;
   return Reason;
 }
 
 CustomTypeAnnotation::AnnotationReason
 CustomTypeAnnotation::tmplArgAnnotationReason(ArrayRef<TemplateArgument> Args) {
   for (const TemplateArgument &Arg : Args) {
     if (Arg.getKind() == TemplateArgument::Type) {
@@ -164,11 +157,11 @@ CustomTypeAnnotation::tmplArgAnnotationR
     } else if (Arg.getKind() == TemplateArgument::Pack) {
       AnnotationReason Reason = tmplArgAnnotationReason(Arg.getPackAsArray());
       if (Reason.Kind != RK_None) {
         return Reason;
       }
     }
   }
 
-  AnnotationReason Reason = {QualType(), RK_None, nullptr, ""};
+  AnnotationReason Reason = {QualType(), RK_None, nullptr};
   return Reason;
 }
--- a/build/clang-plugin/CustomTypeAnnotation.h
+++ b/build/clang-plugin/CustomTypeAnnotation.h
@@ -10,23 +10,21 @@
 class CustomTypeAnnotation {
   enum ReasonKind {
     RK_None,
     RK_Direct,
     RK_ArrayElement,
     RK_BaseClass,
     RK_Field,
     RK_TemplateInherited,
-    RK_Implicit,
   };
   struct AnnotationReason {
     QualType Type;
     ReasonKind Kind;
     const FieldDecl *Field;
-    std::string ImplicitReason;
 
     bool valid() const { return Kind != RK_None; }
   };
   typedef DenseMap<void *, AnnotationReason> ReasonCache;
 
   const char *Spelling;
   const char *Pretty;
   ReasonCache Cache;
@@ -50,24 +48,23 @@ public:
     if (hasEffectiveAnnotation(T)) {
       Check.diag(Loc, Error, DiagnosticIDs::Error) << T;
       Check.diag(Loc, Note, DiagnosticIDs::Note);
       dumpAnnotationReason(Check, T, Loc);
     }
   }
 
 private:
+  bool hasLiteralAnnotation(QualType T) const;
   AnnotationReason directAnnotationReason(QualType T);
   AnnotationReason tmplArgAnnotationReason(ArrayRef<TemplateArgument> Args);
 
 protected:
-  // Allow subclasses to apply annotations for reasons other than a direct
-  // annotation. A non-empty string return value means that the object D is
-  // annotated, and should contain the reason why.
-  virtual std::string getImplicitReason(const TagDecl *D) const { return ""; }
+  // Allow subclasses to apply annotations to external code:
+  virtual bool hasFakeAnnotation(const TagDecl *D) const { return false; }
 };
 
 extern CustomTypeAnnotation StackClass;
 extern CustomTypeAnnotation GlobalClass;
 extern CustomTypeAnnotation NonHeapClass;
 extern CustomTypeAnnotation HeapClass;
 extern CustomTypeAnnotation NonTemporaryClass;
 
--- a/build/clang-plugin/MemMoveAnnotation.h
+++ b/build/clang-plugin/MemMoveAnnotation.h
@@ -12,17 +12,17 @@
 class MemMoveAnnotation final : public CustomTypeAnnotation {
 public:
   MemMoveAnnotation()
       : CustomTypeAnnotation("moz_non_memmovable", "non-memmove()able") {}
 
   virtual ~MemMoveAnnotation() {}
 
 protected:
-  std::string getImplicitReason(const TagDecl *D) const override {
+  bool hasFakeAnnotation(const TagDecl *D) const override {
     // Annotate everything in ::std, with a few exceptions; see bug
     // 1201314 for discussion.
     if (getDeclarationNamespace(D) == "std") {
       // This doesn't check that it's really ::std::pair and not
       // ::std::something_else::pair, but should be good enough.
       StringRef Name = getNameChecked(D);
       if (Name == "pair" ||
           Name == "atomic" ||
@@ -43,19 +43,19 @@ protected:
           Name == "_Atomic_ushort" ||
           Name == "_Atomic_int" ||
           Name == "_Atomic_uint" ||
           Name == "_Atomic_long" ||
           Name == "_Atomic_ulong" ||
           Name == "_Atomic_llong" ||
           Name == "_Atomic_ullong" ||
           Name == "_Atomic_address") {
-        return "";
+        return false;
       }
-      return "it is an stl-provided type not guaranteed to be memmove-able";
+      return true;
     }
-    return "";
+    return false;
   }
 };
 
 extern MemMoveAnnotation NonMemMovable;
 
 #endif
--- a/build/clang-plugin/NonParamInsideFunctionDeclChecker.cpp
+++ b/build/clang-plugin/NonParamInsideFunctionDeclChecker.cpp
@@ -10,38 +10,41 @@ class NonParamAnnotation : public Custom
 public:
   NonParamAnnotation() : CustomTypeAnnotation("moz_non_param", "non-param") {};
 
 protected:
   // Adding alignas(_) on a struct implicitly marks it as MOZ_NON_PARAM, due to
   // MSVC limitations which prevent passing explcitly aligned types by value as
   // parameters. This overload of hasFakeAnnotation injects fake MOZ_NON_PARAM
   // annotations onto these types.
-  std::string getImplicitReason(const TagDecl *D) const override {
+  bool hasFakeAnnotation(const TagDecl *D) const override {
     // Check if the decl itself has an AlignedAttr on it.
     for (const Attr *A : D->attrs()) {
       if (isa<AlignedAttr>(A)) {
-        return "it has an alignas(_) annotation";
+        D->dump();
+        return true;
       }
     }
 
     // Check if any of the decl's fields have an AlignedAttr on them.
     if (auto RD = dyn_cast<RecordDecl>(D)) {
       for (auto F : RD->fields()) {
         for (auto A : F->attrs()) {
           if (isa<AlignedAttr>(A)) {
-            return ("member '" + F->getName() + "' has an alignas(_) annotation").str();
+            D->dump();
+
+            return true;
           }
         }
       }
     }
 
     // We don't need to check the types of fields, as the CustomTypeAnnotation
     // infrastructure will handle that for us.
-    return "";
+    return false;
   }
 };
 NonParamAnnotation NonParam;
 
 void NonParamInsideFunctionDeclChecker::registerMatchers(MatchFinder* AstMatcher) {
   AstMatcher->addMatcher(
       functionDecl(anyOf(allOf(isDefinition(),
                                hasAncestor(classTemplateSpecializationDecl()
@@ -92,13 +95,11 @@ void NonParamInsideFunctionDeclChecker::
            DiagnosticIDs::Note);
 
       if (Spec) {
         diag(Spec->getPointOfInstantiation(),
              "The bad argument was passed to %0 here",
              DiagnosticIDs::Note)
           << Spec->getSpecializedTemplate();
       }
-
-      NonParam.dumpAnnotationReason(*this, T, p->getLocation());
     }
   }
 }
--- a/build/clang-plugin/tests/TestNonMemMovableStd.cpp
+++ b/build/clang-plugin/tests/TestNonMemMovableStd.cpp
@@ -1,20 +1,20 @@
 #define MOZ_NEEDS_MEMMOVABLE_TYPE __attribute__((annotate("moz_needs_memmovable_type")))
 
 template<class T>
 class MOZ_NEEDS_MEMMOVABLE_TYPE Mover { T mForceInst; }; // expected-error-re 4 {{Cannot instantiate 'Mover<{{.*}}>' with non-memmovable template argument '{{.*}}'}}
 
 namespace std {
 // In theory defining things in std:: like this invokes undefined
 // behavior, but in practice it's good enough for this test case.
-template<class C> class basic_string { }; // expected-note 2 {{'std::basic_string<char>' is a non-memmove()able type because it is an stl-provided type not guaranteed to be memmove-able}} expected-note {{'std::string' (aka 'basic_string<char>') is a non-memmove()able type because it is an stl-provided type not guaranteed to be memmove-able}}
+template<class C> class basic_string { };
 typedef basic_string<char> string;
 template<class T, class U> class pair { T mT; U mU; }; // expected-note {{std::pair<bool, std::basic_string<char> >' is a non-memmove()able type because member 'mU' is a non-memmove()able type 'std::basic_string<char>'}}
-class arbitrary_name { }; // expected-note {{'std::arbitrary_name' is a non-memmove()able type because it is an stl-provided type not guaranteed to be memmove-able}}
+class arbitrary_name { };
 }
 
 class HasString { std::string m; }; // expected-note {{'HasString' is a non-memmove()able type because member 'm' is a non-memmove()able type 'std::string' (aka 'basic_string<char>')}}
 
 static Mover<std::string> bad; // expected-note {{instantiation of 'Mover<std::basic_string<char> >' requested here}}
 static Mover<HasString> bad_mem; // expected-note {{instantiation of 'Mover<HasString>' requested here}}
 static Mover<std::arbitrary_name> assumed_bad; // expected-note {{instantiation of 'Mover<std::arbitrary_name>' requested here}}
 static Mover<std::pair<bool, int>> good;
--- a/build/clang-plugin/tests/TestNonParameterChecker.cpp
+++ b/build/clang-plugin/tests/TestNonParameterChecker.cpp
@@ -2,19 +2,19 @@
 
 struct Param {};
 struct MOZ_NON_PARAM NonParam {};
 union MOZ_NON_PARAM NonParamUnion {};
 class MOZ_NON_PARAM NonParamClass {};
 enum MOZ_NON_PARAM NonParamEnum { X, Y, Z };
 enum class MOZ_NON_PARAM NonParamEnumClass { X, Y, Z };
 
-struct HasNonParamStruct { NonParam x; int y; }; // expected-note 14 {{'HasNonParamStruct' is a non-param type because member 'x' is a non-param type 'NonParam'}}
-union HasNonParamUnion { NonParam x; int y; }; // expected-note 18 {{'HasNonParamUnion' is a non-param type because member 'x' is a non-param type 'NonParam'}}
-struct HasNonParamStructUnion { HasNonParamUnion z; }; // expected-note 9 {{'HasNonParamStructUnion' is a non-param type because member 'z' is a non-param type 'HasNonParamUnion'}}
+struct HasNonParamStruct { NonParam x; int y; };
+union HasNonParamUnion { NonParam x; int y; };
+struct HasNonParamStructUnion { HasNonParamUnion z; };
 
 #define MAYBE_STATIC
 #include "NonParameterTestCases.h"
 #undef MAYBE_STATIC
 
 // Do not check typedef and using.
 typedef void (*funcTypeParam)(Param x);
 typedef void (*funcTypeNonParam)(NonParam x);
@@ -175,15 +175,15 @@ void testLambda()
     (void)[](NonParam x) -> void {}; //expected-error {{Type 'NonParam' must not be used as parameter}} expected-note {{Please consider passing a const reference instead}}
     (void)[](HasNonParamStruct x) -> void {}; //expected-error {{Type 'HasNonParamStruct' must not be used as parameter}} expected-note {{Please consider passing a const reference instead}}
     (void)[](HasNonParamUnion x) -> void {}; //expected-error {{Type 'HasNonParamUnion' must not be used as parameter}} expected-note {{Please consider passing a const reference instead}}
     (void)[](HasNonParamStructUnion x) -> void {}; //expected-error {{Type 'HasNonParamStructUnion' must not be used as parameter}} expected-note {{Please consider passing a const reference instead}}
 }
 
 // Check that alignas() implies the MOZ_NON_PARAM attribute.
 
-struct alignas(8) AlignasStruct { char a; }; // expected-note {{'AlignasStruct' is a non-param type because it has an alignas(_) annotation}}
+struct alignas(8) AlignasStruct { char a; };
 void takesAlignasStruct(AlignasStruct x) { } // expected-error {{Type 'AlignasStruct' must not be used as parameter}} expected-note {{Please consider passing a const reference instead}}
 void takesAlignasStructByRef(const AlignasStruct& x) { }
 
-struct AlignasMember { alignas(8) char a; }; // expected-note {{'AlignasMember' is a non-param type because member 'a' has an alignas(_) annotation}}
+struct AlignasMember { alignas(8) char a; };
 void takesAlignasMember(AlignasMember x) { } // expected-error {{Type 'AlignasMember' must not be used as parameter}} expected-note {{Please consider passing a const reference instead}}
 void takesAlignasMemberByRef(const AlignasMember& x) { }