[PATCH] D20113: Fix mangled name of method with ns_consumed parameters.

Sylvain Defresne via cfe-commits cfe-commits at lists.llvm.org
Wed May 11 01:47:30 PDT 2016


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





More information about the cfe-commits mailing list