resolve: Make disambiguators for underscore bindings module-local by petrochenkov · Pull Request #144013 · rust-lang/rust

petrochenkov · 2025-07-16T07:47:44Z

Disambiguators attached to underscore name bindings (like const _: u8 = something;) do not need to be globally unique, they just need to be unique inside the module in which they live, because the bindings in a module are basically kept as Map<BindingKey, SomeData>.

Also, the specific values of the disambiguators are not important, so a glob import of const _ may have a different disambiguator than the original const _ itself.

So in this PR the disambiguator is just set to the current number of bindings in the module.
This removes one more piece of global mutable state from resolver and unblocks #143884.

rustbot · 2025-07-16T07:47:48Z

r? @oli-obk

rustbot has assigned @oli-obk.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

petrochenkov · 2025-07-16T07:50:26Z

compiler/rustc_resolve/src/macros.rs

            // FIXME: Adjust hygiene for idents from globs, like for glob imports.
            if let Some(overriding_keys) = this.impl_binding_keys.get(&impl_def_id)
-                && overriding_keys.contains(&BindingKey::new(ident.normalize_to_macros_2_0(), ns))
+                && overriding_keys.contains(&BindingKey::new(ident, ns))


Drive-by cleanup, the normalization already happens inside BindingKey::new.

oli-obk

r=me with nit and comment

compiler/rustc_resolve/src/lib.rs

compiler/rustc_resolve/src/build_reduced_graph.rs

petrochenkov · 2025-07-17T14:36:35Z

Didn't rename any BindingKey methods, only added some comments.
@bors r=oli-obk

bors · 2025-07-17T14:36:47Z

📌 Commit 998df3a has been approved by oli-obk

It is now in the queue for this repository.

resolve: Make disambiguators for underscore bindings module-local Disambiguators attached to underscore name bindings (like `const _: u8 = something;`) do not need to be globally unique, they just need to be unique inside the module in which they live, because the bindings in a module are basically kept as `Map<BindingKey, SomeData>`. Also, the specific values of the disambiguators are not important, so a glob import of `const _` may have a different disambiguator than the original `const _` itself. So in this PR the disambiguator is just set to the current number of bindings in the module. This removes one more piece of global mutable state from resolver and unblocks rust-lang#143884.

Rollup of 12 pull requests Successful merges: - #143280 (Remove duplicate error about raw underscore lifetime) - #143649 (Add test for `default_field_values` and `const_default`) - #143699 (Make `AsyncDrop` check that it's being implemented on a local ADT) - #143704 (Be a bit more careful around exotic cycles in in the inliner) - #143908 (`tests/ui`: A New Order [0/28] ) - #143909 (docs(alloc::fmt): Make type optional, instead of matching empty string) - #143925 (Make slice comparisons const) - #143997 (Use $crate in macros for rustc_public (aka stable_mir)) - #144013 (resolve: Make disambiguators for underscore bindings module-local) - #144029 (Fix wrong messages from methods with the same name from different traits) - #144063 (Add myself to the `infra-ci` reviewer group and adjust some infra auto-labels) - #144069 (ci: use windows 22 for all free runners) r? `@ghost` `@rustbot` modify labels: rollup

Rollup of 11 pull requests Successful merges: - #143280 (Remove duplicate error about raw underscore lifetime) - #143649 (Add test for `default_field_values` and `const_default`) - #143699 (Make `AsyncDrop` check that it's being implemented on a local ADT) - #143908 (`tests/ui`: A New Order [0/28] ) - #143909 (docs(alloc::fmt): Make type optional, instead of matching empty string) - #143925 (Make slice comparisons const) - #143997 (Use $crate in macros for rustc_public (aka stable_mir)) - #144013 (resolve: Make disambiguators for underscore bindings module-local) - #144029 (Fix wrong messages from methods with the same name from different traits) - #144063 (Add myself to the `infra-ci` reviewer group and adjust some infra auto-labels) - #144069 (ci: use windows 22 for all free runners) r? `@ghost` `@rustbot` modify labels: rollup

Rollup merge of #144013 - petrochenkov:disambunder, r=oli-obk resolve: Make disambiguators for underscore bindings module-local Disambiguators attached to underscore name bindings (like `const _: u8 = something;`) do not need to be globally unique, they just need to be unique inside the module in which they live, because the bindings in a module are basically kept as `Map<BindingKey, SomeData>`. Also, the specific values of the disambiguators are not important, so a glob import of `const _` may have a different disambiguator than the original `const _` itself. So in this PR the disambiguator is just set to the current number of bindings in the module. This removes one more piece of global mutable state from resolver and unblocks #143884.

Prepare revert of 144013 This is a possible revert for #144013 causing issue #144168 (imo p-crit) to give us time to figure out a correct fix for #144013 without pressure. Feel free to close if it's an easy fix instead: r? `@petrochenkov`

