[c-interop] Relax C name validation as warning

And allow expert users to use potentially invalid symbols by specifying
it explicitly.

Author: kateinoigakukun

include/swift/AST/Attr.h

@@ -2342,18 +2342,22 @@ class ExposeAttr : public DeclAttribute {
 /// Define the `@_extern` attribute, used to import external declarations in
 /// the specified way to interoperate with Swift.
 class ExternAttr : public DeclAttribute {
+  SourceLoc LParenLoc, RParenLoc;
+
 public:
-  ExternAttr(llvm::Optional<StringRef> ModuleName, llvm::Optional<StringRef> Name,
-             SourceLoc AtLoc, SourceRange Range, ExternKind Kind, bool Implicit)
-      : DeclAttribute(DAK_Extern, AtLoc, Range, Implicit),
-        ModuleName(ModuleName), Name(Name) {
+  ExternAttr(llvm::Optional<StringRef> ModuleName,
+             llvm::Optional<StringRef> Name, SourceLoc AtLoc,
+             SourceLoc LParenLoc, SourceLoc RParenLoc, SourceRange Range,
+             ExternKind Kind, bool Implicit)
+      : DeclAttribute(DAK_Extern, AtLoc, Range, Implicit), LParenLoc(LParenLoc),
+        RParenLoc(RParenLoc), ModuleName(ModuleName), Name(Name) {
     Bits.ExternAttr.kind = static_cast<unsigned>(Kind);
   }
 
-  ExternAttr(llvm::Optional<StringRef> ModuleName, llvm::Optional<StringRef> Name,
-             ExternKind Kind, bool Implicit)
-      : ExternAttr(ModuleName, Name, SourceLoc(), SourceRange(), Kind,
-                   Implicit) {}
+  ExternAttr(llvm::Optional<StringRef> ModuleName,
+             llvm::Optional<StringRef> Name, ExternKind Kind, bool Implicit)
+      : ExternAttr(ModuleName, Name, SourceLoc(), SourceLoc(), SourceLoc(),
+                   SourceRange(), Kind, Implicit) {}
 
   /// The module name to import the named declaration in it
   /// Used for Wasm import declaration.
@@ -2363,6 +2367,9 @@ class ExternAttr : public DeclAttribute {
   /// std::nullopt if the declaration name is not specified with @_extern(c)
   const llvm::Optional<StringRef> Name;
 
+  SourceLoc getLParenLoc() const { return LParenLoc; }
+  SourceLoc getRParenLoc() const { return RParenLoc; }
+
   /// Returns the kind of extern.
   ExternKind getExternKind() const {
     return static_cast<ExternKind>(Bits.ExternAttr.kind);

include/swift/AST/DiagnosticsSema.def

@@ -1878,8 +1878,10 @@ ERROR(extern_empty_c_name,none,
       "expected non-empty C name in @_extern attribute", ())
 ERROR(extern_only_non_other_attr,none,
       "@_extern attribute cannot be applied to an '@%0' declaration", (StringRef))
-ERROR(extern_c_invalid_name, none,
-      "@_extern attribute has invalid C name '%0'", (StringRef))
+WARNING(extern_c_maybe_invalid_name, none,
+        "C name '%0' may be invalid; explicitly specify the name in @_extern(c) to suppress this warning",
+        (StringRef))
+
 ERROR(c_func_variadic, none,
       "cannot declare variadic argument %0 in %kind1",
       (DeclName, const ValueDecl *))

lib/Parse/ParseDecl.cpp

@@ -1434,6 +1434,7 @@ Parser::parseDifferentiableAttribute(SourceLoc atLoc, SourceLoc loc) {
 bool Parser::parseExternAttribute(DeclAttributes &Attributes,
                                   bool &DiscardAttribute, StringRef AttrName,
                                   SourceLoc AtLoc, SourceLoc Loc) {
+  SourceLoc lParenLoc = Tok.getLoc(), rParenLoc;
 
   // Parse @_extern(<language>, ...)
   if (!consumeIf(tok::l_paren)) {
@@ -1512,6 +1513,7 @@ bool Parser::parseExternAttribute(DeclAttributes &Attributes,
     }
   }
 
+  rParenLoc = Tok.getLoc();
   if (!consumeIf(tok::r_paren)) {
     diagnose(Loc, diag::attr_expected_rparen, AttrName,
              DeclAttribute::isDeclModifier(DAK_Extern));
@@ -1527,9 +1529,10 @@ bool Parser::parseExternAttribute(DeclAttributes &Attributes,
   }
 
   if (!DiscardAttribute) {
-    Attributes.add(new (Context) ExternAttr(importModuleName, importName, AtLoc,
-                                            AttrRange, kind,
-                                            /*Implicit=*/false));
+    Attributes.add(new (Context)
+                       ExternAttr(importModuleName, importName, AtLoc,
+                                  lParenLoc, rParenLoc, AttrRange, kind,
+                                  /*Implicit=*/false));
   }
   return false;
 }

lib/Sema/TypeCheckAttr.cpp

@@ -2134,11 +2134,14 @@ void AttributeChecker::visitExternAttr(ExternAttr *attr) {
     StringRef cName = attr->getCName(FD);
     if (cName.empty()) {
       diagnose(attr->getLocation(), diag::extern_empty_c_name);
-    } else if (!clang::isValidAsciiIdentifier(cName)) {
-      // Conservatively ban non ASCII identifiers. The C standard allows
+    } else if (!attr->Name.has_value() && !clang::isValidAsciiIdentifier(cName)) {
+      // Warn non ASCII identifiers if it's *implicitly* specified. The C standard allows
       // Universal Character Names in identifiers, but clang doesn't provide
-      // an easy way to validate them, so we don't allow them for now.
-      diagnose(attr->getLocation(), diag::extern_c_invalid_name, cName);
+      // an easy way to validate them, so we warn identifers that is potentially
+      // invalid. If it's explicitly specified, we assume the user knows what
+      // they are doing, and don't warn.
+      diagnose(attr->getLocation(), diag::extern_c_maybe_invalid_name, cName)
+          .fixItInsert(attr->getRParenLoc(), (", \"" + cName + "\"").str());
     }
 
     // Ensure the decl has C compatible interface. Otherwise it produces diagnostics.

test/attr/attr_extern.swift

@@ -49,12 +49,16 @@ func underscoredValid()
 @_extern(c, "") // expected-error {{expected non-empty C name in @_extern attribute}}
 func emptyCName()
 
-@_extern(c, "0start_with_digit") // expected-error {{@_extern attribute has invalid C name '0start_with_digit'}}
-func digitPrefixed()
+// Allow specifying any identifier explicitly
+@_extern(c, "0start_with_digit")
+func explicitDigitPrefixed()
 
-@_extern(c) // expected-error {{@_extern attribute has invalid C name '+'}}
+@_extern(c) // expected-warning {{C name '+' may be invalid; explicitly specify the name in @_extern(c) to suppress this warning}}
 func +(a: Int, b: Bool) -> Bool
 
+@_extern(c) // expected-warning {{C name '🥸_implicitInvalid' may be invalid; explicitly specify the name in @_extern(c) to suppress this warning}}
+func 🥸_implicitInvalid()
+
 @_extern(c)
 func omitCName()
 

test/attr/attr_extern_fixit.swift

@@ -0,0 +1,5 @@
+// RUN: %target-typecheck-verify-swift -emit-fixits-path %t.remap -fixit-all -diagnostics-editor-mode
+// RUN: c-arcmt-test %t.remap | arcmt-test -verify-transformed-files %s.result
+
+@_extern(c) // expected-warning {{C name '+' may be invalid; explicitly specify the name in @_extern(c) to suppress this warning}}
+func +(a: Int, b: Bool) -> Bool

test/attr/attr_extern_fixit.swift.result

@@ -0,0 +1,5 @@
+// RUN: %target-typecheck-verify-swift -emit-fixits-path %t.remap -fixit-all -diagnostics-editor-mode
+// RUN: c-arcmt-test %t.remap | arcmt-test -verify-transformed-files %s.result
+
+@_extern(c, "+") // expected-warning {{C name '+' may be invalid; explicitly specify the name in @_extern(c) to suppress this warning}}
+func +(a: Int, b: Bool) -> Bool