[Async Refactoring] Better handle non-refutable patterns #37720
Conversation
hamishknight
changed the title
hamishknight
force-pushed
the
all-the-patter
branch
2 times, most recently
from
d994943 to
8a21fbb
Compare
|
Updated to unify the code paths for single-var patterns and multi-var patterns a little (though a lot of logic is only really interested in single-var patterns), and also changed the behavior to make sure we always print I've left the handling of mixed let and var bindings in a single pattern for a future PR though. |
| // OUT-OF-LINE-CASE-NEXT: print(str1, str2, strs) | ||
|
|
||
| // RUN: %refactor -convert-call-to-async-alternative -dump-text -source-filename %s -pos=%(line+1):3 | %FileCheck -check-prefix=FALLBACK %s | ||
| stringTupleParam { strs, err in |
There was a problem hiding this comment.
Seems weird to fallback here since the guard makes it pretty clear the rest is a success... Is it just because we see the err == nil in an if with another unhandled condition?
There was a problem hiding this comment.
Yeah that's exactly why, and I definitely agree we could do better here. I'll see if I can handle it along with cases like rdar://78564388. For now, I'll add a FIXME and add another fallback test case that's a little more reasonable :)
| // MIXED-TUPLE-RESULT-NEXT: } | ||
|
|
||
| // RUN: %refactor-check-compiles -convert-call-to-async-alternative -dump-text -source-filename %s -pos=%(line+1):3 | %FileCheck -check-prefix=MIXED-TUPLE-RESULT2 %s | ||
| mixedTupleResult { res in |
There was a problem hiding this comment.
There's probably a few tests in here that are a little superfluous given others but... eh.
At some point I'd like to update the %refactor* tools to be similar to the swift-ide-test batch completion ones given the number we're adding 😆.
There was a problem hiding this comment.
Ah, I think what happened here is I ended up adding the MIXED-TUPLE-RESULT test case after this one and then didn't notice the overlap. I'll tweak this one so it's at least testing something slightly different.
At some point I'd like to update the %refactor* tools to be similar to the swift-ide-test batch completion ones given the number we're adding 😆.
That'd be awesome!
hamishknight
force-pushed
the
all-the-patter
branch
from
8a21fbb to
eb80f70
Compare
|
TIL apparently you can do @swift-ci please SourceKit stress test |
|
Hah, that uncovered another ASTWalker bug |
Woo! (sort of :P) |
I don't believe this case currently can come up, but leave it explicitly unhandled for now so we don't perform an erroneous transform if it ever comes up in the future.
These aren't currently supported.
Previously we would silently add these to the success block. For now, let's leave them unhandled.
Add the necessary walking hooks, and fix ReferenceCollector to use it.
Use a DenseMap to track the number of references to a given decl in a scope.
Allow it to iterate over an arbitrary container type.
Keep track of patterns that bind multiple vars and print them out when converting an async call. If the parameter being bound isn't referenced elsewhere, we'll print the pattern inline as e.g: ``` let ((x, y), z) = await foo() ``` Otherwise, if the parameter is referenced elsewhere in the block we'll print the pattern out of line, such as: ``` let (res, z) = await foo() let (x, y) = res ``` In addition, make sure to print var bindings out of line if there's also a let binding, e.g: ``` let x = await foo() var y = x ``` This ensures any mutations to y doesn't affect x. If there's only a single var binding, we'll print it inline. rdar://77802560
If we're in a case stmt body, any DeclRefExprs to vars bound by a pattern will actually be to an implicit var decl that's declared for the body. We therefore need to walk to its "parent" to get to the var referenced by the pattern, which will have the correct entries in the naming and placeholder maps.
hamishknight
force-pushed
the
all-the-patter
branch
from
eb80f70 to
2dac5b2
Compare
|
@swift-ci please test |
|
@swift-ci please SourceKit stress test |
|
Stress tester failure unrelated |
Keep track of patterns that bind multiple vars and print them out when converting an async call. If the parameter being bound isn't referenced elsewhere, we'll print the pattern inline as e.g:
Otherwise, if the parameter is referenced elsewhere in the block we'll print the pattern out of line, such as:
In addition, make sure to print var bindings out of line if there's also a let binding, e.g:
This ensures any mutations to
ydoesn't affectx. If there's only a single var binding, we'll print it inline.Finally this PR ensures that refutable patterns and unclassified switch cases are left unhandled rather than being silently added to the success block.
rdar://77802560