[PATCH] D87534: Sema: introduce `__attribute__((__swift_name__))`
This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
compnerd marked 4 inline comments as done.
Closed by commit rG9bb5ecf1f760: Sema: introduce
`__attribute__((__swift_name__))` (authored by compnerd).
Changed prior to commit:
https://reviews.llvm.org/D87534?vs=292548=293470#toc
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D87534/new/
https://reviews.llvm.org/D87534
Files:
clang/include/clang/Basic/Attr.td
clang/include/clang/Basic/AttrDocs.td
clang/include/clang/Basic/DiagnosticGroups.td
clang/include/clang/Basic/DiagnosticSemaKinds.td
clang/include/clang/Sema/Sema.h
clang/lib/Sema/SemaDecl.cpp
clang/lib/Sema/SemaDeclAttr.cpp
clang/test/SemaObjC/attr-swift_name.m
Index: clang/test/SemaObjC/attr-swift_name.m
===
--- /dev/null
+++ clang/test/SemaObjC/attr-swift_name.m
@@ -0,0 +1,174 @@
+// RUN: %clang_cc1 -verify -fsyntax-only -fobjc-arc %s
+
+#define SWIFT_NAME(name) __attribute__((__swift_name__(name)))
+
+typedef struct {
+ float x, y, z;
+} Point3D;
+
+__attribute__((__swift_name__("PType")))
+@protocol P
+@end
+
+__attribute__((__swift_name__("IClass")))
+@interface I
+- (instancetype)init SWIFT_NAME("init()");
+- (instancetype)initWithValue:(int)value SWIFT_NAME("iWithValue(_:)");
+
++ (void)refresh SWIFT_NAME("refresh()");
+
+- (instancetype)i SWIFT_NAME("i()");
+
+- (I *)iWithValue:(int)value SWIFT_NAME("i(value:)");
+- (I *)iWithValue:(int)value value:(int)value2 SWIFT_NAME("i(value:extra:)");
+- (I *)iWithValueConvertingValue:(int)value value:(int)value2 SWIFT_NAME("i(_:extra:)");
+
++ (I *)iWithOtheValue:(int)value SWIFT_NAME("init");
+// expected-warning@-1 {{'__swift_name__' attribute argument must be a string literal specifying a Swift function name}}
+
++ (I *)iWithAnotherValue:(int)value SWIFT_NAME("i()");
+// expected-warning@-1 {{too few parameters in '__swift_name__' attribute (expected 1; got 0)}}
+
++ (I *)iWithYetAnotherValue:(int)value SWIFT_NAME("i(value:extra:)");
+// expected-warning@-1 {{too many parameters in '__swift_name__' attribute (expected 1; got 2}}
+
++ (I *)iAndReturnErrorCode:(int *)errorCode SWIFT_NAME("i()"); // no-warning
++ (I *)iWithValue:(int)value andReturnErrorCode:(int *)errorCode SWIFT_NAME("i(value:)"); // no-warning
+
++ (I *)iFromErrorCode:(const int *)errorCode SWIFT_NAME("i()");
+// expected-warning@-1 {{too few parameters in '__swift_name__' attribute (expected 1; got 0)}}
+
++ (I *)iWithPointerA:(int *)value andReturnErrorCode:(int *)errorCode SWIFT_NAME("i()"); // no-warning
++ (I *)iWithPointerB:(int *)value andReturnErrorCode:(int *)errorCode SWIFT_NAME("i(pointer:)"); // no-warning
++ (I *)iWithPointerC:(int *)value andReturnErrorCode:(int *)errorCode SWIFT_NAME("i(pointer:errorCode:)"); // no-warning
+
++ (I *)iWithOtherI:(I *)other SWIFT_NAME("i()");
+// expected-warning@-1 {{too few parameters in '__swift_name__' attribute (expected 1; got 0)}}
+
++ (instancetype)specialI SWIFT_NAME("init(options:)");
++ (instancetype)specialJ SWIFT_NAME("init(options:extra:)");
+// expected-warning@-1 {{too many parameters in '__swift_name__' attribute (expected 0; got 2)}}
++ (instancetype)specialK SWIFT_NAME("init(_:)");
+// expected-warning@-1 {{too many parameters in '__swift_name__' attribute (expected 0; got 1)}}
++ (instancetype)specialL SWIFT_NAME("i(options:)");
+// expected-warning@-1 {{too many parameters in '__swift_name__' attribute (expected 0; got 1)}}
+
++ (instancetype)trailingParen SWIFT_NAME("foo(");
+// expected-warning@-1 {{'__swift_name__' attribute argument must be a string literal specifying a Swift function name}}
++ (instancetype)trailingColon SWIFT_NAME("foo:");
+// expected-warning@-1 {{'__swift_name__' attribute argument must be a string literal specifying a Swift function name}}
++ (instancetype)initialIgnore:(int)value SWIFT_NAME("_(value:)");
+// expected-warning@-1 {{'__swift_name__' attribute has invalid identifier for the base name}}
++ (instancetype)middleOmitted:(int)value SWIFT_NAME("i(:)");
+// expected-warning@-1 {{'__swift_name__' attribute has invalid identifier for the parameter name}}
+
+@property(strong) id someProp SWIFT_NAME("prop");
+@end
+
+enum SWIFT_NAME("E") E {
+ value1,
+ value2,
+ value3 SWIFT_NAME("three"),
+ value4 SWIFT_NAME("four()"), // expected-warning {{'__swift_name__' attribute has invalid identifier for the base name}}
+};
+
+struct SWIFT_NAME("TStruct") SStruct {
+ int i, j, k SWIFT_NAME("kay");
+};
+
+int i SWIFT_NAME("g_i");
+
+void f0(int i) SWIFT_NAME("f_0");
+// expected-warning@-1 {{'__swift_name__' attribute argument must be a string literal specifying a Swift function name}}
+
+void f1(int i) SWIFT_NAME("f_1()");
+// expected-warning@-1 {{too few parameters in '__swift_name__' attribute (expected 1; got 0)}}
+
+void f2(int i) SWIFT_NAME("f_2(a:b:)");
+// expected-warning@-1
[PATCH] D87534: Sema: introduce `__attribute__((__swift_name__))`
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. init(foo:bar:baz:) or
controllerForName(_:),
+ /// 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()) {
+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
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D87534: Sema: introduce `__attribute__((__swift_name__))`
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;
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;
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. init(foo:bar:baz:) or
controllerForName(_:),
+ /// 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()) {
+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
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D87534: Sema: introduce `__attribute__((__swift_name__))`
aaron.ballman accepted this revision. aaron.ballman added a comment. This revision is now accepted and ready to land. LGTM, though you may want to get a second set of eyes on the swift name validation bits for a more thorough review of that part. 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; compnerd wrote: > aaron.ballman wrote: > > take -> have > > > > elsewhere `new value` is spelled `'newValue:`, should that be the same here? > `newValue:` is the argument label spelling and "new value" is the argument > value, which is the reason for the difference. That makes sense to me, thanks! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D87534/new/ https://reviews.llvm.org/D87534 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D87534: Sema: introduce `__attribute__((__swift_name__))`
compnerd updated this revision to Diff 292548.
compnerd marked 2 inline comments as done.
compnerd added a comment.
Address feedback
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D87534/new/
https://reviews.llvm.org/D87534
Files:
clang/include/clang/Basic/Attr.td
clang/include/clang/Basic/AttrDocs.td
clang/include/clang/Basic/DiagnosticGroups.td
clang/include/clang/Basic/DiagnosticSemaKinds.td
clang/include/clang/Sema/Sema.h
clang/lib/Sema/SemaDecl.cpp
clang/lib/Sema/SemaDeclAttr.cpp
clang/test/SemaObjC/attr-swift_name.m
Index: clang/test/SemaObjC/attr-swift_name.m
===
--- /dev/null
+++ clang/test/SemaObjC/attr-swift_name.m
@@ -0,0 +1,174 @@
+// RUN: %clang_cc1 -verify -fsyntax-only -fobjc-arc %s
+
+#define SWIFT_NAME(name) __attribute__((__swift_name__(name)))
+
+typedef struct {
+ float x, y, z;
+} Point3D;
+
+__attribute__((__swift_name__("PType")))
+@protocol P
+@end
+
+__attribute__((__swift_name__("IClass")))
+@interface I
+- (instancetype)init SWIFT_NAME("init()");
+- (instancetype)initWithValue:(int)value SWIFT_NAME("iWithValue(_:)");
+
++ (void)refresh SWIFT_NAME("refresh()");
+
+- (instancetype)i SWIFT_NAME("i()");
+
+- (I *)iWithValue:(int)value SWIFT_NAME("i(value:)");
+- (I *)iWithValue:(int)value value:(int)value2 SWIFT_NAME("i(value:extra:)");
+- (I *)iWithValueConvertingValue:(int)value value:(int)value2 SWIFT_NAME("i(_:extra:)");
+
++ (I *)iWithOtheValue:(int)value SWIFT_NAME("init");
+// expected-warning@-1 {{'__swift_name__' attribute argument must be a string literal specifying a Swift function name}}
+
++ (I *)iWithAnotherValue:(int)value SWIFT_NAME("i()");
+// expected-warning@-1 {{too few parameters in '__swift_name__' attribute (expected 1; got 0)}}
+
++ (I *)iWithYetAnotherValue:(int)value SWIFT_NAME("i(value:extra:)");
+// expected-warning@-1 {{too many parameters in '__swift_name__' attribute (expected 1; got 2}}
+
++ (I *)iAndReturnErrorCode:(int *)errorCode SWIFT_NAME("i()"); // no-warning
++ (I *)iWithValue:(int)value andReturnErrorCode:(int *)errorCode SWIFT_NAME("i(value:)"); // no-warning
+
++ (I *)iFromErrorCode:(const int *)errorCode SWIFT_NAME("i()");
+// expected-warning@-1 {{too few parameters in '__swift_name__' attribute (expected 1; got 0)}}
+
++ (I *)iWithPointerA:(int *)value andReturnErrorCode:(int *)errorCode SWIFT_NAME("i()"); // no-warning
++ (I *)iWithPointerB:(int *)value andReturnErrorCode:(int *)errorCode SWIFT_NAME("i(pointer:)"); // no-warning
++ (I *)iWithPointerC:(int *)value andReturnErrorCode:(int *)errorCode SWIFT_NAME("i(pointer:errorCode:)"); // no-warning
+
++ (I *)iWithOtherI:(I *)other SWIFT_NAME("i()");
+// expected-warning@-1 {{too few parameters in '__swift_name__' attribute (expected 1; got 0)}}
+
++ (instancetype)specialI SWIFT_NAME("init(options:)");
++ (instancetype)specialJ SWIFT_NAME("init(options:extra:)");
+// expected-warning@-1 {{too many parameters in '__swift_name__' attribute (expected 0; got 2)}}
++ (instancetype)specialK SWIFT_NAME("init(_:)");
+// expected-warning@-1 {{too many parameters in '__swift_name__' attribute (expected 0; got 1)}}
++ (instancetype)specialL SWIFT_NAME("i(options:)");
+// expected-warning@-1 {{too many parameters in '__swift_name__' attribute (expected 0; got 1)}}
+
++ (instancetype)trailingParen SWIFT_NAME("foo(");
+// expected-warning@-1 {{'__swift_name__' attribute argument must be a string literal specifying a Swift function name}}
++ (instancetype)trailingColon SWIFT_NAME("foo:");
+// expected-warning@-1 {{'__swift_name__' attribute argument must be a string literal specifying a Swift function name}}
++ (instancetype)initialIgnore:(int)value SWIFT_NAME("_(value:)");
+// expected-warning@-1 {{'__swift_name__' attribute has invalid identifier for base name}}
++ (instancetype)middleOmitted:(int)value SWIFT_NAME("i(:)");
+// expected-warning@-1 {{'__swift_name__' attribute has invalid identifier for parameter name}}
+
+@property(strong) id someProp SWIFT_NAME("prop");
+@end
+
+enum SWIFT_NAME("E") E {
+ value1,
+ value2,
+ value3 SWIFT_NAME("three"),
+ value4 SWIFT_NAME("four()"), // expected-warning {{'__swift_name__' attribute has invalid identifier for base name}}
+};
+
+struct SWIFT_NAME("TStruct") SStruct {
+ int i, j, k SWIFT_NAME("kay");
+};
+
+int i SWIFT_NAME("g_i");
+
+void f0(int i) SWIFT_NAME("f_0");
+// expected-warning@-1 {{'__swift_name__' attribute argument must be a string literal specifying a Swift function name}}
+
+void f1(int i) SWIFT_NAME("f_1()");
+// expected-warning@-1 {{too few parameters in '__swift_name__' attribute (expected 1; got 0)}}
+
+void f2(int i) SWIFT_NAME("f_2(a:b:)");
+// expected-warning@-1 {{too many parameters in '__swift_name__' attribute (expected 1; got 2)}}
+
+void f3(int x, int y) SWIFT_NAME("fWithX(_:y:)");
+void f4(int x, int *error) SWIFT_NAME("fWithX(_:)");
+
+typedef int int_t SWIFT_NAME("IntType");
+
+struct
[PATCH] D87534: Sema: introduce `__attribute__((__swift_name__))`
compnerd marked 11 inline comments as done. compnerd added inline comments. Comment at: clang/include/clang/Basic/DiagnosticSemaKinds.td:3977 +// Swift attributes. +def warn_attr_swift_name_function aaron.ballman wrote: > 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. Discussed offline. 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; aaron.ballman wrote: > take -> have > > elsewhere `new value` is spelled `'newValue:`, should that be the same here? `newValue:` is the argument label spelling and "new value" is the argument value, which is the reason for the difference. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D87534/new/ https://reviews.llvm.org/D87534 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D87534: Sema: introduce `__attribute__((__swift_name__))`
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;
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;
`%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;
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;
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;
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;
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;
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;
take -> have
Comment at: clang/include/clang/Basic/DiagnosticSemaKinds.td:4017
+InGroup;
+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 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
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D87534: Sema: introduce `__attribute__((__swift_name__))`
compnerd updated this revision to Diff 292369.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D87534/new/
https://reviews.llvm.org/D87534
Files:
clang/include/clang/Basic/Attr.td
clang/include/clang/Basic/AttrDocs.td
clang/include/clang/Basic/DiagnosticGroups.td
clang/include/clang/Basic/DiagnosticSemaKinds.td
clang/include/clang/Sema/Sema.h
clang/lib/Sema/SemaDecl.cpp
clang/lib/Sema/SemaDeclAttr.cpp
clang/test/SemaObjC/attr-swift_name.m
Index: clang/test/SemaObjC/attr-swift_name.m
===
--- /dev/null
+++ clang/test/SemaObjC/attr-swift_name.m
@@ -0,0 +1,177 @@
+// RUN: %clang_cc1 -verify -fsyntax-only -fobjc-arc %s
+
+#define SWIFT_NAME(name) __attribute__((__swift_name__(name)))
+
+typedef struct {
+ float x, y, z;
+} Point3D;
+
+__attribute__((__swift_name__("PType")))
+@protocol P
+@end
+
+__attribute__((__swift_name__("IClass")))
+@interface I
+- (instancetype)init SWIFT_NAME("init()");
+- (instancetype)initWithValue:(int)value SWIFT_NAME("iWithValue(_:)");
+
++ (void)refresh SWIFT_NAME("refresh()");
+
+- (instancetype)i SWIFT_NAME("i()");
+
+- (I *)iWithValue:(int)value SWIFT_NAME("i(value:)");
+- (I *)iWithValue:(int)value value:(int)value2 SWIFT_NAME("i(value:extra:)");
+- (I *)iWithValueConvertingValue:(int)value value:(int)value2 SWIFT_NAME("i(_:extra:)");
+
++ (I *)iWithOtheValue:(int)value SWIFT_NAME("init");
+// expected-warning@-1 {{parameter of '__swift_name__' attribute must be a Swift function name string}}
+
++ (I *)iWithAnotherValue:(int)value SWIFT_NAME("i()");
+// expected-warning@-1 {{too few parameters in '__swift_name__' attribute (expected 1; got 0)}}
+
++ (I *)iWithYetAnotherValue:(int)value SWIFT_NAME("i(value:extra:)");
+// expected-warning@-1 {{too many parameters in '__swift_name__' attribute (expected 1; got 2}}
+
++ (I *)iAndReturnErrorCode:(int *)errorCode SWIFT_NAME("i()"); // no-warning
++ (I *)iWithValue:(int)value andReturnErrorCode:(int *)errorCode SWIFT_NAME("i(value:)"); // no-warning
+
++ (I *)iFromErrorCode:(const int *)errorCode SWIFT_NAME("i()");
+// expected-warning@-1 {{too few parameters in '__swift_name__' attribute (expected 1; got 0)}}
+
++ (I *)iWithPointerA:(int *)value andReturnErrorCode:(int *)errorCode SWIFT_NAME("i()"); // no-warning
++ (I *)iWithPointerB:(int *)value andReturnErrorCode:(int *)errorCode SWIFT_NAME("i(pointer:)"); // no-warning
++ (I *)iWithPointerC:(int *)value andReturnErrorCode:(int *)errorCode SWIFT_NAME("i(pointer:errorCode:)"); // no-warning
+
++ (I *)iWithOtherI:(I *)other SWIFT_NAME("i()");
+// expected-warning@-1 {{too few parameters in '__swift_name__' attribute (expected 1; got 0)}}
+
++ (instancetype)specialI SWIFT_NAME("init(options:)");
++ (instancetype)specialJ SWIFT_NAME("init(options:extra:)");
+// expected-warning@-1 {{too many parameters in '__swift_name__' attribute (expected 0; got 2)}}
++ (instancetype)specialK SWIFT_NAME("init(_:)");
+// expected-warning@-1 {{too many parameters in '__swift_name__' attribute (expected 0; got 1)}}
++ (instancetype)specialL SWIFT_NAME("i(options:)");
+// expected-warning@-1 {{too many parameters in '__swift_name__' attribute (expected 0; got 1)}}
+
++ (instancetype)trailingParen SWIFT_NAME("foo(");
+// expected-warning@-1 {{parameter of '__swift_name__' attribute must be a Swift function name string}}
++ (instancetype)trailingColon SWIFT_NAME("foo:");
+// expected-warning@-1 {{parameter of '__swift_name__' attribute must be a Swift function name string}}
++ (instancetype)initialIgnore:(int)value SWIFT_NAME("_(value:)");
+// expected-warning@-1 {{'__swift_name__' attribute has invalid identifier for base name}}
++ (instancetype)middleOmitted:(int)value SWIFT_NAME("i(:)");
+// expected-warning@-1 {{'__swift_name__' attribute has invalid identifier for parameter name}}
+
+@property(strong) id someProp SWIFT_NAME("prop");
+@end
+
+enum SWIFT_NAME("E") E {
+ value1,
+ value2,
+ value3 SWIFT_NAME("three"),
+ value4 SWIFT_NAME("four()"), // expected-warning {{'__swift_name__' attribute has invalid identifier for base name}}
+};
+
+struct SWIFT_NAME("TStruct") SStruct {
+ int i, j, k SWIFT_NAME("kay");
+};
+
+int i SWIFT_NAME("g_i");
+
+void f0(int i) SWIFT_NAME("f_0");
+// expected-warning@-1 {{parameter of '__swift_name__' attribute must be a Swift function name string}}
+
+void f1(int i) SWIFT_NAME("f_1()");
+// expected-warning@-1 {{too few parameters in '__swift_name__' attribute (expected 1; got 0)}}
+
+void f2(int i) SWIFT_NAME("f_2(a:b:)");
+// expected-warning@-1 {{too many parameters in '__swift_name__' attribute (expected 1; got 2)}}
+
+void f3(int x, int y) SWIFT_NAME("fWithX(_:y:)");
+void f4(int x, int *error) SWIFT_NAME("fWithX(_:)");
+
+typedef int int_t SWIFT_NAME("IntType");
+
+struct Point3D createPoint3D(float x, float y, float z) SWIFT_NAME("Point3D.init(x:y:z:)");
+struct Point3D rotatePoint3D(Point3D point, float radians)
[PATCH] D87534: Sema: introduce `__attribute__((__swift_name__))`
compnerd updated this revision to Diff 292368.
compnerd added a comment.
Change diagnostics for conflicting `swift_name`, add a test case
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D87534/new/
https://reviews.llvm.org/D87534
Files:
clang/include/clang/Basic/Attr.td
clang/include/clang/Basic/AttrDocs.td
clang/include/clang/Basic/DiagnosticGroups.td
clang/include/clang/Basic/DiagnosticSemaKinds.td
clang/include/clang/Sema/Sema.h
clang/lib/Sema/SemaDecl.cpp
clang/lib/Sema/SemaDeclAttr.cpp
clang/test/SemaObjC/attr-swift_name.m
Index: clang/test/SemaObjC/attr-swift_name.m
===
--- /dev/null
+++ clang/test/SemaObjC/attr-swift_name.m
@@ -0,0 +1,170 @@
+// RUN: %clang_cc1 -verify -fsyntax-only -fobjc-arc %s
+
+#define SWIFT_NAME(name) __attribute__((__swift_name__(name)))
+
+typedef struct {
+ float x, y, z;
+} Point3D;
+
+__attribute__((__swift_name__("PType")))
+@protocol P
+@end
+
+__attribute__((__swift_name__("IClass")))
+@interface I
+- (instancetype)init SWIFT_NAME("init()");
+- (instancetype)initWithValue:(int)value SWIFT_NAME("iWithValue(_:)");
+
++ (void)refresh SWIFT_NAME("refresh()");
+
+- (instancetype)i SWIFT_NAME("i()");
+
+- (I *)iWithValue:(int)value SWIFT_NAME("i(value:)");
+- (I *)iWithValue:(int)value value:(int)value2 SWIFT_NAME("i(value:extra:)");
+- (I *)iWithValueConvertingValue:(int)value value:(int)value2 SWIFT_NAME("i(_:extra:)");
+
++ (I *)iWithOtheValue:(int)value SWIFT_NAME("init");
+// expected-warning@-1 {{parameter of '__swift_name__' attribute must be a Swift function name string}}
+
++ (I *)iWithAnotherValue:(int)value SWIFT_NAME("i()");
+// expected-warning@-1 {{too few parameters in '__swift_name__' attribute (expected 1; got 0)}}
+
++ (I *)iWithYetAnotherValue:(int)value SWIFT_NAME("i(value:extra:)");
+// expected-warning@-1 {{too many parameters in '__swift_name__' attribute (expected 1; got 2}}
+
++ (I *)iAndReturnErrorCode:(int *)errorCode SWIFT_NAME("i()"); // no-warning
++ (I *)iWithValue:(int)value andReturnErrorCode:(int *)errorCode SWIFT_NAME("i(value:)"); // no-warning
+
++ (I *)iFromErrorCode:(const int *)errorCode SWIFT_NAME("i()");
+// expected-warning@-1 {{too few parameters in '__swift_name__' attribute (expected 1; got 0)}}
+
++ (I *)iWithPointerA:(int *)value andReturnErrorCode:(int *)errorCode SWIFT_NAME("i()"); // no-warning
++ (I *)iWithPointerB:(int *)value andReturnErrorCode:(int *)errorCode SWIFT_NAME("i(pointer:)"); // no-warning
++ (I *)iWithPointerC:(int *)value andReturnErrorCode:(int *)errorCode SWIFT_NAME("i(pointer:errorCode:)"); // no-warning
+
++ (I *)iWithOtherI:(I *)other SWIFT_NAME("i()");
+// expected-warning@-1 {{too few parameters in '__swift_name__' attribute (expected 1; got 0)}}
+
++ (instancetype)specialI SWIFT_NAME("init(options:)");
++ (instancetype)specialJ SWIFT_NAME("init(options:extra:)");
+// expected-warning@-1 {{too many parameters in '__swift_name__' attribute (expected 0; got 2)}}
++ (instancetype)specialK SWIFT_NAME("init(_:)");
+// expected-warning@-1 {{too many parameters in '__swift_name__' attribute (expected 0; got 1)}}
++ (instancetype)specialL SWIFT_NAME("i(options:)");
+// expected-warning@-1 {{too many parameters in '__swift_name__' attribute (expected 0; got 1)}}
+
++ (instancetype)trailingParen SWIFT_NAME("foo(");
+// expected-warning@-1 {{parameter of '__swift_name__' attribute must be a Swift function name string}}
++ (instancetype)trailingColon SWIFT_NAME("foo:");
+// expected-warning@-1 {{parameter of '__swift_name__' attribute must be a Swift function name string}}
++ (instancetype)initialIgnore:(int)value SWIFT_NAME("_(value:)");
+// expected-warning@-1 {{'__swift_name__' attribute has invalid identifier for base name}}
++ (instancetype)middleOmitted:(int)value SWIFT_NAME("i(:)");
+// expected-warning@-1 {{'__swift_name__' attribute has invalid identifier for parameter name}}
+
+@property(strong) id someProp SWIFT_NAME("prop");
+@end
+
+enum SWIFT_NAME("E") E {
+ value1,
+ value2,
+ value3 SWIFT_NAME("three"),
+ value4 SWIFT_NAME("four()"), // expected-warning {{'__swift_name__' attribute has invalid identifier for base name}}
+};
+
+struct SWIFT_NAME("TStruct") SStruct {
+ int i, j, k SWIFT_NAME("kay");
+};
+
+int i SWIFT_NAME("g_i");
+
+void f0(int i) SWIFT_NAME("f_0");
+// expected-warning@-1 {{parameter of '__swift_name__' attribute must be a Swift function name string}}
+
+void f1(int i) SWIFT_NAME("f_1()");
+// expected-warning@-1 {{too few parameters in '__swift_name__' attribute (expected 1; got 0)}}
+
+void f2(int i) SWIFT_NAME("f_2(a:b:)");
+// expected-warning@-1 {{too many parameters in '__swift_name__' attribute (expected 1; got 2)}}
+
+void f3(int x, int y) SWIFT_NAME("fWithX(_:y:)");
+void f4(int x, int *error) SWIFT_NAME("fWithX(_:)");
+
+typedef int int_t SWIFT_NAME("IntType");
+
+struct Point3D createPoint3D(float x, float y, float z)
[PATCH] D87534: Sema: introduce `__attribute__((__swift_name__))`
compnerd added inline comments.
Comment at: clang/lib/Sema/SemaDeclAttr.cpp:4289
+if (Inline->getName() != Name && !Inline->isImplicit()) {
+ Diag(Inline->getLocation(), diag::warn_attribute_ignored) << Inline;
+ Diag(CI.getLoc(), diag::note_conflicting_attribute);
compnerd wrote:
> aaron.ballman wrote:
> > compnerd wrote:
> > > aaron.ballman wrote:
> > > > I think it would be more helpful if the diagnostic said why the
> > > > attribute is being ignored (because the arguments don't match).
> > > Does the note below not accomplish that?
> > Not really, no. The warning diagnostic itself just says that the attribute
> > is ignored, which is the effect but not the rationale. The note (which is
> > easier for folks to ignore) says the attribute is conflicting, but
> > conflicting with *what* (there could be a half dozen attributes on the same
> > declaration, for instance).
> Okay, I don't think that there is an existing warning, but I should be able
> to add one.
The existing diagnostic is actually more inline with the existing attribute
handling for `__attribute__((__optnone__))`, `__attribute__((__minsize__))`,
and `__attribute__((__always_inline__))` which merge similarly.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D87534/new/
https://reviews.llvm.org/D87534
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D87534: Sema: introduce `__attribute__((__swift_name__))`
compnerd updated this revision to Diff 292299.
compnerd marked 2 inline comments as done.
compnerd added a comment.
Address everything but warning
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D87534/new/
https://reviews.llvm.org/D87534
Files:
clang/include/clang/Basic/Attr.td
clang/include/clang/Basic/AttrDocs.td
clang/include/clang/Basic/DiagnosticGroups.td
clang/include/clang/Basic/DiagnosticSemaKinds.td
clang/include/clang/Sema/Sema.h
clang/lib/Sema/SemaDecl.cpp
clang/lib/Sema/SemaDeclAttr.cpp
clang/test/SemaObjC/attr-swift_name.m
Index: clang/test/SemaObjC/attr-swift_name.m
===
--- /dev/null
+++ clang/test/SemaObjC/attr-swift_name.m
@@ -0,0 +1,170 @@
+// RUN: %clang_cc1 -verify -fsyntax-only -fobjc-arc %s
+
+#define SWIFT_NAME(name) __attribute__((__swift_name__(name)))
+
+typedef struct {
+ float x, y, z;
+} Point3D;
+
+__attribute__((__swift_name__("PType")))
+@protocol P
+@end
+
+__attribute__((__swift_name__("IClass")))
+@interface I
+- (instancetype)init SWIFT_NAME("init()");
+- (instancetype)initWithValue:(int)value SWIFT_NAME("iWithValue(_:)");
+
++ (void)refresh SWIFT_NAME("refresh()");
+
+- (instancetype)i SWIFT_NAME("i()");
+
+- (I *)iWithValue:(int)value SWIFT_NAME("i(value:)");
+- (I *)iWithValue:(int)value value:(int)value2 SWIFT_NAME("i(value:extra:)");
+- (I *)iWithValueConvertingValue:(int)value value:(int)value2 SWIFT_NAME("i(_:extra:)");
+
++ (I *)iWithOtheValue:(int)value SWIFT_NAME("init");
+// expected-warning@-1 {{parameter of '__swift_name__' attribute must be a Swift function name string}}
+
++ (I *)iWithAnotherValue:(int)value SWIFT_NAME("i()");
+// expected-warning@-1 {{too few parameters in '__swift_name__' attribute (expected 1; got 0)}}
+
++ (I *)iWithYetAnotherValue:(int)value SWIFT_NAME("i(value:extra:)");
+// expected-warning@-1 {{too many parameters in '__swift_name__' attribute (expected 1; got 2}}
+
++ (I *)iAndReturnErrorCode:(int *)errorCode SWIFT_NAME("i()"); // no-warning
++ (I *)iWithValue:(int)value andReturnErrorCode:(int *)errorCode SWIFT_NAME("i(value:)"); // no-warning
+
++ (I *)iFromErrorCode:(const int *)errorCode SWIFT_NAME("i()");
+// expected-warning@-1 {{too few parameters in '__swift_name__' attribute (expected 1; got 0)}}
+
++ (I *)iWithPointerA:(int *)value andReturnErrorCode:(int *)errorCode SWIFT_NAME("i()"); // no-warning
++ (I *)iWithPointerB:(int *)value andReturnErrorCode:(int *)errorCode SWIFT_NAME("i(pointer:)"); // no-warning
++ (I *)iWithPointerC:(int *)value andReturnErrorCode:(int *)errorCode SWIFT_NAME("i(pointer:errorCode:)"); // no-warning
+
++ (I *)iWithOtherI:(I *)other SWIFT_NAME("i()");
+// expected-warning@-1 {{too few parameters in '__swift_name__' attribute (expected 1; got 0)}}
+
++ (instancetype)specialI SWIFT_NAME("init(options:)");
++ (instancetype)specialJ SWIFT_NAME("init(options:extra:)");
+// expected-warning@-1 {{too many parameters in '__swift_name__' attribute (expected 0; got 2)}}
++ (instancetype)specialK SWIFT_NAME("init(_:)");
+// expected-warning@-1 {{too many parameters in '__swift_name__' attribute (expected 0; got 1)}}
++ (instancetype)specialL SWIFT_NAME("i(options:)");
+// expected-warning@-1 {{too many parameters in '__swift_name__' attribute (expected 0; got 1)}}
+
++ (instancetype)trailingParen SWIFT_NAME("foo(");
+// expected-warning@-1 {{parameter of '__swift_name__' attribute must be a Swift function name string}}
++ (instancetype)trailingColon SWIFT_NAME("foo:");
+// expected-warning@-1 {{parameter of '__swift_name__' attribute must be a Swift function name string}}
++ (instancetype)initialIgnore:(int)value SWIFT_NAME("_(value:)");
+// expected-warning@-1 {{'__swift_name__' attribute has invalid identifier for base name}}
++ (instancetype)middleOmitted:(int)value SWIFT_NAME("i(:)");
+// expected-warning@-1 {{'__swift_name__' attribute has invalid identifier for parameter name}}
+
+@property(strong) id someProp SWIFT_NAME("prop");
+@end
+
+enum SWIFT_NAME("E") E {
+ value1,
+ value2,
+ value3 SWIFT_NAME("three"),
+ value4 SWIFT_NAME("four()"), // expected-warning {{'__swift_name__' attribute has invalid identifier for base name}}
+};
+
+struct SWIFT_NAME("TStruct") SStruct {
+ int i, j, k SWIFT_NAME("kay");
+};
+
+int i SWIFT_NAME("g_i");
+
+void f0(int i) SWIFT_NAME("f_0");
+// expected-warning@-1 {{parameter of '__swift_name__' attribute must be a Swift function name string}}
+
+void f1(int i) SWIFT_NAME("f_1()");
+// expected-warning@-1 {{too few parameters in '__swift_name__' attribute (expected 1; got 0)}}
+
+void f2(int i) SWIFT_NAME("f_2(a:b:)");
+// expected-warning@-1 {{too many parameters in '__swift_name__' attribute (expected 1; got 2)}}
+
+void f3(int x, int y) SWIFT_NAME("fWithX(_:y:)");
+void f4(int x, int *error) SWIFT_NAME("fWithX(_:)");
+
+typedef int int_t SWIFT_NAME("IntType");
+
+struct Point3D createPoint3D(float x, float y, float z)
[PATCH] D87534: Sema: introduce `__attribute__((__swift_name__))`
compnerd marked 3 inline comments as done.
compnerd added inline comments.
Comment at: clang/include/clang/Basic/Attr.td:104
+def ObjCClassMethod
+: SubsetSubjectisInstanceMethod()}],
aaron.ballman wrote:
> This change is no longer needed.
Ah, right, I'll move it to the `swift_private` change.
Comment at: clang/lib/Sema/SemaDeclAttr.cpp:4289
+if (Inline->getName() != Name && !Inline->isImplicit()) {
+ Diag(Inline->getLocation(), diag::warn_attribute_ignored) << Inline;
+ Diag(CI.getLoc(), diag::note_conflicting_attribute);
aaron.ballman wrote:
> compnerd wrote:
> > aaron.ballman wrote:
> > > I think it would be more helpful if the diagnostic said why the attribute
> > > is being ignored (because the arguments don't match).
> > Does the note below not accomplish that?
> Not really, no. The warning diagnostic itself just says that the attribute is
> ignored, which is the effect but not the rationale. The note (which is easier
> for folks to ignore) says the attribute is conflicting, but conflicting with
> *what* (there could be a half dozen attributes on the same declaration, for
> instance).
Okay, I don't think that there is an existing warning, but I should be able to
add one.
Comment at: clang/lib/Sema/SemaDeclAttr.cpp:5804-5805
+if (const auto *Method = dyn_cast(D)) {
+ ParamCount = Method->getSelector().getNumArgs();
+ Params = Method->parameters().slice(0, ParamCount);
+} else {
aaron.ballman wrote:
> compnerd wrote:
> > aaron.ballman wrote:
> > > Do you have to worry about functions with `...` variadic parameters and
> > > how those impact counts (for either ObjC or regular methods)?
> > No, they are currently not auto-imported AFAIK.
> Should such function signatures be rejected here though? (If so, can you add
> a test for that case as well as a test using a K C declaration like `void
> f()`?)
I'll add a test case demonstrating that, it is indeed missing.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D87534/new/
https://reviews.llvm.org/D87534
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D87534: Sema: introduce `__attribute__((__swift_name__))`
aaron.ballman added inline comments.
Comment at: clang/include/clang/Basic/Attr.td:104
+def ObjCClassMethod
+: SubsetSubjectisInstanceMethod()}],
This change is no longer needed.
Comment at: clang/include/clang/Basic/AttrDocs.td:3584
+The name may be a compound Swift name. For a type, enum constant, property, or
+variable declaration, the name must be a simple or qualified identifier.
+ }];
A code example showing proper usage would be appreciated.
Comment at: clang/lib/Sema/SemaDeclAttr.cpp:4289
+if (Inline->getName() != Name && !Inline->isImplicit()) {
+ Diag(Inline->getLocation(), diag::warn_attribute_ignored) << Inline;
+ Diag(CI.getLoc(), diag::note_conflicting_attribute);
compnerd wrote:
> aaron.ballman wrote:
> > I think it would be more helpful if the diagnostic said why the attribute
> > is being ignored (because the arguments don't match).
> Does the note below not accomplish that?
Not really, no. The warning diagnostic itself just says that the attribute is
ignored, which is the effect but not the rationale. The note (which is easier
for folks to ignore) says the attribute is conflicting, but conflicting with
*what* (there could be a half dozen attributes on the same declaration, for
instance).
Comment at: clang/lib/Sema/SemaDeclAttr.cpp:5804-5805
+if (const auto *Method = dyn_cast(D)) {
+ ParamCount = Method->getSelector().getNumArgs();
+ Params = Method->parameters().slice(0, ParamCount);
+} else {
compnerd wrote:
> aaron.ballman wrote:
> > Do you have to worry about functions with `...` variadic parameters and how
> > those impact counts (for either ObjC or regular methods)?
> No, they are currently not auto-imported AFAIK.
Should such function signatures be rejected here though? (If so, can you add a
test for that case as well as a test using a K C declaration like `void f()`?)
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D87534/new/
https://reviews.llvm.org/D87534
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D87534: Sema: introduce `__attribute__((__swift_name__))`
compnerd updated this revision to Diff 292253.
compnerd marked an inline comment as done.
compnerd added a comment.
Address feedback from @aaron.ballman
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D87534/new/
https://reviews.llvm.org/D87534
Files:
clang/include/clang/Basic/Attr.td
clang/include/clang/Basic/AttrDocs.td
clang/include/clang/Basic/DiagnosticGroups.td
clang/include/clang/Basic/DiagnosticSemaKinds.td
clang/include/clang/Sema/Sema.h
clang/lib/Sema/SemaDecl.cpp
clang/lib/Sema/SemaDeclAttr.cpp
clang/test/SemaObjC/attr-swift_name.m
Index: clang/test/SemaObjC/attr-swift_name.m
===
--- /dev/null
+++ clang/test/SemaObjC/attr-swift_name.m
@@ -0,0 +1,167 @@
+// RUN: %clang_cc1 -verify -fsyntax-only -fobjc-arc %s
+
+#define SWIFT_NAME(name) __attribute__((__swift_name__(name)))
+
+typedef struct {
+ float x, y, z;
+} Point3D;
+
+__attribute__((__swift_name__("PType")))
+@protocol P
+@end
+
+__attribute__((__swift_name__("IClass")))
+@interface I
+- (instancetype)init SWIFT_NAME("init()");
+- (instancetype)initWithValue:(int)value SWIFT_NAME("iWithValue(_:)");
+
++ (void)refresh SWIFT_NAME("refresh()");
+
+- (instancetype)i SWIFT_NAME("i()");
+
+- (I *)iWithValue:(int)value SWIFT_NAME("i(value:)");
+- (I *)iWithValue:(int)value value:(int)value2 SWIFT_NAME("i(value:extra:)");
+- (I *)iWithValueConvertingValue:(int)value value:(int)value2 SWIFT_NAME("i(_:extra:)");
+
++ (I *)iWithOtheValue:(int)value SWIFT_NAME("init");
+// expected-warning@-1 {{parameter of '__swift_name__' attribute must be a Swift function name string}}
+
++ (I *)iWithAnotherValue:(int)value SWIFT_NAME("i()");
+// expected-warning@-1 {{too few parameters in '__swift_name__' attribute (expected 1; got 0)}}
+
++ (I *)iWithYetAnotherValue:(int)value SWIFT_NAME("i(value:extra:)");
+// expected-warning@-1 {{too many parameters in '__swift_name__' attribute (expected 1; got 2}}
+
++ (I *)iAndReturnErrorCode:(int *)errorCode SWIFT_NAME("i()"); // no-warning
++ (I *)iWithValue:(int)value andReturnErrorCode:(int *)errorCode SWIFT_NAME("i(value:)"); // no-warning
+
++ (I *)iFromErrorCode:(const int *)errorCode SWIFT_NAME("i()");
+// expected-warning@-1 {{too few parameters in '__swift_name__' attribute (expected 1; got 0)}}
+
++ (I *)iWithPointerA:(int *)value andReturnErrorCode:(int *)errorCode SWIFT_NAME("i()"); // no-warning
++ (I *)iWithPointerB:(int *)value andReturnErrorCode:(int *)errorCode SWIFT_NAME("i(pointer:)"); // no-warning
++ (I *)iWithPointerC:(int *)value andReturnErrorCode:(int *)errorCode SWIFT_NAME("i(pointer:errorCode:)"); // no-warning
+
++ (I *)iWithOtherI:(I *)other SWIFT_NAME("i()");
+// expected-warning@-1 {{too few parameters in '__swift_name__' attribute (expected 1; got 0)}}
+
++ (instancetype)specialI SWIFT_NAME("init(options:)");
++ (instancetype)specialJ SWIFT_NAME("init(options:extra:)");
+// expected-warning@-1 {{too many parameters in '__swift_name__' attribute (expected 0; got 2)}}
++ (instancetype)specialK SWIFT_NAME("init(_:)");
+// expected-warning@-1 {{too many parameters in '__swift_name__' attribute (expected 0; got 1)}}
++ (instancetype)specialL SWIFT_NAME("i(options:)");
+// expected-warning@-1 {{too many parameters in '__swift_name__' attribute (expected 0; got 1)}}
+
++ (instancetype)trailingParen SWIFT_NAME("foo(");
+// expected-warning@-1 {{parameter of '__swift_name__' attribute must be a Swift function name string}}
++ (instancetype)trailingColon SWIFT_NAME("foo:");
+// expected-warning@-1 {{parameter of '__swift_name__' attribute must be a Swift function name string}}
++ (instancetype)initialIgnore:(int)value SWIFT_NAME("_(value:)");
+// expected-warning@-1 {{'__swift_name__' attribute has invalid identifier for base name}}
++ (instancetype)middleOmitted:(int)value SWIFT_NAME("i(:)");
+// expected-warning@-1 {{'__swift_name__' attribute has invalid identifier for parameter name}}
+
+@property(strong) id someProp SWIFT_NAME("prop");
+@end
+
+enum SWIFT_NAME("E") E {
+ value1,
+ value2,
+ value3 SWIFT_NAME("three"),
+ value4 SWIFT_NAME("four()"), // expected-warning {{'__swift_name__' attribute has invalid identifier for base name}}
+};
+
+struct SWIFT_NAME("TStruct") SStruct {
+ int i, j, k SWIFT_NAME("kay");
+};
+
+int i SWIFT_NAME("g_i");
+
+void f0(int i) SWIFT_NAME("f_0");
+// expected-warning@-1 {{parameter of '__swift_name__' attribute must be a Swift function name string}}
+
+void f1(int i) SWIFT_NAME("f_1()");
+// expected-warning@-1 {{too few parameters in '__swift_name__' attribute (expected 1; got 0)}}
+
+void f2(int i) SWIFT_NAME("f_2(a:b:)");
+// expected-warning@-1 {{too many parameters in '__swift_name__' attribute (expected 1; got 2)}}
+
+void f3(int x, int y) SWIFT_NAME("fWithX(_:y:)");
+void f4(int x, int *error) SWIFT_NAME("fWithX(_:)");
+
+typedef int int_t SWIFT_NAME("IntType");
+
+struct Point3D createPoint3D(float x, float y, float z)
[PATCH] D87534: Sema: introduce `__attribute__((__swift_name__))`
compnerd marked 8 inline comments as done.
compnerd added inline comments.
Comment at: clang/lib/Sema/SemaDeclAttr.cpp:4289
+if (Inline->getName() != Name && !Inline->isImplicit()) {
+ Diag(Inline->getLocation(), diag::warn_attribute_ignored) << Inline;
+ Diag(CI.getLoc(), diag::note_conflicting_attribute);
aaron.ballman wrote:
> I think it would be more helpful if the diagnostic said why the attribute is
> being ignored (because the arguments don't match).
Does the note below not accomplish that?
Comment at: clang/lib/Sema/SemaDeclAttr.cpp:5804-5805
+if (const auto *Method = dyn_cast(D)) {
+ ParamCount = Method->getSelector().getNumArgs();
+ Params = Method->parameters().slice(0, ParamCount);
+} else {
aaron.ballman wrote:
> Do you have to worry about functions with `...` variadic parameters and how
> those impact counts (for either ObjC or regular methods)?
No, they are currently not auto-imported AFAIK.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D87534/new/
https://reviews.llvm.org/D87534
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D87534: Sema: introduce `__attribute__((__swift_name__))`
compnerd added inline comments. Comment at: clang/include/clang/Basic/Attr.td:2173 + SubjectList<[Enum, EnumConstant, Field, Function, GlobalVar, Struct, TypedefName, + ObjCInterface, ObjCClassMethod, ObjCInstanceMethod, ObjCProperty, ObjCProtocol], + ErrorDiag, "ExpectedSwiftNameSubjects">; doug.gregor wrote: > compnerd wrote: > > Note for @rjmccall and @doug.gregor - this version actually enumerates the > > subjects to which this attribute appertains unlike what was in the original > > swift version. Are there other subjects which this should list? > Hmm. If we enumerate the subjects, we're going to have to update Clang > whenever Swift's Clang importer learns a new trick. For example, this is > probably missing CXXMethod and FunctionTmpl based on work that's going on in > Swift. I suspect we're also missing ObjCCompatibilityAlias. I'm inclined to > treat this more like AsmLabelAttr and not try to enumerate subjects at all. That seems fair to me. I'll try to remove the subject list and try to clean up the fallout. Comment at: clang/test/SemaObjC/attr-swift-name.m:1 +// RUN: %clang_cc1 -verify -fsyntax-only -fobjc-arc -fblocks %s + aaron.ballman wrote: > The `fblocks` makes me wonder -- should this attribute be one you can write > on a block? This is not something that can be applied to blocks. Removing the `-fblocks` is probably a good idea since it isn't actually used. copy-paste failure on my part :-(. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D87534/new/ https://reviews.llvm.org/D87534 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D87534: Sema: introduce `__attribute__((__swift_name__))`
doug.gregor added a comment. Thank you for doing this! Comment at: clang/include/clang/Basic/Attr.td:2173 + SubjectList<[Enum, EnumConstant, Field, Function, GlobalVar, Struct, TypedefName, + ObjCInterface, ObjCClassMethod, ObjCInstanceMethod, ObjCProperty, ObjCProtocol], + ErrorDiag, "ExpectedSwiftNameSubjects">; compnerd wrote: > Note for @rjmccall and @doug.gregor - this version actually enumerates the > subjects to which this attribute appertains unlike what was in the original > swift version. Are there other subjects which this should list? Hmm. If we enumerate the subjects, we're going to have to update Clang whenever Swift's Clang importer learns a new trick. For example, this is probably missing CXXMethod and FunctionTmpl based on work that's going on in Swift. I suspect we're also missing ObjCCompatibilityAlias. I'm inclined to treat this more like AsmLabelAttr and not try to enumerate subjects at all. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D87534/new/ https://reviews.llvm.org/D87534 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D87534: Sema: introduce `__attribute__((__swift_name__))`
aaron.ballman added inline comments.
Comment at: clang/include/clang/Basic/Attr.td:2174
+ ObjCInterface, ObjCClassMethod, ObjCInstanceMethod,
ObjCProperty, ObjCProtocol],
+ ErrorDiag, "ExpectedSwiftNameSubjects">;
+ let Documentation = [SwiftNameDocs];
I don't see anything adding `ExpectedSwiftNameSubjects` to the list of
diagnostic enumerations, are you sure this is needed?
Comment at: clang/include/clang/Basic/DiagnosticSemaKinds.td:3985
+def warn_attr_swift_name_basename_invalid_identifier
+ : Warning<"%0 attribute has invalid identifier for base name">,
+InGroup;
I think several of these can be consolidated into a single diagnostic with
`%select`:
`%0 attribute has invalid identifier for %select{context name|base
name|parameter name}1`
Comment at: clang/include/clang/Sema/Sema.h:1839
+ /// attribute for the decl \p D. Raise a diagnostic if the name is invalid
+ /// for the given declaration.
+ ///
The docs should probably mention what `AL` is used for.
Comment at: clang/lib/Sema/SemaDeclAttr.cpp:4282
+StringRef Name, bool Override) {
+ if (const SwiftNameAttr *Inline = D->getAttr()) {
+if (Override) {
`const auto *`
Comment at: clang/lib/Sema/SemaDeclAttr.cpp:4289
+if (Inline->getName() != Name && !Inline->isImplicit()) {
+ Diag(Inline->getLocation(), diag::warn_attribute_ignored) << Inline;
+ Diag(CI.getLoc(), diag::note_conflicting_attribute);
I think it would be more helpful if the diagnostic said why the attribute is
being ignored (because the arguments don't match).
Comment at: clang/lib/Sema/SemaDeclAttr.cpp:5658
+static Optional
+validateSwiftFunctionName(StringRef Name, unsigned ,
+ bool ) {
FWIW, I'm not particularly qualified to review the logic here. Someone with
more Swift experience should look at this bit.
Comment at: clang/lib/Sema/SemaDeclAttr.cpp:5660
+ bool ) {
+ SwiftParamCount = 0;
+
Set `IsSingleParamInit` as well (nothing sets it before the early return on
line 5673)?
Comment at: clang/lib/Sema/SemaDeclAttr.cpp:5804-5805
+if (const auto *Method = dyn_cast(D)) {
+ ParamCount = Method->getSelector().getNumArgs();
+ Params = Method->parameters().slice(0, ParamCount);
+} else {
Do you have to worry about functions with `...` variadic parameters and how
those impact counts (for either ObjC or regular methods)?
Comment at: clang/test/SemaObjC/attr-swift-name.m:1
+// RUN: %clang_cc1 -verify -fsyntax-only -fobjc-arc -fblocks %s
+
The `fblocks` makes me wonder -- should this attribute be one you can write on
a block?
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D87534/new/
https://reviews.llvm.org/D87534
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D87534: Sema: introduce `__attribute__((__swift_name__))`
compnerd added inline comments. Comment at: clang/include/clang/Basic/Attr.td:2173 + SubjectList<[Enum, EnumConstant, Field, Function, GlobalVar, Struct, TypedefName, + ObjCInterface, ObjCClassMethod, ObjCInstanceMethod, ObjCProperty, ObjCProtocol], + ErrorDiag, "ExpectedSwiftNameSubjects">; Note for @rjmccall and @doug.gregor - this version actually enumerates the subjects to which this attribute appertains unlike what was in the original swift version. Are there other subjects which this should list? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D87534/new/ https://reviews.llvm.org/D87534 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D87534: Sema: introduce `__attribute__((__swift_name__))`
compnerd created this revision.
compnerd added a reviewer: aaron.ballman.
Herald added a project: clang.
compnerd requested review of this revision.
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
name into Swift. It takes a parameter which is the Swift function name.
This parameter is validated to check if it matches the possible
transformed signature in Swift.
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D87534
Files:
clang/include/clang/Basic/Attr.td
clang/include/clang/Basic/AttrDocs.td
clang/include/clang/Basic/DiagnosticGroups.td
clang/include/clang/Basic/DiagnosticSemaKinds.td
clang/include/clang/Sema/Sema.h
clang/lib/Sema/SemaDecl.cpp
clang/lib/Sema/SemaDeclAttr.cpp
clang/test/SemaObjC/attr-swift-name.m
Index: clang/test/SemaObjC/attr-swift-name.m
===
--- /dev/null
+++ clang/test/SemaObjC/attr-swift-name.m
@@ -0,0 +1,167 @@
+// RUN: %clang_cc1 -verify -fsyntax-only -fobjc-arc -fblocks %s
+
+#define SWIFT_NAME(name) __attribute__((__swift_name__(name)))
+
+typedef struct {
+ float x, y, z;
+} Point3D;
+
+__attribute__((__swift_name__("PType")))
+@protocol P
+@end
+
+__attribute__((__swift_name__("IClass")))
+@interface I
+- (instancetype)init SWIFT_NAME("init()");
+- (instancetype)initWithValue:(int)value SWIFT_NAME("iWithValue(_:)");
+
++ (void)refresh SWIFT_NAME("refresh()");
+
+- (instancetype)i SWIFT_NAME("i()");
+
+- (I *)iWithValue:(int)value SWIFT_NAME("i(value:)");
+- (I *)iWithValue:(int)value value:(int)value2 SWIFT_NAME("i(value:extra:)");
+- (I *)iWithValueConvertingValue:(int)value value:(int)value2 SWIFT_NAME("i(_:extra:)");
+
++ (I *)iWithOtheValue:(int)value SWIFT_NAME("init");
+// expected-warning@-1 {{parameter of '__swift_name__' attribute must be a Swift function name string}}
+
++ (I *)iWithAnotherValue:(int)value SWIFT_NAME("i()");
+// expected-warning@-1 {{too few parameters in '__swift_name__' attribute (expected 1; got 0)}}
+
++ (I *)iWithYetAnotherValue:(int)value SWIFT_NAME("i(value:extra:)");
+// expected-warning@-1 {{too many parameters in '__swift_name__' attribute (expected 1; got 2}}
+
++ (I *)iAndReturnErrorCode:(int *)errorCode SWIFT_NAME("i()"); // no-warning
++ (I *)iWithValue:(int)value andReturnErrorCode:(int *)errorCode SWIFT_NAME("i(value:)"); // no-warning
+
++ (I *)iFromErrorCode:(const int *)errorCode SWIFT_NAME("i()");
+// expected-warning@-1 {{too few parameters in '__swift_name__' attribute (expected 1; got 0)}}
+
++ (I *)iWithPointerA:(int *)value andReturnErrorCode:(int *)errorCode SWIFT_NAME("i()"); // no-warning
++ (I *)iWithPointerB:(int *)value andReturnErrorCode:(int *)errorCode SWIFT_NAME("i(pointer:)"); // no-warning
++ (I *)iWithPointerC:(int *)value andReturnErrorCode:(int *)errorCode SWIFT_NAME("i(pointer:errorCode:)"); // no-warning
+
++ (I *)iWithOtherI:(I *)other SWIFT_NAME("i()");
+// expected-warning@-1 {{too few parameters in '__swift_name__' attribute (expected 1; got 0)}}
+
++ (instancetype)specialI SWIFT_NAME("init(options:)");
++ (instancetype)specialJ SWIFT_NAME("init(options:extra:)");
+// expected-warning@-1 {{too many parameters in '__swift_name__' attribute (expected 0; got 2)}}
++ (instancetype)specialK SWIFT_NAME("init(_:)");
+// expected-warning@-1 {{too many parameters in '__swift_name__' attribute (expected 0; got 1)}}
++ (instancetype)specialL SWIFT_NAME("i(options:)");
+// expected-warning@-1 {{too many parameters in '__swift_name__' attribute (expected 0; got 1)}}
+
++ (instancetype)trailingParen SWIFT_NAME("foo(");
+// expected-warning@-1 {{parameter of '__swift_name__' attribute must be a Swift function name string}}
++ (instancetype)trailingColon SWIFT_NAME("foo:");
+// expected-warning@-1 {{parameter of '__swift_name__' attribute must be a Swift function name string}}
++ (instancetype)initialIgnore:(int)value SWIFT_NAME("_(value:)");
+// expected-warning@-1 {{'__swift_name__' attribute has invalid identifier for base name}}
++ (instancetype)middleOmitted:(int)value SWIFT_NAME("i(:)");
+// expected-warning@-1 {{'__swift_name__' attribute has invalid identifier for parameter name}}
+
+@property(strong) id someProp SWIFT_NAME("prop");
+@end
+
+enum SWIFT_NAME("E") E {
+ value1,
+ value2,
+ value3 SWIFT_NAME("three"),
+ value4 SWIFT_NAME("four()"), // expected-warning {{'__swift_name__' attribute has invalid identifier for base name}}
+};
+
+struct SWIFT_NAME("TStruct") SStruct {
+ int i, j, k SWIFT_NAME("kay");
+};
+
+int i SWIFT_NAME("g_i");
+
+void f0(int i) SWIFT_NAME("f_0");
+// expected-warning@-1 {{parameter of '__swift_name__' attribute must be a Swift function name string}}
+
+void f1(int i) SWIFT_NAME("f_1()");
+// expected-warning@-1 {{too few parameters in '__swift_name__' attribute (expected 1; got 0)}}
+
+void f2(int i)
