[PATCH] D87534: Sema: introduce `__attribute__((__swift_name__))`

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added inline comments.


================
Comment at: clang/include/clang/Basic/AttrDocs.td:3582
+
+The parameter takes the single string representation of the Swift function name.
+The name may be a compound Swift name.  For a type, enum constant, property, or
----------------
The parameter takes -> The argument is


================
Comment at: clang/include/clang/Basic/DiagnosticSemaKinds.td:3977
 
+// Swift attributes.
+def warn_attr_swift_name_function
----------------
I want to make sure we're clear with the terminology used in the diagnostics, so there's a fair number of "take -> have" suggestions here, but I want to be sure I'm correct before you apply those suggestions.

I'm used to function declarations having parameters which take arguments from a function call expression. Are the diagnostics about "taking a parameter" talking about "having a parameter" or about "taking an argument"? I believe the string arguments are function signatures rather than function names (perhaps?) and so we should be talking about having a parameter, but I wasn't 100% sure.


================
Comment at: clang/include/clang/Basic/DiagnosticSemaKinds.td:3979
+def warn_attr_swift_name_function
+  : Warning<"parameter of %0 attribute must be a Swift function name string">,
+    InGroup<SwiftNameAttribute>;
----------------
How about: `%0 attribute argument must be a string literal specifying a Swift function name`?


================
Comment at: clang/include/clang/Basic/DiagnosticSemaKinds.td:3988
+def warn_attr_swift_name_subscript_not_accessor
+  : Warning<"%0 attribute for 'subscript' must be a getter or setter">,
+    InGroup<SwiftNameAttribute>;
----------------
`%0 attribute for 'subscript' must %select{be a getter or setter|have at least one parameter|have a 'self' parameter}1` ?


================
Comment at: clang/include/clang/Basic/DiagnosticSemaKinds.td:3994
+def warn_attr_swift_name_subscript_no_parameter
+  : Warning<"%0 attribute for 'subscript' must take at least one parameter">,
+    InGroup<SwiftNameAttribute>;
----------------
take -> have


================
Comment at: clang/include/clang/Basic/DiagnosticSemaKinds.td:3997
+def warn_attr_swift_name_setter_parameters
+  : Warning<"%0 attribute for setter must take one parameter for new value">,
+    InGroup<SwiftNameAttribute>;
----------------
take -> have

elsewhere `new value` is spelled `'newValue:`, should that be the same here?


================
Comment at: clang/include/clang/Basic/DiagnosticSemaKinds.td:4006
+def warn_attr_swift_name_getter_parameters
+  : Warning<"%0 attribute for getter must not take any parameters besides 'self:'">,
+    InGroup<SwiftNameAttribute>;
----------------
take -> have


================
Comment at: clang/include/clang/Basic/DiagnosticSemaKinds.td:4009
+def warn_attr_swift_name_subscript_setter_no_newValue
+  : Warning<"%0 attribute for 'subscript' setter must take a 'newValue:' parameter">,
+    InGroup<SwiftNameAttribute>;
----------------
take -> have


================
Comment at: clang/include/clang/Basic/DiagnosticSemaKinds.td:4012
+def warn_attr_swift_name_subscript_setter_multiple_newValues
+  : Warning<"%0 attribute for 'subscript' setter cannot take multiple 'newValue:' parameters">,
+    InGroup<SwiftNameAttribute>;
----------------
take -> have


================
Comment at: clang/include/clang/Basic/DiagnosticSemaKinds.td:4015
+def warn_attr_swift_name_subscript_getter_newValue
+  : Warning<"%0 attribute for 'subscript' getter cannot take a 'newValue:' parameter">,
+    InGroup<SwiftNameAttribute>;
----------------
take -> have


================
Comment at: clang/include/clang/Basic/DiagnosticSemaKinds.td:4017
+    InGroup<SwiftNameAttribute>;
+def warn_attr_swift_name_function_no_prototype
+  : Warning<"%0 attribute can only be applied to function declarations with prototypes">,
----------------
I don't think you need this diagnostic -- you should be able to use `warn_attribute_wrong_decl_type` (it has a non-K&R-style functions option which is a bit more clear).


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D87534/new/

https://reviews.llvm.org/D87534