diff --git a/Cargo.lock b/Cargo.lock index fb6140e74fea4..842179453a4de 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -3666,6 +3666,7 @@ name = "rustc_hir_typeck" version = "0.1.0" dependencies = [ "rustc_ast", + "rustc_attr", "rustc_data_structures", "rustc_errors", "rustc_graphviz", diff --git a/compiler/rustc_attr/src/builtin.rs b/compiler/rustc_attr/src/builtin.rs index 753f62dd589d0..a7534613d1a31 100644 --- a/compiler/rustc_attr/src/builtin.rs +++ b/compiler/rustc_attr/src/builtin.rs @@ -180,6 +180,50 @@ pub enum StabilityLevel { /// fn foobar() {} /// ``` implied_by: Option, + /// When an unstable method exists on a type and the user is invoking a method that would + /// no longer have priority when the unstable method is stabilized, an "unstable name + /// collision" lint is emitted. In the vast majority of circumstances, this is desirable. + /// + /// However, when adding a new inherent method to the standard library, which deliberately + /// shadows the name of a method in the `Deref` target, then this lint can be triggered, + /// affecting users on stable who don't even know about the new unstable inherent method. + /// As the new method is being added to the standard library, by the library team, it can + /// be known that the lint isn't necessary, as the library team can ensure that the new + /// inherent method has the same behaviour and won't cause any problems for users when it + /// is stabilized. + /// + /// ```pseudo-Rust + /// pub struct Foo; + /// pub struct Bar; + /// + /// impl std::ops::Deref for Foo { + /// type Target = Bar; + /// fn deref(&self) -> &Self::Target { &Bar } + /// } + /// + /// impl Foo { + /// #[unstable(feature = "new_feature", issue = "none", collision_safe)] + /// pub fn example(&self) -> u32 { 4 } + /// } + /// + /// impl Bar { + /// #[stable(feature = "old_feature", since = "1.0.0")] + /// pub fn example(&self) -> u32 { 3 } + /// } + /// + /// // ..in another crate.. + /// fn main() { + /// let foo = Foo; + /// assert_eq!(foo.example(), 3); + /// // still invokes `Bar`'s `example`, as the `new_feature` isn't enabled, but doesn't + /// // trigger a name collision lint (in practice, both `example` functions should + /// // have identical behaviour) + /// } + /// ``` + /// + /// Without this addition, the new inherent method would need to be insta-stable in order + /// to avoid breaking stable users. + collision_safe: bool, }, /// `#[stable]` Stable { @@ -328,6 +372,7 @@ where let mut issue = None; let mut issue_num = None; let mut is_soft = false; + let mut collision_safe = false; let mut implied_by = None; for meta in metas { let Some(mi) = meta.meta_item() else { @@ -383,6 +428,14 @@ where } is_soft = true; } + sym::collision_safe => { + if !mi.is_word() { + sess.emit_err(session_diagnostics::CollisionSafeNoArgs { + span: mi.span, + }); + } + collision_safe = true; + } sym::implied_by => { if !get(mi, &mut implied_by) { continue 'outer; @@ -417,6 +470,7 @@ where issue: issue_num, is_soft, implied_by, + collision_safe, }; if sym::unstable == meta_name { stab = Some((Stability { level, feature }, attr.span)); diff --git a/compiler/rustc_attr/src/session_diagnostics.rs b/compiler/rustc_attr/src/session_diagnostics.rs index edccfa1c8ffa2..63547c30c96b4 100644 --- a/compiler/rustc_attr/src/session_diagnostics.rs +++ b/compiler/rustc_attr/src/session_diagnostics.rs @@ -385,6 +385,13 @@ pub(crate) struct SoftNoArgs { pub span: Span, } +#[derive(Diagnostic)] +#[diag(attr_collision_safe_no_args)] +pub(crate) struct CollisionSafeNoArgs { + #[primary_span] + pub span: Span, +} + #[derive(Diagnostic)] #[diag(attr_unknown_version_literal)] pub(crate) struct UnknownVersionLiteral { diff --git a/compiler/rustc_error_messages/locales/en-US/attr.ftl b/compiler/rustc_error_messages/locales/en-US/attr.ftl index a7f8c993d4225..5b4297397c28a 100644 --- a/compiler/rustc_error_messages/locales/en-US/attr.ftl +++ b/compiler/rustc_error_messages/locales/en-US/attr.ftl @@ -103,5 +103,8 @@ attr_expects_features = attr_soft_no_args = `soft` should not have any arguments +attr_collision_safe_no_args = + `collision_safe` should not have any arguments + attr_unknown_version_literal = unknown version literal format, assuming it refers to a future version diff --git a/compiler/rustc_hir_typeck/Cargo.toml b/compiler/rustc_hir_typeck/Cargo.toml index 093f9bb84486e..66b206c5fde08 100644 --- a/compiler/rustc_hir_typeck/Cargo.toml +++ b/compiler/rustc_hir_typeck/Cargo.toml @@ -9,6 +9,7 @@ edition = "2021" smallvec = { version = "1.8.1", features = ["union", "may_dangle"] } tracing = "0.1" rustc_ast = { path = "../rustc_ast" } +rustc_attr = { path = "../rustc_attr" } rustc_data_structures = { path = "../rustc_data_structures" } rustc_errors = { path = "../rustc_errors" } rustc_graphviz = { path = "../rustc_graphviz" } diff --git a/compiler/rustc_hir_typeck/src/method/probe.rs b/compiler/rustc_hir_typeck/src/method/probe.rs index 9d75ccad133dd..5de78e5d08686 100644 --- a/compiler/rustc_hir_typeck/src/method/probe.rs +++ b/compiler/rustc_hir_typeck/src/method/probe.rs @@ -5,6 +5,8 @@ use super::NoMatchData; use crate::errors::MethodCallOnUnknownType; use crate::FnCtxt; + +use rustc_attr::{Stability, StabilityLevel}; use rustc_data_structures::fx::FxHashSet; use rustc_errors::Applicability; use rustc_hir as hir; @@ -1365,9 +1367,33 @@ impl<'tcx> Pick<'tcx> { span: Span, scope_expr_id: hir::HirId, ) { - if self.unstable_candidates.is_empty() { + if self.unstable_candidates.is_empty() + || !{ + let has_collision_unsafe_candidate = + self.unstable_candidates.iter().any(|(candidate, _)| { + let stab = tcx.lookup_stability(candidate.item.def_id); + !matches!( + stab, + Some(Stability { + level: StabilityLevel::Unstable { collision_safe: true, .. }, + .. + }) + ) + }); + // `collision_safe` shouldn't silence the lint when the selected candidate is from + // user code, only when it is a collision with an item from std (where the + // assumption is that t-libs have confirmed that such a collision is okay). Use the + // existence of a stability attribute on the selected candidate has a heuristic + // for item from std. + let chosen_candidate_has_stability = + tcx.lookup_stability(self.item.def_id).is_some(); + debug!(?has_collision_unsafe_candidate, ?chosen_candidate_has_stability, ?self); + has_collision_unsafe_candidate || !chosen_candidate_has_stability + } + { return; } + let def_kind = self.item.kind.as_def_kind(); tcx.struct_span_lint_hir( lint::builtin::UNSTABLE_NAME_COLLISIONS, diff --git a/compiler/rustc_middle/src/middle/stability.rs b/compiler/rustc_middle/src/middle/stability.rs index 61bc089e431bb..f608a1f5e17a5 100644 --- a/compiler/rustc_middle/src/middle/stability.rs +++ b/compiler/rustc_middle/src/middle/stability.rs @@ -440,7 +440,7 @@ impl<'tcx> TyCtxt<'tcx> { match stability { Some(Stability { - level: attr::Unstable { reason, issue, is_soft, implied_by }, + level: attr::Unstable { reason, issue, is_soft, implied_by, .. }, feature, .. }) => { diff --git a/compiler/rustc_passes/src/stability.rs b/compiler/rustc_passes/src/stability.rs index 88bd655d8d384..a59dfea797f54 100644 --- a/compiler/rustc_passes/src/stability.rs +++ b/compiler/rustc_passes/src/stability.rs @@ -654,6 +654,7 @@ fn stability_index(tcx: TyCtxt<'_>, (): ()) -> Index { issue: NonZeroU32::new(27812), is_soft: false, implied_by: None, + collision_safe: false, }, feature: sym::rustc_private, }; diff --git a/compiler/rustc_resolve/src/macros.rs b/compiler/rustc_resolve/src/macros.rs index 8c7972f8eebb9..edf03a374a4f4 100644 --- a/compiler/rustc_resolve/src/macros.rs +++ b/compiler/rustc_resolve/src/macros.rs @@ -790,7 +790,8 @@ impl<'a> Resolver<'a> { ) { let span = path.span; if let Some(stability) = &ext.stability { - if let StabilityLevel::Unstable { reason, issue, is_soft, implied_by } = stability.level + if let StabilityLevel::Unstable { reason, issue, is_soft, implied_by, .. } = + stability.level { let feature = stability.feature; diff --git a/compiler/rustc_span/src/symbol.rs b/compiler/rustc_span/src/symbol.rs index 1b109ca6b11dd..b5b59e522e6cf 100644 --- a/compiler/rustc_span/src/symbol.rs +++ b/compiler/rustc_span/src/symbol.rs @@ -482,6 +482,7 @@ symbols! { coerce_unsized, cold, collapse_debuginfo, + collision_safe, column, compare_exchange, compare_exchange_weak, diff --git a/src/test/ui/stability-attribute/auxiliary/stability-attribute-collision-safe.rs b/src/test/ui/stability-attribute/auxiliary/stability-attribute-collision-safe.rs new file mode 100644 index 0000000000000..150b8cc6f5e19 --- /dev/null +++ b/src/test/ui/stability-attribute/auxiliary/stability-attribute-collision-safe.rs @@ -0,0 +1,74 @@ +#![crate_type = "lib"] +#![feature(staged_api)] +#![stable(feature = "old_feature", since = "1.0.0")] + +#[stable(feature = "old_feature", since = "1.0.0")] +pub struct Foo; + +#[stable(feature = "old_feature", since = "1.0.0")] +pub struct Bar; + +#[stable(feature = "old_feature", since = "1.0.0")] +impl std::ops::Deref for Foo { + type Target = Bar; + + fn deref(&self) -> &Self::Target { + &Bar + } +} + +impl Foo { + #[unstable(feature = "new_feature", issue = "none")] + pub fn example(&self) -> u32 { + 4 + } + + #[unstable(feature = "new_feature", issue = "none", collision_safe)] + pub fn example_safe(&self) -> u32 { + 2 + } +} + +impl Bar { + #[stable(feature = "old_feature", since = "1.0.0")] + pub fn example(&self) -> u32 { + 3 + } + + #[stable(feature = "old_feature", since = "1.0.0")] + pub fn example_safe(&self) -> u32 { + 2 + } +} + +#[stable(feature = "trait_feature", since = "1.0.0")] +pub trait Trait { + #[unstable(feature = "new_trait_feature", issue = "none")] + fn not_safe(&self) -> u32 { + 0 + } + + #[unstable(feature = "new_trait_feature", issue = "none", collision_safe)] + fn safe(&self) -> u32 { + 0 + } + + #[unstable(feature = "new_trait_feature", issue = "none", collision_safe)] + fn safe_and_shadowing_a_stable_item(&self) -> u32 { + 0 + } +} + +#[stable(feature = "trait_feature", since = "1.0.0")] +pub trait OtherTrait { + #[stable(feature = "trait_feature", since = "1.0.0")] + fn safe_and_shadowing_a_stable_item(&self) -> u32 { + 4 + } +} + +#[stable(feature = "trait_feature", since = "1.0.0")] +impl Trait for char { } + +#[stable(feature = "trait_feature", since = "1.0.0")] +impl OtherTrait for char { } diff --git a/src/test/ui/stability-attribute/stability-attribute-collision-safe-not-word.rs b/src/test/ui/stability-attribute/stability-attribute-collision-safe-not-word.rs new file mode 100644 index 0000000000000..0f24f3a2a7f96 --- /dev/null +++ b/src/test/ui/stability-attribute/stability-attribute-collision-safe-not-word.rs @@ -0,0 +1,20 @@ +#![crate_type = "lib"] +#![feature(staged_api)] +#![stable(feature = "old_feature", since = "1.0.0")] + +#[stable(feature = "old_feature", since = "1.0.0")] +pub struct Foo; + +impl Foo { + #[unstable(feature = "new_feature", issue = "none", collision_safe = "foo")] + //~^ ERROR `collision_safe` should not have any arguments + pub fn bad(&self) -> u32 { + 2 + } + + #[unstable(feature = "new_feature", issue = "none", collision_safe("foo"))] + //~^ ERROR `collision_safe` should not have any arguments + pub fn also_bad(&self) -> u32 { + 2 + } +} diff --git a/src/test/ui/stability-attribute/stability-attribute-collision-safe-not-word.stderr b/src/test/ui/stability-attribute/stability-attribute-collision-safe-not-word.stderr new file mode 100644 index 0000000000000..2508a24becac9 --- /dev/null +++ b/src/test/ui/stability-attribute/stability-attribute-collision-safe-not-word.stderr @@ -0,0 +1,14 @@ +error: `collision_safe` should not have any arguments + --> $DIR/stability-attribute-collision-safe-not-word.rs:9:57 + | +LL | #[unstable(feature = "new_feature", issue = "none", collision_safe = "foo")] + | ^^^^^^^^^^^^^^^^^^^^^^ + +error: `collision_safe` should not have any arguments + --> $DIR/stability-attribute-collision-safe-not-word.rs:15:57 + | +LL | #[unstable(feature = "new_feature", issue = "none", collision_safe("foo"))] + | ^^^^^^^^^^^^^^^^^^^^^ + +error: aborting due to 2 previous errors + diff --git a/src/test/ui/stability-attribute/stability-attribute-collision-safe-user.rs b/src/test/ui/stability-attribute/stability-attribute-collision-safe-user.rs new file mode 100644 index 0000000000000..0027b33482f82 --- /dev/null +++ b/src/test/ui/stability-attribute/stability-attribute-collision-safe-user.rs @@ -0,0 +1,35 @@ +// aux-build:stability-attribute-collision-safe.rs +#![deny(unstable_name_collisions)] + +extern crate stability_attribute_collision_safe; +use stability_attribute_collision_safe::{Trait, OtherTrait}; + +pub trait LocalTrait { + fn not_safe(&self) -> u32 { + 1 + } + + fn safe(&self) -> u32 { + 1 + } +} + +impl LocalTrait for char {} + + +fn main() { + // Despite having `collision_safe` on defn, the fn chosen doesn't have a stability attribute, + // thus could be user code (and is in this test), so the lint is still appropriate.. + assert_eq!('x'.safe(), 1); + //~^ ERROR an associated function with this name may be added to the standard library in the future + //~^^ WARN once this associated item is added to the standard library, the ambiguity may cause an error or change in behavior! + + // ..but with `collision_safe` on defn, if the chosen item has a stability attribute, then + // assumed to be from std or somewhere that's been checked to be okay, so no lint.. + assert_eq!('x'.safe_and_shadowing_a_stable_item(), 4); // okay! + + // ..and not safe functions should, of course, still lint.. + assert_eq!('x'.not_safe(), 1); + //~^ ERROR an associated function with this name may be added to the standard library in the future + //~^^ WARN once this associated item is added to the standard library, the ambiguity may cause an error or change in behavior! +} diff --git a/src/test/ui/stability-attribute/stability-attribute-collision-safe-user.stderr b/src/test/ui/stability-attribute/stability-attribute-collision-safe-user.stderr new file mode 100644 index 0000000000000..8fb1ebd90620f --- /dev/null +++ b/src/test/ui/stability-attribute/stability-attribute-collision-safe-user.stderr @@ -0,0 +1,29 @@ +error: an associated function with this name may be added to the standard library in the future + --> $DIR/stability-attribute-collision-safe-user.rs:23:20 + | +LL | assert_eq!('x'.safe(), 1); + | ^^^^ + | + = warning: once this associated item is added to the standard library, the ambiguity may cause an error or change in behavior! + = note: for more information, see issue #48919 + = help: call with fully qualified syntax `LocalTrait::safe(...)` to keep using the current method + = help: add `#![feature(new_trait_feature)]` to the crate attributes to enable `safe` +note: the lint level is defined here + --> $DIR/stability-attribute-collision-safe-user.rs:2:9 + | +LL | #![deny(unstable_name_collisions)] + | ^^^^^^^^^^^^^^^^^^^^^^^^ + +error: an associated function with this name may be added to the standard library in the future + --> $DIR/stability-attribute-collision-safe-user.rs:32:20 + | +LL | assert_eq!('x'.not_safe(), 1); + | ^^^^^^^^ + | + = warning: once this associated item is added to the standard library, the ambiguity may cause an error or change in behavior! + = note: for more information, see issue #48919 + = help: call with fully qualified syntax `LocalTrait::not_safe(...)` to keep using the current method + = help: add `#![feature(new_trait_feature)]` to the crate attributes to enable `not_safe` + +error: aborting due to 2 previous errors + diff --git a/src/test/ui/stability-attribute/stability-attribute-collision-safe-with-feature.rs b/src/test/ui/stability-attribute/stability-attribute-collision-safe-with-feature.rs new file mode 100644 index 0000000000000..f77a5e026033c --- /dev/null +++ b/src/test/ui/stability-attribute/stability-attribute-collision-safe-with-feature.rs @@ -0,0 +1,13 @@ +// aux-build:stability-attribute-collision-safe.rs +// check-pass +#![feature(new_feature)] + +extern crate stability_attribute_collision_safe; +use stability_attribute_collision_safe::Foo; + +fn main() { + let f = Foo; + assert_eq!(f.example_safe(), 2); // okay! has `collision_safe` on defn + + assert_eq!(f.example(), 4); // okay! have feature! +} diff --git a/src/test/ui/stability-attribute/stability-attribute-collision-safe-without-feature.rs b/src/test/ui/stability-attribute/stability-attribute-collision-safe-without-feature.rs new file mode 100644 index 0000000000000..94786fb7f9e77 --- /dev/null +++ b/src/test/ui/stability-attribute/stability-attribute-collision-safe-without-feature.rs @@ -0,0 +1,14 @@ +// aux-build:stability-attribute-collision-safe.rs +#![deny(unstable_name_collisions)] + +extern crate stability_attribute_collision_safe; +use stability_attribute_collision_safe::Foo; + +fn main() { + let f = Foo; + assert_eq!(f.example_safe(), 2); // okay! has `collision_safe` on defn + + assert_eq!(f.example(), 3); + //~^ ERROR an associated function with this name may be added to the standard library in the future + //~^^ WARN once this associated item is added to the standard library, the ambiguity may cause an error or change in behavior! +} diff --git a/src/test/ui/stability-attribute/stability-attribute-collision-safe-without-feature.stderr b/src/test/ui/stability-attribute/stability-attribute-collision-safe-without-feature.stderr new file mode 100644 index 0000000000000..e618c7549250d --- /dev/null +++ b/src/test/ui/stability-attribute/stability-attribute-collision-safe-without-feature.stderr @@ -0,0 +1,18 @@ +error: an associated function with this name may be added to the standard library in the future + --> $DIR/stability-attribute-collision-safe-without-feature.rs:11:18 + | +LL | assert_eq!(f.example(), 3); + | ^^^^^^^ + | + = warning: once this associated item is added to the standard library, the ambiguity may cause an error or change in behavior! + = note: for more information, see issue #48919 + = help: call with fully qualified syntax `Bar::example(...)` to keep using the current method + = help: add `#![feature(new_feature)]` to the crate attributes to enable `Foo::example` +note: the lint level is defined here + --> $DIR/stability-attribute-collision-safe-without-feature.rs:2:9 + | +LL | #![deny(unstable_name_collisions)] + | ^^^^^^^^^^^^^^^^^^^^^^^^ + +error: aborting due to previous error +