Rollup merge of #149596 - petrochenkov:visambig2, r=yaahc
resolve: Report more visibility-related early resolution ambiguities for imports The new ambiguities are reported when the import's visibility is ambiguous and may depend on the resolution/expansion order. Detailed description: https://github.com/rust-lang/rust/pull/149596#issuecomment-3620449013.
This commit is contained in:
commit
55f75af470
13 changed files with 254 additions and 12 deletions
|
|
@ -20,6 +20,7 @@ declare_lint_pass! {
|
|||
AMBIGUOUS_GLOB_IMPORTED_TRAITS,
|
||||
AMBIGUOUS_GLOB_IMPORTS,
|
||||
AMBIGUOUS_GLOB_REEXPORTS,
|
||||
AMBIGUOUS_IMPORT_VISIBILITIES,
|
||||
AMBIGUOUS_PANIC_IMPORTS,
|
||||
ARITHMETIC_OVERFLOW,
|
||||
ASM_SUB_REGISTER,
|
||||
|
|
@ -4564,6 +4565,55 @@ declare_lint! {
|
|||
};
|
||||
}
|
||||
|
||||
declare_lint! {
|
||||
/// The `ambiguous_import_visibilities` lint detects imports that should report ambiguity
|
||||
/// errors, but previously didn't do that due to rustc bugs.
|
||||
///
|
||||
/// ### Example
|
||||
///
|
||||
/// ```rust,compile_fail
|
||||
/// #![deny(unknown_lints)]
|
||||
/// #![deny(ambiguous_import_visibilities)]
|
||||
/// mod reexport {
|
||||
/// mod m {
|
||||
/// pub struct S {}
|
||||
/// }
|
||||
///
|
||||
/// macro_rules! mac {
|
||||
/// () => { use m::S; }
|
||||
/// }
|
||||
///
|
||||
/// pub use m::*;
|
||||
/// mac!();
|
||||
///
|
||||
/// pub use S as Z; // ambiguous visibility
|
||||
/// }
|
||||
///
|
||||
/// fn main() {
|
||||
/// reexport::Z {};
|
||||
/// }
|
||||
/// ```
|
||||
///
|
||||
/// {{produces}}
|
||||
///
|
||||
/// ### Explanation
|
||||
///
|
||||
/// Previous versions of Rust compile it successfully because it
|
||||
/// fetched the glob import's visibility for `pub use S as Z` import, and ignored the private
|
||||
/// `use m::S` import that appeared later.
|
||||
///
|
||||
/// This is a [future-incompatible] lint to transition this to a
|
||||
/// hard error in the future.
|
||||
///
|
||||
/// [future-incompatible]: ../index.md#future-incompatible-lints
|
||||
pub AMBIGUOUS_IMPORT_VISIBILITIES,
|
||||
Warn,
|
||||
"detects certain glob imports that require reporting an ambiguity error",
|
||||
@future_incompatible = FutureIncompatibleInfo {
|
||||
reason: fcw!(FutureReleaseError #149145),
|
||||
};
|
||||
}
|
||||
|
||||
declare_lint! {
|
||||
/// The `refining_impl_trait_reachable` lint detects `impl Trait` return
|
||||
/// types in method signatures that are refined by a publically reachable
|
||||
|
|
|
|||
|
|
@ -408,6 +408,12 @@ impl<Id: Into<DefId>> Visibility<Id> {
|
|||
}
|
||||
}
|
||||
|
||||
impl<Id: Into<DefId> + Copy> Visibility<Id> {
|
||||
pub fn min(self, vis: Visibility<Id>, tcx: TyCtxt<'_>) -> Visibility<Id> {
|
||||
if self.is_at_least(vis, tcx) { vis } else { self }
|
||||
}
|
||||
}
|
||||
|
||||
impl Visibility<DefId> {
|
||||
pub fn expect_local(self) -> Visibility {
|
||||
self.map_id(|id| id.expect_local())
|
||||
|
|
|
|||
|
|
@ -24,8 +24,8 @@ use rustc_middle::ty::TyCtxt;
|
|||
use rustc_session::Session;
|
||||
use rustc_session::lint::BuiltinLintDiag;
|
||||
use rustc_session::lint::builtin::{
|
||||
ABSOLUTE_PATHS_NOT_STARTING_WITH_CRATE, AMBIGUOUS_GLOB_IMPORTS, AMBIGUOUS_PANIC_IMPORTS,
|
||||
MACRO_EXPANDED_MACRO_EXPORTS_ACCESSED_BY_ABSOLUTE_PATHS,
|
||||
ABSOLUTE_PATHS_NOT_STARTING_WITH_CRATE, AMBIGUOUS_GLOB_IMPORTS, AMBIGUOUS_IMPORT_VISIBILITIES,
|
||||
AMBIGUOUS_PANIC_IMPORTS, MACRO_EXPANDED_MACRO_EXPORTS_ACCESSED_BY_ABSOLUTE_PATHS,
|
||||
};
|
||||
use rustc_session::utils::was_invoked_from_cargo;
|
||||
use rustc_span::edit_distance::find_best_match_for_name;
|
||||
|
|
@ -145,6 +145,7 @@ impl<'ra, 'tcx> Resolver<'ra, 'tcx> {
|
|||
};
|
||||
|
||||
let lint = match ambiguity_warning {
|
||||
_ if ambiguity_error.ambig_vis.is_some() => AMBIGUOUS_IMPORT_VISIBILITIES,
|
||||
AmbiguityWarning::GlobImport => AMBIGUOUS_GLOB_IMPORTS,
|
||||
AmbiguityWarning::PanicImport => AMBIGUOUS_PANIC_IMPORTS,
|
||||
};
|
||||
|
|
@ -1995,7 +1996,8 @@ impl<'ra, 'tcx> Resolver<'ra, 'tcx> {
|
|||
}
|
||||
|
||||
fn ambiguity_diagnostic(&self, ambiguity_error: &AmbiguityError<'ra>) -> errors::Ambiguity {
|
||||
let AmbiguityError { kind, ident, b1, b2, scope1, scope2, .. } = *ambiguity_error;
|
||||
let AmbiguityError { kind, ambig_vis, ident, b1, b2, scope1, scope2, .. } =
|
||||
*ambiguity_error;
|
||||
let extern_prelude_ambiguity = || {
|
||||
// Note: b1 may come from a module scope, as an extern crate item in module.
|
||||
matches!(scope2, Scope::ExternPreludeFlags)
|
||||
|
|
@ -2074,9 +2076,18 @@ impl<'ra, 'tcx> Resolver<'ra, 'tcx> {
|
|||
None
|
||||
};
|
||||
|
||||
let ambig_vis = ambig_vis.map(|(vis1, vis2)| {
|
||||
format!(
|
||||
"{} or {}",
|
||||
vis1.to_string(CRATE_DEF_ID, self.tcx),
|
||||
vis2.to_string(CRATE_DEF_ID, self.tcx)
|
||||
)
|
||||
});
|
||||
|
||||
errors::Ambiguity {
|
||||
ident,
|
||||
help,
|
||||
ambig_vis,
|
||||
kind: kind.descr(),
|
||||
b1_note,
|
||||
b1_help_msgs,
|
||||
|
|
|
|||
|
|
@ -1463,6 +1463,7 @@ pub(crate) struct UnknownDiagnosticAttributeTypoSugg {
|
|||
// FIXME: Make this properly translatable.
|
||||
pub(crate) struct Ambiguity {
|
||||
pub ident: Ident,
|
||||
pub ambig_vis: Option<String>,
|
||||
pub kind: &'static str,
|
||||
pub help: Option<&'static [&'static str]>,
|
||||
pub b1_note: Spanned<String>,
|
||||
|
|
@ -1473,8 +1474,12 @@ pub(crate) struct Ambiguity {
|
|||
|
||||
impl Ambiguity {
|
||||
fn decorate<'a>(self, diag: &mut Diag<'a, impl EmissionGuarantee>) {
|
||||
diag.primary_message(format!("`{}` is ambiguous", self.ident));
|
||||
diag.span_label(self.ident.span, "ambiguous name");
|
||||
if let Some(ambig_vis) = self.ambig_vis {
|
||||
diag.primary_message(format!("ambiguous import visibility: {ambig_vis}"));
|
||||
} else {
|
||||
diag.primary_message(format!("`{}` is ambiguous", self.ident));
|
||||
diag.span_label(self.ident.span, "ambiguous name");
|
||||
}
|
||||
diag.note(format!("ambiguous because of {}", self.kind));
|
||||
diag.span_note(self.b1_note.span, self.b1_note.node);
|
||||
if let Some(help) = self.help {
|
||||
|
|
|
|||
|
|
@ -5,6 +5,7 @@ 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::ty::Visibility;
|
||||
use rustc_middle::{bug, span_bug};
|
||||
use rustc_session::lint::builtin::PROC_MACRO_DERIVE_RESOLUTION_FALLBACK;
|
||||
use rustc_session::parse::feature_err;
|
||||
|
|
@ -485,9 +486,12 @@ impl<'ra, 'tcx> Resolver<'ra, 'tcx> {
|
|||
// 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(decl));
|
||||
}
|
||||
let import_vis = match finalize {
|
||||
None | Some(Finalize { stage: Stage::Late, .. }) => {
|
||||
return ControlFlow::Break(Ok(decl));
|
||||
}
|
||||
Some(Finalize { import_vis, .. }) => import_vis,
|
||||
};
|
||||
|
||||
if let Some(&(innermost_decl, _)) = innermost_results.first() {
|
||||
// Found another solution, if the first one was "weak", report an error.
|
||||
|
|
@ -500,6 +504,7 @@ impl<'ra, 'tcx> Resolver<'ra, 'tcx> {
|
|||
decl,
|
||||
scope,
|
||||
&innermost_results,
|
||||
import_vis,
|
||||
) {
|
||||
// No need to search for more potential ambiguities, one is enough.
|
||||
return ControlFlow::Break(Ok(innermost_decl));
|
||||
|
|
@ -779,12 +784,22 @@ impl<'ra, 'tcx> Resolver<'ra, 'tcx> {
|
|||
decl: Decl<'ra>,
|
||||
scope: Scope<'ra>,
|
||||
innermost_results: &[(Decl<'ra>, Scope<'ra>)],
|
||||
import_vis: Option<Visibility>,
|
||||
) -> bool {
|
||||
let (innermost_decl, innermost_scope) = innermost_results[0];
|
||||
let (res, innermost_res) = (decl.res(), innermost_decl.res());
|
||||
if res == innermost_res {
|
||||
let ambig_vis = if res != innermost_res {
|
||||
None
|
||||
} else if let Some(import_vis) = import_vis
|
||||
&& let min =
|
||||
(|d: Decl<'_>| d.vis().min(import_vis.to_def_id(), self.tcx).expect_local())
|
||||
&& let (min1, min2) = (min(decl), min(innermost_decl))
|
||||
&& min1 != min2
|
||||
{
|
||||
Some((min1, min2))
|
||||
} else {
|
||||
return false;
|
||||
}
|
||||
};
|
||||
|
||||
// FIXME: Use `scope` instead of `res` to detect built-in attrs and derive helpers,
|
||||
// it will exclude imports, make slightly more code legal, and will require lang approval.
|
||||
|
|
@ -871,10 +886,17 @@ impl<'ra, 'tcx> Resolver<'ra, 'tcx> {
|
|||
|| (self.is_specific_builtin_macro(res, sym::core_panic)
|
||||
&& self.is_specific_builtin_macro(innermost_res, sym::std_panic)));
|
||||
|
||||
let warning = is_issue_147319_hack.then_some(AmbiguityWarning::PanicImport);
|
||||
let warning = if ambig_vis.is_some() {
|
||||
Some(AmbiguityWarning::GlobImport)
|
||||
} else if is_issue_147319_hack {
|
||||
Some(AmbiguityWarning::PanicImport)
|
||||
} else {
|
||||
None
|
||||
};
|
||||
|
||||
self.ambiguity_errors.push(AmbiguityError {
|
||||
kind,
|
||||
ambig_vis,
|
||||
ident: ident.orig(orig_ident_span),
|
||||
b1: innermost_decl,
|
||||
b2: decl,
|
||||
|
|
|
|||
|
|
@ -1189,7 +1189,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().decl(),
|
||||
Some(import),
|
||||
);
|
||||
|
|
|
|||
|
|
@ -969,6 +969,7 @@ enum AmbiguityWarning {
|
|||
|
||||
struct AmbiguityError<'ra> {
|
||||
kind: AmbiguityKind,
|
||||
ambig_vis: Option<(Visibility, Visibility)>,
|
||||
ident: Ident,
|
||||
b1: Decl<'ra>,
|
||||
b2: Decl<'ra>,
|
||||
|
|
@ -2087,6 +2088,7 @@ impl<'ra, 'tcx> Resolver<'ra, 'tcx> {
|
|||
if let Some(b2) = used_decl.ambiguity.get() {
|
||||
let ambiguity_error = AmbiguityError {
|
||||
kind: AmbiguityKind::GlobVsGlob,
|
||||
ambig_vis: None,
|
||||
ident,
|
||||
b1: used_decl,
|
||||
b2,
|
||||
|
|
@ -2556,6 +2558,8 @@ struct Finalize {
|
|||
used: Used = Used::Other,
|
||||
/// Finalizing early or late resolution.
|
||||
stage: Stage = Stage::Early,
|
||||
/// Nominal visibility of the import item, in case we are resolving an import's final segment.
|
||||
import_vis: Option<Visibility> = None,
|
||||
}
|
||||
|
||||
impl Finalize {
|
||||
|
|
|
|||
19
tests/ui/imports/ambiguous-import-visibility-macro.rs
Normal file
19
tests/ui/imports/ambiguous-import-visibility-macro.rs
Normal file
|
|
@ -0,0 +1,19 @@
|
|||
//@ check-pass
|
||||
//@ edition:2018
|
||||
//@ proc-macro: same-res-ambigious-extern-macro.rs
|
||||
|
||||
macro_rules! globbing{
|
||||
() => {
|
||||
pub use same_res_ambigious_extern_macro::*;
|
||||
}
|
||||
}
|
||||
|
||||
#[macro_use] // this imports the `RustEmbed` macro with `pub(crate)` visibility
|
||||
extern crate same_res_ambigious_extern_macro;
|
||||
globbing! {} // this imports the same `RustEmbed` macro with `pub` visibility
|
||||
|
||||
pub trait RustEmbed {}
|
||||
|
||||
pub use RustEmbed as Embed; //~ WARN ambiguous import visibility
|
||||
//~| WARN this was previously accepted
|
||||
fn main() {}
|
||||
29
tests/ui/imports/ambiguous-import-visibility-macro.stderr
Normal file
29
tests/ui/imports/ambiguous-import-visibility-macro.stderr
Normal file
|
|
@ -0,0 +1,29 @@
|
|||
warning: ambiguous import visibility: pub(crate) or pub
|
||||
--> $DIR/ambiguous-import-visibility-macro.rs:17:9
|
||||
|
|
||||
LL | pub use RustEmbed as Embed;
|
||||
| ^^^^^^^^^
|
||||
|
|
||||
= warning: this was previously accepted by the compiler but is being phased out; it will become a hard error in a future release!
|
||||
= note: for more information, see issue #149145 <https://github.com/rust-lang/rust/issues/149145>
|
||||
= note: ambiguous because of a conflict between a name from a glob import and an outer scope during import or macro resolution
|
||||
note: `RustEmbed` could refer to the derive macro imported here
|
||||
--> $DIR/ambiguous-import-visibility-macro.rs:7:17
|
||||
|
|
||||
LL | pub use same_res_ambigious_extern_macro::*;
|
||||
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
|
||||
...
|
||||
LL | globbing! {} // this imports the same `RustEmbed` macro with `pub` visibility
|
||||
| ------------ in this macro invocation
|
||||
= help: consider adding an explicit import of `RustEmbed` to disambiguate
|
||||
= help: or use `crate::RustEmbed` to refer to this derive macro unambiguously
|
||||
note: `RustEmbed` could also refer to the derive macro imported here
|
||||
--> $DIR/ambiguous-import-visibility-macro.rs:11:1
|
||||
|
|
||||
LL | #[macro_use] // this imports the `RustEmbed` macro with `pub(crate)` visibility
|
||||
| ^^^^^^^^^^^^
|
||||
= note: `#[warn(ambiguous_import_visibilities)]` (part of `#[warn(future_incompatible)]`) on by default
|
||||
= note: this warning originates in the macro `globbing` (in Nightly builds, run with -Z macro-backtrace for more info)
|
||||
|
||||
warning: 1 warning emitted
|
||||
|
||||
24
tests/ui/imports/ambiguous-import-visibility-module.rs
Normal file
24
tests/ui/imports/ambiguous-import-visibility-module.rs
Normal file
|
|
@ -0,0 +1,24 @@
|
|||
//@ check-pass
|
||||
//@ edition:2018..
|
||||
|
||||
mod reexport {
|
||||
mod m {
|
||||
pub struct S {}
|
||||
}
|
||||
|
||||
macro_rules! mac {
|
||||
() => {
|
||||
use m::S;
|
||||
};
|
||||
}
|
||||
|
||||
pub use m::*;
|
||||
mac!();
|
||||
|
||||
pub use S as Z; //~ WARN ambiguous import visibility
|
||||
//~| WARN this was previously accepted
|
||||
}
|
||||
|
||||
fn main() {
|
||||
reexport::Z {};
|
||||
}
|
||||
29
tests/ui/imports/ambiguous-import-visibility-module.stderr
Normal file
29
tests/ui/imports/ambiguous-import-visibility-module.stderr
Normal file
|
|
@ -0,0 +1,29 @@
|
|||
warning: ambiguous import visibility: pub or pub(in crate::reexport)
|
||||
--> $DIR/ambiguous-import-visibility-module.rs:18:13
|
||||
|
|
||||
LL | pub use S as Z;
|
||||
| ^
|
||||
|
|
||||
= warning: this was previously accepted by the compiler but is being phased out; it will become a hard error in a future release!
|
||||
= note: for more information, see issue #149145 <https://github.com/rust-lang/rust/issues/149145>
|
||||
= note: ambiguous because of a conflict between a macro-expanded name and a less macro-expanded name from outer scope during import or macro resolution
|
||||
note: `S` could refer to the struct imported here
|
||||
--> $DIR/ambiguous-import-visibility-module.rs:11:17
|
||||
|
|
||||
LL | use m::S;
|
||||
| ^^^^
|
||||
...
|
||||
LL | mac!();
|
||||
| ------ in this macro invocation
|
||||
= help: use `self::S` to refer to this struct unambiguously
|
||||
note: `S` could also refer to the struct imported here
|
||||
--> $DIR/ambiguous-import-visibility-module.rs:15:13
|
||||
|
|
||||
LL | pub use m::*;
|
||||
| ^^^^
|
||||
= help: use `self::S` to refer to this struct unambiguously
|
||||
= note: `#[warn(ambiguous_import_visibilities)]` (part of `#[warn(future_incompatible)]`) on by default
|
||||
= note: this warning originates in the macro `mac` (in Nightly builds, run with -Z macro-backtrace for more info)
|
||||
|
||||
warning: 1 warning emitted
|
||||
|
||||
14
tests/ui/imports/ambiguous-import-visibility.rs
Normal file
14
tests/ui/imports/ambiguous-import-visibility.rs
Normal file
|
|
@ -0,0 +1,14 @@
|
|||
//@ check-pass
|
||||
//@ edition:2018
|
||||
//@ proc-macro: same-res-ambigious-extern-macro.rs
|
||||
|
||||
#[macro_use] // this imports the `RustEmbed` macro with `pub(crate)` visibility
|
||||
extern crate same_res_ambigious_extern_macro;
|
||||
// this imports the same `RustEmbed` macro with `pub` visibility
|
||||
pub use same_res_ambigious_extern_macro::*;
|
||||
|
||||
pub trait RustEmbed {}
|
||||
|
||||
pub use RustEmbed as Embed; //~ WARN ambiguous import visibility
|
||||
//~| WARN this was previously accepted
|
||||
fn main() {}
|
||||
25
tests/ui/imports/ambiguous-import-visibility.stderr
Normal file
25
tests/ui/imports/ambiguous-import-visibility.stderr
Normal file
|
|
@ -0,0 +1,25 @@
|
|||
warning: ambiguous import visibility: pub(crate) or pub
|
||||
--> $DIR/ambiguous-import-visibility.rs:12:9
|
||||
|
|
||||
LL | pub use RustEmbed as Embed;
|
||||
| ^^^^^^^^^
|
||||
|
|
||||
= warning: this was previously accepted by the compiler but is being phased out; it will become a hard error in a future release!
|
||||
= note: for more information, see issue #149145 <https://github.com/rust-lang/rust/issues/149145>
|
||||
= note: ambiguous because of a conflict between a name from a glob import and an outer scope during import or macro resolution
|
||||
note: `RustEmbed` could refer to the derive macro imported here
|
||||
--> $DIR/ambiguous-import-visibility.rs:8:9
|
||||
|
|
||||
LL | pub use same_res_ambigious_extern_macro::*;
|
||||
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
|
||||
= help: consider adding an explicit import of `RustEmbed` to disambiguate
|
||||
= help: or use `crate::RustEmbed` to refer to this derive macro unambiguously
|
||||
note: `RustEmbed` could also refer to the derive macro imported here
|
||||
--> $DIR/ambiguous-import-visibility.rs:5:1
|
||||
|
|
||||
LL | #[macro_use] // this imports the `RustEmbed` macro with `pub(crate)` visibility
|
||||
| ^^^^^^^^^^^^
|
||||
= note: `#[warn(ambiguous_import_visibilities)]` (part of `#[warn(future_incompatible)]`) on by default
|
||||
|
||||
warning: 1 warning emitted
|
||||
|
||||
Loading…
Add table
Add a link
Reference in a new issue