r187174 - Fix GNU ObjC ABI for a message returning a struct.

[John McCall]

On Jul 26, 2013, at 2:09 PM, Jonathan Schleifer <js at webkeks.org> wrote:
> thanks for reviewing it.
> 
>> This isn’t really a language extension; it’s an ABI feature that fixes a language
>> feature.  I don’t think it rises to the level of being worth documenting in the
>> language-extensions section.
> 
> I agree here, a lot. However, Eli asked me to document it there as all other __has_feature are documented there.
> 
>> I’m also skeptical that this deserves to be exposed to the user with __has_feature.
>> Do you have a use case for that, or are you just doing it for generic reasons?
> 
> Yes! What I do is check that there is an assembly implementation for of_forward_stret the architecture and that __has_feature(objc_msg_lookup_stret) is true. If that is the case, I set a define so that it's possible to check if forwarding is available with structs. This is important in e.g. the automatic tests: Without such a define, it would just always try to forward for stret - and crash.
> 
>> If it is important to expose this with __has_feature, you should really abstract this
>> into ObjCRuntime, like what we do with hasTerminate() (which I’ll note also does
>> not have a __has_feature check).
> 
> Yes, __has_feature is important as it's the only way to see if Clang supports it. After all, the runtime has to provide of_forward_stret and objc_msg_lookup_stret, but both are useless if Clang does not emit calls to objc_msg_lookup_stret. Thus the __has_feature to check if all three are given. It's the only way to make it possible to let code check if that feature is available instead of just hoping it is and crash.

I feel like making your runtime's unit tests more portable across compiler versions isn’t really a compelling reason to expose something like this that’s essentially a fix to an existing language feature.  We don’t get to remove __has_feature checks ever, so we’ve generally tried to avoid them for improvements like this, because otherwise the logic produces a plausible argument for basically every change to the compiler to get an independent __has_feature check.

John.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20130726/6f05621c/attachment.html>