Skip to content
Open
Show file tree
Hide file tree
Changes from 1 commit
Commits
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
2 changes: 2 additions & 0 deletions compiler/rustc_hir_analysis/src/autoderef.rs
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,8 @@ pub enum AutoderefKind {
Builtin,
/// A type which must dispatch to a `Deref` implementation.
Overloaded,
/// A pinned reference type, such as `Pin<&T>` and `Pin<&mut T>`.
Pin,
}
struct AutoderefSnapshot<'tcx> {
at_start: bool,
Expand Down
4 changes: 4 additions & 0 deletions compiler/rustc_hir_typeck/src/autoderef.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ use itertools::Itertools;
use rustc_hir_analysis::autoderef::{Autoderef, AutoderefKind};
use rustc_infer::infer::InferOk;
use rustc_infer::traits::PredicateObligations;
use rustc_middle::bug;
use rustc_middle::ty::adjustment::{Adjust, Adjustment, DerefAdjustKind, OverloadedDeref};
use rustc_middle::ty::{self, Ty};
use rustc_span::Span;
Expand Down Expand Up @@ -62,6 +63,9 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
})
.unwrap_or(DerefAdjustKind::Builtin)
}
AutoderefKind::Pin => {
bug!("Pin autoderef kind should not be present in the steps")
}
AutoderefKind::Builtin => DerefAdjustKind::Builtin,
})
.zip_eq(targets)
Expand Down
65 changes: 65 additions & 0 deletions compiler/rustc_hir_typeck/src/coercion.rs
Original file line number Diff line number Diff line change
Expand Up @@ -269,12 +269,21 @@ impl<'f, 'tcx> Coerce<'f, 'tcx> {
return self.coerce_to_raw_ptr(a, b, b_mutbl);
}
ty::Ref(r_b, _, mutbl_b) => {
if self.tcx.features().pin_ergonomics()
&& a.pinned_ty().is_some_and(|ty| ty.is_ref())
&& let Ok(coerce) = self.commit_if_ok(|_| self.coerce_maybe_pinned_ref(a, b))
{
return Ok(coerce);
}
Comment on lines 272 to 277
Copy link
Member

Choose a reason for hiding this comment

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

I am sort of thinking that this code (and the lines added under the ty::Adt arm, should go in coerce_to_ref and coerce_to_pin_ref respectively - keeping this function relatively clean.

Copy link
Member

Choose a reason for hiding this comment

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

This also would help to eliminate some span_bugs from coerce_maybe_pinned_ref

return self.coerce_to_ref(a, b, r_b, mutbl_b);
}
ty::Adt(pin, _)
if self.tcx.features().pin_ergonomics()
&& self.tcx.is_lang_item(pin.did(), hir::LangItem::Pin) =>
{
if a.is_ref() && b.pinned_ty().is_some_and(|ty| ty.is_ref()) {
return self.coerce_maybe_pinned_ref(a, b);
}
let pin_coerce = self.commit_if_ok(|_| self.coerce_to_pin_ref(a, b));
if pin_coerce.is_ok() {
return pin_coerce;
Expand Down Expand Up @@ -847,6 +856,62 @@ impl<'f, 'tcx> Coerce<'f, 'tcx> {
self.unify_and(a, b, [], Adjust::ReborrowPin(mut_b), ForceLeakCheck::No)
}

/// Coerce pinned reference to regular reference or vice versa
///
/// - `Pin<&mut T>` <-> `&mut T` when `T: Unpin`
/// - `Pin<&T>` <-> `&T` when `T: Unpin`
/// - `Pin<&mut T>` <-> `Pin<&T>` when `T: Unpin`
#[instrument(skip(self), level = "trace")]
fn coerce_maybe_pinned_ref(&self, a: Ty<'tcx>, b: Ty<'tcx>) -> CoerceResult<'tcx> {
let span = self.cause.span;
let Some((a_ty, a_pinnedness, a_mutbl, a_region)) = a.maybe_pinned_ref() else {
span_bug!(span, "expect pinned reference or reference, found {:?}", a);
};
let Some((_b_ty, b_pinnedness, b_mutbl, _b_region)) = b.maybe_pinned_ref() else {
span_bug!(span, "expect pinned reference or reference, found {:?}", b);
};
Copy link
Member

Choose a reason for hiding this comment

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

Something feels a bit off here to me...I like have one function that covers "all the pinned ref" coercions - but simultaneously all the span_bugs here scream this function being ripe for getting misused or logic changing elsewhere but not updated here.

I sort of wonder if everything before let mut coerce = self.unify_and needs to be done in the calling functions, and necessarily bits get passed in.

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'm not sure if I fully get your ideas. I removed the coerce_maybe_pinned_ref and split it into coerce_to_pin_ref and coerce_pin_ref_to_ref.

Theoretically, coerce_pin_ref_to_ref can be merged into coerce_to_ref, but I have no idea how to add an autoref before the autoderef (I mean, the Autoderef iterator handles &mut Pin<&mut T> to &mut T and &Pin<&T> to &T well, but for Pin<&mut T> and Pin<&T>, there need to be an autoref at the beginning). I'm also not sure if doing that would mess up the existing code.

Copy link
Member

Choose a reason for hiding this comment

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

Skimming over, I think your changes look better, but I'm noticing there are test changes, so there are semantic changes i need to think about. Will do a proper review this week.

use ty::Pinnedness::*;
if a_pinnedness == b_pinnedness {
span_bug!(span, "expect different pinnedness, found {:?} and {:?}", a, b);
}

coerce_mutbls(a_mutbl, b_mutbl)?;

let (deref, borrow) = match (a_pinnedness, b_pinnedness) {
(Not, Not) | (Pinned, Pinned) => {
span_bug!(span, "expect different pinnedness, found {:?} and {:?}", a, b)
}
(Pinned, Not) => {
let mutbl = AutoBorrowMutability::new(b_mutbl, AllowTwoPhase::Yes);
(DerefAdjustKind::Pin, AutoBorrow::Ref(mutbl))
}
(Not, Pinned) => (DerefAdjustKind::Builtin, AutoBorrow::Pin(b_mutbl)),
};
let mut coerce = self.unify_and(
// update a with b's pinnedness and mutability since we'll be coercing pinnedness and mutability
match b_pinnedness {
Pinned => Ty::new_pinned_ref(self.tcx, a_region, a_ty, b_mutbl),
Not => Ty::new_ref(self.tcx, a_region, a_ty, b_mutbl),
},
b,
[Adjustment { kind: Adjust::Deref(deref), target: a_ty }],
Adjust::Borrow(borrow),
ForceLeakCheck::No,
)?;

// Create an obligation for `a_ty: Unpin`.
let cause =
self.cause(self.cause.span, ObligationCauseCode::Coercion { source: a, target: b });
let pred = ty::TraitRef::new(
self.tcx,
self.tcx.require_lang_item(hir::LangItem::Unpin, self.cause.span),
[a_ty],
);
let obligation = Obligation::new(self.tcx, cause, self.fcx.param_env, pred);
coerce.obligations.push(obligation);
Ok(coerce)
}

fn coerce_from_fn_pointer(
&self,
a: Ty<'tcx>,
Expand Down
12 changes: 11 additions & 1 deletion compiler/rustc_hir_typeck/src/expr_use_visitor.rs
Original file line number Diff line number Diff line change
Expand Up @@ -734,7 +734,7 @@ impl<'tcx, Cx: TypeInformationCtxt<'tcx>, D: Delegate<'tcx>> ExprUseVisitor<'tcx
self.consume_or_copy(&place_with_id, place_with_id.hir_id);
}

adjustment::Adjust::Deref(DerefAdjustKind::Builtin) => {}
adjustment::Adjust::Deref(DerefAdjustKind::Builtin | DerefAdjustKind::Pin) => {}

// Autoderefs for overloaded Deref calls in fact reference
// their receiver. That is, if we have `(*x)` where `x`
Expand Down Expand Up @@ -798,6 +798,16 @@ impl<'tcx, Cx: TypeInformationCtxt<'tcx>, D: Delegate<'tcx>> ExprUseVisitor<'tcx
ty::BorrowKind::from_mutbl(m),
);
}

