Skip to content

Allow calling *const methods on *mut values #82436

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

Merged
merged 1 commit into from
Mar 13, 2021
Merged

Allow calling *const methods on *mut values #82436

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
64 changes: 39 additions & 25 deletions compiler/rustc_typeck/src/check/method/confirm.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -155,32 +155,46 @@ impl<'a, 'tcx> ConfirmContext<'a, 'tcx> {
let mut target =
self.structurally_resolved_type(autoderef.span(), autoderef.final_ty(false));

if let Some(mutbl) = pick.autoref {
let region = self.next_region_var(infer::Autoref(self.span, pick.item));
target = self.tcx.mk_ref(region, ty::TypeAndMut { mutbl, ty: target });
let mutbl = match mutbl {
hir::Mutability::Not => AutoBorrowMutability::Not,
hir::Mutability::Mut => AutoBorrowMutability::Mut {
// Method call receivers are the primary use case
// for two-phase borrows.
allow_two_phase_borrow: AllowTwoPhase::Yes,
},
};
adjustments
.push(Adjustment { kind: Adjust::Borrow(AutoBorrow::Ref(region, mutbl)), target });

if let Some(unsize_target) = pick.unsize {
target = self
.tcx
.mk_ref(region, ty::TypeAndMut { mutbl: mutbl.into(), ty: unsize_target });
adjustments.push(Adjustment { kind: Adjust::Pointer(PointerCast::Unsize), target });
match &pick.autoref_or_ptr_adjustment {
Some(probe::AutorefOrPtrAdjustment::Autoref { mutbl, unsize }) => {
let region = self.next_region_var(infer::Autoref(self.span, pick.item));
target = self.tcx.mk_ref(region, ty::TypeAndMut { mutbl: *mutbl, ty: target });
let mutbl = match mutbl {
hir::Mutability::Not => AutoBorrowMutability::Not,
hir::Mutability::Mut => AutoBorrowMutability::Mut {
// Method call receivers are the primary use case
// for two-phase borrows.
allow_two_phase_borrow: AllowTwoPhase::Yes,
},
};
adjustments.push(Adjustment {
kind: Adjust::Borrow(AutoBorrow::Ref(region, mutbl)),
target,
});

if let Some(unsize_target) = unsize {
target = self
.tcx
.mk_ref(region, ty::TypeAndMut { mutbl: mutbl.into(), ty: unsize_target });
adjustments
.push(Adjustment { kind: Adjust::Pointer(PointerCast::Unsize), target });
}
}
Some(probe::AutorefOrPtrAdjustment::ToConstPtr) => {
target = match target.kind() {
ty::RawPtr(ty::TypeAndMut { ty, mutbl }) => {
assert_eq!(*mutbl, hir::Mutability::Mut);
self.tcx.mk_ptr(ty::TypeAndMut { mutbl: hir::Mutability::Not, ty })
}
other => panic!("Cannot adjust receiver type {:?} to const ptr", other),
};

adjustments.push(Adjustment {
kind: Adjust::Pointer(PointerCast::MutToConstPointer),
target,
});
}
} else {
// No unsizing should be performed without autoref (at
// least during method dispach). This is because we
// currently only unsize `[T;N]` to `[T]`, and naturally
// that must occur being a reference.
assert!(pick.unsize.is_none());
None => {}
}

self.register_predicates(autoderef.into_obligations());
Expand Down
103 changes: 82 additions & 21 deletions compiler/rustc_typeck/src/check/method/probe.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -154,6 +154,42 @@ enum ProbeResult {
Match,
}

/// When adjusting a receiver we often want to do one of
///
/// - Add a `&` (or `&mut`), converting the recevier from `T` to `&T` (or `&mut T`)
/// - If the receiver has type `*mut T`, convert it to `*const T`
///
/// This type tells us which one to do.
///
/// Note that in principle we could do both at the same time. For example, when the receiver has
/// type `T`, we could autoref it to `&T`, then convert to `*const T`. Or, when it has type `*mut
/// T`, we could convert it to `*const T`, then autoref to `&*const T`. However, currently we do
/// (at most) one of these. Either the receiver has type `T` and we convert it to `&T` (or with
/// `mut`), or it has type `*mut T` and we convert it to `*const T`.
#[derive(Debug, PartialEq, Clone)]
pub enum AutorefOrPtrAdjustment<'tcx> {
/// Receiver has type `T`, add `&` or `&mut` (it `T` is `mut`), and maybe also "unsize" it.
/// Unsizing is used to convert a `[T; N]` to `[T]`, which only makes sense when autorefing.
Autoref {
mutbl: hir::Mutability,

/// Indicates that the source expression should be "unsized" to a target type. This should
/// probably eventually go away in favor of just coercing method receivers.
unsize: Option<Ty<'tcx>>,
},
/// Receiver has type `*mut T`, convert to `*const T`
ToConstPtr,
}

impl<'tcx> AutorefOrPtrAdjustment<'tcx> {
fn get_unsize(&self) -> Option<Ty<'tcx>> {
match self {
AutorefOrPtrAdjustment::Autoref { mutbl: _, unsize } => unsize.clone(),
AutorefOrPtrAdjustment::ToConstPtr => None,
}
}
}

#[derive(Debug, PartialEq, Clone)]
pub struct Pick<'tcx> {
pub item: ty::AssocItem,
Expand All @@ -165,17 +201,9 @@ pub struct Pick<'tcx> {
/// A = expr | *expr | **expr | ...
pub autoderefs: usize,

/// Indicates that an autoref is applied after the optional autoderefs
///
/// B = A | &A | &mut A
pub autoref: Option<hir::Mutability>,

/// Indicates that the source expression should be "unsized" to a
/// target type. This should probably eventually go away in favor
/// of just coercing method receivers.
///
/// C = B | unsize(B)
pub unsize: Option<Ty<'tcx>>,
/// Indicates that we want to add an autoref (and maybe also unsize it), or if the receiver is
/// `*mut T`, convert it to `*const T`.
pub autoref_or_ptr_adjustment: Option<AutorefOrPtrAdjustment<'tcx>>,
}

