Skip to content

clean up handling of paths in mgca #160216

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

Open
khyperia wants to merge 1 commit into
base: main
Choose a base branch
from
Open

clean up handling of paths in mgca #160216

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
21 changes: 11 additions & 10 deletions compiler/rustc_ast/src/ast.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -141,14 +141,15 @@ impl Path {
self.segments.first().is_some_and(|segment| segment.ident.name == kw::PathRoot)
}

/// Check if this path is potentially a trivial const arg, i.e., one that can _potentially_
/// be represented without an anon const in the HIR.
///
/// Returns true iff the path has exactly one segment, and it has no generic args
/// (i.e., it is _potentially_ a const parameter).
#[tracing::instrument(level = "debug", ret)]
pub fn is_potential_trivial_const_arg(&self) -> bool {
self.segments.len() == 1 && self.segments.iter().all(|seg| seg.args.is_none())
/// Checks if this path is just a simple one-word `PATH` - i.e. the inverse of
/// [`Path::from_ident`]
pub fn is_single_argless_ident(&self) -> bool {
self.segments.len() == 1 && self.segments[0].args.is_none()
}

/// The inverse of [`Path::from_ident`] - if this path is just a simple one-word `PATH`
pub fn as_single_argless_ident(&self) -> Option<Ident> {
self.is_single_argless_ident().then(|| self.segments[0].ident)
}
}

