[NFC] [Parse/SILGen] Switch statement silgen cleanup #19039
Conversation
…ngle callers/callees and simplifying.
…atements correctly bind to the current pattern instead of always to the first pattern. Thus the hacky var decl juggling in where clauses in SILGen can be deleted.
|
@swift-ci Please smoke test. |
|
@swift-ci Please test source compatibility |
|
('Debug' build has a known failure right now, its not your fault) |
| @@ -21,7 +21,7 @@ func test3(arg: Int?) { | |||
| case let .some(x) where x == 0: | |||
| print(x) | |||
| case let .some(xRenamed) where xRenamed == 1, | |||
| let .some(x) where xRenamed == 2: // FIXME: This 'x' in '.some(x)' isn't properly renamed in 'casebind_2' case. | |||
| let .some(x) where x == 2: // FIXME: This 'x' in '.some(x)' isn't properly renamed in 'casebind_2' case. | |||
There was a problem hiding this comment.
Can you look into what's going on here?
There was a problem hiding this comment.
Sure, can do! But this is a pre-existing refactoring bug. The user would expect all the xs to become xRenamed but the .some(x) in the second pattern is a different decl and so it doesn't change. This PR makes it so that the x in the second where clause is the same decl as that second .some(x), so it doesn't change either.
So more 'correct', but still not the user expectation.
There was a problem hiding this comment.
Oh ok, makes sense then. I was just wondering why the behavior changes.
|
I know @gottesmm is working in this area, so you may want to hold off until he's pushed SIL ownership all the way through SILGenPattern. |
|
@gottesmm I don't think that I touched anything near the RAII or forwarding/unforwarding ownership stuff here, but if you'd prefer, I'm fine to hold off merging. |
|
IMO these cleanups still look valuable. |
|
Ah, yeah, I didn't mean that they weren't valuable, just that there might be a merge conflict that'd be easier to resolve in the other direction. But I just remembered that Michael's on vacation, so his WIP shouldn't block this PR. |
|
Yeah, I think you should go ahead and merge this, @gregomni. |
General cleanup, mostly via combining functions with single callees into their callers and simplifying where that can result in cleaner code.
And then fix name lookup of parsing where guards in switch cases so that the where expression references the correct bound pattern variables instead of always those from the first pattern. Which gets rid of the managed value juggling in SILGen for those expressions.