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

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a reviewer: rsmith.
aaron.ballman added a comment.

I'm adding Richard to the review because he may have opinions on this functionality.

Is there a reason we want the second spelling instead of making the current spelling behave in the manner you describe? While the change is fairly fundamental, I don't know that it would break code either (but I could be wrong). I think that forcing the user to choice which spelling of "overloadable" they want to use is not very user friendly.



================
Comment at: include/clang/Basic/Attr.td:1416
 def Overloadable : Attr {
-  let Spellings = [GNU<"overloadable">];
+  let Spellings = [GNU<"overloadable">, GNU<"transparently_overloadable">];
   let Subjects = SubjectList<[Function], ErrorDiag>;
----------------
I'm not too keen on the name, especially since there's already a "transparent_union" attribute, where the "transparent" means something slightly different than it does here. However, my attempts to dream up a better name are coming up short...


================
Comment at: include/clang/Basic/DiagnosticSemaKinds.td:3259
   "'overloadable' function %0 must have a prototype">;
+def err_attribute_overloadable_too_many_transparent_overloads : Error<
+  "only one 'overloadable' overload may be transparent">;
----------------
Why should this be an error instead of simply an ignored attribute?


================
Comment at: test/Sema/overloadable.c:189
+  void to_foo5(int) __attribute__((overloadable)); // expected-note {{previous overload}}
+  void to_foo5(int) __attribute__((transparently_overloadable)); // expected-error{{mismatched transparency}}
+
----------------
Why should this be a mismatch? Attributes can usually be added to redeclarations without an issue, and it's not unheard of for subsequent redeclarations to gain additional attributes. It seems like the behavior the user would expect from this is for the `transparently_overloadable` attribute to "win" and simply replaces, or silently augments, the `overloadable` attribute.


https://reviews.llvm.org/D32332