Resolve associated item bindings by namespace #118668

fmease · 2023-12-06T09:53:44Z

This is the 3rd commit split off from #118360 with tests reblessed (they no longer contain duplicated diags which were caused by 4c0addc) & slightly adapted (removed supertraits from a UI test, cc #118040).

Resolve all assoc item bindings (type, const, fn (feature return_type_notation)) by namespace instead of trying to resolve a type first (in the non-RTN case) and falling back to consts afterwards. This is consistent with RTN. E.g., for Tr<K = {…}> we now always try to look up assoc consts (this extends to supertrait bounds). This gets rid of assoc tys shadowing assoc consts in assoc item bindings which is undesirable & inconsistent (types and consts live in different namespaces after all)

Consolidate the resolution of assoc {ty, const} bindings and RTN (dedup, better diags for RTN)

Fix assoc consts being labeled as assoc types in several diagnostics

Make a bunch of diagnostics translatable

Fixes #112560 (error → pass).

As discussed
r? @compiler-errors

Addendum: What I call “associated item bindings” are commonly referred to as “type bindings” for historical reasons. Nowadays, “type bindings” include assoc type bindings, assoc const bindings and RTN (return type notation) which is why I prefer not to use this outdated term.

fmease · 2023-12-06T13:52:04Z

compiler/rustc_hir_analysis/src/astconv/errors.rs

+            None
+        };
+
+        // FIXME(associated_const_equality): This has quite a few false positives and negatives.


(preexisting)

bors · 2023-12-06T14:01:04Z

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

compiler-errors

💯

I just had one question about hygeine

r=me though if it's something unfixable at the moment

compiler-errors · 2023-12-06T22:58:53Z

compiler/rustc_hir_analysis/src/astconv/bounds.rs

+        let assoc_item = tcx
+            .associated_items(candidate.def_id())
+            .filter_by_name_unhygienic(assoc_ident.name)
+            .find(|i| i.kind == assoc_kind && i.ident(tcx).normalize_to_macros_2_0() == assoc_ident)


Huh, how does this differ from find_by_name_and_kind?

find_by_name_and_kind is semantically identical to the code above but this version is slightly faster in theory since we only normalize the LHS above instead of normalizing both sides (which is what hygienic_eq does). assoc_ident has already been normalized above in adjust_ident_and_get_scope, so no need to do it again (adjust_ident_and_get_scope also returns the def_scope which is used later in is_accessible_from).

pls note this

compiler/rustc_hir_analysis/src/astconv/mod.rs

compiler-errors · 2023-12-06T23:01:54Z

tests/ui/associated-consts/assoc-const-eq-missing.stderr

-LL | fn foo1<F: Foo<Z=3>>() {}
-   |                ^ associated type `Z` not found
+LL | fn foo1<F: Foo<Z = 3>>() {}
+   |                ^ help: there is an associated constant with a similar name: `N`


this is awesome

compiler-errors · 2023-12-06T23:02:13Z

tests/ui/associated-consts/shadowed-const.stderr

-   |              ^^^^^^^^^
+   |                    ^^^ unexpected constant
+   |
+note: the associated constant is defined here


compiler-errors · 2023-12-06T23:02:24Z

tests/ui/associated-type-bounds/consts.stderr

  --> $DIR/consts.rs:3:29
   |
 LL | pub fn accept(_: impl Trait<K: Copy>) {}
-   |                             ^
+   |                             ^------ bounds are not allowed on associated constants


If a const is expected, resolve a const. If a type is expected, resolve a type. Don't try to resolve a type first falling back to consts.

fmease · 2023-12-07T22:25:39Z

@bors r=compiler-errors

bors · 2023-12-07T22:25:42Z

📌 Commit 55559d9 has been approved by compiler-errors

It is now in the queue for this repository.

bors · 2023-12-08T09:30:55Z

⌛ Testing commit 55559d9 with merge 5ea6256...

bors · 2023-12-08T11:26:37Z

☀️ Test successful - checks-actions
Approved by: compiler-errors
Pushing 5ea6256 to master...

rust-timer · 2023-12-08T12:48:03Z

Finished benchmarking commit (5ea6256): comparison URL.

Overall result: no relevant changes - no action needed

@rustbot label: -perf-regression

Instruction count

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

Max RSS (memory usage)

Results

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)	1.2%	[0.5%, 1.9%]	2
Regressions ❌ (secondary)	2.3%	[2.3%, 2.3%]	1
Improvements ✅ (primary)	-0.5%	[-0.5%, -0.5%]	2
Improvements ✅ (secondary)	-2.3%	[-4.1%, -1.1%]	8
All ❌✅ (primary)	0.4%	[-0.5%, 1.9%]	4

Cycles

Results

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)	1.1%	[0.6%, 1.7%]	2
Regressions ❌ (secondary)	-	-	0
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-	-	0
All ❌✅ (primary)	1.1%	[0.6%, 1.7%]	2

