diff --git a/compiler/rustc_resolve/src/ident.rs b/compiler/rustc_resolve/src/ident.rs index f59b5a0aad9a7..1a029745ee5bc 100644 --- a/compiler/rustc_resolve/src/ident.rs +++ b/compiler/rustc_resolve/src/ident.rs @@ -5,7 +5,8 @@ use Namespace::*; use rustc_ast::{self as ast, NodeId}; use rustc_errors::ErrorGuaranteed; use rustc_hir::def::{DefKind, MacroKinds, Namespace, NonMacroAttrKind, PartialRes, PerNS}; -use rustc_middle::{bug, span_bug}; +use rustc_hir::def_id::DefId; +use rustc_middle::{bug, span_bug, ty}; use rustc_session::lint::builtin::PROC_MACRO_DERIVE_RESOLUTION_FALLBACK; use rustc_session::parse::feature_err; use rustc_span::hygiene::{ExpnId, ExpnKind, LocalExpnId, MacroKind, SyntaxContext}; @@ -465,13 +466,34 @@ impl<'ra, 'tcx> Resolver<'ra, 'tcx> { Ok((binding, flags)) if sub_namespace_match(binding.macro_kinds(), macro_kind) => { + if !matches!(scope, Scope::MacroUsePrelude) { + let adjusted_module = if let Scope::Module(module, _) = scope + && !matches!(scope_set, ScopeSet::ModuleAndExternPrelude(..)) + { + module + } else { + parent_scope.module + }; + if !this.is_accessible_from(binding.vis, adjusted_module) { + this.dcx() + .struct_span_err(binding.span, "binding") + .with_span_label(adjusted_module.span, "module") + .emit(); + } + } + // Below we report various ambiguity errors. // We do not need to report them if we are either in speculative resolution, // or in late resolution when everything is already imported and expanded // and no ambiguities exist. - if matches!(finalize, None | Some(Finalize { stage: Stage::Late, .. })) { - return ControlFlow::Break(Ok(binding)); - } + let (significant_visibility, import_vis) = match finalize { + None | Some(Finalize { stage: Stage::Late, .. }) => { + return ControlFlow::Break(Ok(binding)); + } + Some(Finalize { significant_visibility, import_vis, .. }) => { + (significant_visibility, import_vis) + } + }; if let Some((innermost_binding, innermost_flags)) = innermost_result { // Found another solution, if the first one was "weak", report an error. @@ -482,6 +504,8 @@ impl<'ra, 'tcx> Resolver<'ra, 'tcx> { innermost_binding, flags, innermost_flags, + significant_visibility, + import_vis, extern_prelude_item_binding, extern_prelude_flag_binding, ) { @@ -730,11 +754,21 @@ impl<'ra, 'tcx> Resolver<'ra, 'tcx> { innermost_binding: NameBinding<'ra>, flags: Flags, innermost_flags: Flags, + significant_visibility: bool, + import_vis: Option, extern_prelude_item_binding: Option>, extern_prelude_flag_binding: Option>, ) -> bool { let (res, innermost_res) = (binding.res(), innermost_binding.res()); - if res == innermost_res { + + let vis_min = |v1: ty::Visibility, v2: ty::Visibility| { + if v1.is_at_least(v2, self.tcx) { v2 } else { v1 } + }; + let bad_vis = significant_visibility + && vis_min(binding.vis, import_vis.unwrap().to_def_id()) + != vis_min(innermost_binding.vis, import_vis.unwrap().to_def_id()); + + if res == innermost_res && !bad_vis { return false; } @@ -1207,7 +1241,7 @@ impl<'ra, 'tcx> Resolver<'ra, 'tcx> { && shadowing == Shadowing::Restricted && finalize.stage == Stage::Early && binding.expansion != LocalExpnId::ROOT - && binding.res() != shadowed_glob.res() + && (binding.res() != shadowed_glob.res() || finalize.significant_visibility) { self.ambiguity_errors.push(AmbiguityError { kind: AmbiguityKind::GlobVsExpanded, diff --git a/compiler/rustc_resolve/src/imports.rs b/compiler/rustc_resolve/src/imports.rs index f98aaecea18ce..6059a2bfe92bc 100644 --- a/compiler/rustc_resolve/src/imports.rs +++ b/compiler/rustc_resolve/src/imports.rs @@ -394,6 +394,7 @@ impl<'ra, 'tcx> Resolver<'ra, 'tcx> { && non_glob_binding.expansion != LocalExpnId::ROOT && glob_binding.res() != non_glob_binding.res() { + // FIXME: record vis mismatches as well, but breakage in practice. resolution.non_glob_binding = Some(this.new_ambiguity_binding( AmbiguityKind::GlobVsExpanded, non_glob_binding, @@ -413,7 +414,8 @@ impl<'ra, 'tcx> Resolver<'ra, 'tcx> { glob_binding, false, )); - } else if !old_glob_binding.vis.is_at_least(binding.vis, this.tcx) { + } else if !old_glob_binding.vis.is_at_least(glob_binding.vis, this.tcx) + { resolution.glob_binding = Some(glob_binding); } } else { @@ -957,7 +959,7 @@ impl<'ra, 'tcx> Resolver<'ra, 'tcx> { &import.module_path, None, &import.parent_scope, - Some(finalize), + Some(Finalize { significant_visibility: false, ..finalize }), ignore_binding, Some(import), ); @@ -1135,7 +1137,11 @@ impl<'ra, 'tcx> Resolver<'ra, 'tcx> { ident, ns, &import.parent_scope, - Some(Finalize { report_private: false, ..finalize }), + Some(Finalize { + report_private: false, + import_vis: Some(import.vis), + ..finalize + }), bindings[ns].get().binding(), Some(import), ); diff --git a/compiler/rustc_resolve/src/late.rs b/compiler/rustc_resolve/src/late.rs index df620ddb66529..6bb49dbce9493 100644 --- a/compiler/rustc_resolve/src/late.rs +++ b/compiler/rustc_resolve/src/late.rs @@ -1498,7 +1498,7 @@ impl<'a, 'ast, 'ra, 'tcx> LateResolutionVisitor<'a, 'ast, 'ra, 'tcx> { opt_ns, &self.parent_scope, Some(source), - finalize, + finalize.map(|finalize| Finalize { significant_visibility: false, ..finalize }), Some(&self.ribs), None, None, diff --git a/compiler/rustc_resolve/src/lib.rs b/compiler/rustc_resolve/src/lib.rs index 16eeb9229c977..5b4609c56a7f4 100644 --- a/compiler/rustc_resolve/src/lib.rs +++ b/compiler/rustc_resolve/src/lib.rs @@ -2031,6 +2031,8 @@ impl<'ra, 'tcx> Resolver<'ra, 'tcx> { warn_ambiguity: bool, ) { if let Some((b2, kind)) = used_binding.ambiguity { + // FIXME: Need to check if the `AmbiguityKind::GlobVsGlob`s caused by visibility + // mismatch ever passed through an import, but it's too breaking. let ambiguity_error = AmbiguityError { kind, ident, @@ -2503,6 +2505,11 @@ struct Finalize { used: Used = Used::Other, /// Finalizing early or late resolution. stage: Stage = Stage::Early, + /// Ambiguous bindings with different visibilities need to be reported + /// even if they have the same `Res`olutions. + significant_visibility: bool = true, + /// Tralala + import_vis: Option = None, } impl Finalize { diff --git a/compiler/rustc_resolve/src/macros.rs b/compiler/rustc_resolve/src/macros.rs index 1222b40d8b367..255d24fb2cbfd 100644 --- a/compiler/rustc_resolve/src/macros.rs +++ b/compiler/rustc_resolve/src/macros.rs @@ -868,11 +868,13 @@ impl<'ra, 'tcx> Resolver<'ra, 'tcx> { for seg in &mut path { seg.id = None; } + + let finalize = Finalize::new(ast::CRATE_NODE_ID, path_span); match self.cm().resolve_path( &path, Some(ns), &parent_scope, - Some(Finalize::new(ast::CRATE_NODE_ID, path_span)), + Some(Finalize { significant_visibility: false, ..finalize }), None, None, ) { @@ -941,11 +943,12 @@ impl<'ra, 'tcx> Resolver<'ra, 'tcx> { let macro_resolutions = self.single_segment_macro_resolutions.take(self); for (ident, kind, parent_scope, initial_binding, sugg_span) in macro_resolutions { + let finalize = Finalize::new(ast::CRATE_NODE_ID, ident.span); match self.cm().resolve_ident_in_scope_set( ident, ScopeSet::Macro(kind), &parent_scope, - Some(Finalize::new(ast::CRATE_NODE_ID, ident.span)), + Some(Finalize { significant_visibility: false, ..finalize }), true, None, None, @@ -996,11 +999,12 @@ impl<'ra, 'tcx> Resolver<'ra, 'tcx> { let builtin_attrs = mem::take(&mut self.builtin_attrs); for (ident, parent_scope) in builtin_attrs { + let finalize = Finalize::new(ast::CRATE_NODE_ID, ident.span); let _ = self.cm().resolve_ident_in_scope_set( ident, ScopeSet::Macro(MacroKind::Attr), &parent_scope, - Some(Finalize::new(ast::CRATE_NODE_ID, ident.span)), + Some(Finalize { significant_visibility: false, ..finalize }), true, None, None, diff --git a/tests/ui/imports/auxiliary/same-res-ambigious-extern-fail.rs b/tests/ui/imports/auxiliary/same-res-ambigious-extern-fail.rs index 61a8d8f0054a8..401c5cef022c7 100644 --- a/tests/ui/imports/auxiliary/same-res-ambigious-extern-fail.rs +++ b/tests/ui/imports/auxiliary/same-res-ambigious-extern-fail.rs @@ -13,4 +13,4 @@ globbing! {} // this imports the same `RustEmbed` macro with `pub` visibility pub trait RustEmbed {} -pub use RustEmbed as Embed; +pub use self::RustEmbed as Embed; diff --git a/tests/ui/imports/auxiliary/same-res-ambigious-extern.rs b/tests/ui/imports/auxiliary/same-res-ambigious-extern.rs index 5269dcd0b17a2..bcc688737fb79 100644 --- a/tests/ui/imports/auxiliary/same-res-ambigious-extern.rs +++ b/tests/ui/imports/auxiliary/same-res-ambigious-extern.rs @@ -8,4 +8,4 @@ pub use same_res_ambigious_extern_macro::*; pub trait RustEmbed {} -pub use RustEmbed as Embed; +pub use self::RustEmbed as Embed; diff --git a/tests/ui/imports/same-res-ambigious.fail.stderr b/tests/ui/imports/same-res-ambigious.fail.stderr deleted file mode 100644 index dfd7c5a5f94e0..0000000000000 --- a/tests/ui/imports/same-res-ambigious.fail.stderr +++ /dev/null @@ -1,20 +0,0 @@ -error[E0603]: derive macro `Embed` is private - --> $DIR/same-res-ambigious.rs:8:28 - | -LL | #[derive(ambigious_extern::Embed)] - | ^^^^^ private derive macro - | -note: the derive macro `Embed` is defined here - --> $DIR/auxiliary/same-res-ambigious-extern-fail.rs:16:9 - | -LL | pub use RustEmbed as Embed; - | ^^^^^^^^^ -help: import `Embed` directly - | -LL - #[derive(ambigious_extern::Embed)] -LL + #[derive(same_res_ambigious_extern_macro::RustEmbed)] - | - -error: aborting due to 1 previous error - -For more information about this error, try `rustc --explain E0603`. diff --git a/tests/ui/imports/same-res-ambigious.rs b/tests/ui/imports/same-res-ambigious.rs index b5c13a15b7c9c..137050752b156 100644 --- a/tests/ui/imports/same-res-ambigious.rs +++ b/tests/ui/imports/same-res-ambigious.rs @@ -1,11 +1,11 @@ //@ edition: 2018 //@ revisions: fail pass -//@[pass] check-pass +//@ check-pass //@[pass] aux-crate: ambigious_extern=same-res-ambigious-extern.rs //@[fail] aux-crate: ambigious_extern=same-res-ambigious-extern-fail.rs // see https://github.com/rust-lang/rust/pull/147196 -#[derive(ambigious_extern::Embed)] //[fail]~ ERROR: derive macro `Embed` is private +#[derive(ambigious_extern::Embed)] struct Foo{} fn main(){}