Expand Down Expand Up @@ -1407,7 +1408,7 @@ impl Expr {
/// be represented without an anon const in the HIR.
///
/// This will unwrap at most one block level (curly braces). After that, if the expression
/// is a path, it mostly dispatches to [`Path::is_potential_trivial_const_arg`].
/// is a path, it mostly dispatches to [`Path::is_single_argless_ident`].
///
/// This function will only allow paths with no qself, before dispatching to the `Path`
/// function of the same name.
Expand All @@ -1417,7 +1418,7 @@ impl Expr {
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()
&& path.is_single_argless_ident()
{
true
} else {
Expand Down
186 changes: 101 additions & 85 deletions compiler/rustc_ast_lowering/src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1442,25 +1442,22 @@ impl<'hir> LoweringContext<'_, 'hir> {
// type and value namespaces. If we resolved the path in the value namespace, we
// transform it into a generic const argument.
//
// Note that even under `#![feature(min_generic_const_args)]`, only plain paths
// to constants are allowed - e.g. `A::<T::ASSOC_CONST>` and
// `A::<CONST_WITH_PARAM::<2>>` are disallowed (they must be wrapped in `{ }`).
//
// FIXME: Should we be handling `(PATH_TO_CONST)`?
TyKind::Path(None, path) => {
if let Some(res) = self
.get_partial_res(ty.id)
.and_then(|partial_res| partial_res.full_res())
{
if !res.matches_ns(Namespace::TypeNS)
&& path.is_potential_trivial_const_arg()
{
debug!(
"lower_generic_arg: Lowering type argument as const argument: {:?}",
ty,
);

let ct =
self.lower_const_path_to_const_arg(path, res, ty.id, ty.span);
return GenericArg::Const(ct.try_as_ambig_ct().unwrap());
}
}
TyKind::Path(None, path)
if path.is_single_argless_ident()
&& let Some(res) = self
.get_partial_res(ty.id)
.and_then(|partial_res| partial_res.full_res())
&& !res.matches_ns(Namespace::TypeNS) =>
{
let ct =
self.lower_const_path_to_const_arg(&None, path, res, ty.id, ty.span);
let ct = self.arena.alloc(ct);

@bit-aloo bit-aloo Jul 31, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Curious why we do arena alloc here?

@khyperia khyperia Aug 1, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

  • GenericArg::Const(&'hir ConstArg<..>) is arena'd, so it needs an arena alloc somewhere
  • lower_anon_const_to_const_arg returns ConstArg by value (this has been the case since before I started working on this code)
    • there's a lower_anon_const_to_const_arg_and_alloc wrapper, fwiw
  • which requires lower_expr_to_const_arg_direct to return it by value
  • which requires lower_path_to_const_arg_direct to do the same
  • and then I said shrug, make lower_const_path_to_const_arg do the same, since it's a parallel to lower_anon_const_to_const_arg
  • so we arena alloc in the caller of lower_const_path_to_const_arg, here

there's no particular reason why this whole set of functions (lower_anon_const_to_const_arg/lower_expr_to_const_arg_direct/lower_const_path_to_const_arg/lower_path_to_const_arg_direct) returns by value instead of by &'hir, could go the other way around, but they should all be consistently the same IMO.

@khyperia khyperia Aug 1, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I guess in other words:

the diff adds an arena alloc here, because I removed it from lower_const_path_to_const_arg, because I wanted lower_const_path_to_const_arg to be more parallel to lower_anon_const_to_const_arg: they are conceptually extremely similar functions (take an X, attempt to lower it to a direct const arg, and fall back to anon const if that fails), just X is a path for one, and an AST anon const in the other.

@bit-aloo bit-aloo Aug 2, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Makes sense. Thanks for detailed explanation. 😄

return GenericArg::Const(ct.try_as_ambig_ct().unwrap());
}
TyKind::DirectConstArg(expr)
if self.tcx.features().min_generic_const_args() =>
Expand Down Expand Up @@ -2591,6 +2588,10 @@ impl<'hir> LoweringContext<'_, 'hir> {
fn lower_array_length_to_const_arg(&mut self, c: &AnonConst) -> &'hir hir::ConstArg<'hir> {
// We cannot just match on `ExprKind::Underscore` as `(_)` is represented as
// `ExprKind::Paren(ExprKind::Underscore)` and should also be lowered to `GenericArg::Infer`
//
// FIXME(macroless_generic_const_args): Handling of underscores should be moved into
// lower_expr_to_const_arg_direct. It is left here as retaining compatibility of what is
// currently allowed on stable gets hairy and annoying otherwise.
match c.value.peel_parens().kind {
ExprKind::Underscore => {
let ct_kind = hir::ConstArgKind::Infer(());
Expand All @@ -2610,27 +2611,16 @@ impl<'hir> LoweringContext<'_, 'hir> {
#[instrument(level = "debug", skip(self))]
fn lower_const_path_to_const_arg(
&mut self,
qself: &Option<Box<QSelf>>,
path: &Path,
res: Res<NodeId>,
ty_id: NodeId,
id: NodeId,
span: Span,
) -> &'hir hir::ConstArg<'hir> {
let tcx = self.tcx;

let is_trivial_path = path.is_potential_trivial_const_arg()
&& matches!(res, Res::Def(DefKind::ConstParam, _));
let ct_kind = if is_trivial_path || tcx.features().macroless_generic_const_args() {
let qpath = self.lower_qpath(
ty_id,
&None,
path,
ParamMode::Explicit,
AllowReturnTypeNotation::No,
// FIXME(mgca): update for `fn foo() -> Bar<FOO<impl Trait>>` support
ImplTraitContext::Disallowed(ImplTraitPosition::Path),
None,
);
hir::ConstArgKind::Path(qpath)
) -> hir::ConstArg<'hir> {
let context = self.ambient_direct_const_arg_context();
if self.can_lower_path_to_const_arg_direct(qself, path, span, Some(res), context).is_ok() {
let span = self.lower_span(span);
self.lower_path_to_const_arg_direct(id, None, qself, path, span)
} else {
// Construct an AnonConst where the expr is the "ty"'s path.
let node_id = self.next_node_id();
Expand All @@ -2644,8 +2634,8 @@ impl<'hir> LoweringContext<'_, 'hir> {
let hir_id = self.lower_node_id(node_id);

let path_expr = Expr {
id: ty_id,
kind: ExprKind::Path(None, path.clone()),
id,
kind: ExprKind::Path(qself.clone(), path.clone()),
span,
attrs: AttrVec::new(),
tokens: None,
Expand All @@ -2659,14 +2649,12 @@ impl<'hir> LoweringContext<'_, 'hir> {
span,
})
});
hir::ConstArgKind::Anon(ct)
};

self.arena.alloc(hir::ConstArg {
hir_id: self.next_id(),
kind: ct_kind,
span: self.lower_span(span),
})
hir::ConstArg {
hir_id: self.next_id(),
kind: hir::ConstArgKind::Anon(ct),
span: self.lower_span(span),
}
}
}

fn lower_const_item_rhs(
Expand Down Expand Up @@ -2703,9 +2691,39 @@ impl<'hir> LoweringContext<'_, 'hir> {
}
}

fn ambient_direct_const_arg_context(&self) -> DirectConstArgContext {
if self.tcx.features().macroless_generic_const_args() {
DirectConstArgContext::MacrolessMinGenericConstArgs
} else if self.tcx.features().min_generic_const_args() {
DirectConstArgContext::MinGenericConstArgs
} else {
DirectConstArgContext::Stable
}
}

fn can_lower_path_to_const_arg_direct(
&self,
qself: &Option<Box<QSelf>>,
path: &Path,
span: Span,
res: Option<Res<NodeId>>,
context: DirectConstArgContext,
) -> Result<(), UnrepresentableConstArgError> {
if let DirectConstArgContext::MacrolessMinGenericConstArgs = context {
Ok(())
} else if qself.is_none()
&& path.is_single_argless_ident()
&& matches!(res, Some(Res::Def(DefKind::ConstParam, _)))
{
Ok(())
} else {
Err(UnrepresentableConstArgError { span, will_create_def_ids: false })
}
}

#[instrument(level = "debug", skip(self), ret)]
fn can_lower_expr_to_const_arg_direct(
&mut self,
&self,
expr: &Expr,
context: DirectConstArgContext,
) -> Result<(), UnrepresentableConstArgError> {
Expand All @@ -2727,19 +2745,10 @@ impl<'hir> LoweringContext<'_, 'hir> {
}
Ok(())
}
(ExprKind::Path(_, _), MacrolessMinGenericConstArgs) => Ok(()),
(ExprKind::Path(_, path), _) => {
if path.is_potential_trivial_const_arg()
&& matches!(
self.get_partial_res(expr.id)
.and_then(|partial_res| partial_res.full_res()),
Some(Res::Def(DefKind::ConstParam, _))
)
{
Ok(())
} else {
Err(UnrepresentableConstArgError::new(expr))
}
(ExprKind::Path(qself, path), _) => {
let res =
self.get_partial_res(expr.id).and_then(|partial_res| partial_res.full_res());
self.can_lower_path_to_const_arg_direct(qself, path, expr.span, res, context)
}
(ExprKind::Struct(se), MacrolessMinGenericConstArgs) => {
for f in &se.fields {
Expand All @@ -2754,6 +2763,9 @@ impl<'hir> LoweringContext<'_, 'hir> {
Ok(())
}
(ExprKind::Underscore, MacrolessMinGenericConstArgs) => Ok(()),
(ExprKind::Paren(expr), MacrolessMinGenericConstArgs) => {
self.can_lower_expr_to_const_arg_direct(expr, context)
}
(ExprKind::Block(block, _), MacrolessMinGenericConstArgs)
if let [stmt] = block.stmts.as_slice()
&& let StmtKind::Expr(expr) = &stmt.kind =>
Expand All @@ -2776,6 +2788,31 @@ impl<'hir> LoweringContext<'_, 'hir> {
}
}

/// It is not allowed to call this function without checking can_lower_path_to_const_arg_direct
/// first, as we assume all feature gates/etc. have been checked already.
fn lower_path_to_const_arg_direct(
&mut self,
id: NodeId,
id_override: Option<NodeId>,
qself: &Option<Box<QSelf>>,
path: &Path,
span: Span,
) -> hir::ConstArg<'hir> {
let qpath = self.lower_qpath(
id,
qself,
path,
ParamMode::Explicit,
AllowReturnTypeNotation::No,
// FIXME(mgca): update for `fn foo() -> Bar<FOO<impl Trait>>` support
ImplTraitContext::Disallowed(ImplTraitPosition::Path),
None,
);

let node_id = id_override.unwrap_or(id);
ConstArg { hir_id: self.lower_node_id(node_id), kind: hir::ConstArgKind::Path(qpath), span }
}

/// It is not allowed to call this function without checking can_lower_expr_to_const_arg_direct
/// first, as we assume all feature gates/etc. have been checked already.
#[instrument(level = "debug", skip(self), ret)]
Expand Down Expand Up @@ -2822,22 +2859,7 @@ impl<'hir> LoweringContext<'_, 'hir> {
}
}
ExprKind::Path(qself, path) => {
let qpath = self.lower_qpath(
expr.id,
qself,
path,
ParamMode::Explicit,
AllowReturnTypeNotation::No,
// FIXME(mgca): update for `fn foo() -> Bar<FOO<impl Trait>>` support
ImplTraitContext::Disallowed(ImplTraitPosition::Path),
None,
);

ConstArg {
hir_id: self.lower_node_id(node_id),
kind: hir::ConstArgKind::Path(qpath),
span,
}
self.lower_path_to_const_arg_direct(expr.id, id_override, qself, path, span)
}
ExprKind::Struct(se) => {
let path = self.lower_qpath(
Expand Down Expand Up @@ -2896,11 +2918,12 @@ impl<'hir> LoweringContext<'_, 'hir> {
kind: hir::ConstArgKind::Infer(()),
span,
},
ExprKind::Paren(expr) => self.lower_expr_to_const_arg_direct(expr, id_override),
ExprKind::Block(block, _)
if let [stmt] = block.stmts.as_slice()
&& let StmtKind::Expr(expr) = &stmt.kind =>
{
return self.lower_expr_to_const_arg_direct(expr, id_override);
self.lower_expr_to_const_arg_direct(expr, id_override)
}
ExprKind::Lit(literal) => {
let span = self.lower_span(expr.span);
Expand Down Expand Up @@ -2984,14 +3007,7 @@ impl<'hir> LoweringContext<'_, 'hir> {
anon.value.maybe_unwrap_block()
};

let context = if self.tcx.features().macroless_generic_const_args() {
DirectConstArgContext::MacrolessMinGenericConstArgs
} else if self.tcx.features().min_generic_const_args() {
DirectConstArgContext::MinGenericConstArgs
} else {
DirectConstArgContext::Stable
};

let context = self.ambient_direct_const_arg_context();
if self.can_lower_expr_to_const_arg_direct(expr, context).is_ok() {
return self.lower_expr_to_const_arg_direct(expr, Some(anon.id));
}
Expand Down
45 changes: 22 additions & 23 deletions compiler/rustc_resolve/src/late.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1040,6 +1040,11 @@ impl<'ast, 'ra, 'tcx> Visitor<'ast> for LateResolutionVisitor<'_, 'ast, 'ra, 'tc
self.visit_ty(element_ty);
self.resolve_anon_const(length, AnonConstKind::ConstArg(IsRepeatExpr::No));
}
TyKind::DirectConstArg(expr) => self.resolve_anon_const_manual(
true,
AnonConstKind::ConstArg(IsRepeatExpr::No),
|this| this.resolve_expr(expr, None),
),
_ => visit::walk_ty(self, ty),
}
self.diag_metadata.current_trait_object = prev;
Expand Down Expand Up @@ -1301,8 +1306,8 @@ impl<'ast, 'ra, 'tcx> Visitor<'ast> for LateResolutionVisitor<'_, 'ast, 'ra, 'tc
}
}

