Skip to content

mir_build: Avoid some useless work when visiting "primary" bindings #137465

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 4 commits into from
Mar 17, 2025
Merged

mir_build: Avoid some useless work when visiting "primary" bindings #137465

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
bca5f56
Add a mir-opt test that demonstrates user type annotations
Zalathar Feb 23, 2025
977106a
Simplify handling of `visibility_scope` in `declare_bindings`
Zalathar Feb 16, 2025
7805b46
Split `visit_primary_bindings` into two variants
Zalathar Feb 15, 2025
5434242
Build `UserTypeProjections` lazily when visiting bindings
Zalathar Feb 20, 2025
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
85 changes: 3 additions & 82 deletions compiler/rustc_middle/src/mir/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,8 +33,8 @@ use crate::mir::interpret::{AllocRange, Scalar};
use crate::ty::codec::{TyDecoder, TyEncoder};
use crate::ty::print::{FmtPrinter, Printer, pretty_print_const, with_no_trimmed_paths};
use crate::ty::{
self, AdtDef, GenericArg, GenericArgsRef, Instance, InstanceKind, List, Ty, TyCtxt,
TypeVisitableExt, TypingEnv, UserTypeAnnotationIndex,
self, GenericArg, GenericArgsRef, Instance, InstanceKind, List, Ty, TyCtxt, TypeVisitableExt,
TypingEnv, UserTypeAnnotationIndex,
};

mod basic_blocks;
Expand Down Expand Up @@ -1482,53 +1482,10 @@ pub struct UserTypeProjections {
pub contents: Vec<UserTypeProjection>,
}

impl<'tcx> UserTypeProjections {
pub fn none() -> Self {
UserTypeProjections { contents: vec![] }
}

pub fn is_empty(&self) -> bool {
self.contents.is_empty()
}

impl UserTypeProjections {
pub fn projections(&self) -> impl Iterator<Item = &UserTypeProjection> + ExactSizeIterator {
self.contents.iter()
}

pub fn push_user_type(mut self, base_user_type: UserTypeAnnotationIndex) -> Self {
self.contents.push(UserTypeProjection { base: base_user_type, projs: vec![] });
self
}

fn map_projections(mut self, f: impl FnMut(UserTypeProjection) -> UserTypeProjection) -> Self {
self.contents = self.contents.into_iter().map(f).collect();
self
}

pub fn index(self) -> Self {
self.map_projections(|pat_ty_proj| pat_ty_proj.index())
}

pub fn subslice(self, from: u64, to: u64) -> Self {
self.map_projections(|pat_ty_proj| pat_ty_proj.subslice(from, to))
}

pub fn deref(self) -> Self {
self.map_projections(|pat_ty_proj| pat_ty_proj.deref())
}

pub fn leaf(self, field: FieldIdx) -> Self {
self.map_projections(|pat_ty_proj| pat_ty_proj.leaf(field))
}

pub fn variant(
self,
adt_def: AdtDef<'tcx>,
variant_index: VariantIdx,
field_index: FieldIdx,
) -> Self {
self.map_projections(|pat_ty_proj| pat_ty_proj.variant(adt_def, variant_index, field_index))
}
}

/// Encodes the effect of a user-supplied type annotation on the
Expand All @@ -1553,42 +1510,6 @@ pub struct UserTypeProjection {
pub projs: Vec<ProjectionKind>,
}

impl UserTypeProjection {
pub(crate) fn index(mut self) -> Self {
self.projs.push(ProjectionElem::Index(()));
self
}

pub(crate) fn subslice(mut self, from: u64, to: u64) -> Self {
self.projs.push(ProjectionElem::Subslice { from, to, from_end: true });
self
}

pub(crate) fn deref(mut self) -> Self {
self.projs.push(ProjectionElem::Deref);
self
}

pub(crate) fn leaf(mut self, field: FieldIdx) -> Self {
self.projs.push(ProjectionElem::Field(field, ()));
self
}

pub(crate) fn variant(
mut self,
adt_def: AdtDef<'_>,
variant_index: VariantIdx,
field_index: FieldIdx,
) -> Self {
self.projs.push(ProjectionElem::Downcast(
Some(adt_def.variant(variant_index).name),
variant_index,
));
self.projs.push(ProjectionElem::Field(field_index, ()));
self
}
}

