Stop back-patching 'readonly' Objective-C properties with 'readwrite'…

… ones.

A 'readonly' Objective-C property declared in the primary class can
effectively be shadowed by a 'readwrite' property declared within an
extension of that class, so long as the types and attributes of the
two property declarations are compatible.

Previously, this functionality was implemented by back-patching the
original 'readonly' property to make it 'readwrite', destroying source
information and causing some hideously redundant, incorrect
code. Simplify the implementation to express how this should actually
be modeled: as a separate property declaration in the extension that
shadows (via the name lookup rules) the declaration in the primary
class. While here, correct some broken Fix-Its, eliminate a pile of
redundant code, clean up the ARC migrator's handling of properties
declared in extensions, and fix debug info's naming of methods that
come from categories.

A wonderous side effect of doing this write is that it eliminates the
"AddedObjCPropertyInClassExtension" method from the AST mutation
listener, which in turn eliminates the last place where we rewrite
entire declarations in a chained PCH file or a module file. This
change (which fixes rdar://problem/18475765) will allow us to
eliminate the rewritten-decls logic from the serialization library,
and fixes a crash (rdar://problem/23247794) illustrated by the
test/PCH/chain-categories.m example.



git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@251874 91177308-0d34-0410-b5e6-96231b3b80d8

include/clang/AST/ASTMutationListener.h

@@ -92,18 +92,6 @@ class ASTMutationListener {
   virtual void AddedObjCCategoryToInterface(const ObjCCategoryDecl *CatD,
                                             const ObjCInterfaceDecl *IFD) {}
 
-  /// \brief A objc class extension redeclared or introduced a property.
-  ///
-  /// \param Prop the property in the class extension
-  ///
-  /// \param OrigProp the property from the original interface that was declared
-  /// or null if the property was introduced.
-  ///
-  /// \param ClassExt the class extension.
-  virtual void AddedObjCPropertyInClassExtension(const ObjCPropertyDecl *Prop,
-                                            const ObjCPropertyDecl *OrigProp,
-                                            const ObjCCategoryDecl *ClassExt) {}
-
   /// \brief A declaration is marked used which was not previously marked used.
   ///
   /// \param D the declaration marked used

include/clang/AST/DeclObjC.h

@@ -2524,11 +2524,6 @@ class ObjCPropertyDecl : public NamedDecl {
     PropertyAttributesAsWritten = PRVal;
   }
 
- void makeitReadWriteAttribute() {
-    PropertyAttributes &= ~OBJC_PR_readonly;
-    PropertyAttributes |= OBJC_PR_readwrite;
- }
-
   // Helper methods for accessing attributes.
 
   /// isReadOnly - Return true iff the property has a setter.

include/clang/Basic/DiagnosticSemaKinds.td

@@ -884,7 +884,8 @@ def err_property_type : Error<"property cannot have array or function type %0">;
 def error_missing_property_context : Error<
   "missing context for property implementation declaration">;
 def error_bad_property_decl : Error<
-  "property implementation must have its declaration in interface %0">;
+  "property implementation must have its declaration in interface %0 or one of "
+  "its extensions">;
 def error_category_property : Error<
   "property declared in category %0 cannot be implemented in "
   "class implementation">;

include/clang/Sema/Sema.h

@@ -3047,7 +3047,7 @@ class Sema {
   /// warning) when atomic property has one but not the other user-declared
   /// setter or getter.
   void AtomicPropertySetterGetterRules(ObjCImplDecl* IMPDecl,
-                                       ObjCContainerDecl* IDecl);
+                                       ObjCInterfaceDecl* IDecl);
 
   void DiagnoseOwningPropertyGetterSynthesis(const ObjCImplementationDecl *D);
 

include/clang/Serialization/ASTWriter.h

@@ -876,9 +876,6 @@ class ASTWriter : public ASTDeserializationListener,
   void FunctionDefinitionInstantiated(const FunctionDecl *D) override;
   void AddedObjCCategoryToInterface(const ObjCCategoryDecl *CatD,
                                     const ObjCInterfaceDecl *IFD) override;
-  void AddedObjCPropertyInClassExtension(const ObjCPropertyDecl *Prop,
-                                    const ObjCPropertyDecl *OrigProp,
-                                    const ObjCCategoryDecl *ClassExt) override;
   void DeclarationMarkedUsed(const Decl *D) override;
   void DeclarationMarkedOpenMPThreadPrivate(const Decl *D) override;
   void RedefinedHiddenDefinition(const NamedDecl *D, Module *M) override;

lib/ARCMigrate/TransProperties.cpp

@@ -96,6 +96,10 @@ class PropertiesRewriter {
 
     collectProperties(iface, AtProps);
 
+    // Look through extensions.
+    for (auto *Ext : iface->visible_extensions())
+      collectProperties(Ext, AtProps);
+
     typedef DeclContext::specific_decl_iterator<ObjCPropertyImplDecl>
         prop_impl_iterator;
     for (prop_impl_iterator
@@ -137,19 +141,6 @@ class PropertiesRewriter {
       Transaction Trans(Pass.TA);
       rewriteProperty(props, atLoc);
     }
-
-    AtPropDeclsTy AtExtProps;
-    // Look through extensions.
-    for (auto *Ext : iface->visible_extensions())
-      collectProperties(Ext, AtExtProps, &AtProps);
-
-    for (AtPropDeclsTy::iterator
-           I = AtExtProps.begin(), E = AtExtProps.end(); I != E; ++I) {
-      SourceLocation atLoc = SourceLocation::getFromRawEncoding(I->first);
-      PropsTy &props = I->second;
-      Transaction Trans(Pass.TA);
-      doActionForExtensionProp(props, atLoc);
-    }
   }
 
 private:
@@ -177,15 +168,6 @@ class PropertiesRewriter {
     }
   }
 
-  void doActionForExtensionProp(PropsTy &props, SourceLocation atLoc) {
-    llvm::DenseMap<IdentifierInfo *, PropActionKind>::iterator I;
-    I = ActionOnProp.find(props[0].PropD->getIdentifier());
-    if (I == ActionOnProp.end())
-      return;
-
-    doPropAction(I->second, props, atLoc, false);
-  }
-
   void rewriteProperty(PropsTy &props, SourceLocation atLoc) {
     ObjCPropertyDecl::PropertyAttributeKind propAttrs = getPropertyAttrs(props);
 

lib/AST/DeclObjC.cpp

@@ -161,6 +161,15 @@ ObjCPropertyDecl::findPropertyDecl(const DeclContext *DC,
         return nullptr;
   }
 
+  // If context is class, then lookup property in its extensions.
+  // This comes before property is looked up in primary class.
+  if (auto *IDecl = dyn_cast<ObjCInterfaceDecl>(DC)) {
+    for (const auto *Ext : IDecl->known_extensions())
+      if (ObjCPropertyDecl *PD = ObjCPropertyDecl::findPropertyDecl(Ext,
+                                                                    propertyID))
+        return PD;
+  }
+
   DeclContext::lookup_result R = DC->lookup(propertyID);
   for (DeclContext::lookup_iterator I = R.begin(), E = R.end(); I != E;
        ++I)
@@ -190,6 +199,15 @@ ObjCPropertyDecl *ObjCContainerDecl::FindPropertyDeclaration(
       if (Def->isHidden())
         return nullptr;
   }
+
+  // Search the extensions of a class first; they override what's in
+  // the class itself.
+  if (const auto *ClassDecl = dyn_cast<ObjCInterfaceDecl>(this)) {
+    for (const auto *Ext : ClassDecl->visible_extensions()) {
+      if (auto *P = Ext->FindPropertyDeclaration(PropertyId))
+        return P;
+    }
+  }
 
   if (ObjCPropertyDecl *PD =
         ObjCPropertyDecl::findPropertyDecl(cast<DeclContext>(this), PropertyId))
@@ -207,7 +225,7 @@ ObjCPropertyDecl *ObjCContainerDecl::FindPropertyDeclaration(
     }
     case Decl::ObjCInterface: {
       const ObjCInterfaceDecl *OID = cast<ObjCInterfaceDecl>(this);
-      // Look through categories (but not extensions).
+      // Look through categories (but not extensions; they were handled above).
       for (const auto *Cat : OID->visible_categories()) {
         if (!Cat->IsClassExtension())
           if (ObjCPropertyDecl *P = Cat->FindPropertyDeclaration(PropertyId))
@@ -327,6 +345,13 @@ void ObjCInterfaceDecl::collectPropertiesToImplement(PropertyMap &PM,
     PM[Prop->getIdentifier()] = Prop;
     PO.push_back(Prop);
   }
+  for (const auto *Ext : known_extensions()) {
+    const ObjCCategoryDecl *ClassExt = Ext;
+    for (auto *Prop : ClassExt->properties()) {
+      PM[Prop->getIdentifier()] = Prop;
+      PO.push_back(Prop);
+    }
+  }
   for (const auto *PI : all_referenced_protocols())
     PI->collectPropertiesToImplement(PM, PO);
   // Note, the properties declared only in class extensions are still copied
@@ -1182,18 +1207,47 @@ ObjCMethodDecl::findPropertyDecl(bool CheckOverrides) const {
 
   if (isPropertyAccessor()) {
     const ObjCContainerDecl *Container = cast<ObjCContainerDecl>(getParent());
-    // If container is class extension, find its primary class.
-    if (const ObjCCategoryDecl *CatDecl = dyn_cast<ObjCCategoryDecl>(Container))
-      if (CatDecl->IsClassExtension())
-        Container = CatDecl->getClassInterface();
-
     bool IsGetter = (NumArgs == 0);
 
-    for (const auto *I : Container->properties()) {
-      Selector NextSel = IsGetter ? I->getGetterName()
-                                  : I->getSetterName();
-      if (NextSel == Sel)
-        return I;
+    /// Local function that attempts to find a matching property within the
+    /// given Objective-C container.
+    auto findMatchingProperty =
+      [&](const ObjCContainerDecl *Container) -> const ObjCPropertyDecl * {
+
+      for (const auto *I : Container->properties()) {
+        Selector NextSel = IsGetter ? I->getGetterName()
+                                    : I->getSetterName();
+        if (NextSel == Sel)
+          return I;
+      }
+
+      return nullptr;
+    };
+
+    // Look in the container we were given.
+    if (const auto *Found = findMatchingProperty(Container))
+      return Found;
+
+    // If we're in a category or extension, look in the main class.
+    const ObjCInterfaceDecl *ClassDecl = nullptr;
+    if (const auto *Category = dyn_cast<ObjCCategoryDecl>(Container)) {
+      ClassDecl = Category->getClassInterface();
+      if (const auto *Found = findMatchingProperty(ClassDecl))
+        return Found;
+    } else {
+      // Determine whether the container is a class.
+      ClassDecl = dyn_cast<ObjCInterfaceDecl>(Container);
+    }
+
+    // If we have a class, check its visible extensions.
+    if (ClassDecl) {
+      for (const auto *Ext : ClassDecl->visible_extensions()) {
+        if (Ext == Container)
+          continue;
+
+        if (const auto *Found = findMatchingProperty(Ext))
+          return Found;
+      }
     }
 
     llvm_unreachable("Marked as a property accessor but no property found!");

lib/CodeGen/CGDebugInfo.cpp

@@ -216,6 +216,13 @@ StringRef CGDebugInfo::getObjCMethodName(const ObjCMethodDecl *OMD) {
   } else if (const ObjCInterfaceDecl *OID =
                  dyn_cast<const ObjCInterfaceDecl>(DC)) {
     OS << OID->getName();
+  } else if (const ObjCCategoryDecl *OC = dyn_cast<ObjCCategoryDecl>(DC)) {
+    if (OC->IsClassExtension()) {
+      OS << OC->getClassInterface()->getName();
+    } else {
+      OS << ((const NamedDecl *)OC)->getIdentifier()->getNameStart() << '('
+         << OC->getIdentifier()->getNameStart() << ')';
+    }
   } else if (const ObjCCategoryImplDecl *OCD =
                  dyn_cast<const ObjCCategoryImplDecl>(DC)) {
     OS << ((const NamedDecl *)OCD)->getIdentifier()->getNameStart() << '('

lib/Frontend/MultiplexConsumer.cpp

@@ -122,9 +122,6 @@ class MultiplexASTMutationListener : public ASTMutationListener {
   void AddedObjCCategoryToInterface(const ObjCCategoryDecl *CatD,
                                     const ObjCInterfaceDecl *IFD) override;
   void FunctionDefinitionInstantiated(const FunctionDecl *D) override;
-  void AddedObjCPropertyInClassExtension(const ObjCPropertyDecl *Prop,
-                                    const ObjCPropertyDecl *OrigProp,
-                                    const ObjCCategoryDecl *ClassExt) override;
   void DeclarationMarkedUsed(const Decl *D) override;
   void DeclarationMarkedOpenMPThreadPrivate(const Decl *D) override;
   void RedefinedHiddenDefinition(const NamedDecl *D, Module *M) override;
@@ -207,13 +204,6 @@ void MultiplexASTMutationListener::FunctionDefinitionInstantiated(
   for (auto &Listener : Listeners)
     Listener->FunctionDefinitionInstantiated(D);
 }
-void MultiplexASTMutationListener::AddedObjCPropertyInClassExtension(
-                                             const ObjCPropertyDecl *Prop,
-                                             const ObjCPropertyDecl *OrigProp,
-                                             const ObjCCategoryDecl *ClassExt) {
-  for (size_t i = 0, e = Listeners.size(); i != e; ++i)
-    Listeners[i]->AddedObjCPropertyInClassExtension(Prop, OrigProp, ClassExt);
-}
 void MultiplexASTMutationListener::DeclarationMarkedUsed(const Decl *D) {
   for (size_t i = 0, e = Listeners.size(); i != e; ++i)
     Listeners[i]->DeclarationMarkedUsed(D);

lib/Sema/SemaDeclObjC.cpp

@@ -2844,6 +2844,20 @@ void Sema::ImplMethodsVsClassMethods(Scope *S, ObjCImplDecl* IMPDecl,
   for (const auto *I : IMPDecl->instance_methods())
     InsMap.insert(I->getSelector());
 
+  // Add the selectors for getters/setters of @dynamic properties.
+  for (const auto *PImpl : IMPDecl->property_impls()) {
+    // We only care about @dynamic implementations.
+    if (PImpl->getPropertyImplementation() != ObjCPropertyImplDecl::Dynamic)
+      continue;
+
+    const auto *P = PImpl->getPropertyDecl();
+    if (!P) continue;
+
+    InsMap.insert(P->getGetterName());
+    if (!P->getSetterName().isNull())
+      InsMap.insert(P->getSetterName());
+  }
+
   // Check and see if properties declared in the interface have either 1)
   // an implementation or 2) there is a @synthesize/@dynamic implementation
   // of the property in the @implementation.

lib/Sema/SemaExprObjC.cpp

@@ -1779,8 +1779,7 @@ HandleExprPropertyRefExpr(const ObjCObjectPointerType *OPT,
                           diag::err_property_not_found_forward_class,
                           MemberName, BaseRange))
     return ExprError();
-
-  // Search for a declared property first.
+
   if (ObjCPropertyDecl *PD = IFace->FindPropertyDeclaration(Member)) {
     // Check whether we can reference this property.
     if (DiagnoseUseOfDecl(PD, MemberLoc))