Back out revision 6d94504b602d (bug 602122).
authorBoris Zbarsky <bzbarsky@mit.edu>
Tue, 28 Jul 2015 17:19:46 -0400
changeset 286720 6934c180598434cc0a07841aefb658cdfaec3499
parent 286719 08653d62e6c066e1b627336d856328473d056d99
child 286721 f6d75433812ffd08291665e96a2aab873c208f3d
push id5067
push userraliiev@mozilla.com
push dateMon, 21 Sep 2015 14:04:52 +0000
treeherdermozilla-beta@14221ffe5b2f [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
bugs602122
milestone42.0a1
backs out6d94504b602dad135481478ffd3ed4b351e3a277
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
Back out revision 6d94504b602d (bug 602122).
build/clang-plugin/clang-plugin.cpp
build/clang-plugin/tests/TestNoDuplicateRefCntMember.cpp
build/clang-plugin/tests/moz.build
--- a/build/clang-plugin/clang-plugin.cpp
+++ b/build/clang-plugin/clang-plugin.cpp
@@ -91,21 +91,16 @@ private:
     virtual void run(const MatchFinder::MatchResult &Result);
   };
 
   class ExplicitOperatorBoolChecker : public MatchFinder::MatchCallback {
   public:
     virtual void run(const MatchFinder::MatchResult &Result);
   };
 
-  class NoDuplicateRefCntMemberChecker : public MatchFinder::MatchCallback {
-  public:
-    virtual void run(const MatchFinder::MatchResult &Result);
-  };
-
   class NeedsNoVTableTypeChecker : public MatchFinder::MatchCallback {
   public:
     virtual void run(const MatchFinder::MatchResult &Result);
   };
 
   class NonMemMovableChecker : public MatchFinder::MatchCallback {
   public:
     virtual void run(const MatchFinder::MatchResult &Result);
@@ -115,17 +110,16 @@ private:
   ScopeChecker globalClassChecker;
   NonHeapClassChecker nonheapClassChecker;
   ArithmeticArgChecker arithmeticArgChecker;
   TrivialCtorDtorChecker trivialCtorDtorChecker;
   NaNExprChecker nanExprChecker;
   NoAddRefReleaseOnReturnChecker noAddRefReleaseOnReturnChecker;
   RefCountedInsideLambdaChecker refCountedInsideLambdaChecker;
   ExplicitOperatorBoolChecker explicitOperatorBoolChecker;
-  NoDuplicateRefCntMemberChecker noDuplicateRefCntMemberChecker;
   NeedsNoVTableTypeChecker needsNoVTableTypeChecker;
   NonMemMovableChecker nonMemMovableChecker;
   MatchFinder astMatcher;
 };
 
 namespace {
 
 std::string getDeclarationNamespace(const Decl *decl) {
@@ -582,58 +576,16 @@ bool IsInSystemHeader(const ASTContext &
   auto &SourceManager = AC.getSourceManager();
   auto ExpansionLoc = SourceManager.getExpansionLoc(D.getLocStart());
   if (ExpansionLoc.isInvalid()) {
     return false;
   }
   return SourceManager.isInSystemHeader(ExpansionLoc);
 }
 
-const FieldDecl *getClassRefCntMember(const CXXRecordDecl *D) {
-  for (RecordDecl::field_iterator field = D->field_begin(), e = D->field_end();
-       field != e; ++field) {
-    if (field->getName() == "mRefCnt") {
-      return *field;
-    }
-  }
-  return 0;
-}
-
-const FieldDecl *getClassRefCntMember(QualType T) {
-  while (const ArrayType *arrTy = T->getAsArrayTypeUnsafe())
-    T = arrTy->getElementType();
-  CXXRecordDecl *clazz = T->getAsCXXRecordDecl();
-  return clazz ? getClassRefCntMember(clazz) : 0;
-}
-
-const FieldDecl *getBaseRefCntMember(QualType T);
-
-const FieldDecl *getBaseRefCntMember(const CXXRecordDecl *D) {
-  const FieldDecl *refCntMember = getClassRefCntMember(D);
-  if (refCntMember && isClassRefCounted(D)) {
-    return refCntMember;
-  }
-
-  for (CXXRecordDecl::base_class_const_iterator base = D->bases_begin(), e = D->bases_end();
-       base != e; ++base) {
-    refCntMember = getBaseRefCntMember(base->getType());
-    if (refCntMember) {
-      return refCntMember;
-    }
-  }
-  return 0;
-}
-
-const FieldDecl *getBaseRefCntMember(QualType T) {
-  while (const ArrayType *arrTy = T->getAsArrayTypeUnsafe())
-    T = arrTy->getElementType();
-  CXXRecordDecl *clazz = T->getAsCXXRecordDecl();
-  return clazz ? getBaseRefCntMember(clazz) : 0;
-}
-
 bool typeHasVTable(QualType T) {
   while (const ArrayType *arrTy = T->getAsArrayTypeUnsafe())
     T = arrTy->getElementType();
   CXXRecordDecl* offender = T->getAsCXXRecordDecl();
   return offender && offender->hasDefinition() && offender->isDynamicClass();
 }
 
 }
@@ -785,20 +737,16 @@ AST_POLYMORPHIC_MATCHER_P(equalsBoundNod
   };
   Visitor visitor(Node, ID, haveMatchingResult);
   bindings.visitMatches(&visitor);
   return haveMatchingResult;
 }
 
 #endif
 