rustc_index::newtype_index! {
#[derive(HashStable)]
#[encodable]
Expand Down
66 changes: 36 additions & 30 deletions 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 @@ -5,6 +5,11 @@
//! This also includes code for pattern bindings in `let` statements and
//! function parameters.

use std::assert_matches::assert_matches;
use std::borrow::Borrow;
use std::mem;
use std::sync::Arc;

use rustc_abi::VariantIdx;
use rustc_data_structures::fx::FxIndexMap;
use rustc_data_structures::stack::ensure_sufficient_stack;
Expand All @@ -19,6 +24,7 @@ use tracing::{debug, instrument};

use crate::builder::ForGuard::{self, OutsideGuard, RefWithinGuard};
use crate::builder::expr::as_place::PlaceBuilder;
use crate::builder::matches::user_ty::ProjectedUserTypesNode;
use crate::builder::scope::DropKind;
use crate::builder::{
BlockAnd, BlockAndExtension, Builder, GuardFrame, GuardFrameLocal, LocalsForNode,
Expand All @@ -27,13 +33,9 @@ use crate::builder::{
// helper functions, broken out by category:
mod match_pair;
mod test;
mod user_ty;
mod util;

use std::assert_matches::assert_matches;
use std::borrow::Borrow;
use std::mem;
use std::sync::Arc;

/// Arguments to [`Builder::then_else_break_inner`] that are usually forwarded
/// to recursive invocations.
#[derive(Clone, Copy)]
Expand Down Expand Up @@ -757,11 +759,12 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
) -> Option<SourceScope> {
self.visit_primary_bindings_special(
pattern,
UserTypeProjections::none(),
&mut |this, name, mode, var, span, ty, user_ty| {
&ProjectedUserTypesNode::None,
&mut |this, name, mode, var, span, ty, user_tys| {
let vis_scope = *visibility_scope
.get_or_insert_with(|| this.new_source_scope(scope_span, LintLevel::Inherited));
let source_info = SourceInfo { span, scope: this.source_scope };
let user_tys = user_tys.build_user_type_projections();

this.declare_binding(
source_info,
Expand All @@ -770,7 +773,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
mode,
var,
ty,
user_ty,
user_tys,
ArmHasGuard(guard.is_some()),
opt_match_place.map(|(x, y)| (x.cloned(), y)),
pattern.span,
Expand Down Expand Up @@ -874,29 +877,29 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
fn visit_primary_bindings_special(
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 couldn't come up with a good name for this method, so I punted and went with something that signals “please read the docs for this weird method instead of just calling it”.

One of the problems with the old name (visit_primary_bindings) was that it sounded like a very normal visitor method, when in fact it was also doing some non-trivial extra stuff, and I think that's how we ended up in a situation where it was being called in other places that didn't need the extra stuff.

&mut self,
pattern: &Pat<'tcx>,
pattern_user_ty: UserTypeProjections,
user_tys: &ProjectedUserTypesNode<'_>,
f: &mut impl FnMut(
&mut Self,
Symbol,
BindingMode,
LocalVarId,
Span,
Ty<'tcx>,
UserTypeProjections,
&ProjectedUserTypesNode<'_>,
),
) {
// Avoid having to write the full method name at each recursive call.
let visit_subpat = |this: &mut Self, subpat, user_tys, f: &mut _| {
let visit_subpat = |this: &mut Self, subpat, user_tys: &_, f: &mut _| {
this.visit_primary_bindings_special(subpat, user_tys, f)
};

match pattern.kind {
PatKind::Binding { name, mode, var, ty, ref subpattern, is_primary, .. } => {
if is_primary {
f(self, name, mode, var, pattern.span, ty, pattern_user_ty.clone());
f(self, name, mode, var, pattern.span, ty, user_tys);
}
if let Some(subpattern) = subpattern.as_ref() {
visit_subpat(self, subpattern, pattern_user_ty, f);
visit_subpat(self, subpattern, user_tys, f);
}
}

Expand All @@ -905,13 +908,13 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
let from = u64::try_from(prefix.len()).unwrap();
let to = u64::try_from(suffix.len()).unwrap();
for subpattern in prefix.iter() {
visit_subpat(self, subpattern, pattern_user_ty.clone().index(), f);
visit_subpat(self, subpattern, &user_tys.index(), f);
}
if let Some(subpattern) = slice {
visit_subpat(self, subpattern, pattern_user_ty.clone().subslice(from, to), f);
visit_subpat(self, subpattern, &user_tys.subslice(from, to), f);
}
for subpattern in suffix.iter() {
visit_subpat(self, subpattern, pattern_user_ty.clone().index(), f);
visit_subpat(self, subpattern, &user_tys.index(), f);
}
}

Expand All @@ -922,11 +925,11 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
| PatKind::Error(_) => {}

PatKind::Deref { ref subpattern } => {
visit_subpat(self, subpattern, pattern_user_ty.deref(), f);
visit_subpat(self, subpattern, &user_tys.deref(), f);
}

PatKind::DerefPattern { ref subpattern, .. } => {
visit_subpat(self, subpattern, UserTypeProjections::none(), f);
visit_subpat(self, subpattern, &ProjectedUserTypesNode::None, f);
}

PatKind::AscribeUserType {
Expand All @@ -942,28 +945,31 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
// Note that the variance doesn't apply here, as we are tracking the effect
// of `user_ty` on any bindings contained with subpattern.

// Caution: Pushing this user type here is load-bearing even for
// patterns containing no bindings, to ensure that the type ends
// up represented in MIR _somewhere_.
let base_user_ty = self.canonical_user_type_annotations.push(annotation.clone());
let subpattern_user_ty = pattern_user_ty.push_user_type(base_user_ty);
visit_subpat(self, subpattern, subpattern_user_ty, f)
let subpattern_user_tys = user_tys.push_user_type(base_user_ty);
visit_subpat(self, subpattern, &subpattern_user_tys, f)
}

PatKind::ExpandedConstant { ref subpattern, .. } => {
visit_subpat(self, subpattern, pattern_user_ty, f)
visit_subpat(self, subpattern, user_tys, f)
}

PatKind::Leaf { ref subpatterns } => {
for subpattern in subpatterns {
let subpattern_user_ty = pattern_user_ty.clone().leaf(subpattern.field);
debug!("visit_primary_bindings: subpattern_user_ty={:?}", subpattern_user_ty);
visit_subpat(self, &subpattern.pattern, subpattern_user_ty, f);
let subpattern_user_tys = user_tys.leaf(subpattern.field);
debug!("visit_primary_bindings: subpattern_user_tys={subpattern_user_tys:?}");
visit_subpat(self, &subpattern.pattern, &subpattern_user_tys, f);
}
}

PatKind::Variant { adt_def, args: _, variant_index, ref subpatterns } => {
for subpattern in subpatterns {
let subpattern_user_ty =
pattern_user_ty.clone().variant(adt_def, variant_index, subpattern.field);
visit_subpat(self, &subpattern.pattern, subpattern_user_ty, f);
let subpattern_user_tys =
user_tys.variant(adt_def, variant_index, subpattern.field);
visit_subpat(self, &subpattern.pattern, &subpattern_user_tys, f);
}
}
PatKind::Or { ref pats } => {
Expand All @@ -972,7 +978,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
// `let (x | y) = ...`, the primary binding of `y` occurs in
// the right subpattern
for subpattern in pats.iter() {
visit_subpat(self, subpattern, pattern_user_ty.clone(), f);
visit_subpat(self, subpattern, user_tys, f);
}
}
}
Expand Down Expand Up @@ -2764,7 +2770,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
mode: BindingMode,
var_id: LocalVarId,
var_ty: Ty<'tcx>,
user_ty: UserTypeProjections,
user_ty: Option<Box<UserTypeProjections>>,
has_guard: ArmHasGuard,
opt_match_place: Option<(Option<Place<'tcx>>, Span)>,
pat_span: Span,
Expand All @@ -2774,7 +2780,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
let local = LocalDecl {
mutability: mode.1,
ty: var_ty,
user_ty: if user_ty.is_empty() { None } else { Some(Box::new(user_ty)) },
user_ty,
source_info,
local_info: ClearCrossCrate::Set(Box::new(LocalInfo::User(BindingForm::Var(
VarBindingForm {
Expand Down
Loading
Loading