[indexer] Relate properties to their backing ivars and mark implicit getters and setters with the Implicit role

[nathawes]

Summary

Also output property + getter/setter definition occurrences for both explicit and implicit @synthesize nodes.

[akyrtzi]

lib/Index/IndexDecl.cpp

290

You can use D->property_impls() to simplify the code here.

test/Index/Core/index-source.m

238

For this case there is no property definition emitted, so the fooproperty doesn't get associated with the ivar as accessor. We should a emit an implicit property definition, with location pointing at @implementation, and relate it to the ivar.

[nathawes]

Also report an implicit property definition (with relation to the ivar), getter and setter when the @synthesize is implicit.

[akyrtzi]

lib/Index/IndexDecl.cpp

286

I should be ObjCPropertyImplDecl and you don't need the cast, right ?

[nathawes]

Remove pointless cast.

[akyrtzi]

Committed in r295416, thanks!

lib/Index/IndexDecl.cpp

     if (MethodLoc.isInvalid())
       MethodLoc = D->getLocation();
 
-    if (!IndexCtx.handleDecl(D, MethodLoc, (unsigned)SymbolRole::Dynamic, Relations))
+    SymbolRoleSet Roles = (SymbolRoleSet)SymbolRole::Dynamic;
+    if (D->isImplicit())
+      Roles |= (SymbolRoleSet)SymbolRole::Implicit;
+    if (!IndexCtx.handleDecl(D, MethodLoc, Roles, Relations))
       return false;
     IndexCtx.indexTypeSourceInfo(D->getReturnTypeSourceInfo(), D);
     bool hasIBActionAndFirst = D->hasAttr<IBActionAttr>();

 
   bool VisitObjCIvarDecl(const ObjCIvarDecl *D) {
     if (D->getSynthesize()) {
-      // For synthesized ivars, use the location of the ObjC implementation,
-      // not the location of the property.
-      // Otherwise the header file containing the @interface will have different
-      // indexing contents based on whether the @implementation was present or
-      // not in the translation unit.
-      return IndexCtx.handleDecl(D,
-                                 cast<Decl>(D->getDeclContext())->getLocation(),
-                                 (unsigned)SymbolRole::Implicit);
+      // handled in VisitObjCPropertyImplDecl
+      return true;
     }
     if (!IndexCtx.handleDecl(D))
       return false;

     if (!IndexCtx.handleDecl(D))
       return false;
 
-    // Index the ivars first to make sure the synthesized ivars are indexed
-    // before indexing the methods that can reference them.
-    for (const auto *IvarI : D->ivars())
-      IndexCtx.indexDecl(IvarI);
+    // Visit implicit @synthesize property implementations first as their
+    // location is reported at the name of the @implementation block. This
+    // serves no purpose other than to simplify the FileCheck-based tests.
+    for (const auto *I : D->property_impls()) {
+      if (I->getLocation().isInvalid())
+        IndexCtx.indexDecl(I);
+    }
     for (const auto *I : D->decls()) {
-      if (!isa<ObjCIvarDecl>(I))
+      if (!isa<ObjCPropertyImplDecl>(I) ||
+          cast<ObjCPropertyImplDecl>(I)->getLocation().isValid())
         IndexCtx.indexDecl(I);
     }
 

 
   bool VisitObjCPropertyImplDecl(const ObjCPropertyImplDecl *D) {
     ObjCPropertyDecl *PD = D->getPropertyDecl();
-    if (!IndexCtx.handleReference(PD, D->getLocation(),
-                             /*Parent=*/cast<NamedDecl>(D->getDeclContext()),
-                             D->getDeclContext(), SymbolRoleSet(), {},
-                             /*RefE=*/nullptr, D))
+    auto *Container = cast<ObjCImplDecl>(D->getDeclContext());
+    SourceLocation Loc = D->getLocation();
+    SymbolRoleSet Roles = 0;
+    SmallVector<SymbolRelation, 1> Relations;
+
+    if (ObjCIvarDecl *ID = D->getPropertyIvarDecl())
+      Relations.push_back({(SymbolRoleSet)SymbolRole::RelationAccessorOf, ID});
+    if (Loc.isInvalid()) {
+      Loc = Container->getLocation();
+      Roles |= (SymbolRoleSet)SymbolRole::Implicit;
+    }
+    if (!IndexCtx.handleDecl(D, Loc, Roles, Relations))
       return false;
 
     if (D->getPropertyImplementation() == ObjCPropertyImplDecl::Dynamic)
       return true;
-    assert(D->getPropertyImplementation() == ObjCPropertyImplDecl::Synthesize);
-    
-    if (ObjCIvarDecl *IvarD = D->getPropertyIvarDecl()) {
-      if (!IvarD->getSynthesize())
-        IndexCtx.handleReference(IvarD, D->getPropertyIvarDeclLoc(), nullptr,
-                                 D->getDeclContext(), SymbolRoleSet());
-    }
 
-    auto *ImplD = cast<ObjCImplDecl>(D->getDeclContext());
+    assert(D->getPropertyImplementation() == ObjCPropertyImplDecl::Synthesize);
     if (ObjCMethodDecl *MD = PD->getGetterMethodDecl()) {
       if (MD->isPropertyAccessor() &&
-          !hasUserDefined(MD, ImplD))
-        IndexCtx.handleDecl(MD, D->getLocation(), SymbolRoleSet(), {}, ImplD);
+          !hasUserDefined(MD, Container))
+        IndexCtx.handleDecl(MD, Loc, SymbolRoleSet(SymbolRole::Implicit), {},
+                            Container);
     }
     if (ObjCMethodDecl *MD = PD->getSetterMethodDecl()) {
       if (MD->isPropertyAccessor() &&
-          !hasUserDefined(MD, ImplD))
-        IndexCtx.handleDecl(MD, D->getLocation(), SymbolRoleSet(), {}, ImplD);
+          !hasUserDefined(MD, Container))
+        IndexCtx.handleDecl(MD, Loc, SymbolRoleSet(SymbolRole::Implicit), {},
+                            Container);
+    }
+    if (ObjCIvarDecl *IvarD = D->getPropertyIvarDecl()) {
+      if (IvarD->getSynthesize()) {
+        // For synthesized ivars, use the location of its name in the
+        // corresponding @synthesize. If there isn't one, use the containing
+        // @implementation's location, rather than the property's location,
+        // otherwise the header file containing the @interface will have different
+        // indexing contents based on whether the @implementation was present or
+        // not in the translation unit.
+        SymbolRoleSet IvarRoles = 0;
+        SourceLocation IvarLoc = D->getPropertyIvarDeclLoc();
+        if (D->getLocation().isInvalid()) {
+          IvarLoc = Container->getLocation();
+          IvarRoles = (SymbolRoleSet)SymbolRole::Implicit;
+        } else if (D->getLocation() == IvarLoc) {
+          IvarRoles = (SymbolRoleSet)SymbolRole::Implicit;
+        }
+        if(!IndexCtx.handleDecl(IvarD, IvarLoc, IvarRoles))
+          return false;
+      } else {
+        IndexCtx.handleReference(IvarD, D->getPropertyIvarDeclLoc(), nullptr,
+                                 D->getDeclContext(), SymbolRoleSet());
+      }
     }
     return true;
   }

