dexonsmith added a comment.

In D73208#1836722 <https://reviews.llvm.org/D73208#1836722>, @MadCoder wrote:

> In D73208#1836704 <https://reviews.llvm.org/D73208#1836704>, @dexonsmith 
> wrote:
>
> > In D73208#1835264 <https://reviews.llvm.org/D73208#1835264>, @MadCoder 
> > wrote:
> >
> > > In D73208#1835051 <https://reviews.llvm.org/D73208#1835051>, @dexonsmith 
> > > wrote:
> > >
> > > > Why isn't a similar dance needed for non-direct methods?
> > >
> > >
> > > because non direct methods do not need an `llvm::Function` to be 
> > > synthesized at the call-site. direct methods do, and they form one with 
> > > the type of the declaration they see. Then that same `llvm::Function` is 
> > > used when you CodeGen the Implementation, so if there's a mismatch, 
> > > sadness ensues because the LLVM IR verifier will notice the discrepancy 
> > > between the declared return type of the function and the actual types 
> > > coming out of the `ret` codepaths.
> > >
> > > Regular obj-C methods use the _implementation_ types for the codegen (the 
> > > declaration(s) aren't even consulted) and I want to stick at what obj-c 
> > > does as much as I can.
> > >
> > > (as a data point: If you use obj-C types with C functions, the type of 
> > > the first declaration seen is used instead).
> >
> >
> > Okay, that makes sense to me.
> >
> > Another solution would be to change IRGen for the implementation: if the 
> > declaration already exists (`getFunction`), do a bitcast + RAUW dance to 
> > fix it up (and update the `DirectMethodDefinitions` table).  WDYT?
>
>
> I didn't want to do that because that would mean that the type used for the 
> implementation would depart from dynamic Objective-C methods, and it feels 
> that it shouldn't. hence I took this option.


I think we're talking across each other.  The idea is check the type when 
generating the implementation; if it's not correct, you fix it (need to update 
existing uses to bitcast).  So the type used for the implementation would match 
dynamic methods.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D73208



_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to