Merge pull request #84724 from j-hui/invalid-func-tmpl-inst

[cxx-interop] Avoid crashing upon invalid function template instantiation

Author: j-hui

lib/ClangImporter/ClangImporter.cpp

@@ -7300,10 +7300,8 @@ clang::FunctionDecl *ClangImporter::instantiateCXXFunctionTemplate(
   // Instantiate a specialization of this template using the substitution map.
   auto *templateArgList = clang::TemplateArgumentList::CreateCopy(
       func->getASTContext(), templateSubst);
-  auto &sema = getClangInstance().getSema();
-  auto *spec = sema.InstantiateFunctionDeclaration(func, templateArgList,
-                                                   clang::SourceLocation());
-  if (!spec) {
+
+  auto diagnoseSubstFail = [&]() {
     std::string templateParams;
     llvm::raw_string_ostream templateParamsStream(templateParams);
     llvm::interleaveComma(templateArgList->asArray(), templateParamsStream,
@@ -7315,9 +7313,24 @@ clang::FunctionDecl *ClangImporter::instantiateCXXFunctionTemplate(
     Impl.diagnose(HeaderLoc(func->getBeginLoc()),
                   diag::unable_to_substitute_cxx_function_template,
                   getFuncName(), templateParams);
+  };
+
+  auto &sema = getClangInstance().getSema();
+  auto *spec = sema.InstantiateFunctionDeclaration(func, templateArgList,
+                                                   clang::SourceLocation());
+  if (!spec || spec->isInvalidDecl()) {
+    diagnoseSubstFail();
     return nullptr;
   }
+
   sema.InstantiateFunctionDefinition(clang::SourceLocation(), spec);
+  // Even if the declaration can be instantiated, the definition may contain
+  // a substitution failure that renders spec invalid as a side-effect.
+  if (spec->isInvalidDecl()) {
+    diagnoseSubstFail();
+    return nullptr;
+  }
+
   return spec;
 }
 
@@ -7714,6 +7727,12 @@ ClangImporter::getCXXFunctionTemplateSpecialization(SubstitutionMap subst,
   assert(isa<clang::FunctionTemplateDecl>(decl->getClangDecl()) &&
          "This API should only be used with function templates.");
 
+  // If we hit some instantiation failure and expect the compiler to imminently
+  // terminate (with an error), return some reasonable-looking placeholder value
+  // in the meantime because callers expect this function to return some
+  // non-empty ConcreteDeclRef.
+  auto failurePlaceholder = [&]() { return ConcreteDeclRef(decl); };
+
   auto *newFn =
       decl->getASTContext()
           .getClangModuleLoader()
@@ -7722,31 +7741,28 @@ ClangImporter::getCXXFunctionTemplateSpecialization(SubstitutionMap subst,
               const_cast<clang::FunctionTemplateDecl *>(
                   cast<clang::FunctionTemplateDecl>(decl->getClangDecl())),
               subst);
-  // We failed to specialize this function template. The compiler is going to
-  // exit soon. Return something valid in the meantime.
   if (!newFn)
-    return ConcreteDeclRef(decl);
+    return failurePlaceholder();
 
   auto [fnIt, inserted] =
       Impl.specializedFunctionTemplates.try_emplace(newFn, nullptr);
   if (!inserted)
     return ConcreteDeclRef(fnIt->second);
 
-  auto newDecl = cast_or_null<ValueDecl>(
-      decl->getASTContext().getClangModuleLoader()->importDeclDirectly(
-          newFn));
+  auto *newDecl = cast_or_null<ValueDecl>(
+      decl->getASTContext().getClangModuleLoader()->importDeclDirectly(newFn));
+  if (!newDecl)
+    return failurePlaceholder();
 
-  if (auto fn = dyn_cast<AbstractFunctionDecl>(newDecl)) {
+  if (auto *fn = dyn_cast<AbstractFunctionDecl>(newDecl)) {
     if (!subst.empty()) {
       newDecl = rewriteIntegerTypes(subst, decl, fn);
     }
   }
 
-  if (auto fn = dyn_cast<FuncDecl>(decl)) {
+  if (auto *fn = dyn_cast<FuncDecl>(decl)) {
     newDecl = addThunkForDependentTypes(fn, cast<FuncDecl>(newDecl));
-  }
 
-  if (auto fn = dyn_cast<FuncDecl>(decl)) {
     if (newFn->getNumParams() != fn->getParameters()->size()) {
       newDecl = generateThunkForExtraMetatypes(subst, fn,
                                                cast<FuncDecl>(newDecl));

test/Interop/Cxx/templates/static-cast-typecheck.swift

@@ -0,0 +1,44 @@
+// RUN: split-file %s %t
+// RUN: %target-swift-frontend -typecheck -verify -suppress-remarks -suppress-notes \
+// RUN:     -cxx-interoperability-mode=default \
+// RUN:     -I %t%{fs-sep}Inputs %t%{fs-sep}main.swift \
+// RUN:     -verify-additional-file %t%{fs-sep}Inputs%{fs-sep}header.h
+
+//--- Inputs/module.modulemap
+module CxxHeader {
+    header "header.h"
+    requires cplusplus
+}
+
+//--- Inputs/header.h
+#pragma once
+
+// TODO: the following diagnostics should be moved to main.swift at the call site
+//       that triggers the instantiation
+// expected-error@+4 {{could not substitute parameters for C++ function template 'cxxCast': BaseT, UnrelatedT}}
+// expected-error@+3 {{could not substitute parameters for C++ function template 'cxxCast': BaseT, SubClassT}}
+// expected-error@+2 {{could not substitute parameters for C++ function template 'cxxCast': BaseT, SubProtectedT}}
+// expected-error@+1 {{could not substitute parameters for C++ function template 'cxxCast': BaseT, SubPrivateT}}
+template <class O, class I> O cxxCast(I i) { return static_cast<O>(i); }
+// expected-error@-1 {{cannot cast 'const SubPrivateT' to its private base class 'const BaseT'}}
+// expected-error@-2 {{cannot cast 'const SubProtectedT' to its protected base class 'const BaseT'}}
+// expected-error@-3 {{cannot cast 'const SubClassT' to its private base class 'const BaseT'}}
+// expected-error@-4 {{no matching conversion for static_cast from 'UnrelatedT' to 'BaseT'}}
+
+struct BaseT { };
+
+struct SubT          : BaseT            { }; // publicly inherit from BaseT
+struct SubPrivateT   : private BaseT    { }; // privately inherit from BaseT
+struct SubProtectedT : protected BaseT  { }; // privately inherit from BaseT
+class  SubClassT     : BaseT            { }; // privately inherit from BaseT
+struct UnrelatedT                       { }; // does not inherit from BaseT
+
+
+//--- main.swift
+import CxxHeader
+
+let _: BaseT = cxxCast(SubT())          // valid:   upcast to public base
+let _: BaseT = cxxCast(SubPrivateT())   // invalid: upcast to non-public base
+let _: BaseT = cxxCast(SubProtectedT()) // invalid: upcast to non-public base
+let _: BaseT = cxxCast(SubClassT())     // invalid: upcast to non-public base
+let _: BaseT = cxxCast(UnrelatedT())    // invalid: cast to unrelated type