Skip to content

[TypeChecker/NameLookup] SE-0463: A few fixes for SendableCompletionHandlers feature #79478

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 5 commits into from
Mar 4, 2025
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
5 changes: 5 additions & 0 deletions include/swift/AST/DiagnosticsSema.def
Original file line number Diff line number Diff line change
Expand Up @@ -1898,6 +1898,11 @@ ERROR(objc_implementation_type_mismatch,none,
"%kind0 of type %1 does not match type %2 declared by the header",
(ValueDecl *, Type, Type))

ERROR(objc_implementation_sendability_mismatch,none,
"sendability of function types in %kind0 of type %1 does not match "
"type %2 declared by the header",
(ValueDecl *, Type, Type))

ERROR(objc_implementation_required_attr_mismatch,none,
"%kind0 %select{should not|should}2 be 'required' to match %1 declared "
"by the header",
Expand Down
5 changes: 4 additions & 1 deletion include/swift/AST/TypeMatcher.h
Original file line number Diff line number Diff line change
Expand Up @@ -375,7 +375,8 @@ class TypeMatcher {
if (firstFunc->isNoEscape() != secondFunc->isNoEscape())
return mismatch(firstFunc.getPointer(), secondFunc, sugaredFirstType);

if (firstFunc->isSendable() != secondFunc->isSendable())
if (!Matcher.asDerived().allowSendableFunctionMismatch() &&
firstFunc->isSendable() != secondFunc->isSendable())
return mismatch(firstFunc.getPointer(), secondFunc, sugaredFirstType);

auto sugaredFirstFunc = sugaredFirstType->castTo<AnyFunctionType>();
Expand Down Expand Up @@ -554,6 +555,8 @@ class TypeMatcher {

bool alwaysMismatchTypeParameters() const { return false; }

bool allowSendableFunctionMismatch() const { return false; }

void pushPosition(Position pos) {}
void popPosition(Position pos) {}

Expand Down
14 changes: 14 additions & 0 deletions lib/AST/NameLookup.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -826,6 +826,20 @@ static void recordShadowedDeclsAfterSignatureMatch(
else
type = removeThrownError(decl->getInterfaceType()->getCanonicalType());

// Strip `@Sendable` annotations for declarations that come from
// or are exposed to Objective-C to make it possible to introduce
// new annotations without breaking shadowing rules.
//
// This is a narrow fix for specific backwards-incompatible cases
// we know about, it could be extended to use `isObjC()` in the
// future if we find problematic cases were a more general change
// is required.
if (decl->hasClangNode() || decl->getAttrs().hasAttribute<ObjCAttr>()) {
type =
type->stripConcurrency(/*recursive=*/true, /*dropGlobalActor=*/false)
->getCanonicalType();
}

// Record this declaration based on its signature.
auto &known = collisions[type];
if (known.size() == 1) {
Expand Down
19 changes: 19 additions & 0 deletions lib/Sema/AssociatedTypeInference.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2269,6 +2269,25 @@ AssociatedTypeInference::getPotentialTypeWitnessesByMatchingTypes(ValueDecl *req
return true;
}

bool allowSendableFunctionMismatch() const {
// Allow mismatches on `@Sendable` only if the witness comes
// from ObjC as a very narrow fix to avoid introducing new
// ambiguities like this:
//
// protocol P {
// associatedtype T
// func test(_: (T) -> Void)
// }
//
// struct S : P {
// func test(_: @Sendable (Int) -> Void) {}
// func test(_: (Int) -> Void) {}
// }
//
// Currently, there is only one binding for `T` - `Int`.
return Inferred.Witness && Inferred.Witness->hasClangNode();
}

bool mismatch(GenericTypeParamType *selfParamType,
TypeBase *secondType, Type sugaredFirstType) {
if (selfParamType->isEqual(Conformance->getProtocol()->getSelfInterfaceType())) {
Expand Down
9 changes: 7 additions & 2 deletions lib/Sema/ConstraintSystem.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -331,9 +331,14 @@ void ConstraintSystem::recordAppliedDisjunction(
/// Retrieve a dynamic result signature for the given declaration.
static std::tuple<char, ObjCSelector, CanType>
getDynamicResultSignature(ValueDecl *decl) {
// Handle functions.
if (auto func = dyn_cast<AbstractFunctionDecl>(decl)) {
// Handle functions.
auto type = func->getMethodInterfaceType();
// Strip `@Sendable` and `any Sendable` because it should be
// possible to add them without affecting lookup results and
// shadowing. All of the declarations that are processed here
// are `@objc` and hence `@preconcurrency`.
auto type = func->getMethodInterfaceType()->stripConcurrency(
/*recursive=*/true, /*dropGlobalActor=*/false);
return std::make_tuple(func->isStatic(), func->getObjCSelector(),
type->getCanonicalType());
}
Expand Down
122 changes: 83 additions & 39 deletions lib/Sema/TypeCheckAttr.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -3873,62 +3873,106 @@ findSimilarFunction(DeclNameRef replacedFunctionName,
// Note: we might pass a constant attribute when typechecker is nullptr.
// Any modification to attr must be guarded by a null check on TC.
//
SmallVector<ValueDecl *, 4> results;
lookupReplacedDecl(replacedFunctionName, attr, base, results);
SmallVector<ValueDecl *, 4> lookupResults;
lookupReplacedDecl(replacedFunctionName, attr, base, lookupResults);

for (auto *result : results) {
SmallVector<AbstractFunctionDecl *, 4> candidates;
for (auto *result : lookupResults) {
// Protocol requirements are not replaceable.
if (isa<ProtocolDecl>(result->getDeclContext()))
continue;
// Check for static/instance mismatch.
if (result->isStatic() != base->isStatic())
continue;

auto resultTy = result->getInterfaceType();
auto replaceTy = base->getInterfaceType();
TypeMatchOptions matchMode = TypeMatchFlags::AllowABICompatible;
matchMode |= TypeMatchFlags::AllowCompatibleOpaqueTypeArchetypes;
if (resultTy->matches(replaceTy, matchMode)) {
if (forDynamicReplacement && !result->isDynamic()) {
if (Diags) {
Diags->diagnose(attr->getLocation(),
diag::dynamic_replacement_function_not_dynamic,
result->getName());
attr->setInvalid();
}
return nullptr;
if (auto *AFD = dyn_cast<AbstractFunctionDecl>(result))
candidates.push_back(AFD);
}

if (candidates.empty()) {
if (Diags) {
Diags->diagnose(attr->getLocation(),
forDynamicReplacement
? diag::dynamic_replacement_function_not_found
: diag::specialize_target_function_not_found,
replacedFunctionName);
}

attr->setInvalid();
return nullptr;
}

auto replaceTy = base->getInterfaceType();

// Filter based on the exact type match first.
SmallVector<AbstractFunctionDecl *> matches;
llvm::copy_if(candidates, std::back_inserter(matches),
[&replaceTy](AbstractFunctionDecl *F) {
auto resultTy = F->getInterfaceType();
TypeMatchOptions matchMode =
TypeMatchFlags::AllowABICompatible;
matchMode |=
TypeMatchFlags::AllowCompatibleOpaqueTypeArchetypes;
return resultTy->matches(replaceTy, matchMode);
});

// If there are no exact matches, strip sendability annotations
// from functions imported from Objective-C. This is a narrow
// fix for now but it could be extended to cover all `@preconcurrency`
// declarations.
if (matches.empty()) {
llvm::copy_if(candidates, std::back_inserter(matches),
[&replaceTy](AbstractFunctionDecl *F) {
if (!F->hasClangNode())
return false;

auto resultTy = F->getInterfaceType();
TypeMatchOptions matchMode =
TypeMatchFlags::AllowABICompatible;
matchMode |=
TypeMatchFlags::AllowCompatibleOpaqueTypeArchetypes;
matchMode |= TypeMatchFlags::IgnoreFunctionSendability;
matchMode |= TypeMatchFlags::IgnoreSendability;
return resultTy->matches(replaceTy, matchMode);
});
}

if (matches.size() == 1) {
auto result = matches.front();
if (forDynamicReplacement && !result->isDynamic()) {
if (Diags) {
Diags->diagnose(attr->getLocation(),
diag::dynamic_replacement_function_not_dynamic,
result->getName());
attr->setInvalid();
}
return cast<AbstractFunctionDecl>(result);
return nullptr;
}

return result;
}

attr->setInvalid();

if (!Diags)
return nullptr;

if (results.empty()) {
Diags->diagnose(attr->getLocation(),
forDynamicReplacement
? diag::dynamic_replacement_function_not_found
: diag::specialize_target_function_not_found,
replacedFunctionName);
} else {
Diags->diagnose(attr->getLocation(),
forDynamicReplacement
? diag::dynamic_replacement_function_of_type_not_found
: diag::specialize_target_function_of_type_not_found,
replacedFunctionName,
base->getInterfaceType()->getCanonicalType());
Diags->diagnose(attr->getLocation(),
forDynamicReplacement
? diag::dynamic_replacement_function_of_type_not_found
: diag::specialize_target_function_of_type_not_found,
replacedFunctionName,
base->getInterfaceType()->getCanonicalType());

for (auto *result : results) {
Diags->diagnose(SourceLoc(),
forDynamicReplacement
? diag::dynamic_replacement_found_function_of_type
: diag::specialize_found_function_of_type,
result->getName(),
result->getInterfaceType()->getCanonicalType());
}
for (auto *result : matches) {
Diags->diagnose(SourceLoc(),
forDynamicReplacement
? diag::dynamic_replacement_found_function_of_type
: diag::specialize_found_function_of_type,
result->getName(),
result->getInterfaceType()->getCanonicalType());
}
attr->setInvalid();

return nullptr;
}

Expand Down
Loading