Skip to content

[cxx-interop] Builtin functions should ignore return type nullability… #74494

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 3 commits into from
Jun 26, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
26 changes: 26 additions & 0 deletions lib/ClangImporter/ImportType.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2223,6 +2223,32 @@ ImportedType ClangImporter::Implementation::importFunctionReturnType(
if (auto elaborated =
dyn_cast<clang::ElaboratedType>(returnType))
returnType = elaborated->desugar();
// In C interop mode, the return type of library builtin functions
// like 'memcpy' from headers like 'string.h' drops
// any nullability specifiers from their return type, and preserves it on the
// declared return type. Thus, in C mode the imported return type of such
// functions is always optional. However, in C++ interop mode, the return type
// of builtin functions can preseve the nullability specifiers on their return
// type, and thus the imported return type of such functions can be
// non-optional, if the type is annotated with
// `_Nonnull`. The difference between these two modes can break cross-module
// Swift serialization, as Swift will no longer be able to resolve an x-ref
// such as 'memcpy' from a Swift module that uses C interop, within a Swift
// context that uses C++ interop. In order to avoid the x-ref resolution
// failure, normalize the return type's nullability for builtin functions in
// C++ interop mode, to match the imported type in C interop mode.
auto builtinContext = clang::Builtin::Context();
if (SwiftContext.LangOpts.EnableCXXInterop && clangDecl->getBuiltinID() &&
!builtinContext.isTSBuiltin(clangDecl->getBuiltinID()) &&
builtinContext.isPredefinedLibFunction(
clangDecl->getBuiltinID()) &&
builtinContext.getHeaderName(clangDecl->getBuiltinID()) ==
StringRef("string.h")) {
if (const auto ART = dyn_cast<clang::AttributedType>(returnType)) {
if (ART->getImmediateNullability())
clang::AttributedType::stripOuterNullability(returnType);
}
}

// Specialized templates need to match the args/result exactly (i.e.,
// ptr -> ptr not ptr -> Optional<ptr>).
Expand Down
35 changes: 35 additions & 0 deletions test/Interop/Cxx/function/Inputs/custom-string-builtins.h
Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
#pragma once

#include <stddef.h>

#define __attribute_pure__ __attribute__((__pure__))

#ifdef __cplusplus
extern "C" {
#endif

#ifdef __cplusplus
#define TEST_CONST_RETURN const
#else
#define TEST_CONST_RETURN
#endif


void* _Nonnull memcpy(void* _Nonnull, const void* _Nonnull, size_t);

void* _Nonnull memcpy42(void* _Nonnull, const void* _Nonnull, size_t);

void TEST_CONST_RETURN* _Nullable memchr(const void* _Nonnull __s, int __ch, size_t __n) __attribute_pure__;

void* _Nonnull memmove(void* _Nonnull __dst, const void* _Nonnull __src, size_t __n);

void* _Nonnull memset(void* _Nonnull __dst, int __ch, size_t __n);

char TEST_CONST_RETURN* strrchr(const char* __s, int __ch) __attribute_pure__;

char* _Nonnull strcpy(char* _Nonnull __dst, const char* _Nonnull __src);
char* _Nonnull strcat(char* _Nonnull __dst, const char* _Nonnull __src);

#ifdef __cplusplus
}
#endif
5 changes: 5 additions & 0 deletions test/Interop/Cxx/function/Inputs/module.modulemap
Original file line number Diff line number Diff line change
Expand Up @@ -2,3 +2,8 @@ module DefaultArguments {
header "default-arguments.h"
export *
}

module CustomStringBuiltins {
header "custom-string-builtins.h"
export *
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
// RUN: %target-swift-frontend(mock-sdk: %clang-importer-sdk-nosource) -typecheck -verify -verify-ignore-unknown -I %S/Inputs -cxx-interoperability-mode=default -Xcc -D_CRT_SECURE_NO_WARNINGS %s
// RUN: %target-swift-frontend(mock-sdk: %clang-importer-sdk-nosource) -typecheck -verify -verify-ignore-unknown -I %S/Inputs -Xcc -D_CRT_SECURE_NO_WARNINGS %s

import CustomStringBuiltins

public func testMemcpyOptionalReturn(p: UnsafeMutableRawPointer, e: UnsafeRawPointer, c: UnsafePointer<CChar>, pc: UnsafeMutablePointer<CChar>) {
// This 'memcpy' is a builtin and is always an optional, regardless of _Nonnull.
let x = CustomStringBuiltins.memcpy(p, e, 1)!

// Not a builtin, _Nonnull makes it a non-optional.
let y = CustomStringBuiltins.memcpy42(p, e, 1)! // expected-error {{cannot force unwrap value of non-optional type 'UnsafeMutableRawPointer'}}

// other builtins from 'string.h'
let _ = CustomStringBuiltins.memchr(e, 42, 1)!
let _ = CustomStringBuiltins.memmove(p, e, 42)!
let _ = CustomStringBuiltins.memset(p, 1, 42)!
let _ = CustomStringBuiltins.strrchr(c, 0)!
let _ = CustomStringBuiltins.strcpy(pc, c)!
let _ = CustomStringBuiltins.strcat(pc, c)!
}