Use the glob binding in resolve_rustdoc_path process

[mu001999]

Fixes #117920

Returning None seems enough.

I reproduces and tests this locally by cargo +stage1 build, but I cannot reproduce this ICE by putting the following code into tests/ui/... and then compiling it using rustc +stage1 /path/to/test.rs or x.py test:

#![crate_type = "lib"]

use super::Hasher;

/// [`Hasher`]
pub use core::hash::*;

r? @petrochenkov

[petrochenkov]

This change hides some deeper issue, maybe_resolve_path should never return Indeterminate here, and the unreachable is here to check this post-condition.
@rustbot author

[petrochenkov]

cc @bvanjoi

[bvanjoi]

reduced:

// edition: 2021
// compile-flags: --crate-type=lib

use super::A;

mod b {
    pub trait A {}
    pub trait B {}
}

/// [`A`]
pub use b::*;

fn main() {}

This issue arises when the dummy binding for super::A is not updated correctly.

Before invoking import_dummy_binding, the binding of (root, A) is pointing to root::b::A. And it will attempt to define dummy binding but fails due to the prohibition of overrides from Res::Error, see https://github.com/rust-lang/rust/blob/master/compiler/rustc_resolve/src/imports.rs#L308

Therefore, the initial solution for this issue is to delete the following code:

if res == Res::Err && old_binding.res() != Res::Err {
     return Ok(())
}

I believe this solution could work, yet it might not be worth implementing for invalid code. This pertains to core logic and may introduce unexpected behavior.

Another solution is to ensure that the glob binding is used when resolving (root, A) during the resolve_rustdoc_path process. To implement this, you can add the following code to import_dummy_binding after try_define:

this.update_resolution(import.parent_scope.module, key, false, |_, resolution| {
    resolution.single_imports.remove(&import);
})

I think this is a more robust method.

[bvanjoi]

Another point worth investigating is why the following code doesn't trigger a panic:

// edition: 2021
// compile-flags: --crate-type=lib

use super::A;

mod b {
    pub trait A {}
    // pub trait B {}
}

/// [`A`]
pub use b::*;

fn main() {}

edit: this case doesn't trigger a panic because it returns early in resolve_doc_links due to it doesn't re-export anything.

[mu001999]

@rustbot ready

[mu001999]

@petrochenkov hi, I think this PR is ready for now ;)

[petrochenkov]

@mu001999
I didn't forget, just busy, so the review queue is moving slow.

[petrochenkov]

@bors r+

[bors]

📌 Commit 2368b75 has been approved by petrochenkov

It is now in the queue for this repository.

[bors]

⌛ Testing commit 2368b75 with merge d1f3fb2...

[bors]

💔 Test failed - checks-actions

[petrochenkov]

@bors r+

[bors]

📌 Commit 940473a has been approved by petrochenkov

It is now in the queue for this repository.

[bors]

⌛ Testing commit 940473a with merge 1fdfe12...

[bors]

☀️ Test successful - checks-actions
Approved by: petrochenkov
Pushing 1fdfe12 to master...

[rust-timer]

Finished benchmarking commit (1fdfe12): comparison URL.

Overall result: ❌ regressions - ACTION NEEDED

Next Steps: If you can justify the regressions found in this perf run, please indicate this with @rustbot label: +perf-regression-triaged along with sufficient written justification. If you cannot justify the regressions please open an issue or create a new PR that fixes the regressions, add a comment linking to the newly created issue or PR, and then add the perf-regression-triaged label to this PR.

@rustbot label: +perf-regression
cc @rust-lang/wg-compiler-performance

Warning ⚠: The following benchmark(s) failed to build:

• exa-0.10.1
• diesel-1.4.8
• hyper-0.14.18

cc @rust-lang/wg-compiler-performance

Instruction count

This is a highly reliable metric that was used to determine the overall result at the top of this comment.

	mean	range	count
Regressions ❌ (primary)	5.2%	[1.3%, 9.5%]	3
Regressions ❌ (secondary)	1.4%	[1.4%, 1.4%]	1
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-	-	0
All ❌✅ (primary)	5.2%	[1.3%, 9.5%]	3

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

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)	4.6%	[1.1%, 8.2%]	3
Regressions ❌ (secondary)	-	-	0
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-	-	0
All ❌✅ (primary)	4.6%	[1.1%, 8.2%]	3

Binary size

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

Bootstrap: 677.588s -> 676.286s (-0.19%)
Artifact size: 314.19 MiB -> 314.18 MiB (-0.00%)

[Mark-Simulacrum]

All regressions appear to be have been spurious, resolving themselves on the next merge (#118687). Removing perf-regression label.