Skip to content

Commit

Permalink
Auto merge of rust-lang#125915 - camelid:const-arg-refactor, r=BoxyUwU
Browse files Browse the repository at this point in the history 
Represent type-level consts with new-and-improved `hir::ConstArg`

### Summary

This is a step toward `min_generic_const_exprs`. We now represent all const
generic arguments using an enum that differentiates between const *paths*
(temporarily just bare const params) and arbitrary anon consts that may perform
computations. This will enable us to cleanly implement the `min_generic_const_args`
plan of allowing the use of generics in paths used as const args, while
disallowing their use in arbitrary anon consts. Here is a summary of the salient
aspects of this change:

- Add `current_def_id_parent` to `LoweringContext`

  This is needed to track anon const parents properly once we implement
  `ConstArgKind::Path` (which requires moving anon const def-creation
  outside of `DefCollector`).

- Create `hir::ConstArgKind` enum with `Path` and `Anon` variants. Use it in the
  existing `hir::ConstArg` struct, replacing the previous `hir::AnonConst` field.

- Use `ConstArg` for all instances of const args. Specifically, use it instead
  of `AnonConst` for assoc item constraints, array lengths, and const param
  defaults.

- Some `ast::AnonConst`s now have their `DefId`s created in
  rustc_ast_lowering rather than `DefCollector`. This is because in some
  cases they will end up becoming a `ConstArgKind::Path` instead, which
  has no `DefId`. We have to solve this in a hacky way where we guess
  whether the `AnonConst` could end up as a path const since we can't
  know for sure until after name resolution (`N` could refer to a free
  const or a nullary struct). If it has no chance as being a const
  param, then we create a `DefId` in `DefCollector` -- otherwise we
  decide during ast_lowering. This will have to be updated once all path
  consts use `ConstArgKind::Path`.

- We explicitly use `ConstArgHasType` for array lengths, rather than
  implicitly relying on anon const type feeding -- this is due to the
  addition of `ConstArgKind::Path`.

- Some tests have their outputs changed, but the changes are for the
  most part minor (including removing duplicate or almost-duplicate
  errors). One test now ICEs, but it is for an incomplete, unstable
  feature and is now tracked at rust-lang#127009.

### Followup items post-merge

- Use `ConstArgKind::Path` for all const paths, not just const params.
- Fix (no github dont close this issue) rust-lang#127009
- If a path in generic args doesn't resolve as a type, try to resolve as a const
  instead (do this in rustc_resolve). Then remove the special-casing from
  `rustc_ast_lowering`, so that all params will automatically be lowered as
  `ConstArgKind::Path`.
- (?) Consider making `const_evaluatable_unchecked` a hard error, or at least
  trying it in crater

r? `@BoxyUwU`
  • Loading branch information
@bors
bors committed Jul 19, 2024
2 parents 3d68afc + c8457e6 commit 8c3a94a
Show file tree
Hide file tree
Showing 72 changed files with 850 additions and 540 deletions.
23 changes: 13 additions & 10 deletions compiler/rustc_ast_lowering/src/asm.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -187,7 +187,7 @@ impl<'a, 'hir> LoweringContext<'a, 'hir> {
.emit();
}
hir::InlineAsmOperand::Const {
anon_const: self.lower_anon_const(anon_const),
anon_const: self.lower_anon_const_to_anon_const(anon_const),
}
}
InlineAsmOperand::Sym { sym } => {
Expand Down Expand Up @@ -222,18 +222,21 @@ impl<'a, 'hir> LoweringContext<'a, 'hir> {
};

// Wrap the expression in an AnonConst.
let parent_def_id = self.current_hir_id_owner;
let parent_def_id = self.current_def_id_parent;
let node_id = self.next_node_id();
self.create_def(
parent_def_id.def_id,
node_id,
kw::Empty,
DefKind::AnonConst,
*op_sp,
);
// HACK(min_generic_const_args): see lower_anon_const
if !expr.is_potential_trivial_const_arg() {
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(&anon_const),
anon_const: self.lower_anon_const_to_anon_const(&anon_const),
}
}
}
Expand Down
135 changes: 73 additions & 62 deletions compiler/rustc_ast_lowering/src/expr.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -75,10 +75,15 @@ impl<'hir> LoweringContext<'_, 'hir> {
let kind = match &e.kind {
ExprKind::Array(exprs) => hir::ExprKind::Array(self.lower_exprs(exprs)),
ExprKind::ConstBlock(c) => {
let c = self.with_new_scopes(c.value.span, |this| hir::ConstBlock {
def_id: this.local_def_id(c.id),
hir_id: this.lower_node_id(c.id),
body: this.lower_const_body(c.value.span, Some(&c.value)),
let c = self.with_new_scopes(c.value.span, |this| {
let def_id = this.local_def_id(c.id);
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))
}),
}
});
hir::ExprKind::ConstBlock(c)
}
Expand Down Expand Up @@ -377,17 +382,14 @@ impl<'hir> LoweringContext<'_, 'hir> {
let mut generic_args = ThinVec::new();
for (idx, arg) in args.into_iter().enumerate() {
if legacy_args_idx.contains(&idx) {
let parent_def_id = self.current_hir_id_owner;
let parent_def_id = self.current_def_id_parent;
let node_id = self.next_node_id();

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

let anon_const = AnonConst { id: node_id, value: arg };
generic_args.push(AngleBracketedArg::Arg(GenericArg::Const(anon_const)));
Expand Down Expand Up @@ -622,6 +624,7 @@ impl<'hir> LoweringContext<'_, 'hir> {
coroutine_source: hir::CoroutineSource,
body: impl FnOnce(&mut Self) -> hir::Expr<'hir>,
) -> hir::ExprKind<'hir> {
let closure_def_id = self.local_def_id(closure_node_id);
let coroutine_kind = hir::CoroutineKind::Desugared(desugaring_kind, coroutine_source);

// The `async` desugaring takes a resume argument and maintains a `task_context`,
Expand Down Expand Up @@ -672,22 +675,24 @@ impl<'hir> LoweringContext<'_, 'hir> {
lifetime_elision_allowed: false,
});