lib/Index/IndexingContext.cpp

 bool IndexingContext::handleDecl(const Decl *D,
                                  SymbolRoleSet Roles,
                                  ArrayRef<SymbolRelation> Relations) {
-  return handleDeclOccurrence(D, D->getLocation(), /*IsRef=*/false,
-                              cast<Decl>(D->getDeclContext()), Roles, Relations,
-                              nullptr, nullptr, D->getDeclContext());
+  return handleDecl(D, D->getLocation(), Roles, Relations);
 }
 
 bool IndexingContext::handleDecl(const Decl *D, SourceLocation Loc,

                                  const DeclContext *DC) {
   if (!DC)
     DC = D->getDeclContext();
+
+  const Decl *OrigD = D;
+  if (isa<ObjCPropertyImplDecl>(D)) {
+    D = cast<ObjCPropertyImplDecl>(D)->getPropertyDecl();
+  }
   return handleDeclOccurrence(D, Loc, /*IsRef=*/false, cast<Decl>(DC),
                               Roles, Relations,
-                              nullptr, nullptr, DC);
+                              nullptr, OrigD, DC);
 }
 
 bool IndexingContext::handleReference(const NamedDecl *D, SourceLocation Loc,

 
   if (IsRef)
     Roles |= (unsigned)SymbolRole::Reference;
-  else if (isDeclADefinition(D, ContainerDC, *Ctx))
+  else if (isDeclADefinition(OrigD, ContainerDC, *Ctx))
     Roles |= (unsigned)SymbolRole::Definition;
   else
     Roles |= (unsigned)SymbolRole::Declaration;

test/Index/Core/index-source.m

 @property (nonatomic, strong) IBOutletCollection(I1) NSArray *buttons;
 @end
 
-// CHECK: [[@LINE+2]]:17 | field/ObjC | _prop | c:objc(cs)I2@_prop | <no-cgname> | Def,Impl,RelChild | rel: 1
-// CHECK-NEXT: RelChild | I2 | c:objc(cs)I2
 @implementation I2
-// CHECK: [[@LINE+6]]:13 | instance-property/ObjC | prop | c:objc(cs)I2(py)prop | <no-cgname> | Ref,RelCont | rel: 1
-// CHECK-NEXT: RelCont | I2 | c:objc(cs)I2
-// CHECK: [[@LINE+4]]:13 | instance-method/acc-get/ObjC | prop | c:objc(cs)I2(im)prop | -[I2 prop] | Def,RelChild | rel: 1
+// CHECK: [[@LINE+9]]:13 | instance-property/ObjC | prop | c:objc(cs)I2(py)prop | <no-cgname> | Def,RelChild,RelAcc | rel: 2
+// CHECK-NEXT: RelChild | I2 | c:objc(cs)I2
+// CHECK-NEXT: RelAcc | _prop | c:objc(cs)I2@_prop
+// CHECK: [[@LINE+6]]:13 | instance-method/acc-get/ObjC | prop | c:objc(cs)I2(im)prop | -[I2 prop] | Def,Impl,RelChild | rel: 1
 // CHECK-NEXT: RelChild | I2 | c:objc(cs)I2
-// CHECK: [[@LINE+2]]:13 | instance-method/acc-set/ObjC | setProp: | c:objc(cs)I2(im)setProp: | -[I2 setProp:] | Def,RelChild | rel: 1
+// CHECK: [[@LINE+4]]:13 | instance-method/acc-set/ObjC | setProp: | c:objc(cs)I2(im)setProp: | -[I2 setProp:] | Def,Impl,RelChild | rel: 1
+// CHECK-NEXT: RelChild | I2 | c:objc(cs)I2
+// CHECK: [[@LINE+2]]:20 | field/ObjC | _prop | c:objc(cs)I2@_prop | <no-cgname> | Def,RelChild | rel: 1
 // CHECK-NEXT: RelChild | I2 | c:objc(cs)I2
 @synthesize prop = _prop;
 

 
 // CHECK: [[@LINE+1]]:17 | class/ObjC | I3 | c:objc(cs)I3 | <no-cgname> | Def | rel: 0
 @implementation I3
-// CHECK: [[@LINE+4]]:13 | instance-property/ObjC | prop | c:objc(cs)I3(py)prop | <no-cgname> | Ref,RelCont | rel: 1
-// CHECK-NEXT: RelCont | I3 | c:objc(cs)I3
-// CHECK: [[@LINE+2]]:13 | instance-method/acc-get/ObjC | prop | c:objc(cs)I3(im)prop | -[I3 prop] | Def,RelChild | rel: 1
-// CHECK: [[@LINE+1]]:13 | instance-method/acc-set/ObjC | setProp: | c:objc(cs)I3(im)setProp: | -[I3 setProp:] | Def,RelChild | rel: 1
+// CHECK: [[@LINE+5]]:13 | instance-property/ObjC | prop | c:objc(cs)I3(py)prop | <no-cgname> | Def,RelChild,RelAcc | rel: 2
+// CHECK-NEXT: RelChild | I3 | c:objc(cs)I3
+// CHECK-NEXT: RelAcc | _prop | c:objc(cs)I3@_prop
+// CHECK: [[@LINE+2]]:13 | instance-method/acc-get/ObjC | prop | c:objc(cs)I3(im)prop | -[I3 prop] | Def,Impl,RelChild | rel: 1
+// CHECK: [[@LINE+1]]:13 | instance-method/acc-set/ObjC | setProp: | c:objc(cs)I3(im)setProp: | -[I3 setProp:] | Def,Impl,RelChild | rel: 1
 @synthesize prop = _prop;
 @end
 

 // CHECK-NEXT: RelBase,RelCont | PermanentEnumerator | c:objc(cs)PermanentEnumerator
 @interface PermanentEnumerator : MyEnumerator
 @end
+
+@interface I4
+@property id foo;
+@end
+
+@implementation I4 {
+  id _blahfoo; // explicit def
+  // CHECK: [[@LINE-1]]:6 | field/ObjC | _blahfoo | c:objc(cs)I4@_blahfoo | <no-cgname> | Def,RelChild | rel: 1
+}
+@synthesize foo = _blahfoo; // ref of field _blahfoo
+// CHECK: [[@LINE-1]]:13 | instance-property/ObjC | foo | c:objc(cs)I4(py)foo | <no-cgname> | Def,RelChild,RelAcc | rel: 2
+// CHECK-NEXT: RelChild | I4 | c:objc(cs)I4
+// CHECK-NEXT: RelAcc | _blahfoo | c:objc(cs)I4@_blahfoo
+// CHECK: [[@LINE-4]]:13 | instance-method/acc-get/ObjC | foo | c:objc(cs)I4(im)foo | -[I4 foo] | Def,Impl,RelChild | rel: 1
+// CHECK-NEXT: RelChild | I4 | c:objc(cs)I4
+// CHECK: [[@LINE-6]]:13 | instance-method/acc-set/ObjC | setFoo: | c:objc(cs)I4(im)setFoo: | -[I4 setFoo:] | Def,Impl,RelChild | rel: 1
+// CHECK-NEXT: RelChild | I4 | c:objc(cs)I4
+// CHECK: [[@LINE-8]]:19 | field/ObjC | _blahfoo | c:objc(cs)I4@_blahfoo | <no-cgname> | Ref | rel: 0
+
+-(void)method {
+  _blahfoo = 0;
+  // CHECK: [[@LINE-1]]:3 | field/ObjC | _blahfoo | c:objc(cs)I4@_blahfoo | <no-cgname> | Ref,Writ,RelCont | rel: 1
+}
+@end
+
+@interface I5
+@property id foo;
+@end
+
+@implementation I5
+@synthesize foo = _blahfoo; // explicit def of field _blahfoo
+// CHECK: [[@LINE-1]]:13 | instance-property/ObjC | foo | c:objc(cs)I5(py)foo | <no-cgname> | Def,RelChild,RelAcc | rel: 2
+// CHECK-NEXT: RelChild | I5 | c:objc(cs)I5
+// CHECK-NEXT: RelAcc | _blahfoo | c:objc(cs)I5@_blahfoo
+// CHECK: [[@LINE-4]]:13 | instance-method/acc-get/ObjC | foo | c:objc(cs)I5(im)foo | -[I5 foo] | Def,Impl,RelChild | rel: 1
+// CHECK-NEXT: RelChild | I5 | c:objc(cs)I5
+// CHECK: [[@LINE-6]]:13 | instance-method/acc-set/ObjC | setFoo: | c:objc(cs)I5(im)setFoo: | -[I5 setFoo:] | Def,Impl,RelChild | rel: 1
+// CHECK-NEXT: RelChild | I5 | c:objc(cs)I5
+// CHECK: [[@LINE-8]]:19 | field/ObjC | _blahfoo | c:objc(cs)I5@_blahfoo | <no-cgname> | Def,RelChild | rel: 1
+
+-(void)method {
+  _blahfoo = 0;
+  // CHECK: [[@LINE-1]]:3 | field/ObjC | _blahfoo | c:objc(cs)I5@_blahfoo | <no-cgname> | Ref,Writ,RelCont | rel: 1
+}
+@end
+
+@interface I6
+@property id foo;
+@end
+
+@implementation I6
+@synthesize foo; // implicit def of field foo
+// CHECK: [[@LINE-1]]:13 | instance-property/ObjC | foo | c:objc(cs)I6(py)foo | <no-cgname> | Def,RelChild,RelAcc | rel: 2
+// CHECK-NEXT: RelChild | I6 | c:objc(cs)I6
+// CHECK-NEXT: RelAcc | foo | c:objc(cs)I6@foo
+// CHECK: [[@LINE-4]]:13 | instance-method/acc-get/ObjC | foo | c:objc(cs)I6(im)foo | -[I6 foo] | Def,Impl,RelChild | rel: 1
+// CHECK-NEXT: RelChild | I6 | c:objc(cs)I6
+// CHECK: [[@LINE-6]]:13 | instance-method/acc-set/ObjC | setFoo: | c:objc(cs)I6(im)setFoo: | -[I6 setFoo:] | Def,Impl,RelChild | rel: 1
+// CHECK-NEXT: RelChild | I6 | c:objc(cs)I6
+// CHECK: [[@LINE-8]]:13 | field/ObjC | foo | c:objc(cs)I6@foo | <no-cgname> | Def,Impl,RelChild | rel: 1
+
+-(void)method {
+  foo = 0;
+  // CHECK: [[@LINE-1]]:3 | field/ObjC | foo | c:objc(cs)I6@foo | <no-cgname> | Ref,Writ,RelCont | rel: 1
+}
+@end
+
+@interface I7
+@property id foo;
+@end
+
+@implementation I7 // implicit def of field _foo
+// CHECK: [[@LINE-1]]:17 | instance-property/ObjC | foo | c:objc(cs)I7(py)foo | <no-cgname> | Def,Impl,RelChild,RelAcc | rel: 2
+// CHECK-NEXT: RelChild | I7 | c:objc(cs)I7
+// CHECK-NEXT: RelAcc | _foo | c:objc(cs)I7@_foo
+// CHECK: [[@LINE-4]]:17 | instance-method/acc-get/ObjC | foo | c:objc(cs)I7(im)foo | -[I7 foo] | Def,Impl,RelChild | rel: 1
+// CHECK-NEXT: RelChild | I7 | c:objc(cs)I7
+// CHECK: [[@LINE-6]]:17 | instance-method/acc-set/ObjC | setFoo: | c:objc(cs)I7(im)setFoo: | -[I7 setFoo:] | Def,Impl,RelChild | rel: 1
+// CHECK-NEXT: RelChild | I7 | c:objc(cs)I7
+// CHECK: [[@LINE-8]]:17 | field/ObjC | _foo | c:objc(cs)I7@_foo | <no-cgname> | Def,Impl,RelChild | rel: 1
+
+-(void)method {
+  _foo = 0;
+// CHECK: [[@LINE-1]]:3 | field/ObjC | _foo | c:objc(cs)I7@_foo | <no-cgname> | Ref,Writ,RelCont | rel: 1
+}
+@end

test/Index/Core/index-subkinds.m

 @class NSButton;
 @interface IBCls
 
-// CHECK: [[@LINE+2]]:34 | instance-method/acc-get/ObjC | prop | c:objc(cs)IBCls(im)prop | -[IBCls prop] | Decl,Dyn,RelChild,RelAcc | rel: 2
+// CHECK: [[@LINE+2]]:34 | instance-method/acc-get/ObjC | prop | c:objc(cs)IBCls(im)prop | -[IBCls prop] | Decl,Dyn,Impl,RelChild,RelAcc | rel: 2
 // CHECK: [[@LINE+1]]:34 | instance-property(IB)/ObjC | prop | c:objc(cs)IBCls(py)prop | <no-cgname> | Decl,RelChild | rel: 1
 @property (readonly) IBOutlet id prop;
 // CHECK: [[@LINE+1]]:54 | instance-property(IB,IBColl)/ObjC | propColl | c:objc(cs)IBCls(py)propColl | <no-cgname> | Decl,RelChild | rel: 1

test/Index/index-decls.m

 // CHECK: [indexDeclaration]: kind: objc-instance-method | name: setProp: | {{.*}} | loc: 7:33
 // CHECK: [indexDeclaration]: kind: objc-property | name: prop | {{.*}} | loc: 7:33
 
-// CHECK: [indexDeclaration]: kind: objc-ivar | name: _prop | {{.*}} | loc: 11:20
 // CHECK: [indexDeclaration]: kind: objc-instance-method | name: prop | {{.*}} | loc: 11:13 | {{.*}} | lexical-container: [I:10:17]
 // CHECK: [indexDeclaration]: kind: objc-instance-method | name: setProp: | {{.*}} | loc: 11:13 | {{.*}} | lexical-container: [I:10:17]
+// CHECK: [indexDeclaration]: kind: objc-ivar | name: _prop | {{.*}} | loc: 11:20
 
 // CHECK: [indexDeclaration]: kind: objc-ivar | name: _auto_prop | {{.*}} | loc: 20:33
 // CHECK: [indexEntityReference]: kind: objc-ivar | name: _auto_prop | {{.*}} | loc: 25:3