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

[Dmitri Gribenko via Phabricator via cfe-commits]

gribozavr2 accepted this revision.
gribozavr2 added a comment.

> This introduces the new swift_name attribute that allows annotating
> interfaces with an alternate spelling for Swift. This is used as part
> of the importing mechanism to allow interfaces to be imported with a new

s/interfaces/APIs/ (to not confuse with Objective-C `@interface`)



================
Comment at: clang/include/clang/Basic/AttrDocs.td:3579-3580
+  let Content = [{
+The ``swift_name`` attribute provides the spelling for the transformed name when
+the interface is imported into the Swift language.
+
----------------
WDYT about:

The ``swift_name`` attribute specifies the name of the declaration in Swift. If this attribute is absent, the name is transformed according to the algorithm built into the Swift compiler.


================
Comment at: clang/include/clang/Basic/AttrDocs.td:3582
+
+The argument is 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 argument is a string literal that contains the Swift name of the function, variable, or type. When renaming a function, the  name may be a compound Swift name.


================
Comment at: clang/include/clang/Basic/AttrDocs.td:3593
+    void __attribute__((__swift_name__("function()"))) f(void) {
+    }
+  }];
----------------
Could we try a more realistic example?

```
    @interface URL
    - (void) initWithString:(NSString*)s __attribute__((__swift_name__("URL.init(_:)")))
    @end

    double __attribute__((__swift_name__("squareRoot()"))) sqrt(double) {
    }
```


================
Comment at: clang/include/clang/Basic/DiagnosticSemaKinds.td:3982
+def warn_attr_swift_name_invalid_identifier
+  : Warning<"%0 attribute has invalid identifier for %select{base|context|parameter}1 name">,
+    InGroup<SwiftNameAttribute>;
----------------
for => for the


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


================
Comment at: clang/include/clang/Sema/Sema.h:1837-1838
 
+  /// Do a check to make sure \p Name looks like a legal swift_name
+  /// attribute for the decl \p D. Raise a diagnostic if the name is invalid
+  /// for the given declaration.
----------------
"a legal argument for the swift_name attribute applied to decl \p D."


================
Comment at: clang/include/clang/Sema/Sema.h:1843-1844
+  /// e.g. <code>init(foo:bar:baz:)</code> or <code>controllerForName(_:)</code>,
+  /// and the function will output the number of parameter names, and whether
+  /// this is a single-arg initializer.
+  ///
----------------
WDYM by "the function will output"? Which function, `DiagnoseSwiftName`? I think this part of the comment is about `validateSwiftFunctionName`.


================
Comment at: clang/lib/Sema/SemaDeclAttr.cpp:4282
+                                        StringRef Name, bool Override) {
+  if (const auto *Inline = D->getAttr<SwiftNameAttr>()) {
+    if (Override) {
----------------
Why is the variable called `Inline`? Unless there's a reason, I'd suggest something that has a stronger connection, like `PreviousSNA`.


================
Comment at: clang/lib/Sema/SemaDeclAttr.cpp:5717
+  }
+  Parameters = Parameters.drop_back();  // ')'
+
----------------
Could you add an assert that the character being dropped is indeed ')'? There is an error above, but it is sort of far away and the connection between that line and this one is not obvious.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D87534