Sema: add support for `__attribute__((__swift_objc_members__))`

[compnerd]

Summary

This adds the __swift_objc_members__ attribute to the semantic
analysis. It allows for annotating ObjC interfaces to provide Swift
semantics indicating that the types derived from this interface will be
back-bridged to Objective-C to allow interoperability with Objective-C
and Swift.

[gribozavr2]

clang/include/clang/Basic/AttrDocs.td

3481–3483

clang/test/SemaObjC/attr-swift_objc_members.m

5

20

Please also add a test for (erroneously) passing arguments to the attribute.

[aaron.ballman]

clang/include/clang/Basic/Attr.td

2133

Should this be inherited by redeclarations, or is that not a thing with ObjCInterfaceDecls?

clang/include/clang/Basic/AttrDocs.td

3482

The documentation is a bit hard to follow -- the user is going to be adding this attribute to an ObjC interface, so the docs should be talking about what that means from the ObjC side of things more so than the Swift side, no?

clang/lib/Sema/SemaDeclAttr.cpp

7540

Does it matter if the user writes this attribute on an interface that exposes no members? e.g., are there warnings we may want to give the user about using the attribute in a weird way? (Sorry if this should be obvious, but I don't have experience with Swift.)

clang/test/SemaObjC/attr-swift_objc_members.m

5

The typo fix makes sense to me, but for my own understanding, why switch to a string literal?

[gribozavr2]

clang/include/clang/Basic/AttrDocs.td

3482

IIUC, this attribute has no effect on the Objective-C users of this type.

clang/test/SemaObjC/attr-swift_objc_members.m

5

IIUC, as it is now, the message is tokenized by the lexer -- and I think that's not the intent, none of these words are program code.

[aaron.ballman]

clang/include/clang/Basic/AttrDocs.td

3482

It may not have semantic impact on the ObjC side of things, but I'd still expect that since the user is writing it on an ObjC entity, the docs talk about it from that perspective.

e.g., I'd expect the documentation to be more along the lines of "The attribute indicates that the members of this Objective-C class/protocol/whatever, its subclasses and so on, will be exposed as Swift class members that do the awesome thing." instead of "In Swift, this other attribute does things similar to what this one does in Objective-C."

clang/test/SemaObjC/attr-swift_objc_members.m

5

Interesting and somewhat different from my understanding. My mental model for #error is that it "replays" the tokens into the diagnostic message up to the end of the line. Given that I prefer my diagnostics to be warning: you did the wrong thing and not warning: "you did the wrong thing" (with quotes), I usually leave the quotes off so that the error looks more consistent with other errors.

Neither form is more right than the other in this case, so I don't really care for this review (I was interested in it as a standards committee member who recently had to look at the specification for #error though).

[gribozavr2]

clang/include/clang/Basic/AttrDocs.td

3482

Sure, how about this: "This attribute indicates that Swift subclasses and members of Swift extensions of this class will be implicitly marked with the `@objcMembers` Swift attribute."

(This attribute has no effect on this class itself or its members, even when imported in Swift -- only on Swift extensions & subclasses.)

[aaron.ballman]

clang/include/clang/Basic/AttrDocs.td

3482

That sounds better to me, thank you!

[compnerd]

clang/include/clang/Basic/Attr.td

2133

Objective-C interfaces cannot be redeclared, so this actually is correct I believe.

clang/lib/Sema/SemaDeclAttr.cpp

7540

I think that the name is not exactly helpful in this case, but that ship has sailed since this is in production already. AIUI the attribute should apply to empty interfaces as it results in the Swift interface being exposed back to Objective-C.

clang/test/SemaObjC/attr-swift_objc_members.m

5

FWIW, the reason for the warning not being quoted currently is exactly what @aaron.ballman stated ... that is how I process the #error directive as well, and I tend to leave the quotes off to make the error match the other diagnostics. Is the quoting really that important?

[gribozavr2]

clang/test/SemaObjC/attr-swift_objc_members.m

5

No, not at all important.

[compnerd]

address feedback from @aaron.ballman and @gribozavr2

[gribozavr2]

LGTM!

[aaron.ballman]

LGTM!

clang/include/clang/Basic/Attr.td

 def Regparm : TypeAttr {
   let Spellings = [GCC<"regparm">];
   let Args = [UnsignedArgument<"NumParams">];
   let Documentation = [RegparmDocs];
   // Represented as part of the enclosing function type.
   let ASTNode = 0;
 }
 
+def SwiftObjCMembers : Attr {
+  let Spellings = [GNU<"swift_objc_members">];
+  let Subjects = SubjectList<[ObjCInterface], ErrorDiag>;
+  let Documentation = [SwiftObjCMembersDocs];
+}
+
 def SwiftError : InheritableAttr {
   let Spellings = [GNU<"swift_error">];
   let Args = [
       EnumArgument<"Convention", "ConventionKind",
                    ["none", "nonnull_error", "null_result", "zero_result", "nonzero_result"],
                    ["None", "NonNullError", "NullResult", "ZeroResult", "NonZeroResult"]>
   ];
   let Subjects = SubjectList<[Function, ObjCMethod], ErrorDiag>;

clang/include/clang/Basic/AttrDocs.td

 
 def SwiftDocs : DocumentationCategory<"Customizing Swift Import"> {
   let Content = [{
 Clang supports additional attributes for customizing how APIs are imported into
 Swift.
   }];
 }
 
+def SwiftObjCMembersDocs : Documentation {
+  let Category = SwiftDocs;
+  let Heading = "swift_objc_members";
+  let Content = [{
+This attribute indicates that Swift subclasses and members of Swift extensions
+of this class will be implicitly marked with the ``@objcMembers`` Swift
+attribute, exposing them back to Objective-C.
+  }];
+}
+
 def SwiftErrorDocs : Documentation {
   let Category = SwiftDocs;
   let Heading = "swift_error";
   let Content = [{
 The ``swift_error`` attribute controls whether a particular function (or
 Objective-C method) is imported into Swift as a throwing function, and if so,
 which dynamic convention it uses.
 

clang/lib/Sema/SemaDeclAttr.cpp

static void ProcessDeclAttribute(Sema &S, Scope *scope, Decl *D,
   case ParsedAttr::AT_TypeTagForDatatype:
     handleTypeTagForDatatypeAttr(S, D, AL);
     break;
 
   // Swift attributes.
   case ParsedAttr::AT_SwiftError:
     handleSwiftError(S, D, AL);
     break;
+  case ParsedAttr::AT_SwiftObjCMembers:
+    handleSimpleAttribute<SwiftObjCMembersAttr>(S, D, AL);
+    break;
 
   // XRay attributes.
   case ParsedAttr::AT_XRayLogArgs:
     handleXRayLogArgsAttr(S, D, AL);
     break;
 
   case ParsedAttr::AT_PatchableFunctionEntry:
     handlePatchableFunctionEntryAttr(S, D, AL);

clang/test/Misc/pragma-attribute-supported-attributes-list.test

 // CHECK-NEXT: ScopedLockable (SubjectMatchRule_record)
 // CHECK-NEXT: Section (SubjectMatchRule_function, SubjectMatchRule_variable_is_global, SubjectMatchRule_objc_method, SubjectMatchRule_objc_property)
 // CHECK-NEXT: SetTypestate (SubjectMatchRule_function_is_member)
 // CHECK-NEXT: SpeculativeLoadHardening (SubjectMatchRule_function, SubjectMatchRule_objc_method)
 // CHECK-NEXT: SwiftContext (SubjectMatchRule_variable_is_parameter)
 // CHECK-NEXT: SwiftError (SubjectMatchRule_function, SubjectMatchRule_objc_method)
 // CHECK-NEXT: SwiftErrorResult (SubjectMatchRule_variable_is_parameter)
 // CHECK-NEXT: SwiftIndirectResult (SubjectMatchRule_variable_is_parameter)
+// CHECK-NEXT: SwiftObjCMembers (SubjectMatchRule_objc_interface)
 // CHECK-NEXT: TLSModel (SubjectMatchRule_variable_is_thread_local)
 // CHECK-NEXT: Target (SubjectMatchRule_function)
 // CHECK-NEXT: TestTypestate (SubjectMatchRule_function_is_member)
 // CHECK-NEXT: TrivialABI (SubjectMatchRule_record)
 // CHECK-NEXT: Uninitialized (SubjectMatchRule_variable_is_local)
 // CHECK-NEXT: UseHandle (SubjectMatchRule_variable_is_parameter)
 // CHECK-NEXT: VecReturn (SubjectMatchRule_record)
 // CHECK-NEXT: VecTypeHint (SubjectMatchRule_function)

clang/test/SemaObjC/attr-swift_objc_members.m

+// RUN: %clang_cc1 -verify -fsyntax-only %s
+
+#if !__has_attribute(swift_objc_members)
+#error cannot verify presence of swift_objc_members attribute
+#endif
+
+__attribute__((__swift_objc_members__))
+__attribute__((__objc_root_class__))
+@interface I
+@end
+
+__attribute__((swift_objc_members))
+@protocol P
+@end
+// expected-error@-3 {{'swift_objc_members' attribute only applies to Objective-C interfaces}}
+
+__attribute__((swift_objc_members))
+extern void f(void);
+// expected-error@-2 {{'swift_objc_members' attribute only applies to Objective-C interfaces}}
+
+// expected-error@+1 {{'__swift_objc_members__' attribute takes no arguments}}
+__attribute__((__swift_objc_members__("J")))
+@interface J
+@end