#[derive(Clone, Debug, PartialEq, Eq)]
Expand Down Expand Up @@ -714,16 +742,16 @@ impl<'a, 'tcx> ProbeContext<'a, 'tcx> {

debug!("assemble_inherent_impl_probe {:?}", impl_def_id);

let (impl_ty, impl_substs) = self.impl_ty_and_substs(impl_def_id);
let impl_ty = impl_ty.subst(self.tcx, impl_substs);

for item in self.impl_or_trait_item(impl_def_id) {
if !self.has_applicable_self(&item) {
// No receiver declared. Not a candidate.
self.record_static_candidate(ImplSource(impl_def_id));
continue;
}

let (impl_ty, impl_substs) = self.impl_ty_and_substs(impl_def_id);
let impl_ty = impl_ty.subst(self.tcx, impl_substs);

// Determine the receiver type that the method itself expects.
let xform_tys = self.xform_self_ty(&item, impl_ty, impl_substs);

Expand Down Expand Up @@ -1086,6 +1114,7 @@ impl<'a, 'tcx> ProbeContext<'a, 'tcx> {
self.pick_by_value_method(step, self_ty).or_else(|| {
self.pick_autorefd_method(step, self_ty, hir::Mutability::Not)
.or_else(|| self.pick_autorefd_method(step, self_ty, hir::Mutability::Mut))
.or_else(|| self.pick_const_ptr_method(step, self_ty))
})
})
.next()
Expand Down Expand Up @@ -1113,7 +1142,10 @@ impl<'a, 'tcx> ProbeContext<'a, 'tcx> {
// Insert a `&*` or `&mut *` if this is a reference type:
if let ty::Ref(_, _, mutbl) = *step.self_ty.value.value.kind() {
pick.autoderefs += 1;
pick.autoref = Some(mutbl);
pick.autoref_or_ptr_adjustment = Some(AutorefOrPtrAdjustment::Autoref {
mutbl,
unsize: pick.autoref_or_ptr_adjustment.and_then(|a| a.get_unsize()),
})
}

pick
Expand All @@ -1136,8 +1168,39 @@ impl<'a, 'tcx> ProbeContext<'a, 'tcx> {
self.pick_method(autoref_ty).map(|r| {
r.map(|mut pick| {
pick.autoderefs = step.autoderefs;
pick.autoref = Some(mutbl);
pick.unsize = step.unsize.then_some(self_ty);
pick.autoref_or_ptr_adjustment = Some(AutorefOrPtrAdjustment::Autoref {
mutbl,
unsize: step.unsize.then_some(self_ty),
});
pick
})
})
}

/// If `self_ty` is `*mut T` then this picks `*const T` methods. The reason why we have a
/// special case for this is because going from `*mut T` to `*const T` with autoderefs and
/// autorefs would require dereferencing the pointer, which is not safe.
Copy link
Contributor

Choose a reason for hiding this comment

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

IIUC, it's not so much that the pointer would be dereferenced, but more that adding an autoref would create a safe reference, which we don't want to do.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I just tried this:

struct Test {}

impl Test {
    fn x(&self) {
        println!("x called");
    }
}

fn main() {
    let mut test = Test {};
    let mut test_mut_ref = &mut test;
    test_mut_ref.x();
}

the relevant, unoptimized MIR:

        _4 = &(*_2);
        _3 = Test::x(move _4) -> [return: bb1, unwind: bb2];

I thought the * dereferences, no? Similar code for the pointer version would also do &* and dereference.

Copy link
Contributor

Choose a reason for hiding this comment

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

You're not wrong, the * does dereference -- it's just that, when you combine it with a &, the net effect is kind of an "identity transform", at least at runtime. That is, &*x is sort of equivalent to x (the "address of the data that x points to" is kind of just "x") -- but only sort of, because now we've made a shared reference, and that has implications (e.g., if x: &mut T, it implies that *x is frozen while this shared reference is live).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Is this the case even when result of & is a pointer, rather than reference? I'm guessing the final assembly for &* (for pointer-to-pointer conversions) may disappear, but I thought in MIR it's still dereferencing, and has the implications of * operation (whatever those implications are).

Instead what we do here is more like x as *const T (where x : *mut T), which doesn't have unsafe dereferencing in MIR, and as a result would not have the same implications.

fn pick_const_ptr_method(
&mut self,
step: &CandidateStep<'tcx>,
self_ty: Ty<'tcx>,
) -> Option<PickResult<'tcx>> {
// Don't convert an unsized reference to ptr
if step.unsize {
return None;
}

let ty = match self_ty.kind() {
ty::RawPtr(ty::TypeAndMut { ty, mutbl: hir::Mutability::Mut }) => ty,
_ => return None,
};

let const_self_ty = ty::TypeAndMut { ty, mutbl: hir::Mutability::Not };
let const_ptr_ty = self.tcx.mk_ptr(const_self_ty);
self.pick_method(const_ptr_ty).map(|r| {
r.map(|mut pick| {
pick.autoderefs = step.autoderefs;
pick.autoref_or_ptr_adjustment = Some(AutorefOrPtrAdjustment::ToConstPtr);
pick
})
})
Expand Down Expand Up @@ -1510,8 +1573,7 @@ impl<'a, 'tcx> ProbeContext<'a, 'tcx> {
kind: TraitPick,
import_ids: probes[0].0.import_ids.clone(),
autoderefs: 0,
autoref: None,
unsize: None,
autoref_or_ptr_adjustment: None,
})
}

