Implement consecutive shorthand projections (associated type paths) like `T::AssocA::AssocB` #126651

fmease · 2024-06-19T00:01:07Z

Addresses #126360 (comment).
CC #22519 (arbitrary shorthand projections).

FIXME:

Fix ICE on tests/ui/coroutine/coroutine-in-orphaned-anon-const.rs (I'm stumped atm)
Polish
Either deny or properly support consecutive shorthand projections inside super trait & item bound position (currently cycles)
(non-blocking, likely in a separate PR) Introduce "Def::TypeDependent"

lcnr · 2024-06-19T17:17:15Z

compiler/rustc_hir_analysis/src/hir_ty_lowering/mod.rs

+                Res::SelfTyParam { trait_: param_def_id }
+                | Res::Def(DefKind::TyParam, param_def_id),


unrelated: are there ever cases where we have a ty::Param with a different res? and if so, do we want to support them? we can go from the index of the param to its def_id after all

Self params dont resolve to DefKind::TyParam, they resolve to the trait defid and DefKind::Trait iirc

actually wait im blind thats SelfTyParam already

lcnr · 2024-06-19T17:19:29Z

compiler/rustc_hir_analysis/src/hir_ty_lowering/mod.rs

+            // FIXME(fmease): Add UI tests for `feature(more_qualified_paths)` (e.g., `T::A::B { … }`).
+            (ty::Alias(ty::Projection, alias_ty), _res) => {
+                // FIXME: Double-check that this doesn't lead to any unwanted cycle errors.
+                let predicates = tcx.item_bounds(alias_ty.def_id).instantiate(tcx, alias_ty.args);


does this cycle for

trait Foo { type FooAssoc; } trait Bar<T> {} trait Trait { type Assoc: Foo + Bar<Self::Assoc::FooAssoc>; }

Yeah, it does. I guess there's no way around introducing a specialized variant of item_bounds if at all possible (well, HIR ty lowering is notorious for following such an approach).

that may work 🤔 an easier alternative may be to forbid the consecutive shorthand in item bounds and super traits. It's not great, but it does avoid the issue

lcnr · 2024-06-19T17:22:09Z

compiler/rustc_hir_analysis/src/hir_ty_lowering/mod.rs

+            // FIXME(fmease):
+            // Should we require the pre-lowered projectee (the HIR QSelf) to be a `Res::Def(DefKind::AssocTy, _)`?
+            // Rephrased, should we flat out reject `Identity<T::Assoc>::Assoc` even if `T::Assoc::Assoc` typeck's?
+            // For comparison, `Identity<T>::Assoc` gets rejected even if `T::Assoc` passes typeck.


ah, so wrt to my first comment, this is about non-lazy type aliases?

I would expect that this would then break with lazy type aliases unless we explicitly normalize them here. Because of that I feel like we should restrict it based on the res?

Yeah, I'm referring to eager type aliases here. Makes sense to restrict them to have parity with lazy type aliases +1.

As mentioned by in me in one of the source code comments, that's gonna be a bit of a pain to impl because Res is actually Err in this scenario right now but let's see.

As mentioned by in me in one of the source code comments, that's gonna be a bit of pain to impl because Res is actually Err in this scenario right now but let's see.

that's something that has annoyed me for years and should ideally be changed/fixed regardless imo, has also recently affected @camelid's work on const generics iirc?

fmease · 2024-07-03T13:54:59Z

@bors try @rust-timer queue

Implement consecutive shorthand projections (associated type paths) like `T::AssocA::AssocB` I'd like to resolve / get feedback on the `FIXME` concerning "`Identity<T::Assoc>::Assoc`". Then I'll polish up this PR and make it ready for a types FCP (i.e., tests, tests, tests). Addresses rust-lang#126360 (comment). CC rust-lang#22519 (arbitrary shorthand projections).

bors · 2024-07-03T13:56:11Z

⌛ Trying commit ed8def2 with merge 2084985...

bors · 2024-07-03T15:41:50Z

☀️ Try build successful - checks-actions
Build commit: 2084985 (2084985765ef49c15bbc49a1ef84a6db707bfc4b)

rust-timer · 2024-07-03T17:19:02Z

Finished benchmarking commit (2084985): comparison URL.

Overall result: no relevant changes - no action needed

Benchmarking this pull request likely means that it is perf-sensitive, so we're automatically marking it as not fit for rolling up. While you can manually mark this PR as fit for rollup, we strongly recommend not doing so since this PR may lead to changes in compiler perf.

@bors rollup=never
@rustbot label: -S-waiting-on-perf -perf-regression

Instruction count

This benchmark run did not return any relevant results for this metric.

Max RSS (memory usage)

Results (primary -2.1%, secondary -2.9%)

This is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.

	mean	range	count
Regressions ❌ (primary)	-	-	0
Regressions ❌ (secondary)	-	-	0
Improvements ✅ (primary)	-2.1%	[-2.1%, -2.1%]	1
Improvements ✅ (secondary)	-2.9%	[-3.6%, -2.2%]	6
All ❌✅ (primary)	-2.1%	[-2.1%, -2.1%]	1

Cycles

Results (secondary -3.9%)

This is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.

	mean	range	count
Regressions ❌ (primary)	-	-	0
Regressions ❌ (secondary)	-	-	0
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-3.9%	[-3.9%, -3.9%]	1
All ❌✅ (primary)	-	-	0

Binary size

This benchmark run did not return any relevant results for this metric.

Bootstrap: 695.82s -> 696.616s (0.11%)
Artifact size: 327.73 MiB -> 327.77 MiB (0.01%)

bors · 2024-09-22T00:56:35Z

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

fmease assigned compiler-errors and lcnr Jun 19, 2024

rustbot added A-testsuite Area: The testsuite used to check the correctness of rustc S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-bootstrap Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap) labels Jun 19, 2024

fmease changed the title ~~[PoC] Implement consecutive shorthand associated type paths (projections) like T::AssocA::AssocB~~ [PoC] Implement consecutive shorthand projections (associated type paths) like T::AssocA::AssocB Jun 19, 2024

fmease changed the title ~~[PoC] Implement consecutive shorthand projections (associated type paths) like T::AssocA::AssocB~~ Implement consecutive shorthand projections (associated type paths) like T::AssocA::AssocB Jun 19, 2024

This comment has been minimized.

Sign in to view

fmease force-pushed the consec-shorthand-proj branch from 99bfdeb to 6fc5c3c Compare June 19, 2024 00:39

lcnr reviewed Jun 19, 2024

View reviewed changes

fmease 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 Jun 20, 2024

fmease force-pushed the consec-shorthand-proj branch from 6fc5c3c to ed8def2 Compare July 3, 2024 13:54

This comment has been minimized.

Sign in to view

rustbot added the S-waiting-on-perf Status: Waiting on a perf run to be completed. label Jul 3, 2024

This comment has been minimized.

Sign in to view

rustbot removed the S-waiting-on-perf Status: Waiting on a perf run to be completed. label Jul 3, 2024

fmease added the rla-silenced Silences rust-log-analyzer postings to the PR it's added on. label Jul 18, 2024

fmease force-pushed the consec-shorthand-proj branch 2 times, most recently from c81ea1f to d56708a Compare July 19, 2024 15:33

This comment was marked as resolved.

Sign in to view

fmease added 2 commits August 15, 2024 15:47

Implement consecutive shorthand projections

cc8c507

[PoC] Track type-dependent defs in ItemCtxts (minimally)

0664285

fmease force-pushed the consec-shorthand-proj branch from d56708a to 0664285 Compare August 15, 2024 14:20

Dylan-DPC removed the S-experimental Status: Ongoing experiment that does not require reviewing and won't be merged in its current state. label Aug 20, 2024

fmease mentioned this pull request Sep 3, 2024

cycle on inherent associated types in structs #108491

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Implement consecutive shorthand projections (associated type paths) like `T::AssocA::AssocB` #126651

Implement consecutive shorthand projections (associated type paths) like `T::AssocA::AssocB` #126651

fmease commented Jun 19, 2024 •

edited

Loading

This comment has been minimized.

lcnr Jun 19, 2024

compiler-errors Jun 19, 2024

compiler-errors Jun 19, 2024

lcnr Jun 19, 2024

fmease Jul 1, 2024 •

edited

Loading

lcnr Jul 2, 2024 •

edited

Loading

lcnr Jun 19, 2024

fmease Jun 20, 2024 •

edited

Loading

lcnr Jun 20, 2024 •

edited

Loading

fmease commented Jul 3, 2024

This comment has been minimized.

bors commented Jul 3, 2024

This comment has been minimized.

bors commented Jul 3, 2024

This comment has been minimized.

rust-timer commented Jul 3, 2024

This comment was marked as resolved.

bors commented Sep 22, 2024

		Res::SelfTyParam { trait_: param_def_id }
		\| Res::Def(DefKind::TyParam, param_def_id),

Implement consecutive shorthand projections (associated type paths) like T::AssocA::AssocB #126651

Are you sure you want to change the base?

Implement consecutive shorthand projections (associated type paths) like T::AssocA::AssocB #126651

Conversation

fmease commented Jun 19, 2024 • edited Loading

This comment has been minimized.

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

fmease Jul 1, 2024 • edited Loading

Choose a reason for hiding this comment

lcnr Jul 2, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

fmease Jun 20, 2024 • edited Loading

Choose a reason for hiding this comment

lcnr Jun 20, 2024 • edited Loading

Choose a reason for hiding this comment

fmease commented Jul 3, 2024

This comment has been minimized.

bors commented Jul 3, 2024

This comment has been minimized.

bors commented Jul 3, 2024

This comment has been minimized.

rust-timer commented Jul 3, 2024

Overall result: no relevant changes - no action needed

This comment was marked as resolved.

bors commented Sep 22, 2024

Implement consecutive shorthand projections (associated type paths) like `T::AssocA::AssocB` #126651

Implement consecutive shorthand projections (associated type paths) like `T::AssocA::AssocB` #126651

fmease commented Jun 19, 2024 •

edited

Loading

fmease Jul 1, 2024 •

edited

Loading

lcnr Jul 2, 2024 •

edited

Loading

fmease Jun 20, 2024 •

edited

Loading

lcnr Jun 20, 2024 •

edited

Loading