aprantl added a comment.
In D66121#1656442 <https://reviews.llvm.org/D66121#1656442>, @rjmccall wrote:
> Can you prepare an NFC patch with just the changes relating to adding
> `ObjCPropertyImplDecl::get{Getter,Setter}MethodDecl`?
Sure, I will do that.
> I don't get why the redeclaration logic is changing. What happens when a
> normal method is implemented? Is that not linked up as a redeclaration?
From what I can tell from injecting an assertion into
`ObjCMethodDecl::setAsRedeclaration()` only redeclarations within the same
protocol/interface/category are linked up as redeclarations. An implementation
of an interface is not a redeclaration in the SemaObjC sense. Digging further,
this is implemented in
https://github.com/llvm/llvm-project/blob/244e738485445fa4b72bfef9b9b2f9625cee989e/clang/lib/Sema/SemaDeclObjC.cpp#L3930
by checking whether a method's selector has been processed before within the
same property/interface/category.
That's very different from what my patch is doing. The following example is
extracted from `clang/test/CodeGenObjC/newproperty-nested-synthesis-1.m`:
1 @interface Tester : Object
2 @property char PropertyAtomic_char;
3 @end
4
5 @implementation Tester
6 @dynamic PropertyAtomic_char;
7 @end
8
9 @interface SubClass : Tester
10 {
11 char PropertyAtomic_char;
12 }
13 @end
14
15 @implementation SubClass
16 @synthesize PropertyAtomic_char;
17 @end
My patch sets the getter synthesized at line 6 up as a redeclaration of getter
from the interface on line 2 and the getter from `SubClass` on line 16 as a
redeclaration of the getter from `Tester` on line 6. That must be wrong: If
there were another subclass of `Tester` with another getter, the compiler would
assert, because each Objective-C method declaration can have at most one
redeclaration.
So in summary I think what I was trying to do doesn't make sense in the
existing infrastructure and I shouldn't be trying to set up methods as
redeclarations between *different* interfaces/protocols/implementations. Do you
agree with this assessment? If yes I'll try to redo the patch without using the
redeclaration mechanism.
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D66121/new/
https://reviews.llvm.org/D66121
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
