[PATCH] D66121: Debug Info: Nest Objective-C property function decls inside their container.

[John McCall via Phabricator via cfe-commits]

rjmccall added inline comments.


================
Comment at: clang/lib/CodeGen/CGDebugInfo.h:406
+                         CGBuilderTy &Builder,
+                         const ObjCContainerDecl *CD = nullptr);
 
----------------
aprantl wrote:
> rjmccall wrote:
> > aprantl wrote:
> > > rjmccall wrote:
> > > > Why does this have to be an extra parameter?  The method's DC should be the container.
> > > ```
> > > @protocol BarProto
> > > @property struct Bar *bar;
> > > @end
> > > 
> > > @interface Foo <BarProto>
> > > @end
> > > 
> > > @implementation Foo {}
> > > @synthesize bar = _bar;
> > > @end
> > > ```
> > > 
> > > The ObjCMethodDecl for the synthesized `bar` accessors is the method decl in the protocol `BarProto`, as there is no method decl put into the AST inside of `Foo`. Its DeclContext (lexical an regular) therefor is the Protocol, which is not what we want as the decl context for a synthesized method called `-[Foo setBar:]` in the debug info.
> > Wait, really?  That seems pretty unfortunate.  Should we synthesize a real implementation method?
> That certainly sounds like the more elegant solution. I didn't know enough about Clang's ObjC implementation to understand whether this was feasible. Do you happen to know where a good entry point for doing this would be?
It might be a consequential change, but I think the right place would just be in `Sema::ActOnPropertyImplDecl`, near the bottom where it pulls out the getter and setter methods.  You should be able to just create new implicit `ObjCMethodDecl`s as redeclarations of the getter/setter and add them to the `ObjCImplDecl`.  It might also be nice to link them off of the `ObjCPropertyImplDecl`, but that's not as necessary.

If you do that, you'll probably need to update IRGen so that we don't attempt to emit the synthesized methods twice.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D66121/new/

https://reviews.llvm.org/D66121