Closed
Conversation
Contributor
Author
|
I accidentally deleted the branch. |
Closed
Move construction of a CrateContext into a static method on CrateContext.
Remove all the explicit @mut-fields from CrateContext, though many fields are still @-ptrs. This required changing every single function call that explicitly took a @CrateContext, so I took advantage and changed as many as I could get away with to &-ptrs or &mut ptrs.
Contributor
Author
Contributor
|
I'm on a phone today, detailed review outside scope of what I can do. I would not object to you placing r=me on it, if it does in fact do what the comments say, and if you're willing to do followups on anything I or others pick out in later review? |
bors
added a commit
that referenced
this pull request
Jun 15, 2013
This removes all of the explicit `@mut` fields from `CrateContext`. There are still a few that are managed, but no longer do we have `@mut bool` in the structure. Most of the changes are changing `@CrateContext` to `@mut CrateContext`, though I did change as many as I could get away with to `&CrateContext` and `&mut CrateContext`. The biggest thing preventing me from changing to `&[mut]` in most places was the instruction counter thing. In two cases, where I got a static borrow error and a dynamic borrow error, I opted to remove the count call there as it was literally the only thing preventing me from switching to `&mut CrateContext` parameters in both cases. Other things to note: * the EncoderContext uses borrowed pointers with lifetimes, since it can, though that required me to work around the limitation of not being able to move a structure with borrowed pointers into a heap closure. I changed as much as I could to stack closures, but unfortunately I hit the AST visitor and changing that is somewhat outside the scope of this PR. Instead (and there is a comment to this effect) I choose to unsafely get the structure into the heap, this is because I know the lifetimes involved are safe, even though the compiler can't prove it. * Many of the changes are workarounds because of the borrow checker, either dynamically freezing it for too long, or inferring too large a scope. This is mostly just from nested function calls where each borrow is considered to last for the entire statement. Other cases are where `CrateContext` was borrowed in a `match` causing it to be borrowed for the entire length of the match, even though that wasn't wanted (or needed). * I haven't yet tested to see if this changes compilation times in any way. I doubt there will be much of an impact however, as the only major improvements are less indirection and fewer refcount bumps. * This lays the foundations to remove many more heap allocations in trans as many cases can be changed to use lifetimes instead. ===== This change includes some other, minor refactorings, as I am planning a series, however I don't want to submit them all at once as it will be hell to continually rebase.
Contributor
Author
|
@graydon that's fine. I included all the major strokes in the comments, and it seems to work locally. As I said, I am planning on a series of changes to try and modernize trans as best I can so I'm more than willing to follow up any comments. |
flip1995
pushed a commit
to flip1995/rust
that referenced
this pull request
Apr 27, 2021
Fix lintcheck on windows changelog: None
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
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 removes all of the explicit
@mutfields fromCrateContext. There are still a few that are managed, but no longer do we have@mut boolin the structure.Most of the changes are changing
@CrateContextto@mut CrateContext, though I did change as many as I could get away with to&CrateContextand&mut CrateContext. The biggest thing preventing me from changing to&[mut]in most places was the instruction counter thing. In two cases, where I got a static borrow error and a dynamic borrow error, I opted to remove the count call there as it was literally the only thing preventing me from switching to&mut CrateContextparameters in both cases.Other things to note:
CrateContextwas borrowed in amatchcausing it to be borrowed for the entire length of the match, even though that wasn't wanted (or needed).This change includes some other, minor refactorings, as I am planning a series, however I don't want to submit them all at once as it will be hell to continually rebase.