Skip to content

Commit

Permalink
Rollup merge of #111761 - bvanjoi:fix-109148, r=petrochenkov
Browse files Browse the repository at this point in the history
fix(resolve): not defined `extern crate shadow_name`

Fixes #109148

## Why does #109148 panic?

When resolving `use std::xx` it enters `visit_scopes` from `early_resolve_ident_in_lexical_scope`, and iters twice during the loop:

|iter| `scope` | `break_result` | result |
|-|-|-|-|
| 0 | `Module` pointed to root | binding pointed to `Undetermined`, so result is `None` | scope changed to `ExternPrelude` |
| 1 | `ExternPrelude` | binding pointed to `std` | - |

Then, the result of `maybe_resolve_path` is `Module(std)`, so `import.imported_module.set` is executed.

Finally, during the `finalize_import` of `use std::xx`, `resolve_path` returns `NonModule` because `Binding(Ident(std), Module(root)`'s binding points to `extern crate blah as std`, which causes the assertion to fail at `assert!(import.imported_module.get().is_none());`.

## Investigation

The question is why `#[a] extern crate blah as std` is not defined as a binding of `std::xxx`, which causes the iteration twice during `visit_scopes` when resolving `std::xxx`. Ideally, the value of `break_result.is_some()` should have been valid in the first iteration.

After debugging, I found that because `#[a] extern crate blah as std` had been dummied by `placeholder` during `collect_invocations`, so it had lost its attrs, span, etc..., so it will not be defined. However, `expand_invoc` added them back, then the next `build_reduced_graph`, `#[a] extern crate blah as std` would have been defined, so it makes the result of `resolved_path` unexpected, and the program panics.

## Try to solve

I think there has two-way to solve this issue:

- Expand invocations before the first `resolve_imports` during `fully_expand_fragment`. However, I do not think this is a good idea because it would mess up the current design.
- As my PR described: do not define to `extern crate blah as std` during the second `build_reduced_graph`, which is very easy and more reasonable.

r? `@petrochenkov`
  • Loading branch information
matthiaskrgr committed May 23, 2023
2 parents ee08dd8 + c41b208 commit 37c9478
Show file tree
Hide file tree
Showing 4 changed files with 34 additions and 1 deletion.
5 changes: 5 additions & 0 deletions compiler/rustc_resolve/src/build_reduced_graph.rs
Original file line number Diff line number Diff line change
Expand Up @@ -873,6 +873,11 @@ impl<'a, 'b, 'tcx> BuildReducedGraphVisitor<'a, 'b, 'tcx> {
let msg = "macro-expanded `extern crate` items cannot \
shadow names passed with `--extern`";
self.r.tcx.sess.span_err(item.span, msg);
// `return` is intended to discard this binding because it's an
// unregistered ambiguity error which would result in a panic
// caused by inconsistency `path_res`
// more details: https://github.com/rust-lang/rust/pull/111761
return;
}
}
let entry = self.r.extern_prelude.entry(ident.normalize_to_macros_2_0()).or_insert(
Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_resolve/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -106,7 +106,7 @@ impl Determinacy {
/// A specific scope in which a name can be looked up.
/// This enum is currently used only for early resolution (imports and macros),
/// but not for late resolution yet.
#[derive(Clone, Copy)]
#[derive(Clone, Copy, Debug)]
enum Scope<'a> {
DeriveHelpers(LocalExpnId),
DeriveHelpersCompat,
Expand Down
15 changes: 15 additions & 0 deletions tests/ui/imports/issue-109148.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
// edition: 2021

// https://github.com/rust-lang/rust/pull/111761#issuecomment-1557777314
macro_rules! m {
() => {
extern crate core as std;
//~^ ERROR macro-expanded `extern crate` items cannot shadow names passed with `--extern`
}
}

m!();

use std::mem;

fn main() {}
13 changes: 13 additions & 0 deletions tests/ui/imports/issue-109148.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
error: macro-expanded `extern crate` items cannot shadow names passed with `--extern`
--> $DIR/issue-109148.rs:6:9
|
LL | extern crate core as std;
| ^^^^^^^^^^^^^^^^^^^^^^^^^
...
LL | m!();
| ---- in this macro invocation
|
= note: this error originates in the macro `m` (in Nightly builds, run with -Z macro-backtrace for more info)

error: aborting due to previous error

0 comments on commit 37c9478

Please sign in to comment.