Erase integer and float variables before needs_drop calls in HIR generator analysis
#103036
Conversation
This file contains 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
rustbot
added
the
T-compiler
rust-highfive
added
the
S-waiting-on-review
compiler-errors
commented
Oct 14, 2022
| let ty = self.fcx.tcx.erase_regions(ty); | ||
| // All int/float types are trivially drop, so those infer variables can be erased | ||
| let ty = self.fcx.resolve_numeric_literals_with_default(ty); | ||
| if ty.needs_infer() { |
There was a problem hiding this comment.
The resolve_numeric_literals_with_default probably makes this a bit more accurate, too, since integer variables no longer trigger may_need_drop for no reason.
|
@lcnr and I decided this is probably not the best way forward... |
Dylan-DPC
added a commit
to Dylan-DPC/rust
that referenced
this pull request
Oct 23, 2022
…r, r=lcnr Check `needs_infer` before `needs_drop` during HIR generator analysis This is kinda a revival of rust-lang#103036, but with the understanding that after fallback, a generator-interior type will only have `needs_infer` true if there's an error that prevented int or float variable fallback to occur (modulo region variables, which are erased). Therefore the best choice here is to delay a bug and skip the `needs_drop` call altogether. r? `@lcnr` feel free to reassign though
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 is kinda a follow-up to #102695, which only coincidentally fixes #102645.
This is a more proper fix, but it may be a "misuse" of the
InferenceLiteralEraserandInferCtxt::resolve_numeric_literals_with_default, which was originally intended only for diagnostics. However, I do feel like this usage is legitimate. For the purposes of drop tracking, we don't care that an int/float infer variable is unresolved, since all builtin int/float types are trivially drop and copy.Given that there may be a legitimate desire not to use
InferenceLiteralEraser, that's why I split it up -- only need a review on the last commit in the stack.r? @lcnr since you reviewed #102695, feel free to reassign.