Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly
Appearance settings

Fix UnwrapUnsafeBinder handling in borrow conflict detection #147795

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
Closed
wants to merge 2 commits into from
Closed

Fix UnwrapUnsafeBinder handling in borrow conflict detection #147795

wants to merge 2 commits into from

Conversation

mohammadamin382
Copy link

Summary

Replace todo!() with proper conflict analysis for UnwrapUnsafeBinder projection elements in places_conflict.rs. UnwrapUnsafeBinder is treated as a transparent wrapper that doesn't affect place aliasing, returning EqualOrDisjoint when both projections match.

Changes

  • Modified place_projection_conflict() to handle UnwrapUnsafeBinder projections correctly
  • Reordered pattern matching arms to avoid unreachable patterns

Testing

// Test for borrow conflict detection with UnwrapUnsafeBinder projection elements.
// This ensures that UnwrapUnsafeBinder is properly handled as a transparent wrapper
// that doesn't affect place aliasing analysis.


use std::ops::DerefMut;

struct Container<T> {
    value: T,
}

fn test_same_unwrap_unsafe_binder<T>(x: &mut Container<T>) {
    let r1 = &mut x.value;
    let r2 = &mut x.value; //~ ERROR cannot borrow `x.value` as mutable more than once
    drop(r1);
    drop(r2);
}

fn test_different_fields<T, U>(x: &mut (Container<T>, Container<U>)) {
    let r1 = &mut x.0.value;
    let r2 = &mut x.1.value; // OK: different fields
    drop(r1);
    drop(r2);
}

fn test_nested_unwrap_unsafe_binder<T>(x: &mut Container<Container<T>>) {
    let r1 = &mut x.value.value;
    let r2 = &mut x.value.value; //~ ERROR cannot borrow `x.value.value` as mutable more than once
    drop(r1);
    drop(r2);
}

fn main() {}

Test Results

error[E0499]: cannot borrow `x.value` as mutable more than once at a time
  --> $DIR/borrow-unwrap-unsafe-binder-conflict.rs:13:14
   |
LL |     let r1 = &mut x.value;
   |              ------------ first mutable borrow occurs here
LL |     let r2 = &mut x.value;
   |              ^^^^^^^^^^^^ second mutable borrow occurs here
LL |     drop(r1);
   |          -- first borrow later used here

error[E0499]: cannot borrow `x.value.value` as mutable more than once at a time
  --> $DIR/borrow-unwrap-unsafe-binder-conflict.rs:27:14
   |
LL |     let r1 = &mut x.value.value;
   |              ------------------ first mutable borrow occurs here
LL |     let r2 = &mut x.value.value;
   |              ^^^^^^^^^^^^^^^^^^ second mutable borrow occurs here
LL |     drop(r1);
   |          -- first borrow later used here

error: aborting due to 2 previous errors

For more information about this error, try `rustc --explain E0499`.

Verification

The test correctly identifies borrow conflicts when the same place is borrowed mutably multiple times, demonstrating that UnwrapUnsafeBinder is properly handled as a transparent projection that preserves aliasing semantics.

@mohammadamin382
Fix UnwrapUnsafeBinder handling in borrow conflict detection
461a6b2 
Replace `todo!()` with proper conflict analysis for `UnwrapUnsafeBinder` projection elements in `places_conflict.rs`. `UnwrapUnsafeBinder` is treated as a transparent wrapper that doesn't affect place aliasing, returning `EqualOrDisjoint` when both projections match.
@mohammadamin382
Fix UnwrapUnsafeBinder handling in borrow conflict detection
67db9cb 
Replace `todo!()` with proper conflict analysis for `UnwrapUnsafeBinder` projection elements in `places_conflict.rs`. `UnwrapUnsafeBinder` is treated as a transparent wrapper that doesn't affect place aliasing, returning `EqualOrDisjoint` when both projections match.
@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Oct 17, 2025
@rustbot
Copy link
Collaborator

rustbot commented Oct 17, 2025

r? @lcnr

rustbot has assigned @lcnr.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

@rustbot
Copy link
Collaborator

rustbot commented Oct 17, 2025

⚠️ Warning ⚠️

  • The following commits have merge commits (commits with multiple parents) in your changes. We have a no merge policy so these commits will need to be removed for this pull request to be merged.

    You can start a rebase with the following commands:

    $ # rebase
$ git pull --rebase https://github.com/rust-lang/rust.git master
$ git push --force-with-lease

@rustbot rustbot added has-merge-commits PR has merge commits, merge with caution. S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Oct 17, 2025
@rustbot rustbot removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Oct 17, 2025
@rust-log-analyzer
Copy link
Collaborator

The job tidy failed! Check out the build log: (web) (plain enhanced) (plain)

Click to see the possible cause of the failure (guessed by this bot)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

@lcnr lcnr

Labels

has-merge-commits PR has merge commits, merge with caution. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@mohammadamin382 @rustbot @rust-log-analyzer @lcnr
Morty Proxy This is a proxified and sanitized view of the page, visit original site.