Expand Down Expand Up @@ -1748,8 +1810,7 @@ impl<'tcx> Candidate<'tcx> {
},
import_ids: self.import_ids.clone(),
autoderefs: 0,
autoref: None,
unsize: None,
autoref_or_ptr_adjustment: None,
}
}
}
20 changes: 20 additions & 0 deletions src/test/mir-opt/receiver-ptr-mutability.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
// EMIT_MIR receiver_ptr_mutability.main.mir_map.0.mir

#![feature(arbitrary_self_types)]

struct Test {}

impl Test {
fn x(self: *const Self) {
println!("x called");
}
}

fn main() {
let ptr: *mut Test = std::ptr::null_mut();
ptr.x();

// Test autoderefs
let ptr_ref: &&&&*mut Test = &&&&ptr;
ptr_ref.x();
}
96 changes: 96 additions & 0 deletions src/test/mir-opt/receiver_ptr_mutability.main.mir_map.0.mir
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,96 @@
// MIR for `main` 0 mir_map

| User Type Annotations
| 0: Canonical { max_universe: U0, variables: [], value: Ty(*mut Test) } at $DIR/receiver-ptr-mutability.rs:14:14: 14:23
| 1: Canonical { max_universe: U0, variables: [], value: Ty(*mut Test) } at $DIR/receiver-ptr-mutability.rs:14:14: 14:23
| 2: Canonical { max_universe: U0, variables: [CanonicalVarInfo { kind: Region(U0) }, CanonicalVarInfo { kind: Region(U0) }, CanonicalVarInfo { kind: Region(U0) }, CanonicalVarInfo { kind: Region(U0) }], value: Ty(&&&&*mut Test) } at $DIR/receiver-ptr-mutability.rs:18:18: 18:31
| 3: Canonical { max_universe: U0, variables: [CanonicalVarInfo { kind: Region(U0) }, CanonicalVarInfo { kind: Region(U0) }, CanonicalVarInfo { kind: Region(U0) }, CanonicalVarInfo { kind: Region(U0) }], value: Ty(&&&&*mut Test) } at $DIR/receiver-ptr-mutability.rs:18:18: 18:31
|
fn main() -> () {
let mut _0: (); // return place in scope 0 at $DIR/receiver-ptr-mutability.rs:13:11: 13:11
let _1: *mut Test as UserTypeProjection { base: UserType(0), projs: [] }; // in scope 0 at $DIR/receiver-ptr-mutability.rs:14:9: 14:12
let _2: (); // in scope 0 at $DIR/receiver-ptr-mutability.rs:15:5: 15:12
let mut _3: *const Test; // in scope 0 at $DIR/receiver-ptr-mutability.rs:15:5: 15:8
let mut _4: *mut Test; // in scope 0 at $DIR/receiver-ptr-mutability.rs:15:5: 15:8
let _6: &&&&*mut Test; // in scope 0 at $DIR/receiver-ptr-mutability.rs:18:34: 18:41
let _7: &&&*mut Test; // in scope 0 at $DIR/receiver-ptr-mutability.rs:18:35: 18:41
let _8: &&*mut Test; // in scope 0 at $DIR/receiver-ptr-mutability.rs:18:36: 18:41
let _9: &*mut Test; // in scope 0 at $DIR/receiver-ptr-mutability.rs:18:37: 18:41
let _10: (); // in scope 0 at $DIR/receiver-ptr-mutability.rs:19:5: 19:16
let mut _11: *const Test; // in scope 0 at $DIR/receiver-ptr-mutability.rs:19:5: 19:12
let mut _12: *mut Test; // in scope 0 at $DIR/receiver-ptr-mutability.rs:19:5: 19:12
scope 1 {
debug ptr => _1; // in scope 1 at $DIR/receiver-ptr-mutability.rs:14:9: 14:12
let _5: &&&&*mut Test as UserTypeProjection { base: UserType(2), projs: [] }; // in scope 1 at $DIR/receiver-ptr-mutability.rs:18:9: 18:16
scope 2 {
debug ptr_ref => _5; // in scope 2 at $DIR/receiver-ptr-mutability.rs:18:9: 18:16
}
}

bb0: {
StorageLive(_1); // scope 0 at $DIR/receiver-ptr-mutability.rs:14:9: 14:12
_1 = null_mut::<Test>() -> [return: bb1, unwind: bb4]; // scope 0 at $DIR/receiver-ptr-mutability.rs:14:26: 14:46
// mir::Constant
// + span: $DIR/receiver-ptr-mutability.rs:14:26: 14:44
// + literal: Const { ty: fn() -> *mut Test {std::ptr::null_mut::<Test>}, val: Value(Scalar(<ZST>)) }
}

bb1: {
FakeRead(ForLet, _1); // scope 0 at $DIR/receiver-ptr-mutability.rs:14:9: 14:12
AscribeUserType(_1, o, UserTypeProjection { base: UserType(1), projs: [] }); // scope 0 at $DIR/receiver-ptr-mutability.rs:14:14: 14:23
StorageLive(_2); // scope 1 at $DIR/receiver-ptr-mutability.rs:15:5: 15:12
StorageLive(_3); // scope 1 at $DIR/receiver-ptr-mutability.rs:15:5: 15:8
StorageLive(_4); // scope 1 at $DIR/receiver-ptr-mutability.rs:15:5: 15:8
_4 = _1; // scope 1 at $DIR/receiver-ptr-mutability.rs:15:5: 15:8
_3 = move _4 as *const Test (Pointer(MutToConstPointer)); // scope 1 at $DIR/receiver-ptr-mutability.rs:15:5: 15:8
StorageDead(_4); // scope 1 at $DIR/receiver-ptr-mutability.rs:15:7: 15:8
_2 = Test::x(move _3) -> [return: bb2, unwind: bb4]; // scope 1 at $DIR/receiver-ptr-mutability.rs:15:5: 15:12
// mir::Constant
// + span: $DIR/receiver-ptr-mutability.rs:15:9: 15:10
// + literal: Const { ty: fn(*const Test) {Test::x}, val: Value(Scalar(<ZST>)) }
}

bb2: {
StorageDead(_3); // scope 1 at $DIR/receiver-ptr-mutability.rs:15:11: 15:12
StorageDead(_2); // scope 1 at $DIR/receiver-ptr-mutability.rs:15:12: 15:13
StorageLive(_5); // scope 1 at $DIR/receiver-ptr-mutability.rs:18:9: 18:16
StorageLive(_6); // scope 1 at $DIR/receiver-ptr-mutability.rs:18:34: 18:41
StorageLive(_7); // scope 1 at $DIR/receiver-ptr-mutability.rs:18:35: 18:41
StorageLive(_8); // scope 1 at $DIR/receiver-ptr-mutability.rs:18:36: 18:41
StorageLive(_9); // scope 1 at $DIR/receiver-ptr-mutability.rs:18:37: 18:41
_9 = &_1; // scope 1 at $DIR/receiver-ptr-mutability.rs:18:37: 18:41
_8 = &_9; // scope 1 at $DIR/receiver-ptr-mutability.rs:18:36: 18:41
_7 = &_8; // scope 1 at $DIR/receiver-ptr-mutability.rs:18:35: 18:41
_6 = &_7; // scope 1 at $DIR/receiver-ptr-mutability.rs:18:34: 18:41
_5 = &(*_6); // scope 1 at $DIR/receiver-ptr-mutability.rs:18:34: 18:41
FakeRead(ForLet, _5); // scope 1 at $DIR/receiver-ptr-mutability.rs:18:9: 18:16
AscribeUserType(_5, o, UserTypeProjection { base: UserType(3), projs: [] }); // scope 1 at $DIR/receiver-ptr-mutability.rs:18:18: 18:31
StorageDead(_6); // scope 1 at $DIR/receiver-ptr-mutability.rs:18:41: 18:42
StorageLive(_10); // scope 2 at $DIR/receiver-ptr-mutability.rs:19:5: 19:16
StorageLive(_11); // scope 2 at $DIR/receiver-ptr-mutability.rs:19:5: 19:12
StorageLive(_12); // scope 2 at $DIR/receiver-ptr-mutability.rs:19:5: 19:12
_12 = (*(*(*(*_5)))); // scope 2 at $DIR/receiver-ptr-mutability.rs:19:5: 19:12
_11 = move _12 as *const Test (Pointer(MutToConstPointer)); // scope 2 at $DIR/receiver-ptr-mutability.rs:19:5: 19:12
StorageDead(_12); // scope 2 at $DIR/receiver-ptr-mutability.rs:19:11: 19:12
_10 = Test::x(move _11) -> [return: bb3, unwind: bb4]; // scope 2 at $DIR/receiver-ptr-mutability.rs:19:5: 19:16
// mir::Constant
// + span: $DIR/receiver-ptr-mutability.rs:19:13: 19:14
// + literal: Const { ty: fn(*const Test) {Test::x}, val: Value(Scalar(<ZST>)) }
}

bb3: {
StorageDead(_11); // scope 2 at $DIR/receiver-ptr-mutability.rs:19:15: 19:16
StorageDead(_10); // scope 2 at $DIR/receiver-ptr-mutability.rs:19:16: 19:17
_0 = const (); // scope 0 at $DIR/receiver-ptr-mutability.rs:13:11: 20:2
StorageDead(_9); // scope 1 at $DIR/receiver-ptr-mutability.rs:20:1: 20:2
StorageDead(_8); // scope 1 at $DIR/receiver-ptr-mutability.rs:20:1: 20:2
StorageDead(_7); // scope 1 at $DIR/receiver-ptr-mutability.rs:20:1: 20:2
StorageDead(_5); // scope 1 at $DIR/receiver-ptr-mutability.rs:20:1: 20:2
StorageDead(_1); // scope 0 at $DIR/receiver-ptr-mutability.rs:20:1: 20:2
return; // scope 0 at $DIR/receiver-ptr-mutability.rs:20:2: 20:2
}

bb4 (cleanup): {
resume; // scope 0 at $DIR/receiver-ptr-mutability.rs:13:1: 20:2
}
}