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.

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

resolve: mark binding is determined after all macros had been expanded #115269

Merged
merged 1 commit into from
Sep 13, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
2 changes: 1 addition & 1 deletion compiler/rustc_resolve/src/build_reduced_graph.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1239,7 +1239,7 @@ impl<'a, 'b, 'tcx> BuildReducedGraphVisitor<'a, 'b, 'tcx> {
use_span_with_attributes: span,
use_span: span,
root_span: span,
span: span,
span,
module_path: Vec::new(),
vis: Cell::new(Some(vis)),
used: Cell::new(true),
Expand Down
5 changes: 2 additions & 3 deletions compiler/rustc_resolve/src/ident.rs
Original file line number Diff line number Diff line change
Expand Up @@ -972,9 +972,8 @@ impl<'a, 'tcx> Resolver<'a, 'tcx> {
// progress, we have to ignore those potential unresolved invocations from other modules
// and prohibit access to macro-expanded `macro_export` macros instead (unless restricted
// shadowing is enabled, see `macro_expanded_macro_export_errors`).
let unexpanded_macros = !module.unexpanded_invocations.borrow().is_empty();
if let Some(binding) = binding {
if !unexpanded_macros || ns == MacroNS || restricted_shadowing {
if binding.determined() || ns == MacroNS || restricted_shadowing {
return check_usable(self, binding);
} else {
return Err((Undetermined, Weak::No));
Expand All @@ -991,7 +990,7 @@ impl<'a, 'tcx> Resolver<'a, 'tcx> {

// Check if one of unexpanded macros can still define the name,
// if it can then our "no resolution" result is not determined and can be invalidated.
if unexpanded_macros {
if !module.unexpanded_invocations.borrow().is_empty() {
return Err((Undetermined, Weak::Yes));
}

Expand Down
9 changes: 2 additions & 7 deletions compiler/rustc_resolve/src/imports.rs
Original file line number Diff line number Diff line change
Expand Up @@ -319,10 +319,7 @@ impl<'a, 'tcx> Resolver<'a, 'tcx> {
// We should replace the `old_binding` with `binding` regardless
// of whether they has same resolution or not when they are
// imported from the same glob-import statement.
// However we currently using `Some(old_binding)` for back compact
// purposes.
// This case can be removed after once `Undetermined` is prepared
// for glob-imports.
resolution.binding = Some(binding);
} else if res != old_binding.res() {
let binding = if warn_ambiguity {
this.warn_ambiguity(
Expand Down Expand Up @@ -805,13 +802,11 @@ impl<'a, 'tcx> Resolver<'a, 'tcx> {
// For better failure detection, pretend that the import will
// not define any names while resolving its module path.
let orig_vis = import.vis.take();
let binding = this.resolve_ident_in_module(
let binding = this.maybe_resolve_ident_in_module(
module,
source,
ns,
&import.parent_scope,
None,
None,
);
import.vis.set(orig_vis);
source_bindings[ns].set(binding);
Expand Down
13 changes: 13 additions & 0 deletions compiler/rustc_resolve/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -881,6 +881,19 @@ impl<'a> NameBindingData<'a> {
invoc_parent_expansion.is_descendant_of(self_parent_expansion);
!(certainly_before_other_or_simultaneously || certainly_before_invoc_or_simultaneously)
}

// Its purpose is to postpone the determination of a single binding because
// we can't predict whether it will be overwritten by recently expanded macros.
// FIXME: How can we integrate it with the `update_resolution`?
fn determined(&self) -> bool {
bvanjoi marked this conversation as resolved.
Show resolved Hide resolved
match &self.kind {
NameBindingKind::Import { binding, import, .. } if import.is_glob() => {
import.parent_scope.module.unexpanded_invocations.borrow().is_empty()
bvanjoi marked this conversation as resolved.
Show resolved Hide resolved
&& binding.determined()
}
_ => true,
}
}
}

#[derive(Default, Clone)]
Expand Down
17 changes: 17 additions & 0 deletions tests/ui/imports/import-after-macro-expand-10.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
// check-pass

mod b {
pub mod http {
pub struct HeaderMap;
}

pub use self::http::*;
#[derive(Debug)]
pub struct HeaderMap;
}

use crate::b::*;

fn main() {
let h: crate::b::HeaderMap = HeaderMap;
}
21 changes: 21 additions & 0 deletions tests/ui/imports/import-after-macro-expand-11.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
// check-pass

#[derive(Debug)]
struct H;

mod p {
use super::*;

#[derive(Clone)]
struct H;

mod t {
use super::*;

fn f() {
let h: crate::p::H = H;
}
}
}

fn main() {}
34 changes: 34 additions & 0 deletions tests/ui/imports/import-after-macro-expand-12.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
// check-pass
// https://github.com/rust-lang/rust/issues/115377

use module::*;

mod module {
pub enum B {}
impl B {
pub const ASSOC: u8 = 0;
}
}

#[derive()]
pub enum B {}
impl B {
pub const ASSOC: u16 = 0;
}

macro_rules! m {
($right:expr) => {
$right
};
}

fn main() {
let a: u16 = {
use self::*;
B::ASSOC
};
let b: u16 = m!({
use self::*;
B::ASSOC
});
}
22 changes: 22 additions & 0 deletions tests/ui/imports/import-after-macro-expand-13.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
// check-pass
// similar as `import-after-macro-expand-6.rs`

use crate::a::HeaderMap;

mod b {
pub mod http {
pub struct HeaderMap;
}

pub use self::http::*;
#[derive(Debug)]
pub struct HeaderMap;
}

mod a {
pub use crate::b::*;
}

fn main() {
let h: crate::b::HeaderMap = HeaderMap;
petrochenkov marked this conversation as resolved.
Show resolved Hide resolved
}
22 changes: 22 additions & 0 deletions tests/ui/imports/import-after-macro-expand-14.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
// check-pass

use crate::a::HeaderMap;

mod b {
pub mod http {
#[derive(Clone)]
pub struct HeaderMap;
}

pub use self::http::*;
#[derive(Debug)]
pub struct HeaderMap;
}

mod a {
pub use crate::b::*;
}

fn main() {
let h: crate::b::HeaderMap = HeaderMap;
}
4 changes: 1 addition & 3 deletions tests/ui/imports/import-after-macro-expand-2.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,9 +12,7 @@ mod tests {
use super::*;

fn test_thing() {
let thing: crate::thing::Thing = Thing::Bar;
// FIXME: `thing` should refer to `crate::Thing`,
// FIXME: but doesn't currently refer to it due to backward compatibility
let thing: crate::Thing = Thing::Foo;
}
}

Expand Down
11 changes: 1 addition & 10 deletions tests/ui/imports/import-after-macro-expand-4.rs
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
// check-pass
// https://github.com/rust-lang/rust/pull/113242#issuecomment-1616034904
// similar with `import-after-macro-expand-2.rs`

Expand All @@ -10,16 +11,6 @@ pub use a::*;
mod c {
use crate::*;
pub struct S(Vec<P>);
//~^ ERROR the size for values of type
//~| WARNING trait objects without an explicit
//~| WARNING this is accepted in the current edition
//~| WARNING trait objects without an explicit
//~| WARNING this is accepted in the current edition
//~| WARNING trait objects without an explicit
//~| WARNING this is accepted in the current edition

// FIXME: should works, but doesn't currently refer
// to it due to backward compatibility
}

#[derive(Clone)]
Expand Down
53 changes: 0 additions & 53 deletions tests/ui/imports/import-after-macro-expand-4.stderr

This file was deleted.

4 changes: 1 addition & 3 deletions tests/ui/imports/import-after-macro-expand-6.rs
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,5 @@ mod b {
use crate::a::HeaderMap;

fn main() {
let h: crate::b::http::HeaderMap = HeaderMap;
// FIXME: should refer to `crate::b::HeaderMap`,
// FIXME: but doesn't currently refer to it due to backward compatibility
let h: crate::b::HeaderMap = HeaderMap;
}
17 changes: 17 additions & 0 deletions tests/ui/imports/import-after-macro-expand-9.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
// check-pass

use crate::b::*;

mod b {
pub mod http {
pub struct HeaderMap;
}

pub use self::http::*;
#[derive(Debug)]
pub struct HeaderMap;
}

fn main() {
let h: crate::b::HeaderMap = HeaderMap;
}
Loading