Prepare revert of 144013 This is a possible revert for rust-lang/rust#144013 causing issue rust-lang/rust#144168 (imo p-crit) to give us time to figure out a correct fix for rust-lang/rust#144013 without pressure. Feel free to close if it's an easy fix instead: r? `@petrochenkov`

petrochenkov · 2025-07-21T20:09:13Z

Second attempt - #144272.

resolve: Make disambiguators for underscore bindings module-local Disambiguators attached to underscore name bindings (like `const _: u8 = something;`) do not need to be globally unique, they just need to be unique inside the module in which they live, because the bindings in a module are basically kept as `Map<BindingKey, SomeData>`. Also, the specific values of the disambiguators are not important, so a glob import of `const _` may have a different disambiguator than the original `const _` itself. So in this PR the disambiguator is just set to the current number of bindings in the module. This removes one more piece of global mutable state from resolver and unblocks rust-lang#143884.

Prepare revert of 144013 This is a possible revert for rust-lang#144013 causing issue rust-lang#144168 (imo p-crit) to give us time to figure out a correct fix for rust-lang#144013 without pressure. Feel free to close if it's an easy fix instead: r? `@petrochenkov`

…iaskrgr Rollup of 11 pull requests Successful merges: - rust-lang#143280 (Remove duplicate error about raw underscore lifetime) - rust-lang#143649 (Add test for `default_field_values` and `const_default`) - rust-lang#143699 (Make `AsyncDrop` check that it's being implemented on a local ADT) - rust-lang#143908 (`tests/ui`: A New Order [0/28] ) - rust-lang#143909 (docs(alloc::fmt): Make type optional, instead of matching empty string) - rust-lang#143925 (Make slice comparisons const) - rust-lang#143997 (Use $crate in macros for rustc_public (aka stable_mir)) - rust-lang#144013 (resolve: Make disambiguators for underscore bindings module-local) - rust-lang#144029 (Fix wrong messages from methods with the same name from different traits) - rust-lang#144063 (Add myself to the `infra-ci` reviewer group and adjust some infra auto-labels) - rust-lang#144069 (ci: use windows 22 for all free runners) r? `@ghost` `@rustbot` modify labels: rollup

resolve: Make disambiguators for underscore bindings module-local (take 2) The difference with rust-lang#144013 can be seen in the second commit. Now we just keep a separate disambiguator counter in every `Module`, instead of a global counter in `Resolver`. This will be ok for parallel import resolution because we'll need to lock the module anyway when updating `resolutions` and other fields in it. And for external modules the disabmiguator could be just passed as an argument to `define_extern`, without using any cells or locks, once rust-lang#143884 lands. Unblocks rust-lang#143884.

resolve: Make disambiguators for underscore bindings module-local (take 2) The difference with #144013 can be seen in the second commit. Now we just keep a separate disambiguator counter in every `Module`, instead of a global counter in `Resolver`. This will be ok for parallel import resolution because we'll need to lock the module anyway when updating `resolutions` and other fields in it. And for external modules the disabmiguator could be just passed as an argument to `define_extern`, without using any cells or locks, once #143884 lands. Unblocks #143884.

resolve: Make disambiguators for underscore bindings module-local (take 2) The difference with rust-lang/rust#144013 can be seen in the second commit. Now we just keep a separate disambiguator counter in every `Module`, instead of a global counter in `Resolver`. This will be ok for parallel import resolution because we'll need to lock the module anyway when updating `resolutions` and other fields in it. And for external modules the disabmiguator could be just passed as an argument to `define_extern`, without using any cells or locks, once rust-lang/rust#143884 lands. Unblocks rust-lang/rust#143884.

Rollup of 11 pull requests Successful merges: - rust-lang/rust#143280 (Remove duplicate error about raw underscore lifetime) - rust-lang/rust#143649 (Add test for `default_field_values` and `const_default`) - rust-lang/rust#143699 (Make `AsyncDrop` check that it's being implemented on a local ADT) - rust-lang/rust#143908 (`tests/ui`: A New Order [0/28] ) - rust-lang/rust#143909 (docs(alloc::fmt): Make type optional, instead of matching empty string) - rust-lang/rust#143925 (Make slice comparisons const) - rust-lang/rust#143997 (Use $crate in macros for rustc_public (aka stable_mir)) - rust-lang/rust#144013 (resolve: Make disambiguators for underscore bindings module-local) - rust-lang/rust#144029 (Fix wrong messages from methods with the same name from different traits) - rust-lang/rust#144063 (Add myself to the `infra-ci` reviewer group and adjust some infra auto-labels) - rust-lang/rust#144069 (ci: use windows 22 for all free runners) r? `@ghost` `@rustbot` modify labels: rollup