fix(resolve): replace bindings to dummy for unresolved imports

[bvanjoi]

close #109343

In #109343, f in pub use f as g points to:

namespace	binding
type	external crate f
value	None
macro	None

When resolve value_ns during resolve_doc_links, the value of the binding of single_import pub use f as g goes to pub use inner::f, and since it does not satisfy !self.is_accessible_from(binding.vis, single_import.parent_scope.module) and returns Err(Undetermined), which eventually goes to PathResult::Indeterminate => unreachable!.

This PR replace all namespace binding to dummy_binding for indeterminate import, so, the bindings of pub use f as g had been changed to followings after finalize:

namespace	binding
type	dummy
value	dummy
macro	dummy

r?@petrochenkov

[bvanjoi]

In my opinion, there are three solutions:

1. as I described in the PR, skip the binding whose Import marked by dummy.
2. add dummy_binding to the import_dummy_binding function for all namespaces whose binding has the value None, but this doesn't seem like a good way.
3. change the way resolve doc meta is handled to make sure it has the similar logic as the following code:

#![crate_type = "lib"]
extern crate f;

pub use inner::f;

pub use f as g;

g!{}

fn main() {}

[petrochenkov]

2. add dummy_binding to the import_dummy_binding function for all namespaces whose binding has the value None, but this doesn't seem like a good way.

But we are already doing that?
In fn import_dummy_binding, just below dummy.set(true);.

and since it does not satisfy [!self.is_accessible_from(binding.vis, single_import.parent_scope.module)]

Why does is_accessible_from return false?
Everything seems public enough there.
Maybe visibilities are not set correctly for dummy bindings?

[bvanjoi]

But we are already doing that?

No, at this stage we only add dummy_binding to import if the target_binding of all three namespaces is None.

The type namespace of f points to external crate f, so it does not satisfy this condition, so it ends up without value and macro without dummy_binding in it.

[bvanjoi]

Wait a minute, maybe we're not talking about the same import, I need to look at it again

[petrochenkov]

so it ends up without value and macro without dummy_binding in it

And without dummy.set(true) too, so I'm not sure what is changed.

[bvanjoi]

The logic goes like this:

For pub use inner::f, its three bindings point to dummy_binding.

For pub use f as g, since the binding of f's type namespace points to extern crate f, its macro and value both point to None instead of dummy_binding.

When dealing with value namespace, f gets the binding which pointed to pub use inner::f (instead of dummy_binding), which is visible to true, so is_accessible_from returns true and !is_accessible_from returns false

[bvanjoi]

And without dummy.set(true) too, so I'm not sure what is changed.

Yes, for pub use f as g does not work.

Perhaps we could put binding.is_dummy_binding_import to something that doesn't rely on a token value like dummy, for example to import.target_binding.all(|binding| binding.get().is_none())

[petrochenkov]

The issue is not related to the extern crate compatibility case.
Further minimized reproduction:

#![crate_type = "lib"]

pub mod f {}
pub use unresolved::f;

/// [g]
pub use f as g;

[bvanjoi]

In the newest commit, I had replaced all bindings to dummy for indeterminate imports, It seems more reasonable to avoid panic caused by Undetermined, cc @petrochenkov

@rustbot ready

[petrochenkov]

Thanks!
@bors r+

[klensy]

PR in rollup #111577 will be merged soon, testing already.

[bvanjoi]

PR description is very out of date.

Thanks for the tips! @rustbot ready

[petrochenkov]

@bors r+

[bors]

📌 Commit 1e8f887 has been approved by petrochenkov

It is now in the queue for this repository.

[bors]

⌛ Testing commit 1e8f887 with merge 94aba6b5c5ca54c7be1c4f20739fc233ada488c8...

[bors]

💥 Test timed out

[bvanjoi]

timeout again and I rebased latest code. so @rustbot ready

[petrochenkov]

@bors r+

[bors]

📌 Commit f34678c has been approved by petrochenkov

It is now in the queue for this repository.

[bors]

⌛ Testing commit f34678c with merge 92f5dea...

[bors]

☀️ Test successful - checks-actions
Approved by: petrochenkov
Pushing 92f5dea to master...

[rust-timer]

Finished benchmarking commit (92f5dea): comparison URL.

Overall result: ❌✅ regressions and improvements - no action needed

@rustbot label: -perf-regression

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

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

Cycles

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

Binary size

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

Bootstrap: 641.544s -> 641.663s (0.02%)