-AST_MATCHER(CXXRecordDecl, hasRefCntMember) {
-  return isClassRefCounted(&Node) && getClassRefCntMember(&Node);
-}
-
 AST_MATCHER(QualType, hasVTable) {
   return typeHasVTable(Node);
 }
 
 AST_MATCHER(CXXRecordDecl, hasNeedsNoVTableTypeAttr) {
   return MozChecker::hasCustomAnnotation(&Node, "moz_needs_no_vtable_type");
 }
 
@@ -1031,20 +979,16 @@ DiagnosticsMatcher::DiagnosticsMatcher()
 
   // Older clang versions such as the ones used on the infra recognize these
   // conversions as 'operator _Bool', but newer clang versions recognize these
   // as 'operator bool'.
   astMatcher.addMatcher(methodDecl(anyOf(hasName("operator bool"),
                                          hasName("operator _Bool"))).bind("node"),
     &explicitOperatorBoolChecker);
 
-  astMatcher.addMatcher(recordDecl(allOf(decl().bind("decl"),
-                                         hasRefCntMember())),
-                        &noDuplicateRefCntMemberChecker);
-
   astMatcher.addMatcher(classTemplateSpecializationDecl(
              allOf(hasAnyTemplateArgument(refersToType(hasVTable())),
                    hasNeedsNoVTableTypeAttr())).bind("node"),
      &needsNoVTableTypeChecker);
 
   // Handle non-mem-movable template specializations
   astMatcher.addMatcher(classTemplateSpecializationDecl(
       allOf(needsMemMovable(),
@@ -1228,42 +1172,16 @@ void DiagnosticsMatcher::ExplicitOperato
       !MozChecker::hasCustomAnnotation(method, "moz_implicit") &&
       !IsInSystemHeader(method->getASTContext(), *method) &&
       isInterestingDeclForImplicitConversion(method)) {
     Diag.Report(method->getLocStart(), errorID) << clazz;
     Diag.Report(method->getLocStart(), noteID) << "'operator bool'";
   }
 }
 
-void DiagnosticsMatcher::NoDuplicateRefCntMemberChecker::run(
-    const MatchFinder::MatchResult &Result) {
-  DiagnosticsEngine &Diag = Result.Context->getDiagnostics();
-  unsigned warningID = Diag.getDiagnosticIDs()->getCustomDiagID(
-      DiagnosticIDs::Error, "Refcounted record %0 has multiple mRefCnt members");
-  unsigned note1ID = Diag.getDiagnosticIDs()->getCustomDiagID(
-      DiagnosticIDs::Note, "Superclass %0 also has an mRefCnt member");
-  unsigned note2ID = Diag.getDiagnosticIDs()->getCustomDiagID(
-      DiagnosticIDs::Note, "Consider using the _INHERITED macros for AddRef and Release here");
-
-  const CXXRecordDecl *decl = Result.Nodes.getNodeAs<CXXRecordDecl>("decl");
-  const FieldDecl *refCntMember = getClassRefCntMember(decl);
-  assert(refCntMember && "The matcher checked to make sure we have a refCntMember");
-
-  // Check every superclass for whether it has a base with a refcnt member, and warn for those which do
-  for (CXXRecordDecl::base_class_const_iterator base = decl->bases_begin(), e = decl->bases_end();
-       base != e; ++base) {
-    const FieldDecl *baseRefCntMember = getBaseRefCntMember(base->getType());
-    if (baseRefCntMember) {
-      Diag.Report(decl->getLocStart(), warningID) << decl;
-      Diag.Report(baseRefCntMember->getLocStart(), note1ID) << baseRefCntMember->getParent();
-      Diag.Report(refCntMember->getLocStart(), note2ID);
-    }
-  }
-}
-
 void DiagnosticsMatcher::NeedsNoVTableTypeChecker::run(
     const MatchFinder::MatchResult &Result) {
   DiagnosticsEngine &Diag = Result.Context->getDiagnostics();
   unsigned errorID = Diag.getDiagnosticIDs()->getCustomDiagID(
       DiagnosticIDs::Error, "%0 cannot be instantiated because %1 has a VTable");
   unsigned noteID = Diag.getDiagnosticIDs()->getCustomDiagID(
       DiagnosticIDs::Note, "bad instantiation of %0 requested here");
 
deleted file mode 100644
--- a/build/clang-plugin/tests/TestNoDuplicateRefCntMember.cpp
+++ /dev/null
@@ -1,38 +0,0 @@
-class C1 {};
-
-class RC1 {
-public:
-  virtual void AddRef();
-  virtual void Release();
-
-private:
-  int mRefCnt; // expected-note 2 {{Superclass 'RC1' also has an mRefCnt member}}
-};
-
-class RC2 : public RC1 { // expected-error {{Refcounted record 'RC2' has multiple mRefCnt members}}
-public:
-  virtual void AddRef();
-  virtual void Release();
-
-private:
-  int mRefCnt; // expected-note {{Consider using the _INHERITED macros for AddRef and Release here}}
-};
-
-class C2 : public RC1 {};
-
-class RC3 : public RC1 {};
-
-class RC4 : public C2, public RC1 {};
-
-class RC5 : public RC1 {};
-
-class RC6 : public C1, public RC5 { // expected-error {{Refcounted record 'RC6' has multiple mRefCnt members}}
-public:
-  virtual void AddRef();
-  virtual void Release();
-
-private:
-  int mRefCnt; // expected-note {{Consider using the _INHERITED macros for AddRef and Release here}}
-};
-
-class Predecl;
--- a/build/clang-plugin/tests/moz.build
+++ b/build/clang-plugin/tests/moz.build
@@ -12,17 +12,16 @@ SOURCES += [
     'TestMultipleAnnotations.cpp',
     'TestMustOverride.cpp',
     'TestMustUse.cpp',
     'TestNANTestingExpr.cpp',
     'TestNANTestingExprC.c',
     'TestNeedsNoVTableType.cpp',
     'TestNoAddRefReleaseOnReturn.cpp',
     'TestNoArithmeticExprInArgument.cpp',
-    'TestNoDuplicateRefCntMember.cpp',
     'TestNonHeapClass.cpp',
     'TestNonMemMovable.cpp',
     'TestNoRefcountedInsideLambdas.cpp',
     'TestStackClass.cpp',
     'TestTrivialCtorDtor.cpp',
 ]
 
 DISABLE_STL_WRAPPING = True