Lower all trivial const paths as `ConstArgKind::Path` #135186

camelid · 2025-01-07T05:21:38Z

This PR:

Lower all trivial const paths (i.e. no nested anon consts) in arg position to hir::ConstArgKind::Path
Then lower assoc const paths to ty::ConstKind::Unevaluated
(incomplete) Introduce #[type_const] attribute for trait assoc consts that are allowed as const args

Next steps:

Implement code to check that assoc const definitions satisfy #[type_const] if present (basically is it a const path or monomorphic anon const)

r? @BoxyUwU

rustbot · 2025-01-11T02:51:48Z

Some changes occurred in const_evaluatable.rs

cc @BoxyUwU

HIR ty lowering was modified

cc @fmease

BoxyUwU

thx for working on this :3

compiler/rustc_ast_lowering/src/lib.rs

compiler/rustc_ast/src/ast.rs

compiler/rustc_hir_analysis/src/hir_ty_lowering/mod.rs

compiler/rustc_hir_analysis/src/collect.rs

bors · 2025-01-23T01:51:57Z

☔ The latest upstream changes (presumably #135896) made this pull request unmergeable. Please resolve the merge conflicts.

bors · 2025-01-27T15:23:39Z

☔ The latest upstream changes (presumably #136024) made this pull request unmergeable. Please resolve the merge conflicts.

compiler/rustc_ast/src/util/const_args.rs

compiler/rustc_hir_analysis/src/hir_ty_lowering/mod.rs

bors · 2025-02-17T14:58:33Z

☔ The latest upstream changes (presumably #137164) made this pull request unmergeable. Please resolve the merge conflicts.

rustbot · 2025-02-19T20:19:45Z

Some changes occurred in compiler/rustc_passes/src/check_attr.rs

cc @jdonszelmann

Co-authored-by: BoxyUwU <[email protected]>

A bit tricky to support this special case now, so might as well save it for once we're ready to handle the general case. See rust-lang#135186 (comment). Co-authored-by: BoxyUwU <[email protected]>

BoxyUwU · 2025-02-22T00:11:43Z

compiler/rustc_ast_lowering/src/lib.rs

+                            // FIXME: should this only allow single-segment paths?
+                                && path.is_potential_trivial_const_arg(self.tcx.features().min_generic_const_args())


Suggested change

// FIXME: should this only allow single-segment paths?

&& path.is_potential_trivial_const_arg(self.tcx.features().min_generic_const_args())

&& path.is_potential_trivial_const_arg(false)

Yes I think this should only work for mcg paths rn. Generally this needs to be kept in sync with the logic in name resolution that attempts to resolve type args in both valuens and typens and that currently uses false.

BoxyUwU · 2025-02-22T00:14:04Z

compiler/rustc_resolve/src/late.rs

@@ -1200,7 +1200,7 @@ impl<'ra: 'ast, 'ast, 'tcx> Visitor<'ast> for LateResolutionVisitor<'_, 'ast, 'r
                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()
+                    && path.is_potential_trivial_const_arg(false)


this.

I think making this use self.features().min_generic_const_args() is spooky 🤔 We would need to update logic furthur down to call methods that can actually resolve multi-seg paths and idk if such methods exist.

So yeah, tl;dr we just make both of these false and handle it down the road when we support unbraced const arguments of more complex paths (probably under a different feature gate)

BoxyUwU · 2025-02-22T00:18:56Z

tests/ui/feature-gates/feature-gate-min-generic-const-args.rs

    const ASSOC: usize;
 }

-// FIXME(min_generic_const_args): implement support for this, behind the feature gate
+// FIXME(min_generic_const_args): add suggestion for mgca to this error


Suggested change

// FIXME(min_generic_const_args): add suggestion for mgca to this error

// FIXME(mgca): add suggestion for mgca to this error

BoxyUwU · 2025-02-22T00:31:37Z

compiler/rustc_hir_analysis/src/hir_ty_lowering/mod.rs

+                let assoc = tcx.associated_item(def_id);
+                let ty = if matches!(assoc, ty::AssocItem {
+                    container: ty::AssocItemContainer::Impl,
+                    trait_item_def_id: None,
+                    ..
+                }) {
+                    Ty::new_alias(tcx, ty::Inherent, ty::AliasTy::new_from_args(tcx, def_id, args))
+                } else {
+                    Ty::new_projection_from_args(tcx, def_id, args)
+                };
+                Ok((ty, DefKind::AssocTy, def_id))


Hmm yeah you're right I don't think we care about any of the other kinds of aliases here... It's just more concise then I guess 😅 My understanding is that type Foo = impl Trait; is a projection/iat that normalizes to an opaque, rather than T::Foo itself referring to an opaque so doesnt matter here

BoxyUwU · 2025-02-22T00:37:59Z

compiler/rustc_ast/src/ast.rs

-    /// that the path resolves to a const param, the caller should check this.
-    pub fn is_potential_trivial_const_arg(&self) -> bool {
-        matches!(self.segments[..], [PathSegment { args: None, .. }])
+    // FIXME: add docs


BoxyUwU · 2025-02-22T00:38:07Z

compiler/rustc_ast/src/ast.rs

@@ -1177,22 +1178,26 @@ pub struct Expr {
 }

 impl Expr {
+    // FIXME: update docs


BoxyUwU · 2025-02-22T00:40:33Z

I think this is basically good to go now (once minor nits are resolved). I'm really happy to see this feature at the point its at :3 thx for doing this

bors · 2025-02-25T03:00:32Z

☔ The latest upstream changes (presumably #135726) made this pull request unmergeable. Please resolve the merge conflicts.

rustbot assigned BoxyUwU Jan 7, 2025

rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Jan 7, 2025

This comment has been minimized.

Sign in to view

camelid changed the title ~~Start lowering multi-segment const paths as ConstArgKind::Path~~ Lower multi-segment const paths as ConstArgKind::Path Jan 7, 2025

camelid force-pushed the const-path-multi branch from c501669 to 4efbed3 Compare January 7, 2025 18:52

This comment has been minimized.

Sign in to view

camelid force-pushed the const-path-multi branch from 4efbed3 to 0036c2c Compare January 7, 2025 19:13

This comment has been minimized.

Sign in to view

camelid marked this pull request as ready for review January 11, 2025 02:51

camelid changed the title ~~Lower multi-segment const paths as ConstArgKind::Path~~ Lower multi-segment const paths as ConstArgKind::Path Jan 11, 2025

BoxyUwU reviewed Jan 16, 2025

View reviewed changes

camelid force-pushed the const-path-multi branch from 263637d to a296ff8 Compare January 20, 2025 04:47

This comment has been minimized.

Sign in to view

camelid force-pushed the const-path-multi branch from a5a9370 to ece9e77 Compare January 23, 2025 06:13

BoxyUwU added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Jan 23, 2025

camelid added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Jan 27, 2025

camelid changed the title ~~Lower multi-segment const paths as ConstArgKind::Path~~ Lower all trivial const paths as ConstArgKind::Path Jan 27, 2025

BoxyUwU reviewed Jan 28, 2025

View reviewed changes

compiler/rustc_hir_analysis/src/hir_ty_lowering/mod.rs Show resolved Hide resolved

BoxyUwU added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Jan 28, 2025

camelid force-pushed the const-path-multi branch from 7cbecce to 50bd398 Compare February 3, 2025 05:34

camelid added 16 commits February 19, 2025 15:09

Start lowering multi-segment const paths as ConstArgKind::Path

3de14e3

Partially fix parent_args handling for assoc const

15e9f39

Avoid "const uses param" error

9706c7c

Replace TODOs with FIXMEs to allow CI to pass for now

cd21d8c

Properly lower assoc consts from traits

4dcc15c

Avoid "const depends on generics" error for bare paths under mgca

e5cf586

Standardize mgca FIXMEs to "mgca"

b69308e

Start implementing #[type_const]

33c63ae

Allow const paths with generics/qself under mgca

dd2819f

Fix handling of const qpath args

a912b7d

Factor out shared code for lowering assoc item paths

0862150

Properly handle ambiguous assoc const

8f9b82a

Factor out shared code for probing inherent assoc items

cf5e01e

Factor out shared code for lowering assoc item paths

07254c2

Relax overly strict mgca requirements

c0ed1dd

Clean up (most of) the refactoring

25a92ee

camelid force-pushed the const-path-multi branch from 0dc9d63 to 25a92ee Compare February 19, 2025 20:19

rustbot added the A-attributes Area: Attributes (`#[…]`, `#![…]`) label Feb 19, 2025

This comment has been minimized.

Sign in to view

Bless tests from rebase

7d221d5

This comment has been minimized.

Sign in to view

camelid and others added 4 commits February 19, 2025 18:52

Gate mgca codepath for inherent consts under inherent_associated_types

84937e1

Move more logic into lower_qpath_shared

61c5e74

Reuse existing logic for type aliases

f8a4a65

Co-authored-by: BoxyUwU <[email protected]>

mgca: Crash on variant const paths for now

abe7d68

A bit tricky to support this special case now, so might as well save it for once we're ready to handle the general case. See rust-lang#135186 (comment). Co-authored-by: BoxyUwU <[email protected]>

BoxyUwU reviewed Feb 22, 2025

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Lower all trivial const paths as `ConstArgKind::Path` #135186

Lower all trivial const paths as `ConstArgKind::Path` #135186

camelid commented Jan 7, 2025 •

edited

Loading

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

rustbot commented Jan 11, 2025

BoxyUwU left a comment

This comment has been minimized.

bors commented Jan 23, 2025

bors commented Jan 27, 2025

bors commented Feb 17, 2025

rustbot commented Feb 19, 2025

This comment has been minimized.

This comment has been minimized.

BoxyUwU Feb 22, 2025

BoxyUwU Feb 22, 2025

BoxyUwU Feb 22, 2025

BoxyUwU Feb 22, 2025

BoxyUwU Feb 22, 2025

BoxyUwU Feb 22, 2025

BoxyUwU commented Feb 22, 2025

bors commented Feb 25, 2025

		// FIXME: should this only allow single-segment paths?
		&& path.is_potential_trivial_const_arg(self.tcx.features().min_generic_const_args())

	// FIXME: should this only allow single-segment paths?
	&& path.is_potential_trivial_const_arg(self.tcx.features().min_generic_const_args())
	&& path.is_potential_trivial_const_arg(false)

	// FIXME(min_generic_const_args): add suggestion for mgca to this error
	// FIXME(mgca): add suggestion for mgca to this error

Lower all trivial const paths as ConstArgKind::Path #135186

Are you sure you want to change the base?

Lower all trivial const paths as ConstArgKind::Path #135186

Conversation

camelid commented Jan 7, 2025 • edited Loading

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

rustbot commented Jan 11, 2025

BoxyUwU left a comment

Choose a reason for hiding this comment

This comment has been minimized.

bors commented Jan 23, 2025

bors commented Jan 27, 2025

bors commented Feb 17, 2025

rustbot commented Feb 19, 2025

This comment has been minimized.

This comment has been minimized.

BoxyUwU Feb 22, 2025

Choose a reason for hiding this comment

BoxyUwU Feb 22, 2025

Choose a reason for hiding this comment

BoxyUwU Feb 22, 2025

Choose a reason for hiding this comment

BoxyUwU Feb 22, 2025

Choose a reason for hiding this comment

BoxyUwU Feb 22, 2025

Choose a reason for hiding this comment

BoxyUwU Feb 22, 2025

Choose a reason for hiding this comment

BoxyUwU commented Feb 22, 2025

bors commented Feb 25, 2025

Lower all trivial const paths as `ConstArgKind::Path` #135186

Lower all trivial const paths as `ConstArgKind::Path` #135186

camelid commented Jan 7, 2025 •

edited

Loading