r179547 - Revert "Speed-up ObjCMethodDecl::getOverriddenMethods()."

Argyrios Kyrtzidis akyrtzi at gmail.com
Mon Apr 15 11:47:23 PDT 2013


Author: akirtzidis
Date: Mon Apr 15 13:47:22 2013
New Revision: 179547

URL: http://llvm.org/viewvc/llvm-project?rev=179547&view=rev
Log:
Revert "Speed-up ObjCMethodDecl::getOverriddenMethods()."

This reverts commit r179436.

Due to caching, it was possible that we could miss overridden methods that
were introduced by categories later on.

Along with reverting the commit I also included a test case that would have caught this.

Modified:
    cfe/trunk/include/clang/AST/ASTContext.h
    cfe/trunk/lib/AST/ASTContext.cpp
    cfe/trunk/lib/AST/DeclObjC.cpp
    cfe/trunk/test/SemaObjC/arc-repeated-weak.mm

Modified: cfe/trunk/include/clang/AST/ASTContext.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/AST/ASTContext.h?rev=179547&r1=179546&r2=179547&view=diff
==============================================================================
--- cfe/trunk/include/clang/AST/ASTContext.h (original)
+++ cfe/trunk/include/clang/AST/ASTContext.h Mon Apr 15 13:47:22 2013
@@ -328,10 +328,6 @@ class ASTContext : public RefCountedBase
   typedef llvm::TinyPtrVector<const CXXMethodDecl*> CXXMethodVector;
   llvm::DenseMap<const CXXMethodDecl *, CXXMethodVector> OverriddenMethods;
 
-  /// \brief Used to cache results from \c getBaseObjCCategoriesAfterInterface.
-  mutable llvm::DenseMap<const ObjCInterfaceDecl *,
-            llvm::SmallVector<const ObjCCategoryDecl *, 2> > CatsAfterInterface;
-
   /// \brief Mapping from each declaration context to its corresponding lambda 
   /// mangling context.
   llvm::DenseMap<const DeclContext *, LambdaMangleContext> LambdaMangleContexts;
@@ -686,12 +682,7 @@ public:
   void getOverriddenMethods(
                         const NamedDecl *Method,
                         SmallVectorImpl<const NamedDecl *> &Overridden) const;
-
-  /// \brief Returns the ObjC categories of base classes, that were declared
-  /// after the given interface declaration.
-  void getBaseObjCCategoriesAfterInterface(const ObjCInterfaceDecl *D,
-                         SmallVectorImpl<const ObjCCategoryDecl *> &Cats) const;
-
+  
   /// \brief Notify the AST context that a new import declaration has been
   /// parsed or implicitly created within this translation unit.
   void addedLocalImportDecl(ImportDecl *Import);

Modified: cfe/trunk/lib/AST/ASTContext.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/ASTContext.cpp?rev=179547&r1=179546&r2=179547&view=diff
==============================================================================
--- cfe/trunk/lib/AST/ASTContext.cpp (original)
+++ cfe/trunk/lib/AST/ASTContext.cpp Mon Apr 15 13:47:22 2013
@@ -1127,8 +1127,8 @@ void ASTContext::getOverriddenMethods(
   assert(D);
 
   if (const CXXMethodDecl *CXXMethod = dyn_cast<CXXMethodDecl>(D)) {
-    Overridden.append(overridden_methods_begin(CXXMethod),
-                      overridden_methods_end(CXXMethod));
+    Overridden.append(CXXMethod->begin_overridden_methods(),
+                      CXXMethod->end_overridden_methods());
     return;
   }
 
@@ -1141,39 +1141,6 @@ void ASTContext::getOverriddenMethods(
   Overridden.append(OverDecls.begin(), OverDecls.end());
 }
 
-void ASTContext::getBaseObjCCategoriesAfterInterface(
-                        const ObjCInterfaceDecl *D,
-                        SmallVectorImpl<const ObjCCategoryDecl *> &Cats) const {
-  if (!D)
-    return;
-  
-  typedef llvm::SmallVector<const ObjCCategoryDecl *, 2> VecTy;
-  typedef llvm::DenseMap<const ObjCInterfaceDecl *, VecTy> MapTy;
-  
-  std::pair<MapTy::iterator, bool>
-    InsertOp = CatsAfterInterface.insert(std::make_pair(D, VecTy()));
-  VecTy &Vec = InsertOp.first->second;
-  if (!InsertOp.second) {
-    // already in map.
-    Cats.append(Vec.begin(), Vec.end());
-    return;
-  }
-  
-  SourceLocation Loc = D->getLocation();
-  for (const ObjCInterfaceDecl *
-         Class = D->getSuperClass(); Class; Class = Class->getSuperClass()) {
-    for (ObjCInterfaceDecl::known_categories_iterator
-           CatI = Class->known_categories_begin(),
-           CatEnd = Class->known_categories_end();
-         CatI != CatEnd; ++CatI) {
-      if (SourceMgr.isBeforeInTranslationUnit(Loc, CatI->getLocation()))
-        Vec.push_back(*CatI);
-    }
-  }
-
-  Cats.append(Vec.begin(), Vec.end());
-}
-
 void ASTContext::addedLocalImportDecl(ImportDecl *Import) {
   assert(!Import->NextLocalImport && "Import declaration already in the chain");
   assert(!Import->isFromASTFile() && "Non-local import declaration");

Modified: cfe/trunk/lib/AST/DeclObjC.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/DeclObjC.cpp?rev=179547&r1=179546&r2=179547&view=diff
==============================================================================
--- cfe/trunk/lib/AST/DeclObjC.cpp (original)
+++ cfe/trunk/lib/AST/DeclObjC.cpp Mon Apr 15 13:47:22 2013
@@ -959,6 +959,26 @@ static void collectOverriddenMethodsSlow
   }
 }
 
