-
Notifications
You must be signed in to change notification settings - Fork 10.5k
[cherry-pick 5.5] Task locals revisions #37158
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
Changes from all commits
Commits
Show all changes
14 commits
Select commit
Hold shift + click to select a range
918457e
[TaskLocals] property wrapper keys
ktoso 42dfd40
[TaskLocal] Crash on inapropriate use within task group
ktoso cb283f5
[Lit] add target-fail-simple-swift for tests which want to crash
ktoso 0b1d826
[TaskLocal] crash on illegal withValue
ktoso 11b20f6
[TaskLocals] move all tests to new style API
ktoso f3921f4
[TaskLocals] remove TaskLocalInheritance, we'll introduce when needed
ktoso 76cf8b8
[TaskLocals] prettier API thanks to default inits
ktoso b3a7c8f
[TaskLocals] set task local value in synchronous function
ktoso 1b66b51
fix typo
ktoso 5532f20
[TaskLocals] review 2: projected value wrapper
ktoso 2b358d9
[TaskLocals] Enforce @TaskLocal may only be used on static props
ktoso d25ac31
[TaskLocals] rename 'do body' to 'operation' parameter
ktoso f5b3d85
[TaskLocals] Windows: fix missing io.h
ktoso d86a618
Remove debug dumps
DougGregor File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -244,6 +244,24 @@ findSuitableWrapperInit(ASTContext &ctx, NominalTypeDecl *nominal, | |
return viableInitializers.empty() ? nullptr : viableInitializers.front(); | ||
} | ||
|
||
/// Returns true if the enclosingInstance parameter is `Never`, | ||
/// implying that there should be NO enclosing instance. | ||
static bool enclosingInstanceTypeIsNever(ASTContext &ctx, SubscriptDecl *subscript) { | ||
if (!subscript) | ||
return false; | ||
|
||
ParameterList *indices = subscript->getIndices(); | ||
assert(indices && indices->size() > 0); | ||
|
||
ParamDecl *param = indices->get(0); | ||
if (param->getArgumentName() != ctx.Id_enclosingInstance) | ||
return false; | ||
|
||
auto paramTy = param->getType(); | ||
auto neverTy = ctx.getNeverType(); | ||
return neverTy->isEqual(paramTy); | ||
} | ||
|
||
/// Determine whether we have a suitable static subscript to which we | ||
/// can pass along the enclosing self + key-paths. | ||
static SubscriptDecl *findEnclosingSelfSubscript(ASTContext &ctx, | ||
|
@@ -385,6 +403,16 @@ PropertyWrapperTypeInfoRequest::evaluate( | |
} | ||
} | ||
|
||
result.requireNoEnclosingInstance = | ||
enclosingInstanceTypeIsNever(ctx, result.enclosingInstanceWrappedSubscript); | ||
// if (requireNoEnclosingInstance) { // && !valueVar->isStatic()) { | ||
// // this means that the property wrapper must be declared on a static property | ||
// valueVar->diagnose( | ||
// diag::property_wrapper_var_must_be_static, valueVar->getName()); | ||
// return PropertyWrapperTypeInfo(); | ||
// result | ||
// } | ||
Comment on lines
+408
to
+414
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 is this commented? 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. Thanks, will remove -- this is a leftover, it was the wrong place to do the diagnosis after all so moved elsewhere |
||
|
||
bool hasInvalidDynamicSelf = false; | ||
if (result.projectedValueVar && | ||
result.projectedValueVar->getValueInterfaceType()->hasDynamicSelfType()) { | ||
|
@@ -439,6 +467,18 @@ AttachedPropertyWrappersRequest::evaluate(Evaluator &evaluator, | |
continue; | ||
} | ||
|
||
// // If the property wrapper requested an `_enclosingInstance: Never` | ||
// // it must be declared as static. TODO: or global once we allow wrappers on top-level code | ||
// auto wrappedInfo = var->getPropertyWrapperTypeInfo(); | ||
// if (wrappedInfo.requireNoEnclosingInstance) { | ||
// if (!var->isStatic()) { | ||
// ctx.Diags.diagnose(var->getLocation(), | ||
// diag::property_wrapper_var_must_be_static, | ||
// var->getName()); | ||
// continue; | ||
// } | ||
// } | ||
|
||
// Check that the variable is part of a single-variable pattern. | ||
auto binding = var->getParentPatternBinding(); | ||
if (binding && binding->getSingleVar() != var) { | ||
|
@@ -508,7 +548,7 @@ AttachedPropertyWrappersRequest::evaluate(Evaluator &evaluator, | |
continue; | ||
} | ||
} | ||
|
||
result.push_back(mutableAttr); | ||
} | ||
|
||
|
@@ -601,6 +641,20 @@ PropertyWrapperBackingPropertyTypeRequest::evaluate( | |
wrappedValue->setInterfaceType(computeWrappedValueType(var, type)); | ||
} | ||
|
||
{ | ||
auto *nominal = type->getDesugaredType()->getAnyNominal(); | ||
if (auto wrappedInfo = nominal->getPropertyWrapperTypeInfo()) { | ||
if (wrappedInfo.requireNoEnclosingInstance && | ||
!var->isStatic()) { | ||
ctx.Diags.diagnose(var->getNameLoc(), | ||
diag::property_wrapper_var_must_be_static, | ||
var->getName(), type); | ||
// TODO: fixit insert static? | ||
return Type(); | ||
} | ||
} | ||
} | ||
|
||
return type; | ||
} | ||
|
||
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
Might be simpler to just say: "property wrapper ___ can only be applied to static properties"
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.
Hmm, I wanted to include the name of the property, you sure we'd prefer the shorter message?
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.
I think the name of the property isn't needed, because the error message will already be pointing to the var decl with the problematic wrapper on it, whether in the terminal or xcode.