-
Notifications
You must be signed in to change notification settings - Fork 10.5k
[region-isolation] Implement function sub typing rules #74129
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
Changes from all commits
1c9b178
3c166b5
83bcf23
ba6c8af
2ac874e
81100ad
a451fe6
16d0194
c8d9e18
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -474,6 +474,17 @@ enum class FixKind : uint8_t { | |
/// Ignore situations when key path subscript index gets passed an invalid | ||
/// type as an argument (something that is not a key path). | ||
IgnoreKeyPathSubscriptIndexMismatch, | ||
|
||
/// Ignore the following situations: | ||
/// | ||
/// 1. Where we have a function that expects a function typed parameter | ||
/// without a sendable parameter but is passed a function type with a sending | ||
/// parameter. | ||
/// | ||
/// 2. Where we have a function that expects a function typed parameter with a | ||
/// sending result, but is passed a function typeed parameter without a | ||
/// sending result. | ||
AllowSendingMismatch, | ||
}; | ||
|
||
class ConstraintFix { | ||
|
@@ -2648,6 +2659,49 @@ class TreatEphemeralAsNonEphemeral final : public AllowArgumentMismatch { | |
} | ||
}; | ||
|
||
/// Error if a user passes let f: (sending T) -> () as a (T) -> (). | ||
/// | ||
/// This prevents data races since f assumes its parameter if the parameter is | ||
/// non-Sendable is safe to transfer onto other situations. The caller though | ||
/// that this is being sent to does not enforce that invariants within its body. | ||
class AllowSendingMismatch final : public ContextualMismatch { | ||
public: | ||
enum class Kind { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I don't think this is necessary, it could be determined based on the locator associated wit the fix. |
||
Parameter, | ||
Result, | ||
}; | ||
|
||
private: | ||
Kind kind; | ||
|
||
AllowSendingMismatch(ConstraintSystem &cs, Type argType, Type paramType, | ||
ConstraintLocator *locator, Kind kind, | ||
FixBehavior fixBehavior) | ||
: ContextualMismatch(cs, FixKind::AllowSendingMismatch, argType, | ||
paramType, locator, fixBehavior), | ||
kind(kind) {} | ||
|
||
public: | ||
std::string getName() const override { | ||
return "treat a function argument with sending parameter as a function " | ||
"argument without sending parameters"; | ||
} | ||
|
||
bool diagnose(const Solution &solution, bool asNote = false) const override; | ||
|
||
bool diagnoseForAmbiguity(CommonFixesArray commonFixes) const override { | ||
return diagnose(*commonFixes.front().first); | ||
} | ||
|
||
static AllowSendingMismatch *create(ConstraintSystem &cs, | ||
ConstraintLocator *locator, Type srcType, | ||
Type dstType, Kind kind); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Nit: Based on all other |
||
|
||
static bool classof(const ConstraintFix *fix) { | ||
return fix->getKind() == FixKind::AllowSendingMismatch; | ||
} | ||
}; | ||
|
||
class SpecifyBaseTypeForContextualMember final : public ConstraintFix { | ||
DeclNameRef MemberName; | ||
|
||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -1555,6 +1555,8 @@ bool Parser::canParseType() { | |
consumeToken(); | ||
} else if (Tok.isContextualKeyword("each")) { | ||
consumeToken(); | ||
} else if (Tok.isContextualKeyword("sending")) { | ||
consumeToken(); | ||
Comment on lines
+1558
to
+1559
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Is this correct? My impression is that only parameters can be annotated as struct sending {}
let a: sending = sending()
func test(x: sending) {} There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @ahoppen I just tested this locally and it isn't an issue. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I just attached a debugger to it and understand why my example didn’t hit the issue that I was thinking about but this one does struct sending {}
func test() {
_ = { () -> sending in
}
} I think the proper solution here is to check if |
||
} | ||
|
||
switch (Tok.getKind()) { | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -8020,6 +8020,24 @@ bool NonEphemeralConversionFailure::diagnoseAsError() { | |
return true; | ||
} | ||
|
||
bool SendingOnFunctionParameterMismatchFail::diagnoseAsError() { | ||
emitDiagnosticAt(getLoc(), diag::sending_function_wrong_sending, | ||
getFromType(), getToType()) | ||
.warnUntilSwiftVersion(6); | ||
emitDiagnosticAt(getLoc(), | ||
diag::sending_function_param_with_sending_param_note); | ||
return true; | ||
} | ||
|
||
bool SendingOnFunctionResultMismatchFailure::diagnoseAsError() { | ||
emitDiagnosticAt(getLoc(), diag::sending_function_wrong_sending, | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why not just |
||
getFromType(), getToType()) | ||
.warnUntilSwiftVersion(6); | ||
emitDiagnosticAt(getLoc(), | ||
diag::sending_function_result_with_sending_param_note); | ||
return true; | ||
} | ||
|
||
bool AssignmentTypeMismatchFailure::diagnoseMissingConformance() const { | ||
auto srcType = getFromType(); | ||
auto dstType = getToType()->lookThroughAllOptionalTypes(); | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -2306,6 +2306,28 @@ class NonEphemeralConversionFailure final : public ArgumentMismatchFailure { | |
void emitSuggestionNotes() const; | ||
}; | ||
|
||
class SendingOnFunctionParameterMismatchFail final : public ContextualFailure { | ||
public: | ||
SendingOnFunctionParameterMismatchFail(const Solution &solution, Type srcType, | ||
Type dstType, | ||
ConstraintLocator *locator, | ||
FixBehavior fixBehavior) | ||
: ContextualFailure(solution, srcType, dstType, locator, fixBehavior) {} | ||
|
||
bool diagnoseAsError() override; | ||
}; | ||
|
||
class SendingOnFunctionResultMismatchFailure final : public ContextualFailure { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It looks like this split is not necessary, the diagnostic is anchored on a simplified locator so it would point to the right spot. |
||
public: | ||
SendingOnFunctionResultMismatchFailure(const Solution &solution, Type srcType, | ||
Type dstType, | ||
ConstraintLocator *locator, | ||
FixBehavior fixBehavior) | ||
: ContextualFailure(solution, srcType, dstType, locator, fixBehavior) {} | ||
|
||
bool diagnoseAsError() override; | ||
}; | ||
|
||
class AssignmentTypeMismatchFailure final : public ContextualFailure { | ||
public: | ||
AssignmentTypeMismatchFailure(const Solution &solution, | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -1884,6 +1884,34 @@ std::string TreatEphemeralAsNonEphemeral::getName() const { | |
return name; | ||
} | ||
|
||
bool AllowSendingMismatch::diagnose(const Solution &solution, | ||
bool asNote) const { | ||
switch (kind) { | ||
case Kind::Parameter: { | ||
SendingOnFunctionParameterMismatchFail failure( | ||
solution, getFromType(), getToType(), getLocator(), fixBehavior); | ||
return failure.diagnose(asNote); | ||
} | ||
case Kind::Result: { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It doesn't look like this split brings much to the table, the difference could be handled internally to the diagnostic. |
||
SendingOnFunctionResultMismatchFailure failure( | ||
solution, getFromType(), getToType(), getLocator(), fixBehavior); | ||
return failure.diagnose(asNote); | ||
} | ||
} | ||
llvm_unreachable("Covered switch isn't covered?!"); | ||
} | ||
|
||
AllowSendingMismatch *AllowSendingMismatch::create(ConstraintSystem &cs, | ||
ConstraintLocator *locator, | ||
Type srcType, Type dstType, | ||
Kind kind) { | ||
auto fixBehavior = cs.getASTContext().LangOpts.isSwiftVersionAtLeast(6) | ||
? FixBehavior::Error | ||
: FixBehavior::DowngradeToWarning; | ||
return new (cs.getAllocator()) | ||
AllowSendingMismatch(cs, srcType, dstType, locator, kind, fixBehavior); | ||
} | ||
|
||
bool SpecifyBaseTypeForContextualMember::diagnose(const Solution &solution, | ||
bool asNote) const { | ||
MissingContextualBaseInMemberRefFailure failure(solution, MemberName, | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
minor typo: