Merge pull request #85409 from hamishknight/fishers-landing

[ClangImporter] Check for builtin conformance in `importNumericLiteral`

Author: hamishknight

lib/AST/ASTContext.cpp

@@ -525,7 +525,9 @@ struct ASTContext::Implementation {
 
   /// The builtin initializer witness for a literal. Used when building
   /// LiteralExprs in fully-checked AST.
-  llvm::DenseMap<const NominalTypeDecl *, ConcreteDeclRef> BuiltinInitWitness;
+  llvm::DenseMap<std::pair<const NominalTypeDecl *, KnownProtocolKind>,
+                 ConcreteDeclRef>
+      BuiltinInitWitness;
 
   /// Mapping from the function decl to its original body's source range. This
   /// is populated if the body is reparsed from other source buffers.
@@ -1686,27 +1688,26 @@ ASTContext::getStringBuiltinInitDecl(NominalTypeDecl *stringDecl) const {
   return getBuiltinInitDecl(stringDecl, builtinProtocolKind, fn);
 }
 
-ConcreteDeclRef
-ASTContext::getBuiltinInitDecl(NominalTypeDecl *decl,
-                               KnownProtocolKind builtinProtocolKind,
-               llvm::function_ref<DeclName (ASTContext &ctx)> initName) const {
-  auto &witness = getImpl().BuiltinInitWitness[decl];
+ConcreteDeclRef ASTContext::getBuiltinInitDecl(
+    NominalTypeDecl *decl, KnownProtocolKind builtinProtocolKind,
+    llvm::function_ref<DeclName(ASTContext &ctx)> initName) const {
+  // Note the initializer name is expected to be unique for each protocol kind
+  // so we don't need it to be part of the key.
+  auto &witness = getImpl().BuiltinInitWitness[{decl, builtinProtocolKind}];
   if (witness)
     return witness;
 
   auto type = decl->getDeclaredInterfaceType();
   auto builtinProtocol = getProtocol(builtinProtocolKind);
   auto builtinConformance = lookupConformance(type, builtinProtocol);
   if (builtinConformance.isInvalid()) {
-    assert(false && "Missing required conformance");
     witness = ConcreteDeclRef();
     return witness;
   }
 
   auto *ctx = const_cast<ASTContext *>(this);
   witness = builtinConformance.getWitnessByName(initName(*ctx));
   if (!witness) {
-    assert(false && "Missing required witness");
     witness = ConcreteDeclRef();
     return witness;
   }

lib/ClangImporter/ImportMacro.cpp

@@ -125,6 +125,11 @@ static ValueDecl *importNumericLiteral(ClangImporter::Implementation &Impl,
         return nullptr;
     }
 
+    auto &ctx = DC->getASTContext();
+    auto *constantTyNominal = constantType->getAnyNominal();
+    if (!constantTyNominal)
+      return nullptr;
+
     if (auto *integer = dyn_cast<clang::IntegerLiteral>(parsed)) {
       // Determine the value.
       llvm::APSInt value{integer->getValue(), clangTy->isUnsignedIntegerType()};
@@ -140,6 +145,16 @@ static ValueDecl *importNumericLiteral(ClangImporter::Implementation &Impl,
         }
       }
 
+      // Make sure the destination type actually conforms to the builtin literal
+      // protocol before attempting to import, otherwise we'll crash since
+      // `createConstant` expects it to.
+      // FIXME: We ought to be careful checking conformance here since it can
+      // result in cycles. Additionally we ought to consider checking for the
+      // non-builtin literal protocol to allow any ExpressibleByIntegerLiteral
+      // type to be supported.
+      if (!ctx.getIntBuiltinInitDecl(constantTyNominal))
+        return nullptr;
+
       return createMacroConstant(Impl, MI, name, DC, constantType,
                                  clang::APValue(value),
                                  ConstantConvertKind::None,
@@ -158,6 +173,16 @@ static ValueDecl *importNumericLiteral(ClangImporter::Implementation &Impl,
         value.changeSign();
       }
 
+      // Make sure the destination type actually conforms to the builtin literal
+      // protocol before attempting to import, otherwise we'll crash since
+      // `createConstant` expects it to.
+      // FIXME: We ought to be careful checking conformance here since it can
+      // result in cycles. Additionally we ought to consider checking for the
+      // non-builtin literal protocol to allow any ExpressibleByFloatLiteral
+      // type to be supported.
+      if (!ctx.getFloatBuiltinInitDecl(constantTyNominal))
+        return nullptr;
+
       return createMacroConstant(Impl, MI, name, DC, constantType,
                                  clang::APValue(value),
                                  ConstantConvertKind::None,

test/ClangImporter/rdar156524292.swift

@@ -0,0 +1,28 @@
+// RUN: %empty-directory(%t)
+// RUN: split-file %s %t
+// RUN: %target-swift-frontend -typecheck -verify %t/main.swift -I %t -verify-additional-file %t/cmodule.h
+
+// REQUIRES: objc_interop
+// REQUIRES: OS=macosx
+
+//--- cmodule.h
+#import <CoreGraphics/CoreGraphics.h>
+#define intLiteralCGFloat ((CGFloat)0)
+// expected-note@-1 {{invalid numeric literal}}
+// expected-note@-2 {{macro 'intLiteralCGFloat' unavailable (cannot import)}}
+#define floatLiteralCGFloat ((CGFloat)0.0)
+// expected-note@-1 {{invalid numeric literal}}
+// expected-note@-2 {{macro 'floatLiteralCGFloat' unavailable (cannot import)}}
+
+//--- module.modulemap
+module CModule [system] {
+  header "cmodule.h"
+  export *
+}
+
+//--- main.swift
+import CModule
+
+// Make sure we don't crash when attempting to import these.
+_ = intLiteralCGFloat // expected-error {{cannot find 'intLiteralCGFloat' in scope}}
+_ = floatLiteralCGFloat // expected-error {{cannot find 'floatLiteralCGFloat' in scope}}