Invalidate default argument exprs if typechecking fails #8059
Conversation
|
We have a bunch of disabled crashers with 'REQUIRES: deterministic-behavior' and a few with 'REQUIRES: SR-xyz'. Can you check if those pass now? |
|
LGTM, and despite the fact that my (still in-progress changes) would have fixed this I would rather surgically fix it now with your change and make sure it's fixed than wait until those changes are ultimately enabled. As for the deterministic-behavior crashers - I'd suggest enabling the ones that seem to pass in a completely separate commit so it is easy to revert that commit if anything goes wrong while still keeping the goodness here. |
|
I agree, I'll submit a separate patch that checks to see if the non-deterministic crashers actually got fixed. Now that I actually ran |
|
@swift-ci please smoke test |
CodaFi
force-pushed
the
initial-value-problem
branch
from
18907c2 to
4a15237
Compare
|
Make that 23. |
CodaFi
force-pushed
the
initial-value-problem
branch
from
4a15237 to
024a072
Compare
|
@swift-ci please smoke test |
|
Oops - looks like you made a typo when resolving 28716 |
|
Probably for the best, I have to rebase onto @practicalswift's last commit anyway. |
| @@ -6,5 +6,5 @@ | |||
| // See https://swift.org/CONTRIBUTORS.txt for the list of Swift project authors | |||
|
|
|||
| // REQUIRES: OS=linux-gnu | |||
There was a problem hiding this comment.
Also please remove this REQUIRES line
CodaFi
force-pushed
the
initial-value-problem
branch
2 times, most recently
from
641d3da to
84be050
Compare
|
Oh fixed crashers should not have a REQUIRES: asserts since they're fixed - they won't hit asserts. I had a look at the disabled crashers, all seem to be default arg related too. Looking forward to those getting resolved in the next PR :-) |
If typechecking fails, the expression will have unsolved type variables written into it. This Crashes The Compiler. In that case, there’s no reason to keep a tree of dangling references around. Detach the initializer expression from the AST, but continue to typecheck it to see if we can get some useful diagnostics out of it.
CodaFi
force-pushed
the
initial-value-problem
branch
from
84be050 to
1367b0c
Compare
|
@swift-ci please smoke test and merge |
These were leftover from the swiftlang#8059, which plugged a type variable leak caused by type checking invalid default argument expressions.
While looking through the crashers, I noticed a large portion of them involved type checking default argument expressions. Round about the time there was an effort to remove the
ExprHandletype, these crashes showed up. This little bit of code was attempting to type check default argument initializers, then would ask the expression handle to write the old expression value back into the parameter if it failed - though I'm not sure this was correct either. When this got refactored, the error path got ditched. In any case, the right behavior is not to keep a reference to a tree of dangling type variables. Instead, invalidate the initializer expression in the AST, then continue type checking it to see if we can get some useful diagnostics.This patch resolves 22 crashers.
Because this touches dangling type variables: @rudkx, what do you think?