Skip to content

[6.0][Sema/ClangImporter] Improvements to witness matching #72679

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
12 changes: 0 additions & 12 deletions include/swift/Sema/ConstraintLocator.h
Original file line number Diff line number Diff line change
Expand Up @@ -820,18 +820,6 @@ class LocatorPathElt::Witness final : public StoredPointerElement<ValueDecl> {
}
};

class LocatorPathElt::ProtocolRequirement final : public StoredPointerElement<ValueDecl> {
public:
ProtocolRequirement(ValueDecl *decl)
: StoredPointerElement(PathElementKind::ProtocolRequirement, decl) {}

ValueDecl *getDecl() const { return getStoredPointer(); }

static bool classof(const LocatorPathElt *elt) {
return elt->getKind() == PathElementKind::ProtocolRequirement;
}
};

class LocatorPathElt::GenericParameter final : public StoredPointerElement<GenericTypeParamType> {
public:
GenericParameter(GenericTypeParamType *type)
Expand Down
2 changes: 1 addition & 1 deletion include/swift/Sema/ConstraintLocatorPathElts.def
Original file line number Diff line number Diff line change
Expand Up @@ -161,7 +161,7 @@ SIMPLE_LOCATOR_PATH_ELT(ParentType)

/// The requirement that we're matching during protocol conformance
/// checking.
CUSTOM_LOCATOR_PATH_ELT(ProtocolRequirement)
SIMPLE_LOCATOR_PATH_ELT(ProtocolRequirement)

/// Type parameter requirements.
ABSTRACT_LOCATOR_PATH_ELT(AnyRequirement)
Expand Down
3 changes: 3 additions & 0 deletions lib/AST/Type.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -989,6 +989,9 @@ Type TypeBase::stripConcurrency(bool recurse, bool dropGlobalActor) {
existentialType->getConstraintType().getPointer())
return Type(this);

if (newConstraintType->getClassOrBoundGenericClass())
return newConstraintType;

return ExistentialType::get(newConstraintType);
}

Expand Down
8 changes: 7 additions & 1 deletion lib/ClangImporter/ImportType.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1916,7 +1916,13 @@ class GetSendableType :
auto composition =
ProtocolCompositionType::get(ctx, members, inverses, explicitAnyObject);

return { composition, true };
// If we started from a protocol or a composition we should already
// be in an existential context. Otherwise we'd have to wrap a new
// composition into an existential.
if (isa<ProtocolType>(ty) || isa<ProtocolCompositionType>(ty))
return {composition, true};

return {ExistentialType::get(composition), true};
}

/// Visitor action: Recurse into the children of this type and try to add
Expand Down
31 changes: 30 additions & 1 deletion lib/Sema/CSSimplify.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -5578,6 +5578,35 @@ bool ConstraintSystem::repairFailures(
return false;
}

if (auto *VD = getAsDecl<ValueDecl>(anchor)) {
// Matching a witness to a ObjC protocol requirement.
if (VD->isObjC() && VD->isProtocolRequirement() &&
path[0].is<LocatorPathElt::Witness>() &&
// Note that the condition below is very important,
// we need to wait until the very last moment to strip
// the concurrency annotations from the inner most type.
conversionsOrFixes.empty()) {
// Allow requirements to introduce `swift_attr` annotations
// (note that `swift_attr` in type contexts weren't supported
// before) and for witnesses to adopt them gradually by matching
// with a warning in non-strict concurrency mode.
if (!(Context.isSwiftVersionAtLeast(6) ||
Context.LangOpts.StrictConcurrencyLevel ==
StrictConcurrency::Complete)) {
auto strippedLHS = lhs->stripConcurrency(/*resursive=*/true,
/*dropGlobalActor=*/true);
auto strippedRHS = rhs->stripConcurrency(/*resursive=*/true,
/*dropGlobalActor=*/true);
auto result = matchTypes(strippedLHS, strippedRHS, matchKind,
flags | TMF_ApplyingFix, locator);
if (!result.isFailure()) {
increaseScore(SK_MissingSynthesizableConformance, locator);
return true;
}
}
}
}

