[move-function] Address feedback from previous test commits and add even more tests! #3991
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
…lf.assertXXX methods. Thanks to @kastiglione for the suggestions on the more idiomatic method usage!
I forgot to include this in the previous test commit I updated. This is a
particularly important one since it shows given the following code:
```
let k = Klass()
if alwaysFalse() {
let _ = _move(k)
}
// <== (A)
```
that at point A, the debugger views k as being uninitialized. This is the
correct behavior since:
1. The lifetime of k needs to be defined by static PC regions in the binary. We
can not model the notion that k's availability is control dependent on the
conditional alwaysFalse unless we cloned code. So fundamentally, we have to be
conservative and say that since (A) is reachable from the _move that the value
isn't available.
2. move semantics means that k can not be used at any point reachable from the
_move. Thus we know that k can not have any uses after the conditional. Thus it
is reasonable that k would not necessarily be available in the debugger at that
point since it is guaranteed to not have any later uses in the function.
Thus we are forced by 1 into this solution with the solice of 2 that it does fit
the model.
I already had tests for these in Swift itself that check at the IR level things look correct. This just provides a true end<->end test that these continue to work as expected.
|
@swift-ci test |
kastiglione
approved these changes
Feb 23, 2022
|
nice 👍 |
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.
This began as a commit to address @kastiglione's feedback on #3986. After I did that I decided to add even more tests using the same PR. I basically just ported some tests that I already had in Swift that FileChecked LLVM-IR to be LLDB tests of the same sort I have here: walk through programs validating that a value shows up as appropriate when moved.