Diagnose and forbid invalid Swift names on inits

Initializers should always have Swift names that have the special `DeclBaseName::createConstructor()` base name. Although there is an assertion to this effect in the constructor for ConstructorDecl, ClangImporter did not actually reject custom Swift names for initializers that violated this rule. This meant that asserts compilers would crash if they encountered code with an invalid `swift_name` attribute, while release compilers would silently accept them (while excluding these decls from certain checks since lookups that were supposed to find all initializers didn’t find them).

Modify ClangImporter to diagnose this condition and ignore the custom Swift name.

Author: beccadax

CHANGELOG.md

@@ -5,6 +5,11 @@
 
 ## Swift 6.1
 
+* Previous versions of Swift would incorrectly allow Objective-C `-init...`
+  methods with custom Swift names to be imported as initializers, but with base 
+  names other than `init`. The compiler now diagnoses these attributes and
+  infers a name for the initializer as though they are not present.
+
 * Projected value initializers are now correctly injected into calls when
   an argument exactly matches a parameter with an external property wrapper.
 

include/swift/AST/DiagnosticsClangImporter.def

@@ -54,6 +54,10 @@ ERROR(dump_pcm_error,Fatal,
 WARNING(invalid_swift_name_method,none,
   "too %select{few|many}0 parameters in swift_name attribute (expected %1; "
   "got %2)", (bool, unsigned, unsigned))
+WARNING(invalid_swift_name_for_decl,none,
+  "custom Swift name '%0' ignored because it is not valid for %kindonly1; "
+  "imported as %1 instead",
+  (StringRef, ValueDecl *))
 
 NOTE(note_while_importing, none, "while importing '%0'", (StringRef))
 ERROR(swift_name_protocol_static, none, "swift_name cannot be used to define "

lib/ClangImporter/ImportDecl.cpp

@@ -7281,6 +7281,14 @@ ConstructorDecl *SwiftDeclConverter::importConstructor(
   // If this constructor overrides another constructor, mark it as such.
   recordObjCOverride(result);
 
+  // If we ignored a custom Swift name because it wasn't suitable for an init,
+  // diagnose that now.
+  if (importedName.hasInvalidCustomName() && isActiveSwiftVersion()) {
+    if (auto customName = NameImporter::findCustomName(objcMethod, version)) {
+      result->diagnose(diag::invalid_swift_name_for_decl, *customName, result);
+    }
+  }
+
   return result;
 }
 

lib/ClangImporter/ImportName.cpp

@@ -1501,6 +1501,15 @@ static StringRef renameUnsafeMethod(ASTContext &ctx,
   return name;
 }
 
+std::optional<StringRef>
+NameImporter::findCustomName(const clang::Decl *decl,
+                             ImportNameVersion version) {
+  if (auto nameAttr = findSwiftNameAttr(decl, version)) {
+    return nameAttr->name;
+  }
+  return std::nullopt;
+}
+
 ImportedName NameImporter::importNameImpl(const clang::NamedDecl *D,
                                           ImportNameVersion version,
                                           clang::DeclarationName givenName) {
@@ -1676,6 +1685,11 @@ ImportedName NameImporter::importNameImpl(const clang::NamedDecl *D,
           // Note that this is an initializer.
           isInitializer = true;
         }
+      } else if (shouldImportAsInitializer(method, version, initPrefixLength)) {
+        // This is an initializer, but its custom name is ill-formed. Ignore
+        // the swift_name attribute.
+        skipCustomName = true;
+        result.info.hasInvalidCustomName = true;
       }
     }
 

lib/ClangImporter/ImportName.h

@@ -230,12 +230,16 @@ class ImportedName {
 
     unsigned hasAsyncAlternateInfo: 1;
 
+    /// Whether this declaration had a custom name that was ignored because it
+    /// was invalid.
+    unsigned hasInvalidCustomName : 1;
+
     Info()
         : errorInfo(), selfIndex(), initKind(CtorInitializerKind::Designated),
           accessorKind(ImportedAccessorKind::None), hasCustomName(false),
           droppedVariadic(false), importAsMember(false), hasSelfIndex(false),
           hasErrorInfo(false), hasAsyncInfo(false),
-          hasAsyncAlternateInfo(false) {}
+          hasAsyncAlternateInfo(false), hasInvalidCustomName(false) {}
   } info;
 
 public:
@@ -313,6 +317,10 @@ class ImportedName {
   bool hasCustomName() const { return info.hasCustomName; }
   void setHasCustomName() { info.hasCustomName = true; }
 
+  /// Whether this declaration had a custom name that was ignored because it
+  /// was invalid.
+  bool hasInvalidCustomName() const { return info.hasInvalidCustomName; }
+
   /// Whether this was one of a special class of Objective-C
   /// initializers for which we drop the variadic argument rather
   /// than refuse to import the initializer.
@@ -489,6 +497,10 @@ class NameImporter {
     importSymbolicCXXDecls = isEnabled;
   }
 
+  /// Retrieve a purported custom name even if it is invalid.
+  static std::optional<StringRef>
+  findCustomName(const clang::Decl *decl, ImportNameVersion version);
+
 private:
   bool enableObjCInterop() const { return swiftCtx.LangOpts.EnableObjCInterop; }
 

test/ClangImporter/Inputs/custom-modules/SwiftName.h

@@ -43,10 +43,11 @@ enum {
 #if __OBJC__
 @interface Foo
 - (instancetype)init;
+- (instancetype)initWithFoo SWIFT_NAME(initWithFoo()); // expected-warning {{custom Swift name 'initWithFoo()' ignored because it is not valid for initializer; imported as 'init(foo:)' instead}}
 @end
 
-void acceptsClosure(id value, void (*fn)(void)) SWIFT_NAME(Foo.accepts(self:closure:));
-void acceptsClosureStatic(void (*fn)(void)) SWIFT_NAME(Foo.accepts(closure:));
+void acceptsClosure(id value, void (*fn)(void)) SWIFT_NAME(Foo.accepts(self:closure:)); // expected-note * {{'acceptsClosure' was obsoleted in Swift 3}}
+void acceptsClosureStatic(void (*fn)(void)) SWIFT_NAME(Foo.accepts(closure:)); // expected-note * {{'acceptsClosureStatic' was obsoleted in Swift 3}}
 
 enum {
   // Note that there was specifically a crash when renaming onto an ObjC class,

test/ClangImporter/attr-swift_name_renaming-objc.swift

@@ -1,4 +1,4 @@
-// RUN: %target-swift-frontend(mock-sdk: %clang-importer-sdk) -enable-objc-interop -I %S/Inputs/custom-modules -Xcc -w -typecheck -verify %s
+// RUN: %target-swift-frontend(mock-sdk: %clang-importer-sdk) -enable-objc-interop -I %S/Inputs/custom-modules -Xcc -w -typecheck -verify %s -verify-additional-file %S/Inputs/custom-modules/SwiftName.h
 
 import SwiftName
 
@@ -22,4 +22,7 @@ func test() {
 
   _ = AnonymousEnumConstantObjC // expected-error {{'AnonymousEnumConstantObjC' has been renamed to 'Foo.anonymousEnumConstant'}}
   _ = Foo.anonymousEnumConstant // okay
+
+  _ = Foo.initWithFoo() // expected-error {{type 'Foo' has no member 'initWithFoo'}}
+  _ = Foo.init(foo: ())
 }