+static void collectOnCategoriesAfterLocation(SourceLocation Loc,
+                                             const ObjCInterfaceDecl *Class,
+                                             SourceManager &SM,
+                                             const ObjCMethodDecl *Method,
+                             SmallVectorImpl<const ObjCMethodDecl *> &Methods) {
+  if (!Class)
+    return;
+
+  for (ObjCInterfaceDecl::known_categories_iterator
+         Cat = Class->known_categories_begin(),
+         CatEnd = Class->known_categories_end();
+       Cat != CatEnd; ++Cat) {
+    if (SM.isBeforeInTranslationUnit(Loc, Cat->getLocation()))
+      CollectOverriddenMethodsRecurse(*Cat, Method, Methods, true);
+  }
+  
+  collectOnCategoriesAfterLocation(Loc, Class->getSuperClass(), SM,
+                                   Method, Methods);
+}
+
 /// \brief Faster collection that is enabled when ObjCMethodDecl::isOverriding()
 /// returns false.
 /// You'd think that in that case there are no overrides but categories can
@@ -968,7 +988,7 @@ static void collectOverriddenMethodsSlow
 /// further in super classes.
 /// Methods in an implementation can overide methods in super class's category
 /// but not in current class's category. But, such methods
-static void collectOverriddenMethodsFast(ASTContext &Ctx,
+static void collectOverriddenMethodsFast(SourceManager &SM,
                                          const ObjCMethodDecl *Method,
                              SmallVectorImpl<const ObjCMethodDecl *> &Methods) {
   assert(!Method->isOverriding());
@@ -981,11 +1001,8 @@ static void collectOverriddenMethodsFast
   if (!Class)
     return;
 
-  SmallVector<const ObjCCategoryDecl *, 32> Cats;
-  Ctx.getBaseObjCCategoriesAfterInterface(Class, Cats);
-  for (SmallVectorImpl<const ObjCCategoryDecl *>::iterator
-         I = Cats.begin(), E = Cats.end(); I != E; ++I)
-    CollectOverriddenMethodsRecurse(*I, Method, Methods, true);
+  collectOnCategoriesAfterLocation(Class->getLocation(), Class->getSuperClass(),
+                                   SM, Method, Methods);
 }
 
 void ObjCMethodDecl::getOverriddenMethods(
@@ -998,7 +1015,8 @@ void ObjCMethodDecl::getOverriddenMethod
   }
 
   if (!Method->isOverriding()) {
-    collectOverriddenMethodsFast(getASTContext(), Method, Overridden);
+    collectOverriddenMethodsFast(getASTContext().getSourceManager(),
+                                 Method, Overridden);
   } else {
     collectOverriddenMethodsSlow(Method, Overridden);
     assert(!Overridden.empty() &&

Modified: cfe/trunk/test/SemaObjC/arc-repeated-weak.mm
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaObjC/arc-repeated-weak.mm?rev=179547&r1=179546&r2=179547&view=diff
==============================================================================
--- cfe/trunk/test/SemaObjC/arc-repeated-weak.mm (original)
+++ cfe/trunk/test/SemaObjC/arc-repeated-weak.mm Mon Apr 15 13:47:22 2013
@@ -327,6 +327,28 @@ void doWhileLoop(Test *a) {
 }
 @end
 
+ at interface Base1
+ at end
+ at interface Sub1 : Base1
+ at end
+ at interface Sub1(cat)
+-(id)prop;
+ at end
+
+void test1(Sub1 *s) {
+  use([s prop]);
+  use([s prop]);
+}
+
+ at interface Base1(cat)
+ at property (weak) id prop;
+ at end
+
+void test2(Sub1 *s) {
+  use([s prop]); // expected-warning{{weak property 'prop' is accessed multiple times}}
+  use([s prop]); // expected-note{{also accessed here}}
+}
+
 
 class Wrapper {
   Test *a;





More information about the cfe-commits mailing list