Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(resolve): not defined extern crate shadow_name #111761

Merged
merged 1 commit into from
May 23, 2023
Merged

fix(resolve): not defined extern crate shadow_name #111761

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions compiler/rustc_resolve/src/build_reduced_graph.rs
View file
Open in desktop
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;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, this is a good solution too.

Could you add a comment saying that this is an ambiguity error, but it's not registered in ambiguity_errors, and that triggers some asserts later on if the binding is added, so we don't add the binding.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it's worth discussing the actual binding of use std::xxx.

When we add return here, std::xxx ultimately refers to prelude::std.

However, based on your comment, it seems that std::xxx will point to extern crate blash(I guess, but not entirely sure).

There might be a difference between these two resolutions, so in my opinion, we should determine which action is better first.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The choice is not that important because it only happens during failing compilation anyway.
The difference may only happen in some secondary diagnostics.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If this case is migrated to regular ambiguity errors, then extern crate blah will be selected as a resolution, yes, because it's closer in scopes at the resolution point.
(This migration is probably a better long term solution, but it's a minor language change, so it's better done separately from fixing an ICE.)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If the migration to regular ambiguity errors happens then this test case:

macro_rules! m {
    () => {
        extern crate core as std;
        
        use std::mem;
    }
}

m!();

fn main() {}

won't produce any errors.

}
}
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
View file
Open in desktop
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
View file
Open in desktop
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
View file
Open in desktop
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