Skip to content

Commit 8644b90

Browse files
davidtwcodavidtwco
committed
typeck: silence lint for collision safe items
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. ```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. Signed-off-by: David Wood <[email protected]>
1 parent 736c675 commit 8644b90

16 files changed

+209
-2
lines changed

‎Cargo.lock

+1
Original file line numberDiff line numberDiff line change
@@ -3666,6 +3666,7 @@ name = "rustc_hir_typeck"
36663666
version = "0.1.0"
36673667
dependencies = [
36683668
"rustc_ast",
3669+
"rustc_attr",
36693670
"rustc_data_structures",
36703671
"rustc_errors",
36713672
"rustc_graphviz",

‎compiler/rustc_attr/src/builtin.rs

+54
Original file line numberDiff line numberDiff line change
@@ -180,6 +180,50 @@ pub enum StabilityLevel {
180180
/// fn foobar() {}
181181
/// ```
182182
implied_by: Option<Symbol>,
183+
/// When an unstable method exists on a type and the user is invoking a method that would
184+
/// no longer have priority when the unstable method is stabilized, an "unstable name
185+
/// collision" lint is emitted. In the vast majority of circumstances, this is desirable.
186+
///
187+
/// However, when adding a new inherent method to the standard library, which deliberately
188+
/// shadows the name of a method in the `Deref` target, then this lint can be triggered,
189+
/// affecting users on stable who don't even know about the new unstable inherent method.
190+
/// As the new method is being added to the standard library, by the library team, it can
191+
/// be known that the lint isn't necessary, as the library team can ensure that the new
192+
/// inherent method has the same behaviour and won't cause any problems for users when it
193+
/// is stabilized.
194+
///
195+
/// ```pseudo-Rust
196+
/// pub struct Foo;
197+
/// pub struct Bar;
198+
///
199+
/// impl std::ops::Deref for Foo {
200+
/// type Target = Bar;
201+
/// fn deref(&self) -> &Self::Target { &Bar }
202+
/// }
203+
///
204+
/// impl Foo {
205+
/// #[unstable(feature = "new_feature", issue = "none", collision_safe)]
206+
/// pub fn example(&self) -> u32 { 4 }
207+
/// }
208+
///
209+
/// impl Bar {
210+
/// #[stable(feature = "old_feature", since = "1.0.0")]
211+
/// pub fn example(&self) -> u32 { 3 }
212+
/// }
213+
///
214+
/// // ..in another crate..
215+
/// fn main() {
216+
/// let foo = Foo;
217+
/// assert_eq!(foo.example(), 3);
218+
/// // still invokes `Bar`'s `example`, as the `new_feature` isn't enabled, but doesn't
219+
/// // trigger a name collision lint (in practice, both `example` functions should
220+
/// // have identical behaviour)
221+
/// }
222+
/// ```
223+
///
224+
/// Without this addition, the new inherent method would need to be insta-stable in order
225+
/// to avoid breaking stable users.
226+
collision_safe: bool,
183227
},
184228
/// `#[stable]`
185229
Stable {
@@ -328,6 +372,7 @@ where
328372
let mut issue = None;
329373
let mut issue_num = None;
330374
let mut is_soft = false;
375+
let mut collision_safe = false;
331376
let mut implied_by = None;
332377
for meta in metas {
333378
let Some(mi) = meta.meta_item() else {
@@ -383,6 +428,14 @@ where
383428
}
384429
is_soft = true;
385430
}
431+
sym::collision_safe => {
432+
if !mi.is_word() {
433+
sess.emit_err(session_diagnostics::CollisionSafeNoArgs {
434+
span: mi.span,
435+
});
436+
}
437+
collision_safe = true;
438+
}
386439
sym::implied_by => {
387440
if !get(mi, &mut implied_by) {
388441
continue 'outer;
@@ -417,6 +470,7 @@ where
417470
issue: issue_num,
418471
is_soft,
419472
implied_by,
473+
collision_safe,
420474
};
421475
if sym::unstable == meta_name {
422476
stab = Some((Stability { level, feature }, attr.span));

‎compiler/rustc_attr/src/session_diagnostics.rs

+7
Original file line numberDiff line numberDiff line change
@@ -385,6 +385,13 @@ pub(crate) struct SoftNoArgs {
385385
pub span: Span,
386386
}
387387

388+
#[derive(Diagnostic)]
389+
#[diag(attr_collision_safe_no_args)]
390+
pub(crate) struct CollisionSafeNoArgs {
391+
#[primary_span]
392+
pub span: Span,
393+
}
394+
388395
#[derive(Diagnostic)]
389396
#[diag(attr_unknown_version_literal)]
390397
pub(crate) struct UnknownVersionLiteral {

‎compiler/rustc_error_messages/locales/en-US/attr.ftl

+3
Original file line numberDiff line numberDiff line change
@@ -103,5 +103,8 @@ attr_expects_features =
103103
attr_soft_no_args =
104104
`soft` should not have any arguments
105105
106+
attr_collision_safe_no_args =
107+
`collision_safe` should not have any arguments
108+
106109
attr_unknown_version_literal =
107110
unknown version literal format, assuming it refers to a future version

‎compiler/rustc_hir_typeck/Cargo.toml

+1
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ edition = "2021"
99
smallvec = { version = "1.8.1", features = ["union", "may_dangle"] }
1010
tracing = "0.1"
1111
rustc_ast = { path = "../rustc_ast" }
12+
rustc_attr = { path = "../rustc_attr" }
1213
rustc_data_structures = { path = "../rustc_data_structures" }
1314
rustc_errors = { path = "../rustc_errors" }
1415
rustc_graphviz = { path = "../rustc_graphviz" }

‎compiler/rustc_hir_typeck/src/method/probe.rs

+17
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,8 @@ use super::NoMatchData;
55

66
use crate::errors::MethodCallOnUnknownType;
77
use crate::FnCtxt;
8+
9+
use rustc_attr::{Stability, StabilityLevel};
810
use rustc_data_structures::fx::FxHashSet;
911
use rustc_errors::Applicability;
1012
use rustc_hir as hir;
@@ -1368,6 +1370,21 @@ impl<'tcx> Pick<'tcx> {
13681370
if self.unstable_candidates.is_empty() {
13691371
return;
13701372
}
1373+
1374+
if self.unstable_candidates.iter().all(|(candidate, _)| {
1375+
let stab = tcx.lookup_stability(candidate.item.def_id);
1376+
debug!(?candidate, ?stab);
1377+
matches!(
1378+
stab,
1379+
Some(Stability {
1380+
level: StabilityLevel::Unstable { collision_safe: true, .. },
1381+
..
1382+
})
1383+
)
1384+
}) {
1385+
return;
1386+
}
1387+
13711388
let def_kind = self.item.kind.as_def_kind();
13721389
tcx.struct_span_lint_hir(
13731390
lint::builtin::UNSTABLE_NAME_COLLISIONS,

‎compiler/rustc_middle/src/middle/stability.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -440,7 +440,7 @@ impl<'tcx> TyCtxt<'tcx> {
440440

441441
match stability {
442442
Some(Stability {
443-
level: attr::Unstable { reason, issue, is_soft, implied_by },
443+
level: attr::Unstable { reason, issue, is_soft, implied_by, .. },
444444
feature,
445445
..
446446
}) => {

‎compiler/rustc_passes/src/stability.rs

+1
Original file line numberDiff line numberDiff line change
@@ -654,6 +654,7 @@ fn stability_index(tcx: TyCtxt<'_>, (): ()) -> Index {
654654
issue: NonZeroU32::new(27812),
655655
is_soft: false,
656656
implied_by: None,
657+
collision_safe: false,
657658
},
658659
feature: sym::rustc_private,
659660
};

‎compiler/rustc_resolve/src/macros.rs

+2-1
Original file line numberDiff line numberDiff line change
@@ -790,7 +790,8 @@ impl<'a> Resolver<'a> {
790790
) {
791791
let span = path.span;
792792
if let Some(stability) = &ext.stability {
793-
if let StabilityLevel::Unstable { reason, issue, is_soft, implied_by } = stability.level
793+
if let StabilityLevel::Unstable { reason, issue, is_soft, implied_by, .. } =
794+
stability.level
794795
{
795796
let feature = stability.feature;
796797

‎compiler/rustc_span/src/symbol.rs

+1
Original file line numberDiff line numberDiff line change
@@ -482,6 +482,7 @@ symbols! {
482482
coerce_unsized,
483483
cold,
484484
collapse_debuginfo,
485+
collision_safe,
485486
column,
486487
compare_exchange,
487488
compare_exchange_weak,

‎src/test/ui/stability-attribute/auxiliary/stability-attribute-collision-safe.rs

+42
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,42 @@
1+
#![crate_type = "lib"]
2+
#![feature(staged_api)]
3+
#![stable(feature = "old_feature", since = "1.0.0")]
4+
5+
#[stable(feature = "old_feature", since = "1.0.0")]
6+
pub struct Foo;
7+
8+
#[stable(feature = "old_feature", since = "1.0.0")]
9+
pub struct Bar;
10+
11+
#[stable(feature = "old_feature", since = "1.0.0")]
12+
impl std::ops::Deref for Foo {
13+
type Target = Bar;
14+
15+
fn deref(&self) -> &Self::Target {
16+
&Bar
17+
}
18+
}
19+
20+
impl Foo {
21+
#[unstable(feature = "new_feature", issue = "none")]
22+
pub fn example(&self) -> u32 {
23+
4
24+
}
25+
26+
#[unstable(feature = "new_feature", issue = "none", collision_safe)]
27+
pub fn example_safe(&self) -> u32 {
28+
2
29+
}
30+
}
31+
32+
impl Bar {
33+
#[stable(feature = "old_feature", since = "1.0.0")]
34+
pub fn example(&self) -> u32 {
35+
3
36+
}
37+
38+
#[stable(feature = "old_feature", since = "1.0.0")]
39+
pub fn example_safe(&self) -> u32 {
40+
2
41+
}
42+
}

‎src/test/ui/stability-attribute/stability-attribute-collision-safe-not-word.rs

+20
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,20 @@
1+
#![crate_type = "lib"]
2+
#![feature(staged_api)]
3+
#![stable(feature = "old_feature", since = "1.0.0")]
4+
5+
#[stable(feature = "old_feature", since = "1.0.0")]
6+
pub struct Foo;
7+
8+
impl Foo {
9+
#[unstable(feature = "new_feature", issue = "none", collision_safe = "foo")]
10+
//~^ ERROR `collision_safe` should not have any arguments
11+
pub fn bad(&self) -> u32 {
12+
2
13+
}
14+
15+
#[unstable(feature = "new_feature", issue = "none", collision_safe("foo"))]
16+
//~^ ERROR `collision_safe` should not have any arguments
17+
pub fn also_bad(&self) -> u32 {
18+
2
19+
}
20+
}

‎src/test/ui/stability-attribute/stability-attribute-collision-safe-not-word.stderr

+14
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,14 @@
1+
error: `collision_safe` should not have any arguments
2+
--> $DIR/stability-attribute-collision-safe-not-word.rs:9:57
3+
|
4+
LL | #[unstable(feature = "new_feature", issue = "none", collision_safe = "foo")]
5+
| ^^^^^^^^^^^^^^^^^^^^^^
6+
7+
error: `collision_safe` should not have any arguments
8+
--> $DIR/stability-attribute-collision-safe-not-word.rs:15:57
9+
|
10+
LL | #[unstable(feature = "new_feature", issue = "none", collision_safe("foo"))]
11+
| ^^^^^^^^^^^^^^^^^^^^^
12+
13+
error: aborting due to 2 previous errors
14+

‎src/test/ui/stability-attribute/stability-attribute-collision-safe-with-feature.rs

+13
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,13 @@
1+
// aux-build:stability-attribute-collision-safe.rs
2+
// check-pass
3+
#![feature(new_feature)]
4+
5+
extern crate stability_attribute_collision_safe;
6+
use stability_attribute_collision_safe::Foo;
7+
8+
fn main() {
9+
let f = Foo;
10+
assert_eq!(f.example_safe(), 2); // okay! has `collision_safe` on defn
11+
12+
assert_eq!(f.example(), 4); // okay! have feature!
13+
}

‎src/test/ui/stability-attribute/stability-attribute-collision-safe-without-feature.rs

+14
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,14 @@
1+
// aux-build:stability-attribute-collision-safe.rs
2+
#![deny(unstable_name_collisions)]
3+
4+
extern crate stability_attribute_collision_safe;
5+
use stability_attribute_collision_safe::Foo;
6+
7+
fn main() {
8+
let f = Foo;
9+
assert_eq!(f.example_safe(), 2); // okay! has `collision_safe` on defn
10+
11+
assert_eq!(f.example(), 3);
12+
//~^ ERROR an associated function with this name may be added to the standard library in the future
13+
//~^^ WARN once this associated item is added to the standard library, the ambiguity may cause an error or change in behavior!
14+
}

‎src/test/ui/stability-attribute/stability-attribute-collision-safe-without-feature.stderr

+18
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,18 @@
1+
error: an associated function with this name may be added to the standard library in the future
2+
--> $DIR/stability-attribute-collision-safe-without-feature.rs:11:18
3+
|
4+
LL | assert_eq!(f.example(), 3);
5+
| ^^^^^^^
6+
|
7+
= warning: once this associated item is added to the standard library, the ambiguity may cause an error or change in behavior!
8+
= note: for more information, see issue #48919 <https://github.com/rust-lang/rust/issues/48919>
9+
= help: call with fully qualified syntax `Bar::example(...)` to keep using the current method
10+
= help: add `#![feature(new_feature)]` to the crate attributes to enable `Foo::example`
11+
note: the lint level is defined here
12+
--> $DIR/stability-attribute-collision-safe-without-feature.rs:2:9
13+
|
14+
LL | #![deny(unstable_name_collisions)]
15+
| ^^^^^^^^^^^^^^^^^^^^^^^^
16+
17+
error: aborting due to previous error
18+

0 commit comments

Comments
 (0)