Merge pull request #28479 from varungandhi-apple/vg-allow-clang-types-in-convention-attr

Allow spelling out a C type in the convention attribute.

Author: varungandhi-apple

include/swift/AST/Attr.h

@@ -65,8 +65,24 @@ class TypeAttributes {
   /// AtLoc - This is the location of the first '@' in the attribute specifier.
   /// If this is an empty attribute specifier, then this will be an invalid loc.
   SourceLoc AtLoc;
-  Optional<StringRef> convention = None;
-  Optional<StringRef> conventionWitnessMethodProtocol = None;
+
+  struct Convention {
+    StringRef Name = {};
+    StringRef WitnessMethodProtocol = {};
+    StringRef ClangType = {};
+    // Carry the source location for diagnostics.
+    SourceLoc ClangTypeLoc = {};
+
+    /// Convenience factory function to create a Swift convention.
+    ///
+    /// Don't use this function if you are creating a C convention as you
+    /// probably need a ClangType field as well.
+    static Convention makeSwiftConvention(StringRef name) {
+      return {name, "", "", {}};
+    }
+  };
+
+  Optional<Convention> ConventionArguments;
 
   // Indicates whether the type's '@differentiable' attribute has a 'linear'
   // argument.
@@ -135,8 +151,19 @@ class TypeAttributes {
     return true;
   }
 
-  bool hasConvention() const { return convention.hasValue(); }
-  StringRef getConvention() const { return *convention; }
+  bool hasConvention() const { return ConventionArguments.hasValue(); }
+
+  /// Returns the primary calling convention string.
+  ///
+  /// Note: For C conventions, this may not represent the full convention.
+  StringRef getConventionName() const {
+    return ConventionArguments.getValue().Name;
+  }
+
+  /// Show the string enclosed between @convention(..)'s parentheses.
+  ///
+  /// For example, @convention(foo, bar) will give the string "foo, bar".
+  void getConventionArguments(SmallVectorImpl<char> &buffer) const;
 
   bool hasOwnership() const {
     return getOwnership() != ReferenceOwnership::Strong;

include/swift/AST/DiagnosticsParse.def

@@ -1430,6 +1430,13 @@ ERROR(convention_attribute_expected_name,none,
       "expected convention name identifier in 'convention' attribute", ())
 ERROR(convention_attribute_expected_rparen,none,
       "expected ')' after convention name for 'convention' attribute", ())
+ERROR(convention_attribute_ctype_expected_label,none,
+      "expected 'cType' label in 'convention' attribute", ())
+ERROR(convention_attribute_ctype_expected_colon,none,
+      "expected ':' after 'cType' for 'convention' attribute", ())
+ERROR(convention_attribute_ctype_expected_string,none,
+      "expected string literal containing clang type for 'cType' in "
+      "'convention' attribute", ())
 ERROR(convention_attribute_witness_method_expected_colon,none,
       "expected ':' after 'witness_method' for 'convention' attribute", ())
 ERROR(convention_attribute_witness_method_expected_protocol,none,

include/swift/Parse/Parser.h

@@ -1021,6 +1021,10 @@ class Parser {
   bool parseTypeAttributeListPresent(ParamDecl::Specifier &Specifier,
                                      SourceLoc &SpecifierLoc,
                                      TypeAttributes &Attributes);
+
+  bool parseConventionAttributeInternal(bool justChecking,
+                                        TypeAttributes::Convention &convention);
+
   bool parseTypeAttribute(TypeAttributes &Attributes, SourceLoc AtLoc,
                           bool justChecking = false);
 

lib/AST/Attr.cpp

@@ -26,6 +26,7 @@
 #include "swift/AST/TypeRepr.h"
 #include "swift/AST/Types.h"
 #include "swift/Basic/Defer.h"
+#include "swift/Basic/QuotedString.h"
 #include "llvm/ADT/SmallString.h"
 #include "llvm/ADT/StringSwitch.h"
 #include "llvm/Support/ErrorHandling.h"
@@ -83,6 +84,18 @@ StringRef swift::getAccessLevelSpelling(AccessLevel value) {
   llvm_unreachable("Unhandled AccessLevel in switch.");
 }
 
+void TypeAttributes::getConventionArguments(SmallVectorImpl<char> &buf) const {
+  llvm::raw_svector_ostream stream(buf);
+  auto &convention = ConventionArguments.getValue();
+  stream << convention.Name;
+  if (!convention.WitnessMethodProtocol.empty()) {
+    stream << ": " << convention.WitnessMethodProtocol;
+    return;
+  }
+  if (!convention.ClangType.empty())
+    stream << ", cType: " << QuotedString(convention.ClangType);
+}
+
 /// Given a name like "autoclosure", return the type attribute ID that
 /// corresponds to it.  This returns TAK_Count on failure.
 ///

lib/AST/TypeRepr.cpp

@@ -312,10 +312,12 @@ void AttributedTypeRepr::printAttrs(ASTPrinter &Printer,
   if (hasAttr(TAK_thick))
     Printer.printSimpleAttr("@thick") << " ";
 
-  if (hasAttr(TAK_convention) && Attrs.convention.hasValue()) {
+  if (hasAttr(TAK_convention) && Attrs.hasConvention()) {
     Printer.callPrintStructurePre(PrintStructureKind::BuiltinAttribute);
     Printer.printAttrName("@convention");
-    Printer << "(" << Attrs.convention.getValue() << ")";
+    SmallString<32> convention;
+    Attrs.getConventionArguments(convention);
+    Printer << "(" << convention << ")";
     Printer.printStructurePost(PrintStructureKind::BuiltinAttribute);
     Printer << " ";
   }

lib/Parse/ParseDecl.cpp

@@ -2370,6 +2370,91 @@ static bool parseDifferentiableAttributeArgument(Parser &P,
   return false;
 }
 
+/// Parse the inside of a convention attribute '(...)'.
+///
+/// The '@convention' prefix should've been parsed by the caller.
+/// See `Parser::parseTypeAttribute` for the justChecking argument.
+///
+/// Returns true if there was an error.
+bool Parser::parseConventionAttributeInternal(
+    bool justChecking, TypeAttributes::Convention &convention) {
+  SourceLoc LPLoc;
+  if (!consumeIfNotAtStartOfLine(tok::l_paren)) {
+    if (!justChecking)
+      diagnose(Tok, diag::convention_attribute_expected_lparen);
+    return true;
+  }
+
+  if (Tok.isNot(tok::identifier)) {
+    if (!justChecking)
+      diagnose(Tok, diag::convention_attribute_expected_name);
+    return true;
+  }
+
+  convention.Name = Tok.getText();
+  consumeToken(tok::identifier);
+
+  // Consume extra (optional) ', cType: " blah blah "'
+  if (consumeIf(tok::comma)) {
+    if (Tok.isNot(tok::identifier)) {
+      if (!justChecking)
+        diagnose(Tok, diag::convention_attribute_ctype_expected_label);
+      return true;
+    }
+    auto cTypeLabel = Tok.getText();
+    consumeToken(tok::identifier);
+    if (cTypeLabel != "cType") {
+      if (!justChecking)
+        diagnose(Tok, diag::convention_attribute_ctype_expected_label);
+      return true;
+    }
+    if (!consumeIf(tok::colon)) {
+      if (!justChecking)
+        diagnose(Tok, diag::convention_attribute_ctype_expected_colon);
+      return true;
+    }
+    if (Tok.isNot(tok::string_literal)) {
+      if (!justChecking)
+        diagnose(Tok, diag::convention_attribute_ctype_expected_string);
+      return true;
+    }
+    if (auto ty = getStringLiteralIfNotInterpolated(Tok.getLoc(), "(C type)")) {
+      convention.ClangType = ty.getValue();
+      convention.ClangTypeLoc = Tok.getLoc();
+    }
+    consumeToken(tok::string_literal);
+  }
+
+  if (convention.Name == "witness_method") {
+    if (!consumeIf(tok::colon)) {
+      if (!justChecking)
+        diagnose(Tok,
+                 diag::convention_attribute_witness_method_expected_colon);
+      return true;
+    }
+    if (Tok.isNot(tok::identifier)) {
+      if (!justChecking)
+        diagnose(Tok,
+                 diag::convention_attribute_witness_method_expected_protocol);
+      return true;
+    }
+
+    convention.WitnessMethodProtocol = Tok.getText();
+    consumeToken(tok::identifier);
+  }
+  
+  // Parse the ')'.  We can't use parseMatchingToken if we're in
+  // just-checking mode.
+  if (justChecking && Tok.isNot(tok::r_paren))
+    return true;
+
+  SourceLoc RPLoc;
+  parseMatchingToken(tok::r_paren, RPLoc,
+                     diag::convention_attribute_expected_rparen,
+                     LPLoc);
+  return false;
+}
+
 /// \verbatim
 ///   attribute-type:
 ///     'noreturn'
@@ -2443,57 +2528,17 @@ bool Parser::parseTypeAttribute(TypeAttributes &Attributes, SourceLoc AtLoc,
   StringRef Text = Tok.getText();
   consumeToken();
 
-  StringRef conventionName;
-  StringRef witnessMethodProtocol;
-
+  TypeAttributes::Convention convention;
   if (attr == TAK_convention) {
-    SourceLoc LPLoc;
-    if (!consumeIfNotAtStartOfLine(tok::l_paren)) {
-      if (!justChecking)
-        diagnose(Tok, diag::convention_attribute_expected_lparen);
-      return true;
-    }
-
-    if (Tok.isNot(tok::identifier)) {
-      if (!justChecking)
-        diagnose(Tok, diag::convention_attribute_expected_name);
+    bool failedToParse =
+      parseConventionAttributeInternal(justChecking, convention);
+    if (failedToParse) {
+      if (Tok.is(tok::r_paren))
+        consumeToken();
       return true;
     }
-
-    conventionName = Tok.getText();
-    consumeToken(tok::identifier);
-
-    if (conventionName == "witness_method") {
-      if (Tok.isNot(tok::colon)) {
-        if (!justChecking)
-          diagnose(Tok,
-                   diag::convention_attribute_witness_method_expected_colon);
-        return true;
-      }
-      consumeToken(tok::colon);
-      if (Tok.isNot(tok::identifier)) {
-        if (!justChecking)
-          diagnose(Tok,
-                   diag::convention_attribute_witness_method_expected_protocol);
-        return true;
-      }
-
-      witnessMethodProtocol = Tok.getText();
-      consumeToken(tok::identifier);
-    }
-
-    // Parse the ')'.  We can't use parseMatchingToken if we're in
-    // just-checking mode.
-    if (justChecking && Tok.isNot(tok::r_paren))
-      return true;
-
-    SourceLoc RPLoc;
-    parseMatchingToken(tok::r_paren, RPLoc,
-                       diag::convention_attribute_expected_rparen,
-                       LPLoc);
   }
 
-
   // In just-checking mode, we only need to consume the tokens, and we don't
   // want to do any other analysis.
   if (justChecking)
@@ -2592,8 +2637,7 @@ bool Parser::parseTypeAttribute(TypeAttributes &Attributes, SourceLoc AtLoc,
 
   // Convention attribute.
   case TAK_convention:
-    Attributes.convention = conventionName;
-    Attributes.conventionWitnessMethodProtocol = witnessMethodProtocol;
+    Attributes.ConventionArguments = convention;
     break;
 
   case TAK__opaqueReturnTypeOf: {

lib/ParseSIL/ParseSIL.cpp

@@ -1272,7 +1272,8 @@ bool SILParser::parseSILType(SILType &Result,
   if (IsFuncDecl && !attrs.has(TAK_convention)) {
     // Use a random location.
     attrs.setAttr(TAK_convention, P.PreviousLoc);
-    attrs.convention = "thin";
+    attrs.ConventionArguments =
+      TypeAttributes::Convention::makeSwiftConvention("thin");
   }
 
   ParserResult<TypeRepr> TyR = P.parseType(diag::expected_sil_type,

lib/Sema/TypeCheckType.cpp

@@ -2168,7 +2168,7 @@ Type TypeResolver::resolveAttributedType(TypeAttributes &attrs,
       if (!attrs.hasConvention()) {
         rep = SILFunctionType::Representation::Thick;
       } else {
-        auto convention = attrs.getConvention();
+        auto convention = attrs.getConventionName();
         // SIL exposes a greater number of conventions than Swift source.
         auto parsedRep =
             llvm::StringSwitch<Optional<SILFunctionType::Representation>>(
@@ -2185,14 +2185,15 @@ Type TypeResolver::resolveAttributedType(TypeAttributes &attrs,
                 .Default(None);
         if (!parsedRep) {
           diagnose(attrs.getLoc(TAK_convention),
-                   diag::unsupported_sil_convention, attrs.getConvention());
+                   diag::unsupported_sil_convention, attrs.getConventionName());
           rep = SILFunctionType::Representation::Thin;
         } else {
           rep = *parsedRep;
         }
 
         if (rep == SILFunctionType::Representation::WitnessMethod) {
-          auto protocolName = *attrs.conventionWitnessMethodProtocol;
+          auto protocolName =
+            attrs.ConventionArguments.getValue().WitnessMethodProtocol;
           witnessMethodProtocol = new (Context) SimpleIdentTypeRepr(
               SourceLoc(), Context.getIdentifier(protocolName));
         }
@@ -2225,15 +2226,15 @@ Type TypeResolver::resolveAttributedType(TypeAttributes &attrs,
       if (attrs.hasConvention()) {
         auto parsedRep =
             llvm::StringSwitch<Optional<FunctionType::Representation>>(
-                attrs.getConvention())
+                attrs.getConventionName())
                 .Case("swift", FunctionType::Representation::Swift)
                 .Case("block", FunctionType::Representation::Block)
                 .Case("thin", FunctionType::Representation::Thin)
                 .Case("c", FunctionType::Representation::CFunctionPointer)
                 .Default(None);
         if (!parsedRep) {
           diagnose(attrs.getLoc(TAK_convention), diag::unsupported_convention,
-                   attrs.getConvention());
+                   attrs.getConventionName());
           rep = FunctionType::Representation::Swift;
         } else {
           rep = *parsedRep;
@@ -2244,7 +2245,7 @@ Type TypeResolver::resolveAttributedType(TypeAttributes &attrs,
                 rep == FunctionType::Representation::Block) {
               diagnose(attrs.getLoc(TAK_convention),
                        diag::invalid_autoclosure_and_convention_attributes,
-                       attrs.getConvention());
+                       attrs.getConventionName());
               attrs.clearAttribute(TAK_convention);
             }
           }
@@ -2357,7 +2358,7 @@ Type TypeResolver::resolveAttributedType(TypeAttributes &attrs,
     // Remove the function attributes from the set so that we don't diagnose.
     for (auto i : FunctionAttrs)
       attrs.clearAttribute(i);
-    attrs.convention = None;
+    attrs.ConventionArguments = None;
   }
 
   // In SIL, handle @opened (n), which creates an existential archetype.

test/Parse/c_function_pointers.swift

@@ -47,3 +47,11 @@ if true {
 func genericFunc<T>(_ t: T) -> T { return t }
 
 let f: @convention(c) (Int) -> Int = genericFunc // expected-error{{cannot be formed from a reference to a generic function}}
+
+func ct1() -> () { print("") }
+
+let ct1ref0 : @convention(c, cType: "void *(void)") () -> () = ct1
+let ct1ref1 : @convention(c, cType: "void *(void)") = ct1 // expected-error{{expected type}}
+let ct1ref2 : @convention(c, ) () -> () = ct1 // expected-error{{expected 'cType' label in 'convention' attribute}}
+let ct1ref3 : @convention(c, cType) () -> () = ct1 // expected-error{{expected ':' after 'cType' for 'convention' attribute}}
+let ct1ref4 : @convention(c, cType: ) () -> () = ct1 // expected-error{{expected string literal containing clang type for 'cType' in 'convention' attribute}}