r295416 - [index] Improvde how we handle synthesized ObjC properties and the associated ivars.

Argyrios Kyrtzidis via cfe-commits cfe-commits at lists.llvm.org
Thu Feb 16 20:49:41 PST 2017


Author: akirtzidis
Date: Thu Feb 16 22:49:41 2017
New Revision: 295416

URL: http://llvm.org/viewvc/llvm-project?rev=295416&view=rev
Log:
[index] Improvde how we handle synthesized ObjC properties and the associated ivars.

Related synthesized properties with the ivar they use with the 'accessor' relation, and make sure
we mark them 'implicit' when appropriate.

Patch by Nathan Hawes!
https://reviews.llvm.org/D30012

Modified:
    cfe/trunk/lib/Index/IndexDecl.cpp
    cfe/trunk/lib/Index/IndexingContext.cpp
    cfe/trunk/test/Index/Core/index-source.m
    cfe/trunk/test/Index/Core/index-subkinds.m
    cfe/trunk/test/Index/index-decls.m

Modified: cfe/trunk/lib/Index/IndexDecl.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Index/IndexDecl.cpp?rev=295416&r1=295415&r2=295416&view=diff
==============================================================================
--- cfe/trunk/lib/Index/IndexDecl.cpp (original)
+++ cfe/trunk/lib/Index/IndexDecl.cpp Thu Feb 16 22:49:41 2017
@@ -98,7 +98,10 @@ public:
     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>();
@@ -178,14 +181,8 @@ public:
 
   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;
@@ -281,12 +278,16 @@ public:
     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);
     }
 
@@ -355,32 +356,58 @@ public:
 
   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;
   }

Modified: cfe/trunk/lib/Index/IndexingContext.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Index/IndexingContext.cpp?rev=295416&r1=295415&r2=295416&view=diff
==============================================================================
--- cfe/trunk/lib/Index/IndexingContext.cpp (original)
+++ cfe/trunk/lib/Index/IndexingContext.cpp Thu Feb 16 22:49:41 2017
@@ -24,9 +24,7 @@ bool IndexingContext::shouldIndexFunctio
 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,
@@ -35,9 +33,14 @@ bool IndexingContext::handleDecl(const D
                                  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,
@@ -286,7 +289,7 @@ bool IndexingContext::handleDeclOccurren
 
   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;

Modified: cfe/trunk/test/Index/Core/index-source.m
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Index/Core/index-source.m?rev=295416&r1=295415&r2=295416&view=diff
==============================================================================
--- cfe/trunk/test/Index/Core/index-source.m (original)
+++ cfe/trunk/test/Index/Core/index-source.m Thu Feb 16 22:49:41 2017
@@ -106,14 +106,15 @@ extern int setjmp(jmp_buf);
 @property (nonatomic, strong) IBOutletCollection(I1) NSArray *buttons;
 @end
 
-// CHECK: [[@LINE+2]]:17 | field/ObjC | _prop | c:objc(cs)I2 at _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 at _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 at _prop | <no-cgname> | Def,RelChild | rel: 1
 // CHECK-NEXT: RelChild | I2 | c:objc(cs)I2
 @synthesize prop = _prop;
 
@@ -139,10 +140,11 @@ extern int setjmp(jmp_buf);
 
 // 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 at _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
 
@@ -182,3 +184,89 @@ typedef MyGenCls<Base *><MyEnumerating>
 // CHECK-NEXT: RelBase,RelCont | PermanentEnumerator | c:objc(cs)PermanentEnumerator
 @interface PermanentEnumerator : MyEnumerator
 @end
+
+ at interface I4
+ at property id foo;
+ at end
+
+ at implementation I4 {
+  id _blahfoo; // explicit def
+  // CHECK: [[@LINE-1]]:6 | field/ObjC | _blahfoo | c:objc(cs)I4 at _blahfoo | <no-cgname> | Def,RelChild | rel: 1
+}
+ at 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 at _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 at _blahfoo | <no-cgname> | Ref | rel: 0
+
+-(void)method {
+  _blahfoo = 0;
+  // CHECK: [[@LINE-1]]:3 | field/ObjC | _blahfoo | c:objc(cs)I4 at _blahfoo | <no-cgname> | Ref,Writ,RelCont | rel: 1
+}
+ at end
+
+ at interface I5
+ at property id foo;
+ at end
+
+ at implementation I5
+ at 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 at _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 at _blahfoo | <no-cgname> | Def,RelChild | rel: 1
+
+-(void)method {
+  _blahfoo = 0;
+  // CHECK: [[@LINE-1]]:3 | field/ObjC | _blahfoo | c:objc(cs)I5 at _blahfoo | <no-cgname> | Ref,Writ,RelCont | rel: 1
+}
+ at end
+
+ at interface I6
+ at property id foo;
+ at end
+
+ at implementation I6
+ at 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 at 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 at foo | <no-cgname> | Def,Impl,RelChild | rel: 1
+
+-(void)method {
+  foo = 0;
+  // CHECK: [[@LINE-1]]:3 | field/ObjC | foo | c:objc(cs)I6 at foo | <no-cgname> | Ref,Writ,RelCont | rel: 1
+}
+ at end
+
+ at interface I7
+ at property id foo;
+ at end
+
+ at 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 at _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 at _foo | <no-cgname> | Def,Impl,RelChild | rel: 1
+
+-(void)method {
+  _foo = 0;
+// CHECK: [[@LINE-1]]:3 | field/ObjC | _foo | c:objc(cs)I7 at _foo | <no-cgname> | Ref,Writ,RelCont | rel: 1
+}
+ at end

Modified: cfe/trunk/test/Index/Core/index-subkinds.m
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Index/Core/index-subkinds.m?rev=295416&r1=295415&r2=295416&view=diff
==============================================================================
--- cfe/trunk/test/Index/Core/index-subkinds.m (original)
+++ cfe/trunk/test/Index/Core/index-subkinds.m Thu Feb 16 22:49:41 2017
@@ -42,7 +42,7 @@
 @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

Modified: cfe/trunk/test/Index/index-decls.m
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Index/index-decls.m?rev=295416&r1=295415&r2=295416&view=diff
==============================================================================
--- cfe/trunk/test/Index/index-decls.m (original)
+++ cfe/trunk/test/Index/index-decls.m Thu Feb 16 22:49:41 2017
@@ -64,9 +64,9 @@ int test1() {
 // 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




More information about the cfe-commits mailing list