#[instrument(level = "debug", skip(self))]
fn visit_generic_arg(&mut self, arg: &'ast GenericArg) {
debug!("visit_generic_arg({:?})", arg);
let prev = replace(&mut self.diag_metadata.currently_processing_generic_args, true);
match arg {
GenericArg::Type(ty) => {
Expand All @@ -1311,31 +1316,25 @@ impl<'ast, 'ra, 'tcx> Visitor<'ast> for LateResolutionVisitor<'_, 'ast, 'ra, 'tc
// namespace first, and if that fails we try again in the value namespace. If
// resolution in the value namespace succeeds, we have an generic const argument on
// our hands.
//
// We cannot disambiguate multi-segment paths right now as that requires type
// checking.
if let TyKind::Path(None, ref path) = ty.kind
// We cannot disambiguate multi-segment paths right now as that requires type
// checking.
&& path.is_potential_trivial_const_arg()
&& let Some(ident) = path.as_single_argless_ident()
&& self.maybe_resolve_ident_in_lexical_scope(ident, TypeNS).is_none()
&& self.maybe_resolve_ident_in_lexical_scope(ident, ValueNS).is_some()
{
let mut check_ns = |ns| {
self.maybe_resolve_ident_in_lexical_scope(path.segments[0].ident, ns)
.is_some()
};
if !check_ns(TypeNS) && check_ns(ValueNS) {
self.resolve_anon_const_manual(
true,
AnonConstKind::ConstArg(IsRepeatExpr::No),
|this| {
this.smart_resolve_path(ty.id, &None, path, PathSource::Expr(None));
this.visit_path(path);
},
);

self.diag_metadata.currently_processing_generic_args = prev;
return;
}
self.resolve_anon_const_manual(
true,
AnonConstKind::ConstArg(IsRepeatExpr::No),
|this| {
this.smart_resolve_path(ty.id, &None, path, PathSource::Expr(None));
this.visit_path(path);
},
)
} else {
self.visit_ty(ty)
}

self.visit_ty(ty);
}
GenericArg::Lifetime(lt) => self.visit_lifetime(lt, visit::LifetimeCtxt::GenericArg),
GenericArg::Const(ct) => {
Expand Down
Loading
Loading