Skip to content

Simplify some Deref impls. #155536

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

Closed
nnethercote wants to merge 1 commit into from
Closed

Simplify some Deref impls. #155536

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
28 changes: 10 additions & 18 deletions compiler/rustc_abi/src/layout.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -85,11 +85,11 @@ rustc_index::newtype_index! {
// but *not* an encoding of the discriminant (e.g., a tag value).
// See issue #49298 for more details on the need to leave space
// for non-ZST uninhabited data (mostly partial initialization).
fn absent<'a, FieldIdx, VariantIdx, F>(fields: &IndexSlice<FieldIdx, F>) -> bool
fn absent<FieldIdx, VariantIdx, F>(fields: &IndexSlice<FieldIdx, F>) -> bool
where
FieldIdx: Idx,
VariantIdx: Idx,
F: Deref<Target = &'a LayoutData<FieldIdx, VariantIdx>> + fmt::Debug,
F: Deref<Target = LayoutData<FieldIdx, VariantIdx>> + fmt::Debug,
{
let uninhabited = fields.iter().any(|f| f.is_uninhabited());
// We cannot ignore alignment; that might lead us to entirely discard a variant and
Expand Down Expand Up @@ -239,8 +239,7 @@ impl<Cx: HasDataLayout> LayoutCalculator<Cx> {
/// This uses dedicated code instead of [`Self::layout_of_struct_or_enum`], as coroutine
/// fields may be shared between multiple variants (see the [`coroutine`] module for details).
pub fn coroutine<
'a,
F: Deref<Target = &'a LayoutData<FieldIdx, VariantIdx>> + fmt::Debug + Copy,
F: Deref<Target = LayoutData<FieldIdx, VariantIdx>> + fmt::Debug + Copy,
VariantIdx: Idx,
FieldIdx: Idx,
LocalIdx: Idx,
Expand All @@ -263,10 +262,9 @@ impl<Cx: HasDataLayout> LayoutCalculator<Cx> {
}

pub fn univariant<
'a,
FieldIdx: Idx,
VariantIdx: Idx,
F: Deref<Target = &'a LayoutData<FieldIdx, VariantIdx>> + fmt::Debug + Copy,
F: Deref<Target = LayoutData<FieldIdx, VariantIdx>> + fmt::Debug + Copy,
>(
&self,
fields: &IndexSlice<FieldIdx, F>,
Expand Down Expand Up @@ -338,10 +336,9 @@ impl<Cx: HasDataLayout> LayoutCalculator<Cx> {
}

pub fn layout_of_struct_or_enum<
'a,
FieldIdx: Idx,
VariantIdx: Idx,
F: Deref<Target = &'a LayoutData<FieldIdx, VariantIdx>> + fmt::Debug + Copy,
F: Deref<Target = LayoutData<FieldIdx, VariantIdx>> + fmt::Debug + Copy,
>(
&self,
repr: &ReprOptions,
Expand Down Expand Up @@ -394,10 +391,9 @@ impl<Cx: HasDataLayout> LayoutCalculator<Cx> {
}

pub fn layout_of_union<
'a,
FieldIdx: Idx,
VariantIdx: Idx,
F: Deref<Target = &'a LayoutData<FieldIdx, VariantIdx>> + fmt::Debug + Copy,
F: Deref<Target = LayoutData<FieldIdx, VariantIdx>> + fmt::Debug + Copy,
>(
&self,
repr: &ReprOptions,
Expand Down Expand Up @@ -520,10 +516,9 @@ impl<Cx: HasDataLayout> LayoutCalculator<Cx> {

/// single-variant enums are just structs, if you think about it
fn layout_of_struct<
'a,
FieldIdx: Idx,
VariantIdx: Idx,
F: Deref<Target = &'a LayoutData<FieldIdx, VariantIdx>> + fmt::Debug + Copy,
F: Deref<Target = LayoutData<FieldIdx, VariantIdx>> + fmt::Debug + Copy,
>(
&self,
repr: &ReprOptions,
Expand Down Expand Up @@ -620,10 +615,9 @@ impl<Cx: HasDataLayout> LayoutCalculator<Cx> {
}

fn layout_of_enum<
'a,
FieldIdx: Idx,
VariantIdx: Idx,
F: Deref<Target = &'a LayoutData<FieldIdx, VariantIdx>> + fmt::Debug + Copy,
F: Deref<Target = LayoutData<FieldIdx, VariantIdx>> + fmt::Debug + Copy,
>(
&self,
repr: &ReprOptions,
Expand Down Expand Up @@ -1158,10 +1152,9 @@ impl<Cx: HasDataLayout> LayoutCalculator<Cx> {
}

fn univariant_biased<
'a,
FieldIdx: Idx,
VariantIdx: Idx,
F: Deref<Target = &'a LayoutData<FieldIdx, VariantIdx>> + fmt::Debug + Copy,
F: Deref<Target = LayoutData<FieldIdx, VariantIdx>> + fmt::Debug + Copy,
>(
&self,
fields: &IndexSlice<FieldIdx, F>,
Expand Down Expand Up @@ -1499,10 +1492,9 @@ impl<Cx: HasDataLayout> LayoutCalculator<Cx> {
}

fn format_field_niches<
'a,
FieldIdx: Idx,
VariantIdx: Idx,
F: Deref<Target = &'a LayoutData<FieldIdx, VariantIdx>> + fmt::Debug,
F: Deref<Target = LayoutData<FieldIdx, VariantIdx>> + fmt::Debug,
>(
&self,
layout: &LayoutData<FieldIdx, VariantIdx>,
Expand Down
3 changes: 1 addition & 2 deletions compiler/rustc_abi/src/layout/coroutine.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -137,8 +137,7 @@ fn coroutine_saved_local_eligibility<VariantIdx: Idx, FieldIdx: Idx, LocalIdx: I

/// Compute the full coroutine layout.
pub(super) fn layout<
'a,
F: core::ops::Deref<Target = &'a LayoutData<FieldIdx, VariantIdx>> + core::fmt::Debug + Copy,
F: core::ops::Deref<Target = LayoutData<FieldIdx, VariantIdx>> + core::fmt::Debug + Copy,
VariantIdx: Idx,
FieldIdx: Idx,
LocalIdx: Idx,
Expand Down
12 changes: 6 additions & 6 deletions compiler/rustc_abi/src/layout/ty.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,9 +24,9 @@ impl<'a> fmt::Debug for Layout<'a> {
}

impl<'a> Deref for Layout<'a> {
type Target = &'a LayoutData<FieldIdx, VariantIdx>;
fn deref(&self) -> &&'a LayoutData<FieldIdx, VariantIdx> {
&self.0.0
type Target = LayoutData<FieldIdx, VariantIdx>;
fn deref(&self) -> &Self::Target {
Copy link
Copy Markdown
Member

@RalfJung RalfJung Apr 20, 2026 •
edited by rustbot
Loading

Choose a reason for hiding this comment

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

This does make the type strictly weaker -- previously one could copy out the inner, longer-lived shared reference to get a 'a reference; now that information is lost. I am not sure I agree with this being an improvement, it seems more like a step backwards to me?

View changes since the review

self.0.0
}
}

Expand Down Expand Up @@ -88,9 +88,9 @@ impl<'a, Ty: fmt::Display> fmt::Debug for TyAndLayout<'a, Ty> {
}

impl<'a, Ty> Deref for TyAndLayout<'a, Ty> {
type Target = &'a LayoutData<FieldIdx, VariantIdx>;
fn deref(&self) -> &&'a LayoutData<FieldIdx, VariantIdx> {
&self.layout.0.0
type Target = LayoutData<FieldIdx, VariantIdx>;
fn deref(&self) -> &Self::Target {
self.layout.0.0
}
}

Expand Down
3 changes: 2 additions & 1 deletion compiler/rustc_const_eval/src/interpret/discriminant.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,8 @@ impl<'tcx, M: Machine<'tcx>> InterpCx<'tcx, M> {
// declared list of variants -- they can differ with explicitly assigned discriminants.
// We use "tag" to refer to how the discriminant is encoded in memory, which can be either
// straight-forward (`TagEncoding::Direct`) or with a niche (`TagEncoding::Niche`).
let (tag_scalar_layout, tag_encoding, tag_field) = match op.layout().variants {
let layout = op.layout();
let (tag_scalar_layout, tag_encoding, tag_field) = match layout.variants {
Copy link
Copy Markdown
Member

@RalfJung RalfJung Apr 20, 2026 •
edited by rustbot
Loading

Choose a reason for hiding this comment

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

Looks like here it doesn't actually simplify things, it forces us to manually spell out the temporary?

View changes since the review

Copy link
Copy Markdown
Contributor Author

@nnethercote nnethercote Apr 20, 2026

Choose a reason for hiding this comment

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

The trade-off is that it's arguably worth having a vanilla Deref impl if it only causes one tiny piece of additional friction elsewhere.

Copy link
Copy Markdown
Contributor

@mejrs mejrs Apr 20, 2026

Choose a reason for hiding this comment

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

It doesn't look that bad to me. If I were to write this function, I'd probably hoist it regardless:

let layout = op.layout();
let ty = layout.ty;

and then op.layout() -> layout throughout this function.

Copy link
Copy Markdown
Member

@RalfJung RalfJung Apr 20, 2026 •
edited
Loading

Choose a reason for hiding this comment

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

I agree the cost is small, but I also don't see any downside to the current Deref impls. They are a bit non-standard, sure, but... so what?

Variants::Empty => {
throw_ub!(UninhabitedEnumVariantRead(None));
}
Expand Down
4 changes: 2 additions & 2 deletions compiler/rustc_hir_typeck/src/fn_ctxt/checks.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3074,10 +3074,10 @@ struct CallCtxt<'a, 'b, 'tcx> {
}

impl<'a, 'b, 'tcx> Deref for CallCtxt<'a, 'b, 'tcx> {
Copy link
Copy Markdown
Contributor

@mejrs mejrs Apr 20, 2026

Choose a reason for hiding this comment

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

These can be changed to placeholder lifetimes:

Suggested change
impl<'a, 'b, 'tcx> Deref for CallCtxt<'a, 'b, 'tcx> {
impl<'b, 'tcx> Deref for CallCtxt<'_, 'b, 'tcx> {

Same for the other Deref impls you touched.

Copy link
Copy Markdown
Contributor Author

@nnethercote nnethercote Apr 20, 2026

Choose a reason for hiding this comment

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

In #155549 I eliminaetd one of the lifetimes anyway.

type Target = &'a FnCtxt<'b, 'tcx>;
type Target = FnCtxt<'b, 'tcx>;

fn deref(&self) -> &Self::Target {
&self.fn_ctxt
self.fn_ctxt
}
}

Expand Down
14 changes: 7 additions & 7 deletions compiler/rustc_middle/src/ty/consts/valtree.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -63,11 +63,11 @@ impl<'tcx> ValTree<'tcx> {
}

impl<'tcx> Deref for ValTree<'tcx> {
type Target = &'tcx ty::ValTreeKind<TyCtxt<'tcx>>;
type Target = ty::ValTreeKind<TyCtxt<'tcx>>;

#[inline]
fn deref(&self) -> &&'tcx ty::ValTreeKind<TyCtxt<'tcx>> {
&self.0.0
fn deref(&self) -> &Self::Target {
self.0.0
}
}

Expand Down Expand Up @@ -168,7 +168,7 @@ impl<'tcx> Value<'tcx> {
/// if this constant is some other kind.
#[inline]
pub fn to_leaf(self) -> ScalarInt {
match &**self.valtree {
match &*self.valtree {
ValTreeKind::Leaf(s) => *s,
ValTreeKind::Branch(..) => bug!("expected leaf, got {:?}", self),
}
Expand All @@ -178,15 +178,15 @@ impl<'tcx> Value<'tcx> {
/// if this constant is some other kind.
#[inline]
pub fn to_branch(self) -> &'tcx [ty::Const<'tcx>] {
match &**self.valtree {
match &*self.valtree {
ValTreeKind::Branch(branch) => &**branch,
ValTreeKind::Leaf(..) => bug!("expected branch, got {:?}", self),
}
}

/// Attempts to convert to a `ValTreeKind::Leaf` value.
pub fn try_to_leaf(self) -> Option<ScalarInt> {
match &**self.valtree {
match &*self.valtree {
ValTreeKind::Leaf(s) => Some(*s),
ValTreeKind::Branch(_) => None,
}
Expand All @@ -199,7 +199,7 @@ impl<'tcx> Value<'tcx> {

/// Attempts to convert to a `ValTreeKind::Branch` value.
pub fn try_to_branch(self) -> Option<&'tcx [ty::Const<'tcx>]> {
match &**self.valtree {
match &*self.valtree {
ValTreeKind::Branch(branch) => Some(&**branch),
ValTreeKind::Leaf(_) => None,
}
Expand Down
4 changes: 2 additions & 2 deletions compiler/rustc_middle/src/ty/print/pretty.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2005,10 +2005,10 @@ pub trait PrettyPrinter<'tcx>: Printer<'tcx> + fmt::Write {
}
(ty::ValTreeKind::Leaf(leaf), ty::Ref(_, inner_ty, _)) => {
write!(self, "&")?;
return self.pretty_print_const_scalar_int(*leaf, inner_ty, print_ty);
return self.pretty_print_const_scalar_int(leaf, inner_ty, print_ty);
}
(ty::ValTreeKind::Leaf(leaf), _) => {
return self.pretty_print_const_scalar_int(*leaf, cv.ty, print_ty);
return self.pretty_print_const_scalar_int(leaf, cv.ty, print_ty);
}
(_, ty::FnDef(def_id, args)) => {
// Never allowed today, but we still encounter them in invalid const args.
Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_mir_build/src/builder/matches/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2947,7 +2947,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
if branch.len() != 1 {
bug!("malformed valtree for an enum")
};
let ValTreeKind::Leaf(actual_variant_idx) = **branch[0].to_value().valtree else {
let ValTreeKind::Leaf(actual_variant_idx) = *branch[0].to_value().valtree else {
bug!("malformed valtree for an enum")
};

Expand Down
Loading