Binary size

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

Bootstrap: 674.492s -> 674.729s (0.04%)
Artifact size: 314.05 MiB -> 314.07 MiB (0.01%)

…-bindings, r=compiler-errors AstConv: Refactor lowering of associated item bindings a bit Split off from rust-lang#119385 as discussed, namely the first two commits (modulo one `FIXME` getting turned into a `NOTE`). The second commit removes `astconv::ConvertedBinding{,Kind}` in favor of `hir::TypeBinding{,Kind}`. The former was a — in my opinion — super useless intermediary. As you can tell from the diff, its removal shaves off some code. Furthermore, yeeting it makes it easier to implement the changes in rust-lang#119385. Nothing in this PR should have any semantic effect. r? `@compiler-errors` **Addendum** as in rust-lang#118668: What I call “associated item bindings” are commonly referred to as “type bindings” for historical reasons. Nowadays, “type bindings” include assoc type bindings, assoc const bindings and RTN (return type notation) which is why I prefer not to use this outdated term.

…-bindings, r=compiler-errors AstConv: Refactor lowering of associated item bindings a bit Split off from rust-lang#119385 as discussed, namely the first two commits (modulo one `FIXME` getting turned into a `NOTE`). The second commit removes `astconv::ConvertedBinding{,Kind}` in favor of `hir::TypeBinding{,Kind}`. The former was a — in my opinion — super useless intermediary. As you can tell from the diff, its removal shaves off some code. Furthermore, yeeting it will make it easier to implement the type resolution fixes in rust-lang#119385. Nothing in this PR should have any semantic effect. r? `@compiler-errors` **Addendum** as in rust-lang#118668: What I call “associated item bindings” are commonly referred to as “type bindings” for historical reasons. Nowadays, “type bindings” include assoc type bindings, assoc const bindings and RTN (return type notation) which is why I prefer not to use this outdated term.

Rollup merge of rust-lang#121221 - fmease:refactor-astconv-assoc-item-bindings, r=compiler-errors AstConv: Refactor lowering of associated item bindings a bit Split off from rust-lang#119385 as discussed, namely the first two commits (modulo one `FIXME` getting turned into a `NOTE`). The second commit removes `astconv::ConvertedBinding{,Kind}` in favor of `hir::TypeBinding{,Kind}`. The former was a — in my opinion — super useless intermediary. As you can tell from the diff, its removal shaves off some code. Furthermore, yeeting it will make it easier to implement the type resolution fixes in rust-lang#119385. Nothing in this PR should have any semantic effect. r? `@compiler-errors` **Addendum** as in rust-lang#118668: What I call “associated item bindings” are commonly referred to as “type bindings” for historical reasons. Nowadays, “type bindings” include assoc type bindings, assoc const bindings and RTN (return type notation) which is why I prefer not to use this outdated term.

…li-obk,cjgillot Fix type resolution of associated const equality bounds (take 2) Instead of trying to re-resolve the type of assoc const bindings inside the `type_of` query impl in an incomplete manner, transfer the already (correctly) resolved type from `add_predicates_for_ast_type_binding` to `type_of`/`anon_type_of` through query feeding. --- Together with rust-lang#118668 (merged) and rust-lang#121258, this supersedes rust-lang#118360. Fixes rust-lang#118040. r? `@ghost`

…li-obk,cjgillot Fix type resolution of associated const equality bounds (take 2) Instead of trying to re-resolve the type of assoc const bindings inside the `type_of` query impl in an incomplete manner, transfer the already (correctly) resolved type from `add_predicates_for_ast_type_binding` to `type_of`/`anon_type_of` through query feeding. --- Together with rust-lang#118668 (merged) and rust-lang#121258, this supersedes rust-lang#118360. Fixes rust-lang#118040. r? ``@ghost``

Rollup merge of rust-lang#119385 - fmease:assoc-const-eq-fixes-2, r=oli-obk,cjgillot Fix type resolution of associated const equality bounds (take 2) Instead of trying to re-resolve the type of assoc const bindings inside the `type_of` query impl in an incomplete manner, transfer the already (correctly) resolved type from `add_predicates_for_ast_type_binding` to `type_of`/`anon_type_of` through query feeding. --- Together with rust-lang#118668 (merged) and rust-lang#121258, this supersedes rust-lang#118360. Fixes rust-lang#118040. r? ``@ghost``

…y-generic-tys, r=compiler-errors Reject overly generic assoc const binding types Split off from rust-lang#119385 to make rust-lang#119385 easier to review. --- In the *instantiated* type of assoc const bindings 1. reject **early-bound generic params** * Provide a rich error message instead of ICE'ing ([rust-lang#108271](rust-lang#108271)). * This is a temporary and semi-artificial restriction until the arrival of *generic const generics*. * It's quite possible that rustc could already perfectly support this subset of generic const generics if we just removed some checks (some `.no_bound_vars().expect(…)`) but even if that was the case, I'd rather gate it behind a new feature flag. Reporting an error instead of ICE'ing is a good first step towards an eventual feature gate error. 2. reject **escaping late-bound generic params** * They lead to ICEs before & I'm pretty sure that they remain incorrect even in a world with *generic const generics* --- Together with rust-lang#118668 & rust-lang#119385, this supersedes rust-lang#118360. Fixes rust-lang#108271.

