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.

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

librustc: WIP patch for upgrading the way rvalues are treated. #16009

Closed
wants to merge 1 commit into from

Conversation

pcwalton
Copy link
Contributor

WIP. Do not merge.

@nikomatsakis I'd like to get your feedback on this. I've implemented a patch which fixes #14587, but it has some nasty fallout in other parts of the codebase.

I first tried to make all rvalues treated like lvalues. This ran into problems with method chaining: foo.bar().baz() would not work anymore if baz required &mut self, as it thought it was a double borrow. I believe this is because of the rvalue temporary rules.

The next thing, which is in this patch, was to formulate the notion of "aliasable rvalues". These are rvalues that could be aliasable because they're about to be unpacked into a pattern. Local initializers, match values, and for loop heads fall into this list. This causes a few strange errors:

/Users/pcwalton/Source/rust/src/libcollections/treemap.rs:554:17: 554:18 error: cannot move out of `<rvalue>` because it is borrowed
/Users/pcwalton/Source/rust/src/libcollections/treemap.rs:554             let n: &mut TreeNode<K, V> = &mut **n;
                                                                              ^
/Users/pcwalton/Source/rust/src/libcollections/treemap.rs:554:42: 554:50 note: borrow of `*<rvalue>` occurs here
/Users/pcwalton/Source/rust/src/libcollections/treemap.rs:554             let n: &mut TreeNode<K, V> = &mut **n;

This may be related to the rvalue scope extension rules. I haven't dug into that to see whether it's the case.

Any thoughts as to the right way to go here?

@pcwalton
Copy link
Contributor Author

One alternative approach I just thought of is to forbid @ patterns where both sides have bindings.

@pcwalton
Copy link
Contributor Author

Closing since we took the sledgehammer approach.

@pcwalton pcwalton closed this Aug 14, 2014
bors added a commit to rust-lang-ci/rust that referenced this pull request Dec 11, 2023
fix: bug in extract_function.rs

There is a little bug in extract_function: It appends `use path::to::ControlFlow;` if the function created contains string "ControlFlow".

 A case below (also in the test named `does_not_import_control_flow` which will fail in the original code)

<img width="322" alt="image" src="https://github.com/rust-lang/rust-analyzer/assets/53432474/4b80bb58-0cfd-4d56-b64c-d9649eed336e">
<img width="391" alt="image" src="https://github.com/rust-lang/rust-analyzer/assets/53432474/3d7262f4-8a4c-44ea-822d-304b8b23fe28">

Now I have changed the condition determining whether adding import statement. Only when the new function body contains ControlFlow::Break or ControlFlow::Continue can the import statement be added.

Last related PR: rust-lang/rust-analyzer#10309
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Binding subpattern with @ allows for illegal mutation
1 participant