sdefresne added a comment.

In http://reviews.llvm.org/D20113#425984, @rjmccall wrote:

> This is a good catch, thanks!


Thank you for the quick reply.

Please excuse me if I misunderstood you or if my remark appear off the mark, 
this is my first time sending patches to clang. I'm not yet completely familiar 
with all clang concepts :-)

> As a slight adjustment, It's probably better to just ignore this attribute 
> when mangling the function type of an entity, the same way that we generally 
> don't mangle return types because they don't affect overloading.  That will 
> require an extra flag to be passed down in a few places, but that's pretty 
> reasonable.  This will generally allow NS_CONSUMED and NS_RETURNS_RETAINED to 
> be placed on existing APIs without changing their mangling unless the 
> attribute is used in a secondary position (such as the type of an argument).

> 

> Finally, you should give ns_returns_retained the same treatment, as well as 
> the parameter-ABI attributes.


I'm not really sure what you mean by "ignore this attribute when mangling the 
function type of an entity". I read this as "we should ensure that the 
ns_consumed attribute is only mangled if it is applied to a parameter". Is this 
correct?

If so, then there is nothing to do, as "ns_consumed" attibute is ignored if 
applied to a non-parameter type as far as I can tell (see below for compilation 
warning clang already emits regarding ignored attributes). So, my understanding 
is that the ns_consumed attribute is only used if applied to a parameter, and 
thus only when it is relevant to include it in the mangled name.

Regarding ns_returns_retained, it is ignored if not applied to the function 
itself.

  $ clang++ -fobjc-arc -o x.o -c x.mm
  x.mm:1:19: warning: 'ns_consumed' attribute only applies to parameters
        [-Wignored-attributes]
  id __attribute__((ns_consumed)) f() { return 0; }
                    ^
  x.mm:2:23: warning: 'ns_consumed' attribute only applies to parameters
        [-Wignored-attributes]
  id g() __attribute__((ns_consumed)) { return 0; }
                        ^
  x.mm:7:26: warning: 'ns_returns_retained' only applies to function types; type
        here is 'id' [-Wignored-attributes]
  void k(id __attribute__((ns_returns_retained))) {}

So, could you elaborate a bit more on what additional changes I need to make to 
my patch?


http://reviews.llvm.org/D20113



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

Reply via email to