From 74790484b53c3824b25eddc005d0caf1310433a6 Mon Sep 17 00:00:00 2001 From: Argyrios Kyrtzidis Date: Fri, 17 Feb 2017 04:49:41 +0000 Subject: [PATCH] [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 llvm-svn: 295416 --- clang/lib/Index/IndexDecl.cpp | 87 +++++++++++++------- clang/lib/Index/IndexingContext.cpp | 13 +-- clang/test/Index/Core/index-source.m | 108 ++++++++++++++++++++++--- clang/test/Index/Core/index-subkinds.m | 2 +- clang/test/Index/index-decls.m | 2 +- 5 files changed, 165 insertions(+), 47 deletions(-) diff --git a/clang/lib/Index/IndexDecl.cpp b/clang/lib/Index/IndexDecl.cpp index 3b4f3f81399d..6f9d69386329 100644 --- a/clang/lib/Index/IndexDecl.cpp +++ b/clang/lib/Index/IndexDecl.cpp @@ -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(); @@ -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(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(I)) + if (!isa(I) || + cast(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(D->getDeclContext()), - D->getDeclContext(), SymbolRoleSet(), {}, - /*RefE=*/nullptr, D)) + auto *Container = cast(D->getDeclContext()); + SourceLocation Loc = D->getLocation(); + SymbolRoleSet Roles = 0; + SmallVector 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(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; } diff --git a/clang/lib/Index/IndexingContext.cpp b/clang/lib/Index/IndexingContext.cpp index 6dd6c0cfb28e..06b24cedb35b 100644 --- a/clang/lib/Index/IndexingContext.cpp +++ b/clang/lib/Index/IndexingContext.cpp @@ -24,9 +24,7 @@ bool IndexingContext::shouldIndexFunctionLocalSymbols() const { bool IndexingContext::handleDecl(const Decl *D, SymbolRoleSet Roles, ArrayRef Relations) { - return handleDeclOccurrence(D, D->getLocation(), /*IsRef=*/false, - cast(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 Decl *D, SourceLocation Loc, const DeclContext *DC) { if (!DC) DC = D->getDeclContext(); + + const Decl *OrigD = D; + if (isa(D)) { + D = cast(D)->getPropertyDecl(); + } return handleDeclOccurrence(D, Loc, /*IsRef=*/false, cast(DC), Roles, Relations, - nullptr, nullptr, DC); + nullptr, OrigD, DC); } bool IndexingContext::handleReference(const NamedDecl *D, SourceLocation Loc, @@ -286,7 +289,7 @@ bool IndexingContext::handleDeclOccurrence(const Decl *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; diff --git a/clang/test/Index/Core/index-source.m b/clang/test/Index/Core/index-source.m index 880028df6310..d172e709f12f 100644 --- a/clang/test/Index/Core/index-source.m +++ b/clang/test/Index/Core/index-source.m @@ -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@_prop | | 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 | | 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 | | Def,RelChild,RelAcc | rel: 2 // 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-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+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 | | 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 | | Def | rel: 0 @implementation I3 -// CHECK: [[@LINE+4]]:13 | instance-property/ObjC | prop | c:objc(cs)I3(py)prop | | 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 | | 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 @@ -182,3 +184,89 @@ typedef MyGenCls MyEnumerator; // 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 | | 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 | | 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 | | Ref | rel: 0 + +-(void)method { + _blahfoo = 0; + // CHECK: [[@LINE-1]]:3 | field/ObjC | _blahfoo | c:objc(cs)I4@_blahfoo | | 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 | | 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 | | Def,RelChild | rel: 1 + +-(void)method { + _blahfoo = 0; + // CHECK: [[@LINE-1]]:3 | field/ObjC | _blahfoo | c:objc(cs)I5@_blahfoo | | 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 | | 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 | | Def,Impl,RelChild | rel: 1 + +-(void)method { + foo = 0; + // CHECK: [[@LINE-1]]:3 | field/ObjC | foo | c:objc(cs)I6@foo | | 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 | | 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 | | Def,Impl,RelChild | rel: 1 + +-(void)method { + _foo = 0; +// CHECK: [[@LINE-1]]:3 | field/ObjC | _foo | c:objc(cs)I7@_foo | | Ref,Writ,RelCont | rel: 1 +} +@end diff --git a/clang/test/Index/Core/index-subkinds.m b/clang/test/Index/Core/index-subkinds.m index 15d60b151896..bb3a37c95105 100644 --- a/clang/test/Index/Core/index-subkinds.m +++ b/clang/test/Index/Core/index-subkinds.m @@ -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 | | 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 | | Decl,RelChild | rel: 1 diff --git a/clang/test/Index/index-decls.m b/clang/test/Index/index-decls.m index 7f7f11576ab8..a5368ecb0c0d 100644 --- a/clang/test/Index/index-decls.m +++ b/clang/test/Index/index-decls.m @@ -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