Skip to content

refactor: Fold hygiene map into bindings themselves #19655

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.

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
Apr 21, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
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
14 changes: 1 addition & 13 deletions crates/hir-def/src/expr_store.rs
Original file line number Diff line number Diff line change
Expand Up @@ -98,14 +98,6 @@ pub struct ExpressionStore {
/// Block expressions in this store that may contain inner items.
block_scopes: Box<[BlockId]>,

/// A map from binding to its hygiene ID.
///
/// Bindings that don't come from macro expansion are not allocated to save space, so not all bindings appear here.
/// If a binding does not appear here it has `SyntaxContextId::ROOT`.
///
/// Note that this may not be the direct `SyntaxContextId` of the binding's expansion, because transparent
/// expansions are attributed to their parent expansion (recursively).
binding_hygiene: FxHashMap<BindingId, HygieneId>,
/// A map from an variable usages to their hygiene ID.
///
/// Expressions (and destructuing patterns) that can be recorded here are single segment path, although not all single segments path refer
Expand Down Expand Up @@ -155,7 +147,6 @@ pub struct ExpressionStoreBuilder {
pub binding_owners: FxHashMap<BindingId, ExprId>,
pub types: Arena<TypeRef>,
block_scopes: Vec<BlockId>,
binding_hygiene: FxHashMap<BindingId, HygieneId>,
ident_hygiene: FxHashMap<ExprOrPatId, HygieneId>,
}

Expand Down Expand Up @@ -192,7 +183,6 @@ impl ExpressionStoreBuilder {
mut pats,
mut bindings,
mut binding_owners,
mut binding_hygiene,
mut ident_hygiene,
mut types,
} = self;
Expand All @@ -201,7 +191,6 @@ impl ExpressionStoreBuilder {
pats.shrink_to_fit();
bindings.shrink_to_fit();
binding_owners.shrink_to_fit();
binding_hygiene.shrink_to_fit();
ident_hygiene.shrink_to_fit();
types.shrink_to_fit();

Expand All @@ -213,7 +202,6 @@ impl ExpressionStoreBuilder {
binding_owners,
types,
block_scopes: block_scopes.into_boxed_slice(),
binding_hygiene,
ident_hygiene,
}
}
Expand Down Expand Up @@ -556,7 +544,7 @@ impl ExpressionStore {
}

fn binding_hygiene(&self, binding: BindingId) -> HygieneId {
self.binding_hygiene.get(&binding).copied().unwrap_or(HygieneId::ROOT)
self.bindings[binding].hygiene
}

pub fn expr_path_hygiene(&self, expr: ExprId) -> HygieneId {
Expand Down
50 changes: 30 additions & 20 deletions crates/hir-def/src/expr_store/lower.rs
Original file line number Diff line number Diff line change
Expand Up @@ -104,9 +104,14 @@ pub(super) fn lower_body(
{
let is_mutable =
self_param_syn.mut_token().is_some() && self_param_syn.amp_token().is_none();
let hygiene = self_param_syn
.name()
.map(|name| collector.hygiene_id_for(name.syntax().text_range()))
.unwrap_or(HygieneId::ROOT);
let binding_id: la_arena::Idx<Binding> = collector.alloc_binding(
Name::new_symbol_root(sym::self_),
BindingAnnotation::new(is_mutable, false),
hygiene,
);
self_param = Some(binding_id);
source_map_self_param =
Expand Down Expand Up @@ -136,17 +141,15 @@ pub(super) fn lower_body(
{
let is_mutable =
self_param_syn.mut_token().is_some() && self_param_syn.amp_token().is_none();
let binding_id: la_arena::Idx<Binding> = collector.alloc_binding(
Name::new_symbol_root(sym::self_),
BindingAnnotation::new(is_mutable, false),
);
let hygiene = self_param_syn
.name()
.map(|name| collector.hygiene_id_for(name.syntax().text_range()))
.unwrap_or(HygieneId::ROOT);
if !hygiene.is_root() {
collector.store.binding_hygiene.insert(binding_id, hygiene);
}
let binding_id: la_arena::Idx<Binding> = collector.alloc_binding(
Name::new_symbol_root(sym::self_),
BindingAnnotation::new(is_mutable, false),
hygiene,
);
self_param = Some(binding_id);
source_map_self_param = Some(collector.expander.in_file(AstPtr::new(&self_param_syn)));
}
Expand Down Expand Up @@ -486,13 +489,10 @@ impl BindingList {
hygiene: HygieneId,
mode: BindingAnnotation,
) -> BindingId {
let id = *self.map.entry((name, hygiene)).or_insert_with_key(|(name, _)| {
let id = ec.alloc_binding(name.clone(), mode);
if !hygiene.is_root() {
ec.store.binding_hygiene.insert(id, hygiene);
}
id
});
let id = *self
.map
.entry((name, hygiene))
.or_insert_with_key(|(name, hygiene)| ec.alloc_binding(name.clone(), mode, *hygiene));
if ec.store.bindings[id].mode != mode {
ec.store.bindings[id].problems = Some(BindingProblems::BoundInconsistently);
}
Expand Down Expand Up @@ -1770,7 +1770,8 @@ impl ExprCollector<'_> {
);
let loop_outer = self
.alloc_expr(Expr::Loop { body: loop_inner, label: label.map(|it| it.1) }, syntax_ptr);
let iter_binding = self.alloc_binding(iter_name, BindingAnnotation::Mutable);
let iter_binding =
self.alloc_binding(iter_name, BindingAnnotation::Mutable, HygieneId::ROOT);
let iter_pat = self.alloc_pat_desugared(Pat::Bind { id: iter_binding, subpat: None });
self.add_definition_to_binding(iter_binding, iter_pat);
self.alloc_expr(
Expand Down Expand Up @@ -1803,8 +1804,11 @@ impl ExprCollector<'_> {
let expr = self
.alloc_expr(Expr::Call { callee: try_branch, args: Box::new([operand]) }, syntax_ptr);
let continue_name = Name::generate_new_name(self.store.bindings.len());
let continue_binding =
self.alloc_binding(continue_name.clone(), BindingAnnotation::Unannotated);
let continue_binding = self.alloc_binding(
continue_name.clone(),
BindingAnnotation::Unannotated,
HygieneId::ROOT,
);
let continue_bpat =
self.alloc_pat_desugared(Pat::Bind { id: continue_binding, subpat: None });
self.add_definition_to_binding(continue_binding, continue_bpat);
Expand All @@ -1818,7 +1822,8 @@ impl ExprCollector<'_> {
expr: self.alloc_expr(Expr::Path(Path::from(continue_name)), syntax_ptr),
};
let break_name = Name::generate_new_name(self.store.bindings.len());
let break_binding = self.alloc_binding(break_name.clone(), BindingAnnotation::Unannotated);
let break_binding =
self.alloc_binding(break_name.clone(), BindingAnnotation::Unannotated, HygieneId::ROOT);
let break_bpat = self.alloc_pat_desugared(Pat::Bind { id: break_binding, subpat: None });
self.add_definition_to_binding(break_binding, break_bpat);
let break_arm = MatchArm {
Expand Down Expand Up @@ -3137,8 +3142,13 @@ impl ExprCollector<'_> {
self.alloc_expr_desugared(Expr::Missing)
}

fn alloc_binding(&mut self, name: Name, mode: BindingAnnotation) -> BindingId {
let binding = self.store.bindings.alloc(Binding { name, mode, problems: None });
fn alloc_binding(
&mut self,
name: Name,
mode: BindingAnnotation,
hygiene: HygieneId,
) -> BindingId {
let binding = self.store.bindings.alloc(Binding { name, mode, problems: None, hygiene });
if let Some(owner) = self.current_binding_owner {
self.store.binding_owners.insert(binding, owner);
}
Expand Down
8 changes: 7 additions & 1 deletion crates/hir-def/src/hir.rs
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,10 @@ use type_ref::TypeRefId;
use crate::{
BlockId,
builtin_type::{BuiltinFloat, BuiltinInt, BuiltinUint},
expr_store::path::{GenericArgs, Path},
expr_store::{
HygieneId,
path::{GenericArgs, Path},
},
type_ref::{Mutability, Rawness},
};

Expand Down Expand Up @@ -552,6 +555,9 @@ pub struct Binding {
pub name: Name,
pub mode: BindingAnnotation,
pub problems: Option<BindingProblems>,
/// Note that this may not be the direct `SyntaxContextId` of the binding's expansion, because transparent
/// expansions are attributed to their parent expansion (recursively).
pub hygiene: HygieneId,
}

#[derive(Debug, Clone, Eq, PartialEq)]
Expand Down