Skip to content

Commit 1230201

Browse files
tmandrytmandry
committed
Split refining_impl_trait lint into _reachable, _internal variants
1 parent 3c02972 commit 1230201

File tree

12 files changed

+169
-28
lines changed

12 files changed

+169
-28
lines changed

‎compiler/rustc_hir_analysis/messages.ftl

+1
Original file line numberDiff line numberDiff line change
@@ -347,6 +347,7 @@ hir_analysis_rpitit_refined = impl trait in impl method signature does not match
347347
.label = return type from trait method defined here
348348
.unmatched_bound_label = this bound is stronger than that defined on the trait
349349
.note = add `#[allow(refining_impl_trait)]` if it is intended for this to be part of the public API of this crate
350+
.feedback_note = we are soliciting feedback, see issue #121718 <https://github.com/rust-lang/rust/issues/121718> for more information
350351
351352
hir_analysis_self_in_impl_self =
352353
`Self` is not valid in the self type of an impl block

‎compiler/rustc_hir_analysis/src/check/compare_impl_item/refine.rs

+17-16
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@ use rustc_data_structures::fx::FxIndexSet;
22
use rustc_hir as hir;
33
use rustc_hir::def_id::DefId;
44
use rustc_infer::infer::{outlives::env::OutlivesEnvironment, TyCtxtInferExt};
5-
use rustc_lint_defs::builtin::REFINING_IMPL_TRAIT;
5+
use rustc_lint_defs::builtin::{REFINING_IMPL_TRAIT_INTERNAL, REFINING_IMPL_TRAIT_REACHABLE};
66
use rustc_middle::traits::{ObligationCause, Reveal};
77
use rustc_middle::ty::{
88
self, Ty, TyCtxt, TypeFoldable, TypeFolder, TypeSuperVisitable, TypeVisitable, TypeVisitor,
@@ -24,26 +24,23 @@ pub(super) fn check_refining_return_position_impl_trait_in_trait<'tcx>(
2424
if !tcx.impl_method_has_trait_impl_trait_tys(impl_m.def_id) {
2525
return;
2626
}
27+
2728
// unreachable traits don't have any library guarantees, there's no need to do this check.
28-
if trait_m
29+
let is_internal = trait_m
2930
.container_id(tcx)
3031
.as_local()
3132
.is_some_and(|trait_def_id| !tcx.effective_visibilities(()).is_reachable(trait_def_id))
32-
{
33-
return;
34-
}
33+
// If a type in the trait ref is private, then there's also no reason to do this check.
34+
|| impl_trait_ref.args.iter().any(|arg| {
35+
if let Some(ty) = arg.as_type()
36+
&& let Some(self_visibility) = type_visibility(tcx, ty)
37+
{
38+
return !self_visibility.is_public();
39+
}
40+
false
41+
});
3542

36-
// If a type in the trait ref is private, then there's also no reason to do this check.
3743
let impl_def_id = impl_m.container_id(tcx);
38-
for arg in impl_trait_ref.args {
39-
if let Some(ty) = arg.as_type()
40-
&& let Some(self_visibility) = type_visibility(tcx, ty)
41-
&& !self_visibility.is_public()
42-
{
43-
return;
44-
}
45-
}
46-
4744
let impl_m_args = ty::GenericArgs::identity_for_item(tcx, impl_m.def_id);
4845
let trait_m_to_impl_m_args = impl_m_args.rebase_onto(tcx, impl_def_id, impl_trait_ref.args);
4946
let bound_trait_m_sig = tcx.fn_sig(trait_m.def_id).instantiate(tcx, trait_m_to_impl_m_args);
@@ -86,6 +83,7 @@ pub(super) fn check_refining_return_position_impl_trait_in_trait<'tcx>(
8683
trait_m.def_id,
8784
impl_m.def_id,
8885
None,
86+
is_internal,
8987
);
9088
return;
9189
};
@@ -105,6 +103,7 @@ pub(super) fn check_refining_return_position_impl_trait_in_trait<'tcx>(
105103
trait_m.def_id,
106104
impl_m.def_id,
107105
None,
106+
is_internal,
108107
);
109108
return;
110109
}
@@ -199,6 +198,7 @@ pub(super) fn check_refining_return_position_impl_trait_in_trait<'tcx>(
199198
trait_m.def_id,
200199
impl_m.def_id,
201200
Some(span),
201+
is_internal,
202202
);
203203
return;
204204
}
@@ -239,6 +239,7 @@ fn report_mismatched_rpitit_signature<'tcx>(
239239
trait_m_def_id: DefId,
240240
impl_m_def_id: DefId,
241241
unmatched_bound: Option<Span>,
242+
is_internal: bool,
242243
) {
243244
let mapping = std::iter::zip(
244245
tcx.fn_sig(trait_m_def_id).skip_binder().bound_vars(),
@@ -291,7 +292,7 @@ fn report_mismatched_rpitit_signature<'tcx>(
291292

292293
let span = unmatched_bound.unwrap_or(span);
293294
tcx.emit_node_span_lint(
294-
REFINING_IMPL_TRAIT,
295+
if is_internal { REFINING_IMPL_TRAIT_INTERNAL } else { REFINING_IMPL_TRAIT_REACHABLE },
295296
tcx.local_def_id_to_hir_id(impl_m_def_id.expect_local()),
296297
span,
297298
crate::errors::ReturnPositionImplTraitInTraitRefined {

‎compiler/rustc_hir_analysis/src/errors.rs

+1
Original file line numberDiff line numberDiff line change
@@ -1072,6 +1072,7 @@ pub struct UnusedAssociatedTypeBounds {
10721072
#[derive(LintDiagnostic)]
10731073
#[diag(hir_analysis_rpitit_refined)]
10741074
#[note]
1075+
#[note(hir_analysis_feedback_note)]
10751076
pub(crate) struct ReturnPositionImplTraitInTraitRefined<'tcx> {
10761077
#[suggestion(applicability = "maybe-incorrect", code = "{pre}{return_ty}{post}")]
10771078
pub impl_return_span: Span,

‎compiler/rustc_lint/src/lib.rs

+6
Original file line numberDiff line numberDiff line change
@@ -313,6 +313,12 @@ fn register_builtins(store: &mut LintStore) {
313313
// MACRO_USE_EXTERN_CRATE
314314
);
315315

316+
add_lint_group!(
317+
"refining_impl_trait",
318+
REFINING_IMPL_TRAIT_REACHABLE,
319+
REFINING_IMPL_TRAIT_INTERNAL
320+
);
321+
316322
// Register renamed and removed lints.
317323
store.register_renamed("single_use_lifetime", "single_use_lifetimes");
318324
store.register_renamed("elided_lifetime_in_path", "elided_lifetimes_in_paths");

‎compiler/rustc_lint_defs/src/builtin.rs

+56-8
Original file line numberDiff line numberDiff line change
@@ -77,7 +77,8 @@ declare_lint_pass! {
7777
PROC_MACRO_BACK_COMPAT,
7878
PROC_MACRO_DERIVE_RESOLUTION_FALLBACK,
7979
PUB_USE_OF_PRIVATE_EXTERN_CRATE,
80-
REFINING_IMPL_TRAIT,
80+
REFINING_IMPL_TRAIT_INTERNAL,
81+
REFINING_IMPL_TRAIT_REACHABLE,
8182
RENAMED_AND_REMOVED_LINTS,
8283
REPR_TRANSPARENT_EXTERNAL_PRIVATE_FIELDS,
8384
RUST_2021_INCOMPATIBLE_CLOSURE_CAPTURES,
@@ -4311,7 +4312,9 @@ declare_lint! {
43114312

43124313
declare_lint! {
43134314
/// The `refining_impl_trait` lint detects usages of return-position impl
4314-
/// traits in trait signatures which are refined by implementations.
4315+
/// traits in trait signatures which are refined by implementations, meaning
4316+
/// the implementation adds information about the return type that is not
4317+
/// present in the trait.
43154318
///
43164319
/// ### Example
43174320
///
@@ -4341,13 +4344,58 @@ declare_lint! {
43414344
///
43424345
/// ### Explanation
43434346
///
4344-
/// Return-position impl trait in traits (RPITITs) desugar to associated types,
4345-
/// and callers of methods for types where the implementation is known are
4347+
/// Callers of methods for types where the implementation is known are
43464348
/// able to observe the types written in the impl signature. This may be
4347-
/// intended behavior, but may also pose a semver hazard for authors of libraries
4348-
/// who do not wish to make stronger guarantees about the types than what is
4349-
/// written in the trait signature.
4350-
pub REFINING_IMPL_TRAIT,
4349+
/// intended behavior, but may also lead to implementation details being
4350+
/// revealed unintentionally. In particular, it may pose a semver hazard
4351+
/// for authors of libraries who do not wish to make stronger guarantees
4352+
/// about the types than what is written in the trait signature.
4353+
pub REFINING_IMPL_TRAIT_REACHABLE,
4354+
Warn,
4355+
"impl trait in impl method signature does not match trait method signature",
4356+
}
4357+
4358+
declare_lint! {
4359+
/// The `refining_impl_trait` lint detects usages of return-position impl
4360+
/// traits in trait signatures which are refined by implementations, meaning
4361+
/// the implementation adds information about the return type that is not
4362+
/// present in the trait.
4363+
///
4364+
/// ### Example
4365+
///
4366+
/// ```rust,compile_fail
4367+
/// #![deny(refining_impl_trait)]
4368+
///
4369+
/// use std::fmt::Display;
4370+
///
4371+
/// pub trait AsDisplay {
4372+
/// fn as_display(&self) -> impl Display;
4373+
/// }
4374+
///
4375+
/// impl<'s> AsDisplay for &'s str {
4376+
/// fn as_display(&self) -> Self {
4377+
/// *self
4378+
/// }
4379+
/// }
4380+
///
4381+
/// fn main() {
4382+
/// // users can observe that the return type of
4383+
/// // `<&str as AsDisplay>::as_display()` is `&str`.
4384+
/// let x: &str = "".as_display();
4385+
/// }
4386+
/// ```
4387+
///
4388+
/// {{produces}}
4389+
///
4390+
/// ### Explanation
4391+
///
4392+
/// Callers of methods for types where the implementation is known are
4393+
/// able to observe the types written in the impl signature. This may be
4394+
/// intended behavior, but may also lead to implementation details being
4395+
/// revealed unintentionally. In particular, it may pose a semver hazard
4396+
/// for authors of libraries who do not wish to make stronger guarantees
4397+
/// about the types than what is written in the trait signature.
4398+
pub REFINING_IMPL_TRAIT_INTERNAL,
43514399
Warn,
43524400
"impl trait in impl method signature does not match trait method signature",
43534401
}

‎tests/ui/async-await/in-trait/async-example-desugared-boxed.stderr

+2
Original file line numberDiff line numberDiff line change
@@ -8,11 +8,13 @@ LL | fn foo(&self) -> Pin<Box<dyn Future<Output = i32> + '_>> {
88
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
99
|
1010
= note: add `#[allow(refining_impl_trait)]` if it is intended for this to be part of the public API of this crate
11+
= note: we are soliciting feedback, see issue #121718 <https://github.com/rust-lang/rust/issues/121718> for more information
1112
note: the lint level is defined here
1213
--> $DIR/async-example-desugared-boxed.rs:13:12
1314
|
1415
LL | #[warn(refining_impl_trait)]
1516
| ^^^^^^^^^^^^^^^^^^^
17+
= note: `#[warn(refining_impl_trait_reachable)]` implied by `#[warn(refining_impl_trait)]`
1618
help: replace the return type so that it matches the trait
1719
|
1820
LL | fn foo(&self) -> impl Future<Output = i32> {

‎tests/ui/async-await/in-trait/async-example-desugared-manual.stderr

+2
Original file line numberDiff line numberDiff line change
@@ -8,11 +8,13 @@ LL | fn foo(&self) -> MyFuture {
88
| ^^^^^^^^
99
|
1010
= note: add `#[allow(refining_impl_trait)]` if it is intended for this to be part of the public API of this crate
11+
= note: we are soliciting feedback, see issue #121718 <https://github.com/rust-lang/rust/issues/121718> for more information
1112
note: the lint level is defined here
1213
--> $DIR/async-example-desugared-manual.rs:21:12
1314
|
1415
LL | #[warn(refining_impl_trait)]
1516
| ^^^^^^^^^^^^^^^^^^^
17+
= note: `#[warn(refining_impl_trait_reachable)]` implied by `#[warn(refining_impl_trait)]`
1618
help: replace the return type so that it matches the trait
1719
|
1820
LL | fn foo(&self) -> impl Future<Output = i32> {

‎tests/ui/impl-trait/in-trait/bad-item-bound-within-rpitit.stderr

+2-1
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,8 @@ LL | fn iter(&self) -> impl 'a + Iterator<Item = I::Item<'a>> {
2222
| ^^ this bound is stronger than that defined on the trait
2323
|
2424
= note: add `#[allow(refining_impl_trait)]` if it is intended for this to be part of the public API of this crate
25-
= note: `#[warn(refining_impl_trait)]` on by default
25+
= note: we are soliciting feedback, see issue #121718 <https://github.com/rust-lang/rust/issues/121718> for more information
26+
= note: `#[warn(refining_impl_trait_reachable)]` on by default
2627
help: replace the return type so that it matches the trait
2728
|
2829
LL | fn iter(&self) -> impl Iterator<Item = <Self as Iterable>::Item<'_>> + '_ {

‎tests/ui/impl-trait/in-trait/foreign.rs

+2-1
Original file line numberDiff line numberDiff line change
@@ -19,8 +19,9 @@ impl Foo for Local {
1919

2020
struct LocalIgnoreRefining;
2121
impl Foo for LocalIgnoreRefining {
22-
#[deny(refining_impl_trait)]
22+
#[warn(refining_impl_trait)]
2323
fn bar(self) -> Arc<String> {
24+
//~^ WARN impl method signature does not match trait method signature
2425
Arc::new(String::new())
2526
}
2627
}

‎tests/ui/impl-trait/in-trait/foreign.stderr

+21
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,21 @@
1+
warning: impl trait in impl method signature does not match trait method signature
2+
--> $DIR/foreign.rs:23:21
3+
|
4+
LL | fn bar(self) -> Arc<String> {
5+
| ^^^^^^^^^^^
6+
|
7+
= note: add `#[allow(refining_impl_trait)]` if it is intended for this to be part of the public API of this crate
8+
= note: we are soliciting feedback, see issue #121718 <https://github.com/rust-lang/rust/issues/121718> for more information
9+
note: the lint level is defined here
10+
--> $DIR/foreign.rs:22:12
11+
|
12+
LL | #[warn(refining_impl_trait)]
13+
| ^^^^^^^^^^^^^^^^^^^
14+
= note: `#[warn(refining_impl_trait_internal)]` implied by `#[warn(refining_impl_trait)]`
15+
help: replace the return type so that it matches the trait
16+
|
17+
LL | fn bar(self) -> impl Deref<Target = impl Sized> {
18+
| ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
19+
20+
warning: 1 warning emitted
21+

‎tests/ui/impl-trait/in-trait/refine.rs

+3
Original file line numberDiff line numberDiff line change
@@ -25,13 +25,15 @@ impl Foo for C {
2525
struct Private;
2626
impl Foo for Private {
2727
fn bar() -> () {}
28+
//~^ ERROR impl method signature does not match trait method signature
2829
}
2930

3031
pub trait Arg<A> {
3132
fn bar() -> impl Sized;
3233
}
3334
impl Arg<Private> for A {
3435
fn bar() -> () {}
36+
//~^ ERROR impl method signature does not match trait method signature
3537
}
3638

3739
pub trait Late {
@@ -52,6 +54,7 @@ mod unreachable {
5254
struct E;
5355
impl UnreachablePub for E {
5456
fn bar() {}
57+
//~^ ERROR impl method signature does not match trait method signature
5558
}
5659
}
5760

‎tests/ui/impl-trait/in-trait/refine.stderr

+56-2
Original file line numberDiff line numberDiff line change
@@ -8,11 +8,13 @@ LL | fn bar() -> impl Copy {}
88
| ^^^^ this bound is stronger than that defined on the trait
99
|
1010
= note: add `#[allow(refining_impl_trait)]` if it is intended for this to be part of the public API of this crate
11+
= note: we are soliciting feedback, see issue #121718 <https://github.com/rust-lang/rust/issues/121718> for more information
1112
note: the lint level is defined here
1213
--> $DIR/refine.rs:1:9
1314
|
1415
LL | #![deny(refining_impl_trait)]
1516
| ^^^^^^^^^^^^^^^^^^^
17+
= note: `#[deny(refining_impl_trait_reachable)]` implied by `#[deny(refining_impl_trait)]`
1618
help: replace the return type so that it matches the trait
1719
|
1820
LL | fn bar() -> impl Sized {}
@@ -28,6 +30,7 @@ LL | fn bar() {}
2830
| ^^^^^^^^
2931
|
3032
= note: add `#[allow(refining_impl_trait)]` if it is intended for this to be part of the public API of this crate
33+
= note: we are soliciting feedback, see issue #121718 <https://github.com/rust-lang/rust/issues/121718> for more information
3134
help: replace the return type so that it matches the trait
3235
|
3336
LL | fn bar()-> impl Sized {}
@@ -43,13 +46,47 @@ LL | fn bar() -> () {}
4346
| ^^
4447
|
4548
= note: add `#[allow(refining_impl_trait)]` if it is intended for this to be part of the public API of this crate
49+
= note: we are soliciting feedback, see issue #121718 <https://github.com/rust-lang/rust/issues/121718> for more information
4650
help: replace the return type so that it matches the trait
4751
|
4852
LL | fn bar() -> impl Sized {}
4953
| ~~~~~~~~~~
5054

5155
error: impl trait in impl method signature does not match trait method signature
52-
--> $DIR/refine.rs:43:27
56+
--> $DIR/refine.rs:27:17
57+
|
58+
LL | fn bar() -> impl Sized;
59+
| ---------- return type from trait method defined here
60+
...
61+
LL | fn bar() -> () {}
62+
| ^^
63+
|
64+
= note: add `#[allow(refining_impl_trait)]` if it is intended for this to be part of the public API of this crate
65+
= note: we are soliciting feedback, see issue #121718 <https://github.com/rust-lang/rust/issues/121718> for more information
66+
= note: `#[deny(refining_impl_trait_internal)]` implied by `#[deny(refining_impl_trait)]`
67+
help: replace the return type so that it matches the trait
68+
|
69+
LL | fn bar() -> impl Sized {}
70+
| ~~~~~~~~~~
71+
72+
error: impl trait in impl method signature does not match trait method signature
73+
--> $DIR/refine.rs:35:17
74+
|
75+
LL | fn bar() -> impl Sized;
76+
| ---------- return type from trait method defined here
77+
...
78+
LL | fn bar() -> () {}
79+
| ^^
80+
|
81+
= note: add `#[allow(refining_impl_trait)]` if it is intended for this to be part of the public API of this crate
82+
= note: we are soliciting feedback, see issue #121718 <https://github.com/rust-lang/rust/issues/121718> for more information
83+
help: replace the return type so that it matches the trait
84+
|
85+
LL | fn bar() -> impl Sized {}
86+
| ~~~~~~~~~~
87+
88+
error: impl trait in impl method signature does not match trait method signature
89+
--> $DIR/refine.rs:45:27
5390
|
5491
LL | fn bar<'a>(&'a self) -> impl Sized + 'a;
5592
| --------------- return type from trait method defined here
@@ -58,10 +95,27 @@ LL | fn bar(&self) -> impl Copy + '_ {}
5895
| ^^^^ this bound is stronger than that defined on the trait
5996
|
6097
= note: add `#[allow(refining_impl_trait)]` if it is intended for this to be part of the public API of this crate
98+
= note: we are soliciting feedback, see issue #121718 <https://github.com/rust-lang/rust/issues/121718> for more information
6199
help: replace the return type so that it matches the trait
62100
|
63101
LL | fn bar(&self) -> impl Sized + '_ {}
64102
| ~~~~~~~~~~~~~~~
65103

66-
error: aborting due to 4 previous errors
104+
error: impl trait in impl method signature does not match trait method signature
105+
--> $DIR/refine.rs:56:9
106+
|
107+
LL | fn bar() -> impl Sized;
108+
| ---------- return type from trait method defined here
109+
...
110+
LL | fn bar() {}
111+
| ^^^^^^^^
112+
|
113+
= note: add `#[allow(refining_impl_trait)]` if it is intended for this to be part of the public API of this crate
114+
= note: we are soliciting feedback, see issue #121718 <https://github.com/rust-lang/rust/issues/121718> for more information
115+
help: replace the return type so that it matches the trait
116+
|
117+
LL | fn bar()-> impl Sized {}
118+
| +++++++++++++
119+
120+
error: aborting due to 7 previous errors
67121

0 commit comments

Comments
 (0)