Fix regression with lazy properties and add another test case [4.0] #10911
Conversation
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
|
@swift-ci Please test |
|
This is failing in ASan unfortunately. I'll have a corrected version soon. |
…ymous closure parameters Even if we don't apply the solution, we still end up writing types into ParamDecls of closures contained the expression. Make sure this is idempotent by disabling 'cleanup', which avoids setting them to ErrorTypes, and teaching ExprCleaner to clear out types of VarDecls. This is a hack that will get better once the constraint system type map stuff is further along. Fixes <rdar://problem/33219081>.
Due to a serendipitous confluence of interesting behaviors, Swift 3 allowed you to define a lazy property with an IUO type, and setting the property to nil would 'reset' it, re-evaluating the lazy getter the next time the property was accessed. This was not intended behavior and it no longer works, so add a test that it no longer works. Fixes <rdar://problem/32687168> and <https://bugs.swift.org/browse/SR-5172>.
slavapestov
force-pushed
the
raii-scrubbers-are-bad-and-should-die-4.0
branch
from
9365ada to
a12b3f8
Compare
|
@swift-ci Please test |
1 similar comment
|
@swift-ci Please test |
|
Build failed |
There are two "RAII cleaners" here: - CleanupIllFormedExpressionRAII cleans up the Expr in its final state - ExprCleanser walks the Expr before it is mutated and collects sub-expressions, then cleans those up after The subtle difference comes into play if we started to apply the solution (which can fail, leaving the AST in an inconsistent state) or if preCheckExpression() modified the AST. The latter case was causing an ASan failure because we were not cleaning up type variables in new nodes introduced by preCheckExpression(). Fix this by moving the preCheckExpression() call out of solveForExpression(), so that if solveForExpression() is called with TypeCheckExprFlags::SkipApplyingSolution, we don't mutate the AST at all. Sigh... Fixes <rdar://problem/33277279>.
slavapestov
force-pushed
the
raii-scrubbers-are-bad-and-should-die-4.0
branch
from
a12b3f8 to
a0d7055
Compare
|
@swift-ci Please test |
1 similar comment
|
@swift-ci Please test |
|
Build failed |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Description: When type checking an expression without applying the solution, we would set the types of anonymous closure parameters to ErrorType. This resulted in SILGen crashes because of un-typechecked AST when a lazy property was initialized by an expression containing a closure with anonymous parameters.
Origination: Regression from another set of fixes to 'lazy' in 4.0.
Tested: New test added. Also add another test for an odd behavior with 'lazy' that worked in 3.1 but we don't want to allow anymore. This second part is not really necessary for 4.0 branch, but it's just a new test.
Reviewed by: Pavel Yaskevich (first two patches), Mark Lacey (ASan fix)
Radar: rdar://problem/33219081