let body = self.lower_body(move |this| {
this.coroutine_kind = Some(coroutine_kind);
let body = self.with_def_id_parent(closure_def_id, move |this| {
this.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> }`:
hir::ExprKind::Closure(self.arena.alloc(hir::Closure {
def_id: self.local_def_id(closure_node_id),
def_id: closure_def_id,
binder: hir::ClosureBinder::Default,
capture_clause,
bound_generic_params: &[],
Expand Down Expand Up @@ -966,35 +971,38 @@ impl<'hir> LoweringContext<'_, 'hir> {
fn_decl_span: Span,
fn_arg_span: Span,
) -> hir::ExprKind<'hir> {
let closure_def_id = self.local_def_id(closure_id);
let (binder_clause, generic_params) = self.lower_closure_binder(binder);

let (body_id, closure_kind) = self.with_new_scopes(fn_decl_span, 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)
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 bound_generic_params = self.lower_lifetime_binder(closure_id, generic_params);
// Lower outside new scope to preserve `is_in_loop_condition`.
let fn_decl = self.lower_fn_decl(decl, closure_id, fn_decl_span, FnDeclKind::Closure, None);

let c = self.arena.alloc(hir::Closure {
def_id: self.local_def_id(closure_id),
def_id: closure_def_id,
binder: binder_clause,
capture_clause,
bound_generic_params,
Expand Down Expand Up @@ -1066,6 +1074,7 @@ impl<'hir> LoweringContext<'_, 'hir> {
fn_decl_span: Span,
fn_arg_span: Span,
) -> hir::ExprKind<'hir> {
let closure_def_id = self.local_def_id(closure_id);
let (binder_clause, generic_params) = self.lower_closure_binder(binder);

assert_matches!(
Expand All @@ -1075,27 +1084,29 @@ impl<'hir> LoweringContext<'_, 'hir> {
);

let body = self.with_new_scopes(fn_decl_span, |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,
);
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 hir_id = this.lower_node_id(coroutine_kind.closure_id());
this.maybe_forward_track_caller(body.span, closure_hir_id, hir_id);
let hir_id = this.lower_node_id(coroutine_kind.closure_id());
this.maybe_forward_track_caller(body.span, closure_hir_id, hir_id);

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

let bound_generic_params = self.lower_lifetime_binder(closure_id, generic_params);
Expand All @@ -1106,7 +1117,7 @@ impl<'hir> LoweringContext<'_, 'hir> {
self.lower_fn_decl(&decl, closure_id, fn_decl_span, FnDeclKind::Closure, None);

let c = self.arena.alloc(hir::Closure {
def_id: self.local_def_id(closure_id),
def_id: closure_def_id,
binder: binder_clause,
capture_clause,
bound_generic_params,
Expand Down
12 changes: 11 additions & 1 deletion compiler/rustc_ast_lowering/src/index.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -181,7 +181,7 @@ impl<'a, 'hir> Visitor<'hir> for NodeCollector<'a, 'hir> {
intravisit::walk_generic_param(self, param);
}

fn visit_const_param_default(&mut self, param: HirId, ct: &'hir AnonConst) {
fn visit_const_param_default(&mut self, param: HirId, ct: &'hir ConstArg<'hir>) {
self.with_parent(param, |this| {
intravisit::walk_const_param_default(this, ct);
})
Expand Down Expand Up @@ -229,6 +229,7 @@ impl<'a, 'hir> Visitor<'hir> for NodeCollector<'a, 'hir> {
}

fn visit_anon_const(&mut self, constant: &'hir AnonConst) {
// FIXME: use real span?
self.insert(DUMMY_SP, constant.hir_id, Node::AnonConst(constant));

self.with_parent(constant.hir_id, |this| {
Expand All @@ -244,6 +245,15 @@ impl<'a, 'hir> Visitor<'hir> for NodeCollector<'a, 'hir> {
});
}

fn visit_const_arg(&mut self, const_arg: &'hir ConstArg<'hir>) {
// FIXME: use real span?
self.insert(DUMMY_SP, const_arg.hir_id, Node::ConstArg(const_arg));

self.with_parent(const_arg.hir_id, |this| {
intravisit::walk_const_arg(this, const_arg);
});
}

fn visit_expr(&mut self, expr: &'hir Expr<'hir>) {
self.insert(expr.span, expr.hir_id, Node::Expr(expr));

Expand Down
29 changes: 19 additions & 10 deletions compiler/rustc_ast_lowering/src/item.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,10 @@ impl<'a, 'hir> ItemLowerer<'a, 'hir> {

for (def_id, info) in lctx.children {
let owner = self.owners.ensure_contains_elem(def_id, || hir::MaybeOwner::Phantom);
debug_assert!(matches!(owner, hir::MaybeOwner::Phantom));
debug_assert!(
matches!(owner, hir::MaybeOwner::Phantom),
"duplicate copy of {def_id:?} in lctx.children"
);
*owner = info;
}
}
Expand Down Expand Up @@ -713,7 +716,7 @@ impl<'hir> LoweringContext<'_, 'hir> {
hir_id,
def_id: self.local_def_id(v.id),
data: self.lower_variant_data(hir_id, &v.data),
disr_expr: v.disr_expr.as_ref().map(|e| self.lower_anon_const(e)),
disr_expr: v.disr_expr.as_ref().map(|e| self.lower_anon_const_to_anon_const(e)),
ident: self.lower_ident(v.ident),
span: self.lower_span(v.span),
}
Expand Down Expand Up @@ -1601,15 +1604,15 @@ impl<'hir> LoweringContext<'_, 'hir> {

if let Some((span, hir_id, def_id)) = host_param_parts {
let const_node_id = self.next_node_id();
let anon_const =
let anon_const_did =
self.create_def(def_id, const_node_id, kw::Empty, DefKind::AnonConst, span);

let const_id = self.next_id();
let const_expr_id = self.next_id();
let bool_id = self.next_id();

self.children.push((def_id, hir::MaybeOwner::NonOwner(hir_id)));
self.children.push((anon_const, hir::MaybeOwner::NonOwner(const_id)));
self.children.push((anon_const_did, hir::MaybeOwner::NonOwner(const_id)));

let const_body = self.lower_body(|this| {
(
Expand All @@ -1624,6 +1627,17 @@ impl<'hir> LoweringContext<'_, 'hir> {
)
});

let default_ac = self.arena.alloc(hir::AnonConst {
def_id: anon_const_did,
hir_id: const_id,
body: const_body,
span,
});
let default_ct = self.arena.alloc(hir::ConstArg {
hir_id: self.next_id(),
kind: hir::ConstArgKind::Anon(default_ac),
is_desugared_from_effects: false,
});
let param = hir::GenericParam {
def_id,
hir_id,
Expand All @@ -1648,12 +1662,7 @@ impl<'hir> LoweringContext<'_, 'hir> {
)),
)),
// FIXME(effects) we might not need a default.
default: Some(self.arena.alloc(hir::AnonConst {
def_id: anon_const,
hir_id: const_id,
body: const_body,
span,
})),
default: Some(default_ct),
is_host_effect: true,
synthetic: true,
},
Expand Down
Loading

0 comments on commit 8c3a94a

Please sign in to comment.