-
Notifications
You must be signed in to change notification settings - Fork 13.1k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Remove uses of last-use analysis #3754
Merged
Merged
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
Had to remove the buffalo example. It was awkward to update for explicit moves.
...they require copying noncopyable fields.
and require explicit moves. Also provide more info in some error messages. Also: check that non-copyable struct fields don't get copied. Closes rust-lang#3481
r+ |
One thought: to avoid errors with inconsistent messages (like "Try adding a move"), you could use a |
bors
pushed a commit
to rust-lang-ci/rust
that referenced
this pull request
May 15, 2021
RalfJung
pushed a commit
to RalfJung/rust
that referenced
this pull request
Aug 18, 2024
Make unused states of Reserved unrepresentable In the [previous TB update](rust-lang/miri#3742) we discovered that the existence of `Reserved + !ty_is_freeze + protected` is undesirable. This has the side effect of making `Reserved { conflicted: true, ty_is_freeze: false }` unreachable. As such it is desirable that this state would also be unrepresentable. This PR eliminates the unused configuration by changing ```rs enum PermissionPriv { Reserved { ty_is_freeze: bool, conflicted: bool }, ... } ``` into ```rs enum PermissionPriv { ReservedFrz { conflicted: bool }, ReservedIM, ... } ``` but this is not the only solution and `Reserved(Activable | Conflicted | InteriorMut)` could be discussed. In addition to making the unreachable state not representable anymore, this change has the nice side effect of enabling `foreign_read` to no longer depend explicitly on the `protected` flag. Currently waiting for - `@JoJoDeveloping` to confirm that this is the same representation of `Reserved` as what is being implemented in simuliris, - `@RalfJung` to approve that this does not introduce too much overhead in the trusted codebase.
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 pull request removes the reliance of kind checking and trans on last-use analysis. This means two main things:
There is also one separate change (I know, I know) which is to change the kind checker to make struct fields non-copyable as per #3481.
Only 9abc7f0 , c4155f5, and c6780fb need any review. The other patches are mostly just inserting implicit moves.
I had said that these patches caused a performance regression, but on further investigation, it doesn't. Either the performance hit comes from code I already checked in long ago (adding explicit moves), or -- more likely -- I was comparing between optimized and unoptimized builds :-( So I'm pretty confident that merging this pull request won't harm performance significantly.
I know we are potentially going to implement @nmatsakis's type-based move proposal, but in the meantime, this work is done and we might as well merge it to avoid the problems described in #2633.