Merge pull request #66805 from apple/egorzhdan/5.9-string-append

🍒[cxx-interop] Make `std::string::append` usable from Swift and avoid crashing for substitution failures

Author: egorzhdan

include/swift/AST/DiagnosticsSema.def

@@ -2056,6 +2056,9 @@ ERROR(unable_to_convert_generic_swift_types,none,
       "the following Swift type(s) provided to '%0' could not be "
       "converted: %1",
       (StringRef, StringRef))
+ERROR(unable_to_substitute_cxx_function_template,none,
+      "could not substitute parameters for C++ function template '%0': %1",
+      (StringRef, StringRef))
 
 // Type aliases
 ERROR(type_alias_underlying_type_access,none,

lib/ClangImporter/ClangImporter.cpp

@@ -5749,20 +5749,24 @@ clang::FunctionDecl *ClangImporter::instantiateCXXFunctionTemplate(
   std::unique_ptr<TemplateInstantiationError> error =
       ctx.getClangTemplateArguments(func->getTemplateParameters(),
                                     subst.getReplacementTypes(), templateSubst);
-  if (error) {
-    std::string failedTypesStr;
-    llvm::raw_string_ostream failedTypesStrStream(failedTypesStr);
-    llvm::interleaveComma(error->failedTypes, failedTypesStrStream);
 
+  auto getFuncName = [&]() -> std::string {
     std::string funcName;
     llvm::raw_string_ostream funcNameStream(funcName);
     func->printQualifiedName(funcNameStream);
+    return funcName;
+  };
+
+  if (error) {
+    std::string failedTypesStr;
+    llvm::raw_string_ostream failedTypesStrStream(failedTypesStr);
+    llvm::interleaveComma(error->failedTypes, failedTypesStrStream);
 
     // TODO: Use the location of the apply here.
     // TODO: This error message should not reference implementation details.
     // See: https://github.com/apple/swift/pull/33053#discussion_r477003350
     ctx.Diags.diagnose(SourceLoc(), diag::unable_to_convert_generic_swift_types,
-                       funcName, failedTypesStr);
+                       getFuncName(), failedTypesStr);
     return nullptr;
   }
 
@@ -5772,6 +5776,20 @@ clang::FunctionDecl *ClangImporter::instantiateCXXFunctionTemplate(
   auto &sema = getClangInstance().getSema();
   auto *spec = sema.InstantiateFunctionDeclaration(func, templateArgList,
                                                    clang::SourceLocation());
+  if (!spec) {
+    std::string templateParams;
+    llvm::raw_string_ostream templateParamsStream(templateParams);
+    llvm::interleaveComma(templateArgList->asArray(), templateParamsStream,
+                          [&](const clang::TemplateArgument &arg) {
+                            arg.print(func->getASTContext().getPrintingPolicy(),
+                                      templateParamsStream,
+                                      /*IncludeType*/ true);
+                          });
+    ctx.Diags.diagnose(SourceLoc(),
+                       diag::unable_to_substitute_cxx_function_template,
+                       getFuncName(), templateParams);
+    return nullptr;
+  }
   sema.InstantiateFunctionDefinition(clang::SourceLocation(), spec);
   return spec;
 }

stdlib/public/Cxx/std/String.swift

@@ -77,7 +77,12 @@ extension std.string: Equatable {
   }
 
   public static func +=(lhs: inout std.string, rhs: std.string) {
-    lhs.__appendUnsafe(rhs) // ignore the returned pointer
+    lhs.append(rhs)
+  }
+
+  @inlinable
+  public mutating func append(_ other: std.string) {
+    __appendUnsafe(other) // ignore the returned pointer
   }
 
   public static func +(lhs: std.string, rhs: std.string) -> std.string {
@@ -93,7 +98,12 @@ extension std.u16string: Equatable {
   }
 
   public static func +=(lhs: inout std.u16string, rhs: std.u16string) {
-    lhs.__appendUnsafe(rhs) // ignore the returned pointer
+    lhs.append(rhs)
+  }
+
+  @inlinable
+  public mutating func append(_ other: std.u16string) {
+    __appendUnsafe(other) // ignore the returned pointer
   }
 
   public static func +(lhs: std.u16string, rhs: std.u16string) -> std.u16string {

test/Interop/Cxx/stdlib/overlay/std-string-overlay.swift

@@ -85,6 +85,20 @@ StdStringOverlayTestSuite.test("std::u16string operators") {
   expectTrue(s1 == "something123literal")
 }
 
+StdStringOverlayTestSuite.test("std::string::append") {
+  var s1 = std.string("0123")
+  let s2 = std.string("abc")
+  s1.append(s2)
+  expectEqual(s1, std.string("0123abc"))
+}
+
+StdStringOverlayTestSuite.test("std::u16string::append") {
+  var s1 = std.u16string("0123")
+  let s2 = std.u16string("abc")
+  s1.append(s2)
+  expectEqual(s1, std.u16string("0123abc"))
+}
+
 StdStringOverlayTestSuite.test("std::string as Hashable") {
   let s0 = std.string()
   let h0 = s0.hashValue

test/Interop/Cxx/templates/Inputs/enable-if.h

@@ -0,0 +1,29 @@
+#ifndef TEST_INTEROP_CXX_TEMPLATES_INPUTS_ENABLE_IF_H
+#define TEST_INTEROP_CXX_TEMPLATES_INPUTS_ENABLE_IF_H
+
+template <bool B, class T = void>
+struct enable_if {};
+
+template <class T>
+struct enable_if<true, T> {
+  typedef T type;
+};
+
+template <class T>
+struct is_bool {
+  static const bool value = false;
+};
+
+template <>
+struct is_bool<bool> {
+  static const bool value = true;
+};
+
+struct HasMethodWithEnableIf {
+  template <typename T>
+  typename enable_if<is_bool<T>::value, bool>::type onlyEnabledForBool(T t) const {
+    return !t;
+  }
+};
+
+#endif // TEST_INTEROP_CXX_TEMPLATES_INPUTS_ENABLE_IF_H

test/Interop/Cxx/templates/Inputs/module.modulemap

@@ -133,6 +133,11 @@ module TemplateTypeParameterNotInSignature {
   requires cplusplus
 }
 
+module EnableIf {
+  header "enable-if.h"
+  requires cplusplus
+}
+
 module DefineReferencedInline {
   header "define-referenced-inline.h"
   requires cplusplus

test/Interop/Cxx/templates/enable-if-typechecker.swift

@@ -0,0 +1,9 @@
+// RUN: not %target-swift-emit-ir %s -I %S/Inputs -enable-experimental-cxx-interop -enable-objc-interop 2>&1 | %FileCheck %s
+// REQUIRES: objc_interop
+
+import StdlibUnittest
+import EnableIf
+
+let x = HasMethodWithEnableIf()
+x.onlyEnabledForBool("a")
+// CHECK: error: could not substitute parameters for C++ function template 'HasMethodWithEnableIf::onlyEnabledForBool': NSString *