Skip to content

Conversation

HKalbasi
Copy link
Member

@HKalbasi HKalbasi commented Apr 6, 2023

Dependency of #14470

@rustbot rustbot added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Apr 6, 2023
Comment on lines 965 to 966
// if `&&T` and `&T` are both applicable, we should prefer `&*&&T` to `&*&T`, so this
// flag exists
Copy link
Member

@Veykril Veykril Apr 6, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am somewhat confused by why we need this weird flag at all. This feels very wrong to have. Why did the earlier version not suffice? (where we just introduced a reborrow)

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This test is the problem:

#[test]
fn method_resolution_by_value_before_autoref() {
    check_types(
        r#"
trait Clone { fn clone(&self) -> Self; }
struct S;
impl Clone for S {}
impl Clone for &S {}
fn test() { (S.clone(), (&S).clone(), (&&S).clone()); }
          //^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ (S, S, &S)
"#,
    );
}

We should continue for the reborrow only one time, and this flag serve that propose. Without it, we will get (S, S, S).

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, now I see what we want to do here. We need to set the autoref field of the ReceiverAdjustments first_adjustment as well as increment the autoderefs field once for this iterate_method_candidates_by_receiver call when the receiver_ty is a reference

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That then also mimics what rustc does here

@Veykril
Copy link
Member

Veykril commented Apr 6, 2023

@bors r+

@bors
Copy link
Contributor

bors commented Apr 6, 2023

📌 Commit 7ba93cb has been approved by Veykril

It is now in the queue for this repository.

@bors
Copy link
Contributor

bors commented Apr 6, 2023

⌛ Testing commit 7ba93cb with merge e739c99...

@bors
Copy link
Contributor

bors commented Apr 6, 2023

☀️ Test successful - checks-actions
Approved by: Veykril
Pushing e739c99 to master...

@bors bors merged commit e739c99 into rust-lang:master Apr 6, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-review Status: Awaiting review from the assignee but also interested parties.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants