Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Fix mutable aliasing uncovered by MIR borrow-checking #61

Merged
merged 1 commit into from Jan 25, 2018
Merged

Fix mutable aliasing uncovered by MIR borrow-checking #61

merged 1 commit into from Jan 25, 2018

Conversation

SimonSapin
Copy link
Contributor

To reproduce, build on Nightly with RUSTFLAGS=-Znll

@SimonSapin
Fix mutable aliasing uncovered by MIR borrow-checking
b05704b 
To reproduce, build on Nightly with `RUSTFLAGS=-Znll`
@SimonSapin
Copy link
Contributor Author

Sorry, I thought this was already fixed in master when I opened #60 but I must have made a mistake.

@SimonSapin SimonSapin mentioned this pull request Jan 24, 2018
Closed
@alexcrichton
Copy link
Member

Will this be necessary in the long run? Is [patch] a viable option in the short term?

@SimonSapin
Copy link
Contributor Author

Yes, I can use [patch] to continue testing NLL with Servo’s dependency graph.

However this particular compiler behavior change was ruled in rust-lang/rust#47707 as not a bug, but rather fixing a previous soundness bug. So this change (or something like it) will be needed when NLL is enabled in Nightly by default. Why wait?

@SimonSapin
Copy link
Contributor Author

SimonSapin commented Jan 24, 2018 •
edited

Telling a soundess fix like this apart from a NLL bug requires manual triaging. Fixing them as we find them will likely help reduce such manual work in future NLL crater runs.

@alexcrichton
Copy link
Member

Er sorry yeah I didn't realize that this was going to become a hard error in the future! I'm ok merging this for that alone yeah.

I'm mostly surprised that this wasn't discovered in the previous PR? I think [patch] would allow local testing, right? Is the NLL usage in Servo getting committed in that a release is needed to be on crates.io soon?

@SimonSapin
Copy link
Contributor Author

I’ve been trying NLL by running RUSTFLAGS=-Znll cargo +nightly build. Nothing’s in the repo.

After seeing that this crate’s build errors was not a rust bug I reproduced it in this repo (not with [patch] in servo) at 0.2.14 and 0.2.15 and then saw a successful build at master and convinced myself that the issue was already fixed, but I must have run the wrong command for that one.

@alexcrichton alexcrichton merged commit 9b84689 into rust-lang:master Jan 25, 2018
@SimonSapin SimonSapin deleted the nll branch January 25, 2018 16:45
@KamilaBorowska KamilaBorowska mentioned this pull request Jan 21, 2022
Open
phimuemue added a commit to phimuemue/hanabi.rs that referenced this pull request Jul 3, 2022
@phimuemue
Fix getopts version
6795fde 
The pinned version does not compile anymore because of mutable aliasing:
* rust-lang/getopts#61
* rust-lang/getopts#110

This was achieved by temporarily setting the getopts version to "0.2.21"
in Cargo.toml, and running `cargo check`.

Note that this also converts the Cargo.lock to a new format.
WuTheFWasThat pushed a commit to WuTheFWasThat/hanabi.rs that referenced this pull request Jul 5, 2022
@phimuemue
Compile on rust 1.61, resolve warnings, fix some lints (#12)
c9b21fa 
* Fix getopts version

The pinned version does not compile anymore because of mutable aliasing:
* rust-lang/getopts#61
* rust-lang/getopts#110

This was achieved by temporarily setting the getopts version to "0.2.21"
in Cargo.toml, and running `cargo check`.

Note that this also converts the Cargo.lock to a new format.

* Fix warning: Instead of deprecated macro try!, use question mark operator

* Fix warning: Avoid anonymous parameters

* Fix warning: Use dyn on trait objects

* Fix warning: Avoid unneeded mutability

* Fix warning: Avoid redundant format in panic or assert

* Fix lint: Avoid redundant field names in initializers

* Fix lint: Avoid redundant clone

* Fix lint: Avoid literal cast

* Fix lint: Collapse if/else where applicable

I left some if/else branches in place, if there was a certain symmetry between the branches.

* Fix lint: Avoid needless borrow

I left some if/else branches in place, if there was a certain symmetry between the branches.

* Fix lint: Use cloned instead of custom closure

* Fix lint: Avoid unneeded trait bound

* Fix lint: Avoid unneeded trait bound (2) avoid redundant clone

* Fix lint: Use &[T] instead of &Vec<T>

* Fix lint: Avoid & on each pattern

* Fix lint: Avoid manual assign

* Fix lint: Use implicit return

* Fix lint: Merge if/else branches with same value

I left one complicated branch in place.

* Fix lint: Use is_empty instead of comparing len against 0

* Fix lint: Use sum instead of fold

* Fix lint: Avoid clone on Copy types
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
2 participants
@SimonSapin @alexcrichton