Rollup merge of rust-lang#121258 - fmease:assoc-const-eq-reject-overly-generic-tys, r=compiler-errors Reject overly generic assoc const binding types Split off from rust-lang#119385 to make rust-lang#119385 easier to review. --- In the *instantiated* type of assoc const bindings 1. reject **early-bound generic params** * Provide a rich error message instead of ICE'ing ([rust-lang#108271](rust-lang#108271)). * This is a temporary and semi-artificial restriction until the arrival of *generic const generics*. * It's quite possible that rustc could already perfectly support this subset of generic const generics if we just removed some checks (some `.no_bound_vars().expect(…)`) but even if that was the case, I'd rather gate it behind a new feature flag. Reporting an error instead of ICE'ing is a good first step towards an eventual feature gate error. 2. reject **escaping late-bound generic params** * They lead to ICEs before & I'm pretty sure that they remain incorrect even in a world with *generic const generics* --- Together with rust-lang#118668 & rust-lang#119385, this supersedes rust-lang#118360. Fixes rust-lang#108271.

rustbot assigned compiler-errors Dec 6, 2023

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 Dec 6, 2023

fmease commented Dec 6, 2023

View reviewed changes

fmease force-pushed the resolve-assoc-item-bindings-by-namespace branch from 4dd99ae to 1f7caff Compare December 6, 2023 15:31

compiler-errors approved these changes Dec 6, 2023

View reviewed changes

compiler-errors 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 Dec 6, 2023

fmease force-pushed the resolve-assoc-item-bindings-by-namespace branch 3 times, most recently from 8301b05 to 5761e6c Compare December 7, 2023 19:10

This comment has been minimized.

Sign in to view

fmease force-pushed the resolve-assoc-item-bindings-by-namespace branch from 5761e6c to d82e780 Compare December 7, 2023 21:27

Resolve assoc item bindings by namespace

55559d9

If a const is expected, resolve a const. If a type is expected, resolve a type. Don't try to resolve a type first falling back to consts.

fmease force-pushed the resolve-assoc-item-bindings-by-namespace branch from d82e780 to 55559d9 Compare December 7, 2023 21:34

bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Dec 7, 2023

bors added the merged-by-bors This PR was explicitly merged by bors. label Dec 8, 2023

bors merged commit 5ea6256 into rust-lang:master Dec 8, 2023
12 checks passed

rustbot added this to the 1.76.0 milestone Dec 8, 2023

fmease deleted the resolve-assoc-item-bindings-by-namespace branch December 8, 2023 20:33

fmease mentioned this pull request Dec 15, 2023

Tracking issue for associated const equality #92827

Open

8 tasks

fmease mentioned this pull request Dec 28, 2023

Fix type resolution of associated const equality bounds (take 2) #119385

Merged

fmease mentioned this pull request Feb 17, 2024

AstConv: Refactor lowering of associated item bindings a bit #121221

Merged

fmease mentioned this pull request Feb 18, 2024

Reject overly generic assoc const binding types #121258

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Resolve associated item bindings by namespace #118668

Resolve associated item bindings by namespace #118668

fmease commented Dec 6, 2023 •

edited

Loading

fmease Dec 6, 2023

bors commented Dec 6, 2023

compiler-errors left a comment

compiler-errors Dec 6, 2023

fmease Dec 7, 2023 •

edited

Loading

compiler-errors Dec 7, 2023

compiler-errors Dec 6, 2023

compiler-errors Dec 6, 2023

compiler-errors Dec 6, 2023

This comment has been minimized.

fmease commented Dec 7, 2023

bors commented Dec 7, 2023

bors commented Dec 8, 2023

bors commented Dec 8, 2023

rust-timer commented Dec 8, 2023

Resolve associated item bindings by namespace #118668

Resolve associated item bindings by namespace #118668

Conversation

fmease commented Dec 6, 2023 • edited Loading

Choose a reason for hiding this comment

bors commented Dec 6, 2023

compiler-errors left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

fmease Dec 7, 2023 • edited Loading

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

Choose a reason for hiding this comment

This comment has been minimized.

fmease commented Dec 7, 2023

bors commented Dec 7, 2023

bors commented Dec 8, 2023

bors commented Dec 8, 2023

rust-timer commented Dec 8, 2023

Overall result: no relevant changes - no action needed

fmease commented Dec 6, 2023 •

edited

Loading

fmease Dec 7, 2023 •

edited

Loading