Skip to content

Commit

Permalink
typeck: always lint for user's shadows
Browse files Browse the repository at this point in the history
`collision_safe` works when the new-unstable item is only shadowing a
from-std stable item, but it could also shadow a user-defined item from
a extension trait, in which case the lint should still fire. Use the
presence of a stability attribute on a chosen item as a heuristic for a
std item and continue to lint if the chosen item isn't from std even if
the unstable items are collision-safe.

Signed-off-by: David Wood <david.wood@huawei.com>
  • Loading branch information
davidtwco committed Nov 21, 2022
1 parent 8644b90 commit f552299
Show file tree
Hide file tree
Showing 4 changed files with 120 additions and 15 deletions.
39 changes: 24 additions & 15 deletions compiler/rustc_hir_typeck/src/method/probe.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1367,21 +1367,30 @@ impl<'tcx> Pick<'tcx> {
span: Span,
scope_expr_id: hir::HirId,
) {
if self.unstable_candidates.is_empty() {
return;
}

if self.unstable_candidates.iter().all(|(candidate, _)| {
let stab = tcx.lookup_stability(candidate.item.def_id);
debug!(?candidate, ?stab);
matches!(
stab,
Some(Stability {
level: StabilityLevel::Unstable { collision_safe: true, .. },
..
})
)
}) {
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;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -40,3 +40,35 @@ impl Bar {
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 { }
Original file line number Diff line number Diff line change
@@ -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!
}
Original file line number Diff line number Diff line change
@@ -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 <https://github.com/rust-lang/rust/issues/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 <https://github.com/rust-lang/rust/issues/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

0 comments on commit f552299

Please sign in to comment.