[move-only] Refactor CanonicalizeOSSA so we can emit nicer non-consuming use notes and increase quality of errors. #63429
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
…ime into an API that computes liveness and a second API that rewrites copies/destroys and fix up MoveOnly checkers to use it. For those who are unaware, CanonicalizeOSSALifetime::canonicalizeValueLifetime() is really a high level driver routine for the functionality of CanonicalizeOSSALifetime that computes liveness and then rewrites copies using boundary information. This change introduces splits the implementation of canonicalizeValueLifetime into two parts: a first part called computeLiveness and a second part called rewriteLifetimes. Internally canonicalizeValueLifetime still just calls these two methods. The reason why I am doing this is that it lets the move only object checker use the raw liveness information computed before the rewriting mucks with the analysis information. This information is used by the checker to compute the raw liveness boundary of a value and use that information to determine the list of consuming uses not on the boundary, consuming uses on the boundary, and non-consuming uses on the boundary. This is then used by later parts of the checker to emit our errors. Some additional benefits of doing this are: 1. I was able to eliminate callbacks in the rewriting stage of CanonicalOSSALifetimes which previously gave the checker this information. 2. Previously the move checker did not have access to the non-consuming boundary uses causing us to always fail appropriately, but sadly not emit a note showing the non-consuming use. I am going to wire this up in a subsequent commit. The other change to the implementation of the move checker that this caused is that I needed to add an extra diagnostic check for instructions that consume the value twice or consume the value and use the value. The reason why this must be done is that liveness does not distinguish in between different operands on the same instruction meaning such an error would be lost.
…or objects and emit more precise errors for consuming use errors. Specifically, previously if we emitted an error we just dumped all of the consuming uses. Now instead for each consuming use that needs a copy, we perform a search for a specific boundary use (consuming or non-consuming) that is reachable from the former and emit a specialized error for it. Thus we emit for the two consuming case the normal consumed twice error, and now for non-consuming errors we emit the "use after consume" error.
|
@swift-ci smoke test |
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.
Specifically: