about summary refs log tree commit diff
diff options
context:
space:
mode:
authorMatthias Krüger <matthias.krueger@famsik.de>2023-05-23 19:53:42 +0200
committerGitHub <noreply@github.com>2023-05-23 19:53:42 +0200
commit37c9478b1a45e6dbcac9ba2c27bca4af32ed6d89 (patch)
tree11eb8d943f8a0828cf308c43381bbf93f4b94ea7
parentee08dd86582a15cf2755951f40c9e50adf72d39d (diff)
parentc41b2089c7a059a7ce86107b8f3528c05ca11254 (diff)
downloadrust-37c9478b1a45e6dbcac9ba2c27bca4af32ed6d89.tar.gz
rust-37c9478b1a45e6dbcac9ba2c27bca4af32ed6d89.zip
Rollup merge of #111761 - bvanjoi:fix-109148, r=petrochenkov
fix(resolve): not defined `extern crate shadow_name`

Fixes https://github.com/rust-lang/rust/issues/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`
-rw-r--r--compiler/rustc_resolve/src/build_reduced_graph.rs5
-rw-r--r--compiler/rustc_resolve/src/lib.rs2
-rw-r--r--tests/ui/imports/issue-109148.rs15
-rw-r--r--tests/ui/imports/issue-109148.stderr13
4 files changed, 34 insertions, 1 deletions
diff --git a/compiler/rustc_resolve/src/build_reduced_graph.rs b/compiler/rustc_resolve/src/build_reduced_graph.rs
index 08b73ebb694..b3d0e4ba258 100644
--- a/compiler/rustc_resolve/src/build_reduced_graph.rs
+++ b/compiler/rustc_resolve/src/build_reduced_graph.rs
@@ -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(
diff --git a/compiler/rustc_resolve/src/lib.rs b/compiler/rustc_resolve/src/lib.rs
index 3cdc3f0ecf8..1e31a0ff278 100644
--- a/compiler/rustc_resolve/src/lib.rs
+++ b/compiler/rustc_resolve/src/lib.rs
@@ -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,
diff --git a/tests/ui/imports/issue-109148.rs b/tests/ui/imports/issue-109148.rs
new file mode 100644
index 00000000000..694cb494a15
--- /dev/null
+++ b/tests/ui/imports/issue-109148.rs
@@ -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() {}
diff --git a/tests/ui/imports/issue-109148.stderr b/tests/ui/imports/issue-109148.stderr
new file mode 100644
index 00000000000..6cc1221cfe9
--- /dev/null
+++ b/tests/ui/imports/issue-109148.stderr
@@ -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
+