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

[Saleem Abdulrasool via Phabricator via cfe-commits]

compnerd marked 8 inline comments as done.
compnerd added inline comments.


================
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.
+
----------------
gribozavr2 wrote:
> 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.
I think that is more approachable and less technical.  I'll take it.


================
Comment at: clang/include/clang/Basic/AttrDocs.td:3593
+    void __attribute__((__swift_name__("function()"))) f(void) {
+    }
+  }];
----------------
gribozavr2 wrote:
> 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) {
>     }
> ```
I like that!


================
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.
+  ///
----------------
gribozavr2 wrote:
> WDYM by "the function will output"? Which function, `DiagnoseSwiftName`? I think this part of the comment is about `validateSwiftFunctionName`.
Ah, right, that is about `validateSwiftFunctionName`.  I'll move the description.


================
Comment at: clang/lib/Sema/SemaDeclAttr.cpp:4282
+                                        StringRef Name, bool Override) {
+  if (const auto *Inline = D->getAttr<SwiftNameAttr>()) {
+    if (Override) {
----------------
gribozavr2 wrote:
> Why is the variable called `Inline`? Unless there's a reason, I'd suggest something that has a stronger connection, like `PreviousSNA`.
The previous one was declared inline in the declaration, but sure, I can rename it to `PrevSNA`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D87534