Skip to content

Commit

Permalink
resolve: determined binding after parent module macro expand
Browse files Browse the repository at this point in the history
  • Loading branch information
bvanjoi committed Sep 13, 2023
1 parent 1fe747c commit f153650
Show file tree
Hide file tree
Showing 14 changed files with 154 additions and 80 deletions.
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 {
match &self.kind {
NameBindingKind::Import { binding, import, .. } if import.is_glob() => {
import.parent_scope.module.unexpanded_invocations.borrow().is_empty()
&& 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;
}
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;
}

0 comments on commit f153650

Please sign in to comment.