auto elt = path.back();
switch (elt.getKind()) {
case ConstraintLocator::LValueConversion: {
Expand Down Expand Up @@ -8552,7 +8581,7 @@ ConstraintSystem::SolutionKind ConstraintSystem::simplifyConformsToConstraint(
auto synthesizeConformance = [&]() {
ProtocolConformanceRef synthesized(protocol);
auto witnessLoc = getConstraintLocator(
/*anchor=*/{}, LocatorPathElt::Witness(witness));
locator.getAnchor(), LocatorPathElt::Witness(witness));
SynthesizedConformances.insert({witnessLoc, synthesized});
return recordConformance(synthesized);
};
Expand Down
4 changes: 1 addition & 3 deletions lib/Sema/ConstraintLocator.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -304,9 +304,7 @@ void LocatorPathElt::dump(raw_ostream &out) const {
break;
}
case ConstraintLocator::ProtocolRequirement: {
auto reqElt = elt.castTo<LocatorPathElt::ProtocolRequirement>();
out << "protocol requirement ";
reqElt.getDecl()->dumpRef(out);
out << "protocol requirement";
break;
}
case ConstraintLocator::Witness: {
Expand Down
10 changes: 10 additions & 0 deletions lib/Sema/ConstraintSystem.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -7985,6 +7985,16 @@ void constraints::dumpAnchor(ASTNode anchor, SourceManager *SM,
out << '@';
pattern->getLoc().print(out, *SM);
}
} else if (auto *decl = anchor.dyn_cast<Decl *>()) {
if (auto *VD = dyn_cast<ValueDecl>(decl)) {
VD->dumpRef(out);
} else {
out << "<<" << Decl::getKindName(decl->getKind()) << ">>";
if (SM) {
out << "@";
decl->getLoc().print(out, *SM);
}
}
}
// TODO(diagnostics): Implement the rest of the cases.
}
Expand Down
84 changes: 31 additions & 53 deletions lib/Sema/TypeCheckProtocol.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1027,7 +1027,6 @@ swift::matchWitness(WitnessChecker::RequirementEnvironmentCache &reqEnvCache,

// Initialized by the setup operation.
std::optional<ConstraintSystem> cs;
ConstraintLocator *locator = nullptr;
ConstraintLocator *reqLocator = nullptr;
ConstraintLocator *witnessLocator = nullptr;
Type witnessType, openWitnessType;
Expand Down Expand Up @@ -1116,8 +1115,8 @@ swift::matchWitness(WitnessChecker::RequirementEnvironmentCache &reqEnvCache,
selfTy = syntheticEnv->mapTypeIntoContext(selfTy);

// Open up the type of the requirement.
reqLocator = cs->getConstraintLocator(
static_cast<Expr *>(nullptr), LocatorPathElt::ProtocolRequirement(req));
reqLocator =
cs->getConstraintLocator(req, ConstraintLocator::ProtocolRequirement);
OpenedTypeMap reqReplacements;
reqType = cs->getTypeOfMemberReference(selfTy, req, dc,
/*isDynamicResult=*/false,
Expand Down Expand Up @@ -1152,10 +1151,8 @@ swift::matchWitness(WitnessChecker::RequirementEnvironmentCache &reqEnvCache,

// Open up the witness type.
witnessType = witness->getInterfaceType();
// FIXME: witness as a base locator?
locator = cs->getConstraintLocator({});
witnessLocator = cs->getConstraintLocator({},
LocatorPathElt::Witness(witness));
witnessLocator =
cs->getConstraintLocator(req, LocatorPathElt::Witness(witness));
if (witness->getDeclContext()->isTypeContext()) {
openWitnessType = cs->getTypeOfMemberReference(
selfTy, witness, dc, /*isDynamicResult=*/false,
Expand All @@ -1175,44 +1172,8 @@ swift::matchWitness(WitnessChecker::RequirementEnvironmentCache &reqEnvCache,
// Match a type in the requirement to a type in the witness.
auto matchTypes = [&](Type reqType,
Type witnessType) -> std::optional<RequirementMatch> {
// `swift_attr` attributes in the type context were ignored before,
// which means that we need to maintain status quo to avoid breaking
// witness matching by stripping everything concurrency related from
// inner types.
auto shouldStripConcurrency = [&]() {
if (!req->isObjC())
return false;

auto &ctx = dc->getASTContext();
return !(ctx.isSwiftVersionAtLeast(6) ||
ctx.LangOpts.StrictConcurrencyLevel ==
StrictConcurrency::Complete);
};

if (shouldStripConcurrency()) {
if (reqType->is<FunctionType>()) {
auto *fnTy = reqType->castTo<FunctionType>();
SmallVector<AnyFunctionType::Param, 4> params;
llvm::transform(fnTy->getParams(), std::back_inserter(params),
[&](const AnyFunctionType::Param &param) {
return param.withType(
param.getPlainType()->stripConcurrency(
/*recursive=*/true,
/*dropGlobalActor=*/true));
});

auto resultTy =
fnTy->getResult()->stripConcurrency(/*recursive=*/true,
/*dropGlobalActor=*/true);

reqType = FunctionType::get(params, resultTy, fnTy->getExtInfo());
} else {
reqType = reqType->stripConcurrency(/*recursive=*/true,
/*dropGlobalActor=*/true);
}
}

cs->addConstraint(ConstraintKind::Bind, reqType, witnessType, locator);
cs->addConstraint(ConstraintKind::Bind, reqType, witnessType,
witnessLocator);
// FIXME: Check whether this has already failed.
return std::nullopt;
};
Expand All @@ -1236,14 +1197,31 @@ swift::matchWitness(WitnessChecker::RequirementEnvironmentCache &reqEnvCache,
return *result;
}
}
bool requiresNonSendable = false;
if (!solution || solution->Fixes.size()) {
/// If the *only* problems are that `@Sendable` attributes are missing,
/// allow the match in some circumstances.
requiresNonSendable = solution
&& llvm::all_of(solution->Fixes, [](constraints::ConstraintFix *fix) {
return fix->getKind() == constraints::FixKind::AddSendableAttribute;
});

bool requiresNonSendable = [&]() {
if (!solution)
return false;

// If the *only* problems are that `@Sendable` attributes are missing,
// allow the match in some circumstances.
if (!solution->Fixes.empty()) {
return llvm::all_of(solution->Fixes,
[](constraints::ConstraintFix *fix) {
return fix->getKind() ==
constraints::FixKind::AddSendableAttribute;
});
}

// If there are no other issues, let's check whether this are
// missing Sendable conformances when matching ObjC requirements.
// This is not an error until Swift 6 because `swift_attr` wasn't
// allowed in type contexts initially.
return req->isObjC() &&
solution->getFixedScore()
.Data[SK_MissingSynthesizableConformance] > 0;
}();

if (!solution || !solution->Fixes.empty()) {
if (!requiresNonSendable)
return RequirementMatch(witness, MatchKind::TypeConflict,
witnessType);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,7 @@ void doSomethingConcurrently(__attribute__((noescape)) void SWIFT_SENDABLE (^blo
@end

@protocol InnerSendableTypes
-(void) testComposition:(SWIFT_SENDABLE MyValue *)composition;
-(void) test:(NSDictionary<NSString *, SWIFT_SENDABLE id> *)options;
-(void) testWithCallback:(NSString *)name handler:(MAIN_ACTOR void (^)(NSDictionary<NSString *, SWIFT_SENDABLE id> *, NSError * _Nullable))handler;
@end
Expand Down Expand Up @@ -109,21 +110,26 @@ func test_sendable_attr_in_type_context(test: Test) {
}

class TestConformanceWithStripping : InnerSendableTypes {
func test(_ options: [String: Any]) { // Ok
func testComposition(_: MyValue) {
// expected-warning@-1 {{sendability of function types in instance method 'testComposition' does not match requirement in protocol 'InnerSendableTypes'; this is an error in the Swift 6 language mode}}
}

func test(_ options: [String: Any]) {
// expected-warning@-1 {{sendability of function types in instance method 'test' does not match requirement in protocol 'InnerSendableTypes'; this is an error in the Swift 6 language mode}}
}

func test(withCallback name: String, handler: @escaping @MainActor ([String : Any], (any Error)?) -> Void) { // Ok
// expected-warning@-1 {{sendability of function types in instance method 'test(withCallback:handler:)' does not match requirement in protocol 'InnerSendableTypes'; this is an error in the Swift 6 language mode}}
}
}

class TestConformanceWithoutStripping : InnerSendableTypes {
// expected-error@-1 {{type 'TestConformanceWithoutStripping' does not conform to protocol 'InnerSendableTypes'}}
func testComposition(_: any MyValue & Sendable) { // Ok
}

func test(_ options: [String: any Sendable]) {
// expected-note@-1 {{candidate has non-matching type '([String : any Sendable]) -> ()'}}
func test(_ options: [String: any Sendable]) { // Ok
}

func test(withCallback name: String, handler: @escaping @MainActor ([String : any Sendable], (any Error)?) -> Void) {
// expected-note@-1 {{candidate has non-matching type '(String, @escaping @MainActor ([String : any Sendable], (any Error)?) -> Void) -> ()'}}
func test(withCallback name: String, handler: @escaping @MainActor ([String : any Sendable], (any Error)?) -> Void) { // Ok
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,7 @@ void doSomethingConcurrently(__attribute__((noescape)) void SWIFT_SENDABLE (^blo
@end

@protocol InnerSendableTypes
-(void) testComposition:(SWIFT_SENDABLE MyValue *)composition;
-(void) test:(NSDictionary<NSString *, SWIFT_SENDABLE id> *)options;
-(void) testWithCallback:(NSString *)name handler:(MAIN_ACTOR void (^)(NSDictionary<NSString *, SWIFT_SENDABLE id> *, NSError * _Nullable))handler;
@end
Expand Down Expand Up @@ -116,6 +117,10 @@ func test_sendable_attr_in_type_context(test: Test) {
class TestConformanceWithStripping : InnerSendableTypes {
// expected-error@-1 {{type 'TestConformanceWithStripping' does not conform to protocol 'InnerSendableTypes'}}

func testComposition(_: MyValue) {
// expected-note@-1 {{candidate has non-matching type '(MyValue) -> ()'}}
}

func test(_ options: [String: Any]) {
// expected-note@-1 {{candidate has non-matching type '([String : Any]) -> ()'}}
}
Expand All @@ -126,6 +131,9 @@ class TestConformanceWithStripping : InnerSendableTypes {
}

class TestConformanceWithoutStripping : InnerSendableTypes {
func testComposition(_: MyValue & Sendable) { // Ok
}

func test(_ options: [String: any Sendable]) { // Ok
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,7 @@ void doSomethingConcurrently(__attribute__((noescape)) void SWIFT_SENDABLE (^blo
@end

@protocol InnerSendableTypes
-(void) testComposition:(SWIFT_SENDABLE MyValue *)composition;
-(void) test:(NSDictionary<NSString *, SWIFT_SENDABLE id> *)options;
-(void) testWithCallback:(NSString *)name handler:(MAIN_ACTOR void (^)(NSDictionary<NSString *, SWIFT_SENDABLE id> *, NSError * _Nullable))handler;
@end
Expand Down Expand Up @@ -115,6 +116,10 @@ func test_sendable_attr_in_type_context(test: Test) {
class TestConformanceWithStripping : InnerSendableTypes {
// expected-error@-1 {{type 'TestConformanceWithStripping' does not conform to protocol 'InnerSendableTypes'}}

func testComposition(_: MyValue) {
// expected-note@-1 {{candidate has non-matching type '(MyValue) -> ()'}}
}

func test(_ options: [String: Any]) {
// expected-note@-1 {{candidate has non-matching type '([String : Any]) -> ()'}}
}
Expand All @@ -125,6 +130,9 @@ class TestConformanceWithStripping : InnerSendableTypes {
}

class TestConformanceWithoutStripping : InnerSendableTypes {
func testComposition(_: any MyValue & Sendable) { // Ok
}

func test(_ options: [String: any Sendable]) { // Ok
}

Expand Down
7 changes: 3 additions & 4 deletions test/IDE/print_clang_objc_async.swift
Original file line number Diff line number Diff line change
Expand Up @@ -129,13 +129,12 @@ import _Concurrency
// CHECK-LABEL: class NXSender :
// CHECK: func sendAny(_ obj: any Sendable) -> any Sendable
// CHECK: func sendOptionalAny(_ obj: (any Sendable)?) -> (any Sendable)?
// FIXME(https://github.com/apple/swift/issues/65730): Compositions not wrappped in existentials
// CHECK: func sendSendable(_ sendable: SendableClass & Sendable) -> SendableClass & Sendable
// CHECK: func sendSendableSubclasses(_ sendableSubclass: NonSendableClass & Sendable) -> NonSendableClass & Sendable
// CHECK: func sendSendable(_ sendable: any SendableClass & Sendable) -> any SendableClass & Sendable
// CHECK: func sendSendableSubclasses(_ sendableSubclass: any NonSendableClass & Sendable) -> any NonSendableClass & Sendable
// CHECK: func sendProto(_ obj: any LabellyProtocol & Sendable) -> any LabellyProtocol & Sendable
// CHECK: func sendProtos(_ obj: any LabellyProtocol & ObjCClub & Sendable) -> any LabellyProtocol & ObjCClub & Sendable
// CHECK: func sendAnyArray(_ array: [any Sendable]) -> [any Sendable]
// CHECK: func sendGeneric(_ generic: GenericObject<SendableClass> & Sendable) -> GenericObject<SendableClass> & Sendable
// CHECK: func sendGeneric(_ generic: any GenericObject<SendableClass> & Sendable) -> any GenericObject<SendableClass> & Sendable
// CHECK: func sendPtr(_ val: UnsafeMutableRawPointer) -> UnsafeMutableRawPointer
// CHECK: func sendStringArray(_ obj: [String]) -> [String]
// CHECK: func sendAnyTypedef(_ obj: any Sendable) -> any Sendable
Expand Down