Skip to content

Commit

Permalink
Auto merge of #133468 - lcnr:uwu4, r=BoxyUwU
Browse files Browse the repository at this point in the history 
always create `DefId`s for anon consts

but don't use them anywhere, we intentionally don't encode them in the crate metadata.

best reviewed by disabling whitespace.

This pretty much reimplements #133285 while adding the tests of #133455. Fixes #133064

r? `@BoxyUwU` `@compiler-errors`
  • Loading branch information
@bors
bors committed Nov 28, 2024
2 parents 9b4d7c6 + d401a07 commit 7e565cc
Show file tree
Hide file tree
Showing 14 changed files with 485 additions and 318 deletions.
7 changes: 4 additions & 3 deletions compiler/rustc_ast/src/ast.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1184,14 +1184,15 @@ pub struct Expr {
}

impl Expr {
/// Is this expr either `N`, or `{ N }`.
/// Could this expr be either `N`, or `{ N }`, where `N` is a const parameter.
///
/// If this is not the case, name resolution does not resolve `N` when using
/// `min_const_generics` as more complex expressions are not supported.
///
/// Does not ensure that the path resolves to a const param, the caller should check this.
pub fn is_potential_trivial_const_arg(&self, strip_identity_block: bool) -> bool {
let this = if strip_identity_block { self.maybe_unwrap_block() } else { self };
/// This also does not consider macros, so it's only correct after macro-expansion.
pub fn is_potential_trivial_const_arg(&self) -> bool {
let this = self.maybe_unwrap_block();

if let ExprKind::Path(None, path) = &this.kind
&& path.is_potential_trivial_const_arg()
Expand Down
19 changes: 8 additions & 11 deletions compiler/rustc_ast_lowering/src/asm.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -227,18 +227,15 @@ impl<'a, 'hir> LoweringContext<'a, 'hir> {
};

// Wrap the expression in an AnonConst.
let parent_def_id = self.current_def_id_parent;
let parent_def_id = self.current_hir_id_owner.def_id;
let node_id = self.next_node_id();
// HACK(min_generic_const_args): see lower_anon_const
if !expr.is_potential_trivial_const_arg(true) {
self.create_def(
parent_def_id,
node_id,
kw::Empty,
DefKind::AnonConst,
*op_sp,
);
}
self.create_def(
parent_def_id,
node_id,
kw::Empty,
DefKind::AnonConst,
*op_sp,
);
let anon_const = AnonConst { id: node_id, value: P(expr) };
hir::InlineAsmOperand::SymFn {
anon_const: self.lower_anon_const_to_anon_const(&anon_const),
Expand Down
110 changes: 48 additions & 62 deletions compiler/rustc_ast_lowering/src/expr.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -109,9 +109,7 @@ impl<'hir> LoweringContext<'_, 'hir> {
hir::ConstBlock {
def_id,
hir_id: this.lower_node_id(c.id),
body: this.with_def_id_parent(def_id, |this| {
this.lower_const_body(c.value.span, Some(&c.value))
}),
body: this.lower_const_body(c.value.span, Some(&c.value)),
}
});
hir::ExprKind::ConstBlock(c)
Expand Down Expand Up @@ -452,15 +450,9 @@ impl<'hir> LoweringContext<'_, 'hir> {
let mut generic_args = ThinVec::new();
for (idx, arg) in args.iter().cloned().enumerate() {
if legacy_args_idx.contains(&idx) {
let parent_def_id = self.current_def_id_parent;
let parent_def_id = self.current_hir_id_owner.def_id;
let node_id = self.next_node_id();

// HACK(min_generic_const_args): see lower_anon_const
if !arg.is_potential_trivial_const_arg(true) {
// Add a definition for the in-band const def.
self.create_def(parent_def_id, node_id, kw::Empty, DefKind::AnonConst, f.span);
}

self.create_def(parent_def_id, node_id, kw::Empty, DefKind::AnonConst, f.span);
let mut visitor = WillCreateDefIdsVisitor {};
let const_value = if let ControlFlow::Break(span) = visitor.visit_expr(&arg) {
AstP(Expr {
Expand Down Expand Up @@ -759,19 +751,17 @@ impl<'hir> LoweringContext<'_, 'hir> {
lifetime_elision_allowed: false,
});

let body = self.with_def_id_parent(closure_def_id, move |this| {
this.lower_body(move |this| {
this.coroutine_kind = Some(coroutine_kind);
let body = self.lower_body(move |this| {
this.coroutine_kind = Some(coroutine_kind);

let old_ctx = this.task_context;
if task_context.is_some() {
this.task_context = task_context;
}
let res = body(this);
this.task_context = old_ctx;
let old_ctx = this.task_context;
if task_context.is_some() {
this.task_context = task_context;
}
let res = body(this);
this.task_context = old_ctx;

(params, res)
})
(params, res)
});

// `static |<_task_context?>| -> <return_ty> { <body> }`:
Expand Down Expand Up @@ -1056,26 +1046,24 @@ impl<'hir> LoweringContext<'_, 'hir> {
let (binder_clause, generic_params) = self.lower_closure_binder(binder);

let (body_id, closure_kind) = self.with_new_scopes(fn_decl_span, move |this| {
this.with_def_id_parent(closure_def_id, move |this| {
let mut coroutine_kind = if this
.attrs
.get(&closure_hir_id.local_id)
.is_some_and(|attrs| attrs.iter().any(|attr| attr.has_name(sym::coroutine)))
{
Some(hir::CoroutineKind::Coroutine(Movability::Movable))
} else {
None
};
let body_id = this.lower_fn_body(decl, |this| {
this.coroutine_kind = coroutine_kind;
let e = this.lower_expr_mut(body);
coroutine_kind = this.coroutine_kind;
e
});
let coroutine_option =
this.closure_movability_for_fn(decl, fn_decl_span, coroutine_kind, movability);
(body_id, coroutine_option)
})
let mut coroutine_kind = if this
.attrs
.get(&closure_hir_id.local_id)
.is_some_and(|attrs| attrs.iter().any(|attr| attr.has_name(sym::coroutine)))
{
Some(hir::CoroutineKind::Coroutine(Movability::Movable))
} else {
None
};
let body_id = this.lower_fn_body(decl, |this| {
this.coroutine_kind = coroutine_kind;
let e = this.lower_expr_mut(body);
coroutine_kind = this.coroutine_kind;
e
});
let coroutine_option =
this.closure_movability_for_fn(decl, fn_decl_span, coroutine_kind, movability);
(body_id, coroutine_option)
});

let bound_generic_params = self.lower_lifetime_binder(closure_id, generic_params);
Expand Down Expand Up @@ -1165,28 +1153,26 @@ impl<'hir> LoweringContext<'_, 'hir> {
);

let body = self.with_new_scopes(fn_decl_span, |this| {
this.with_def_id_parent(closure_def_id, |this| {
let inner_decl =
FnDecl { inputs: decl.inputs.clone(), output: FnRetTy::Default(fn_decl_span) };

// Transform `async |x: u8| -> X { ... }` into
// `|x: u8| || -> X { ... }`.
let body_id = this.lower_body(|this| {
let (parameters, expr) = this.lower_coroutine_body_with_moved_arguments(
&inner_decl,
|this| this.with_new_scopes(fn_decl_span, |this| this.lower_expr_mut(body)),
fn_decl_span,
body.span,
coroutine_kind,
hir::CoroutineSource::Closure,
);
let inner_decl =
FnDecl { inputs: decl.inputs.clone(), output: FnRetTy::Default(fn_decl_span) };

// Transform `async |x: u8| -> X { ... }` into
// `|x: u8| || -> X { ... }`.
let body_id = this.lower_body(|this| {
let (parameters, expr) = this.lower_coroutine_body_with_moved_arguments(
&inner_decl,
|this| this.with_new_scopes(fn_decl_span, |this| this.lower_expr_mut(body)),
fn_decl_span,
body.span,
coroutine_kind,
hir::CoroutineSource::Closure,
);

this.maybe_forward_track_caller(body.span, closure_hir_id, expr.hir_id);
this.maybe_forward_track_caller(body.span, closure_hir_id, expr.hir_id);

(parameters, expr)
});
body_id
})
(parameters, expr)
});
body_id
});

let bound_generic_params = self.lower_lifetime_binder(closure_id, generic_params);
Expand Down
73 changes: 18 additions & 55 deletions compiler/rustc_ast_lowering/src/lib.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -117,18 +117,6 @@ struct LoweringContext<'a, 'hir> {
is_in_dyn_type: bool,

current_hir_id_owner: hir::OwnerId,
/// Why do we need this in addition to [`Self::current_hir_id_owner`]?
///
/// Currently (as of June 2024), anonymous constants are not HIR owners; however,
/// they do get their own DefIds. Some of these DefIds have to be created during
/// AST lowering, rather than def collection, because we can't tell until after
/// name resolution whether an anonymous constant will end up instead being a
/// [`hir::ConstArgKind::Path`]. However, to compute which generics are
/// available to an anonymous constant nested inside another, we need to make
/// sure that the parent is recorded as the parent anon const, not the enclosing
/// item. So we need to track parent defs differently from HIR owners, since they
/// will be finer-grained in the case of anon consts.
current_def_id_parent: LocalDefId,
item_local_id_counter: hir::ItemLocalId,
trait_map: ItemLocalMap<Box<[TraitCandidate]>>,

Expand Down Expand Up @@ -161,7 +149,6 @@ impl<'a, 'hir> LoweringContext<'a, 'hir> {
attrs: SortedMap::default(),
children: Vec::default(),
current_hir_id_owner: hir::CRATE_OWNER_ID,
current_def_id_parent: CRATE_DEF_ID,
item_local_id_counter: hir::ItemLocalId::ZERO,
ident_and_label_to_local_id: Default::default(),
#[cfg(debug_assertions)]
Expand Down Expand Up @@ -565,7 +552,7 @@ impl<'a, 'hir> LoweringContext<'a, 'hir> {
debug_assert_eq!(_old, None);
}

let item = self.with_def_id_parent(def_id, f);
let item = f(self);
debug_assert_eq!(def_id, item.def_id().def_id);
// `f` should have consumed all the elements in these vectors when constructing `item`.
debug_assert!(self.impl_trait_defs.is_empty());
Expand All @@ -590,13 +577,6 @@ impl<'a, 'hir> LoweringContext<'a, 'hir> {
self.children.push((def_id, hir::MaybeOwner::Owner(info)));
}

fn with_def_id_parent<T>(&mut self, parent: LocalDefId, f: impl FnOnce(&mut Self) -> T) -> T {
let current_def_id_parent = std::mem::replace(&mut self.current_def_id_parent, parent);
let result = f(self);
self.current_def_id_parent = current_def_id_parent;
result
}

fn make_owner_info(&mut self, node: hir::OwnerNode<'hir>) -> &'hir hir::OwnerInfo<'hir> {
let attrs = std::mem::take(&mut self.attrs);
let mut bodies = std::mem::take(&mut self.bodies);
Expand Down Expand Up @@ -773,7 +753,7 @@ impl<'a, 'hir> LoweringContext<'a, 'hir> {
LifetimeRes::Fresh { param, kind, .. } => {
// Late resolution delegates to us the creation of the `LocalDefId`.
let _def_id = self.create_def(
self.current_hir_id_owner.def_id, // FIXME: should this use self.current_def_id_parent?
self.current_hir_id_owner.def_id,
param,
kw::UnderscoreLifetime,
DefKind::LifetimeParam,
Expand Down Expand Up @@ -1466,17 +1446,15 @@ impl<'a, 'hir> LoweringContext<'a, 'hir> {
let opaque_ty_hir_id = self.lower_node_id(opaque_ty_node_id);
debug!(?opaque_ty_def_id, ?opaque_ty_hir_id);

let opaque_ty_def = self.with_def_id_parent(opaque_ty_def_id, |this| {
let bounds = lower_item_bounds(this);
let opaque_ty_def = hir::OpaqueTy {
hir_id: opaque_ty_hir_id,
def_id: opaque_ty_def_id,
bounds,
origin,
span: this.lower_span(opaque_ty_span),
};
this.arena.alloc(opaque_ty_def)
});
let bounds = lower_item_bounds(self);
let opaque_ty_def = hir::OpaqueTy {
hir_id: opaque_ty_hir_id,
def_id: opaque_ty_def_id,
bounds,
origin,
span: self.lower_span(opaque_ty_span),
};
let opaque_ty_def = self.arena.alloc(opaque_ty_def);

hir::TyKind::OpaqueDef(opaque_ty_def)
}
Expand Down Expand Up @@ -2084,7 +2062,7 @@ impl<'a, 'hir> LoweringContext<'a, 'hir> {
} else {
// Construct an AnonConst where the expr is the "ty"'s path.

let parent_def_id = self.current_def_id_parent;
let parent_def_id = self.current_hir_id_owner.def_id;
let node_id = self.next_node_id();
let span = self.lower_span(span);

Expand All @@ -2108,9 +2086,7 @@ impl<'a, 'hir> LoweringContext<'a, 'hir> {
self.arena.alloc(hir::AnonConst {
def_id,
hir_id,
body: this.with_def_id_parent(def_id, |this| {
this.lower_const_body(path_expr.span, Some(&path_expr))
}),
body: this.lower_const_body(path_expr.span, Some(&path_expr)),
span,
})
});
Expand Down Expand Up @@ -2159,7 +2135,10 @@ impl<'a, 'hir> LoweringContext<'a, 'hir> {
None,
);

return ConstArg { hir_id: self.next_id(), kind: hir::ConstArgKind::Path(qpath) };
return ConstArg {
hir_id: self.lower_node_id(anon.id),
kind: hir::ConstArgKind::Path(qpath),
};
}

let lowered_anon = self.lower_anon_const_to_anon_const(anon);
Expand All @@ -2169,29 +2148,13 @@ impl<'a, 'hir> LoweringContext<'a, 'hir> {
/// See [`hir::ConstArg`] for when to use this function vs
/// [`Self::lower_anon_const_to_const_arg`].
fn lower_anon_const_to_anon_const(&mut self, c: &AnonConst) -> &'hir hir::AnonConst {
if c.value.is_potential_trivial_const_arg(true) {
// HACK(min_generic_const_args): see DefCollector::visit_anon_const
// Over there, we guess if this is a bare param and only create a def if
// we think it's not. However we may can guess wrong (see there for example)
// in which case we have to create the def here.
self.create_def(
self.current_def_id_parent,
c.id,
kw::Empty,
DefKind::AnonConst,
c.value.span,
);
}

self.arena.alloc(self.with_new_scopes(c.value.span, |this| {
let def_id = this.local_def_id(c.id);
let hir_id = this.lower_node_id(c.id);
hir::AnonConst {
def_id,
hir_id,
body: this.with_def_id_parent(def_id, |this| {
this.lower_const_body(c.value.span, Some(&c.value))
}),
body: this.lower_const_body(c.value.span, Some(&c.value)),
span: this.lower_span(c.value.span),
}
}))
Expand Down
11 changes: 10 additions & 1 deletion compiler/rustc_hir/src/def.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -109,7 +109,16 @@ pub enum DefKind {
Use,
/// An `extern` block.
ForeignMod,
/// Anonymous constant, e.g. the `1 + 2` in `[u8; 1 + 2]`
/// Anonymous constant, e.g. the `1 + 2` in `[u8; 1 + 2]`.
///
/// Not all anon-consts are actually still relevant in the HIR. We lower
/// trivial const-arguments directly to `hir::ConstArgKind::Path`, at which
/// point the definition for the anon-const ends up unused and incomplete.
///
/// We do not provide any a `Span` for the definition and pretty much all other
/// queries also ICE when using this `DefId`. Given that the `DefId` of such
/// constants should only be reachable by iterating all definitions of a
/// given crate, you should not have to worry about this.
AnonConst,
/// An inline constant, e.g. `const { 1 + 2 }`
InlineConst,
Expand Down
14 changes: 14 additions & 0 deletions compiler/rustc_metadata/src/rmeta/encoder.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1383,6 +1383,20 @@ impl<'a, 'tcx> EncodeContext<'a, 'tcx> {
let def_id = local_id.to_def_id();
let def_kind = tcx.def_kind(local_id);
self.tables.def_kind.set_some(def_id.index, def_kind);

// The `DefCollector` will sometimes create unnecessary `DefId`s
// for trivial const arguments which are directly lowered to
// `ConstArgKind::Path`. We never actually access this `DefId`
// anywhere so we don't need to encode it for other crates.
if def_kind == DefKind::AnonConst
&& matches!(
tcx.hir_node_by_def_id(local_id),
hir::Node::ConstArg(hir::ConstArg { kind: hir::ConstArgKind::Path(..), .. })
)
{
continue;
}

if should_encode_span(def_kind) {
let def_span = tcx.def_span(local_id);
record!(self.tables.def_span[def_id] <- def_span);
Expand Down
Loading

0 comments on commit 7e565cc

Please sign in to comment.