[PATCH] D32332: Add support for transparent overloadable functions in clang

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

In https://reviews.llvm.org/D32332#742796, @george.burgess.iv wrote:

> thanks for the feedback!
>
> fwiw, at a high level, the main problem i'm trying to solve with this is that you can't really make a `__overloadable_without_mangling` macro without handing it the function name, since you currently need to use `__asm__("name-you'd-like-the-function-to-have")` to rename the function. if you can think of a better way to go about this, even if it requires that we drop the "no `overloadable` required on some redeclarations" feature this adds, i'm all ears. :)


Yeah, it would require the function name (then you could use the stringize preprocessor directive).

>> Is there a reason we want the second spelling instead of making the current spelling behave in the manner you describe?
> 
> there are two features this is adding, and i'm not sure which you want `overloadable` to imply.
> 
> 1. this new spelling brings with it the promise that we won't mangle function names; if we made every `overloadable` act this way, we'd have:
> 
>   ``` void foo(int) __attribute__((overloadable)); // emitted as `declare void @foo(i32)` void foo(long) __attribute__((overloadable)); // emitted as `declare void @foo(i64)`. LLVM gets angry, since we have different declarations for @foo. ```
> 
>   the only way i can think of to get "no indication of which overloads aren't mangled" to work is choosing which overload to mangle based on source order, and that seems overly subtle to me. so, i think this behavior needs to be spelled out in the source.

I'd like to understand this use case a little bit better. If you don't perform the mangling, then choosing which overload gets called is already based on the source order, isn't it? See https://godbolt.org/g/8b10Ns

> 2. for the "no overloadable required on redecls" feature this adds: i wasn't there for the initial design of the `overloadable` attribute, so i can't say for certain, but i think part of the reason that we required `overloadable` to be on every redecl was so that people would have an indication of "hey, by the way, this name will probably be mangled and there may be other functions you end up calling." in C, i can totally see that being valuable. if you're suggesting we relax that for all overloads, i'll find who originally implemented all of this to figure out what their reasoning was and get back to you. :)

The logic as to why overloadable is required on every redeclaration makes sense, but also flies in the face of your rational for why you want a new spelling of the same attribute that doesn't need to be on every redeclaration. I worry about this difference being mildly confusing to user.

> in any case, whether we actually do this as a new spelling, an optional argument to `overloadable`, etc. makes no difference to me. i chose a new spelling because AFAICT, we don't currently have a standard way for a user to query clang for if it has support for specific features in an attribute. (outside of checking clang's version, but that's discouraged AFAIK.) if we decide an optional arg would be a better approach than a new spelling, i'm happy to add something like `__has_attribute_ext` so you can do `__has_attribute_ext(overloadable, transparent_overloads)`.

That's a fair reason for a new spelling rather than an argument.

>>   I think that forcing the user to choice which spelling of "overloadable" they want to use is not very user friendly.
> 
> yeah. the idea was that users should only need to reach for `transparently_overloadable` if they're trying to make a previously non-`overloadable` function `overloadable`. like said, if you think there's a better way to surface this functionality, i'm all for it.


https://reviews.llvm.org/D32332