adjustment::AutoBorrow::Pin(m) => {
debug!("walk_autoref: expr.hir_id={} base_place={:?}", expr.hir_id, base_place);

self.delegate.borrow_mut().borrow(
base_place,
base_place.hir_id,
ty::BorrowKind::from_mutbl(m),
);
}
}
}

Expand Down
3 changes: 3 additions & 0 deletions compiler/rustc_hir_typeck/src/fn_ctxt/_impl.rs
Original file line number Diff line number Diff line change
Expand Up @@ -279,6 +279,9 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
Adjust::Deref(DerefAdjustKind::Builtin) => {
// FIXME(const_trait_impl): We *could* enforce `&T: [const] Deref` here.
}
Adjust::Deref(DerefAdjustKind::Pin) => {
// FIXME(const_trait_impl): We *could* enforce `Pin<&T>: [const] Deref` here.
}
Adjust::Pointer(_pointer_coercion) => {
// FIXME(const_trait_impl): We should probably enforce these.
}
Expand Down
4 changes: 2 additions & 2 deletions compiler/rustc_lint/src/autorefs.rs
Original file line number Diff line number Diff line change
Expand Up @@ -174,7 +174,7 @@ fn has_implicit_borrow(Adjustment { kind, .. }: &Adjustment<'_>) -> Option<(Muta
Adjust::NeverToAny
| Adjust::Pointer(..)
| Adjust::ReborrowPin(..)
| Adjust::Deref(DerefAdjustKind::Builtin)
| Adjust::Borrow(AutoBorrow::RawPtr(..)) => None,
| Adjust::Deref(DerefAdjustKind::Builtin | DerefAdjustKind::Pin)
| Adjust::Borrow(AutoBorrow::RawPtr(..) | AutoBorrow::Pin(..)) => None,
}
}
5 changes: 5 additions & 0 deletions compiler/rustc_middle/src/ty/adjustment.rs
Original file line number Diff line number Diff line change
Expand Up @@ -105,13 +105,15 @@ pub enum Adjust {
Pointer(PointerCoercion),

/// Take a pinned reference and reborrow as a `Pin<&mut T>` or `Pin<&T>`.
// FIXME(pin_ergonomics): This can be replaced with a `Deref(Pin)` followed by a `Borrow(Pin)`
ReborrowPin(hir::Mutability),
}

#[derive(Copy, Clone, Debug, TyEncodable, TyDecodable, HashStable, TypeFoldable, TypeVisitable)]
pub enum DerefAdjustKind {
Builtin,
Overloaded(OverloadedDeref),
Pin,
}

/// An overloaded autoderef step, representing a `Deref(Mut)::deref(_mut)`
Expand Down Expand Up @@ -196,6 +198,9 @@ pub enum AutoBorrow {

/// Converts from T to *T.
RawPtr(hir::Mutability),

/// Converts from T to Pin<&T>.
Pin(hir::Mutability),
}

/// Information for `CoerceUnsized` impls, storing information we
Expand Down
44 changes: 44 additions & 0 deletions compiler/rustc_mir_build/src/thir/cx/expr.rs
Original file line number Diff line number Diff line change
Expand Up @@ -144,6 +144,19 @@ impl<'tcx> ThirBuildCx<'tcx> {
adjust_span(&mut expr);
ExprKind::Deref { arg: self.thir.exprs.push(expr) }
}
Adjust::Deref(DerefAdjustKind::Pin) => {
adjust_span(&mut expr);
// pointer = ($expr).pointer
let pin_ty = expr.ty.pinned_ty().expect("Deref(Pin) with non-Pin type");
let pointer_target = ExprKind::Field {
lhs: self.thir.exprs.push(expr),
variant_index: FIRST_VARIANT,
name: FieldIdx::ZERO,
};
let expr = Expr { temp_scope_id, ty: pin_ty, span, kind: pointer_target };
// expr = *pointer
ExprKind::Deref { arg: self.thir.exprs.push(expr) }
}
Adjust::Deref(DerefAdjustKind::Overloaded(deref)) => {
// We don't need to do call adjust_span here since
// deref coercions always start with a built-in deref.
Expand Down Expand Up @@ -178,6 +191,37 @@ impl<'tcx> ThirBuildCx<'tcx> {
Adjust::Borrow(AutoBorrow::RawPtr(mutability)) => {
ExprKind::RawBorrow { mutability, arg: self.thir.exprs.push(expr) }
}
Adjust::Borrow(AutoBorrow::Pin(mutbl)) => {
// expr = &pin (mut|const|) arget
let borrow_kind = match mutbl {
hir::Mutability::Mut => BorrowKind::Mut { kind: mir::MutBorrowKind::Default },
hir::Mutability::Not => BorrowKind::Shared,
};
let new_pin_target =
Ty::new_ref(self.tcx, self.tcx.lifetimes.re_erased, expr.ty, mutbl);
let arg = self.thir.exprs.push(expr);
let expr = self.thir.exprs.push(Expr {
temp_scope_id,
ty: new_pin_target,
span,
kind: ExprKind::Borrow { borrow_kind, arg },
});

// kind = Pin { pointer }
let pin_did = self.tcx.require_lang_item(rustc_hir::LangItem::Pin, span);
let args = self.tcx.mk_args(&[new_pin_target.into()]);
let kind = ExprKind::Adt(Box::new(AdtExpr {
adt_def: self.tcx.adt_def(pin_did),
variant_index: FIRST_VARIANT,
args,
fields: Box::new([FieldExpr { name: FieldIdx::ZERO, expr }]),
user_ty: None,
base: AdtExprBase::None,
}));

debug!(?kind);
kind
}
Adjust::ReborrowPin(mutbl) => {
debug!("apply ReborrowPin adjustment");
// Rewrite `$expr` as `Pin { __pointer: &(mut)? *($expr).__pointer }`
Expand Down
Loading
Loading