From e10262ca0aa2ff434b33898d1d03413fdde60edf Mon Sep 17 00:00:00 2001 From: Michael Goulet Date: Sat, 2 Sep 2023 04:02:11 +0000 Subject: [PATCH 1/4] Implement refinement lint for RPITIT --- compiler/rustc_errors/src/lib.rs | 6 +- compiler/rustc_hir_analysis/messages.ftl | 5 + .../src/check/compare_impl_item.rs | 8 + .../src/check/compare_impl_item/refine.rs | 271 ++++++++++++++++++ compiler/rustc_hir_analysis/src/errors.rs | 15 + compiler/rustc_lint_defs/src/builtin.rs | 46 ++- compiler/rustc_middle/src/ty/generic_args.rs | 2 +- .../in-trait/async-example-desugared-extra.rs | 3 +- .../in-trait/async-example-desugared.rs | 2 +- .../impl-trait/in-trait/auxiliary/rpitit.rs | 1 + .../in-trait/bad-item-bound-within-rpitit.rs | 1 + .../bad-item-bound-within-rpitit.stderr | 17 +- .../impl-trait/in-trait/deep-match-works.rs | 1 + tests/ui/impl-trait/in-trait/foreign.rs | 1 + tests/ui/impl-trait/in-trait/issue-102571.rs | 8 - .../impl-trait/in-trait/issue-102571.stderr | 2 +- tests/ui/impl-trait/in-trait/nested-rpitit.rs | 2 + tests/ui/impl-trait/in-trait/object-safety.rs | 2 +- tests/ui/impl-trait/in-trait/refine.rs | 36 +++ tests/ui/impl-trait/in-trait/refine.stderr | 63 ++++ tests/ui/impl-trait/in-trait/reveal.rs | 1 + .../rpitit-shadowed-by-missing-adt.rs | 1 + .../signature-mismatch.failure.stderr | 2 +- .../impl-trait/in-trait/signature-mismatch.rs | 4 +- .../in-trait/specialization-substs-remap.rs | 1 + tests/ui/impl-trait/in-trait/success.rs | 3 + 26 files changed, 483 insertions(+), 21 deletions(-) create mode 100644 compiler/rustc_hir_analysis/src/check/compare_impl_item/refine.rs create mode 100644 tests/ui/impl-trait/in-trait/refine.rs create mode 100644 tests/ui/impl-trait/in-trait/refine.stderr diff --git a/compiler/rustc_errors/src/lib.rs b/compiler/rustc_errors/src/lib.rs index 55c4ec66cd9ed..31547f19be0a2 100644 --- a/compiler/rustc_errors/src/lib.rs +++ b/compiler/rustc_errors/src/lib.rs @@ -44,7 +44,7 @@ use rustc_fluent_macro::fluent_messages; pub use rustc_lint_defs::{pluralize, Applicability}; use rustc_span::source_map::SourceMap; pub use rustc_span::ErrorGuaranteed; -use rustc_span::{Loc, Span}; +use rustc_span::{Loc, Span, DUMMY_SP}; use std::borrow::Cow; use std::error::Report; @@ -1754,7 +1754,7 @@ impl DelayedDiagnostic { BacktraceStatus::Captured => { let inner = &self.inner; self.inner.subdiagnostic(DelayedAtWithNewline { - span: inner.span.primary_span().unwrap(), + span: inner.span.primary_span().unwrap_or(DUMMY_SP), emitted_at: inner.emitted_at.clone(), note: self.note, }); @@ -1764,7 +1764,7 @@ impl DelayedDiagnostic { _ => { let inner = &self.inner; self.inner.subdiagnostic(DelayedAtWithoutNewline { - span: inner.span.primary_span().unwrap(), + span: inner.span.primary_span().unwrap_or(DUMMY_SP), emitted_at: inner.emitted_at.clone(), note: self.note, }); diff --git a/compiler/rustc_hir_analysis/messages.ftl b/compiler/rustc_hir_analysis/messages.ftl index 597cae6ff33ca..6a08fc528d70c 100644 --- a/compiler/rustc_hir_analysis/messages.ftl +++ b/compiler/rustc_hir_analysis/messages.ftl @@ -222,6 +222,11 @@ hir_analysis_return_type_notation_on_non_rpitit = .note = function returns `{$ty}`, which is not compatible with associated type return bounds .label = this function must be `async` or return `impl Trait` +hir_analysis_rpitit_refined = impl trait in impl method signature does not match trait method signature + .suggestion = replace the return type so that it matches the trait + .label = return type from trait method defined here + .unmatched_bound_label = this bound is stronger than that defined on the trait + hir_analysis_self_in_impl_self = `Self` is not valid in the self type of an impl block .note = replace `Self` with a different type diff --git a/compiler/rustc_hir_analysis/src/check/compare_impl_item.rs b/compiler/rustc_hir_analysis/src/check/compare_impl_item.rs index bd0ab6463f08a..92cc9759304fc 100644 --- a/compiler/rustc_hir_analysis/src/check/compare_impl_item.rs +++ b/compiler/rustc_hir_analysis/src/check/compare_impl_item.rs @@ -28,6 +28,8 @@ use rustc_trait_selection::traits::{ use std::borrow::Cow; use std::iter; +mod refine; + /// Checks that a method from an impl conforms to the signature of /// the same method as declared in the trait. /// @@ -53,6 +55,12 @@ pub(super) fn compare_impl_method<'tcx>( impl_trait_ref, CheckImpliedWfMode::Check, )?; + refine::check_refining_return_position_impl_trait_in_trait( + tcx, + impl_m, + trait_m, + impl_trait_ref, + ); }; } diff --git a/compiler/rustc_hir_analysis/src/check/compare_impl_item/refine.rs b/compiler/rustc_hir_analysis/src/check/compare_impl_item/refine.rs new file mode 100644 index 0000000000000..bc80cd62057f7 --- /dev/null +++ b/compiler/rustc_hir_analysis/src/check/compare_impl_item/refine.rs @@ -0,0 +1,271 @@ +use rustc_data_structures::fx::FxIndexSet; +use rustc_hir as hir; +use rustc_hir::def_id::DefId; +use rustc_infer::infer::{outlives::env::OutlivesEnvironment, TyCtxtInferExt}; +use rustc_lint_defs::builtin::REFINING_IMPL_TRAIT; +use rustc_middle::traits::{ObligationCause, Reveal}; +use rustc_middle::ty::{ + self, Ty, TyCtxt, TypeFoldable, TypeSuperVisitable, TypeVisitable, TypeVisitor, +}; +use rustc_span::{Span, DUMMY_SP}; +use rustc_trait_selection::traits::{ + elaborate, normalize_param_env_or_error, outlives_bounds::InferCtxtExt, ObligationCtxt, +}; +use std::ops::ControlFlow; + +/// Check that an implementation does not refine an RPITIT from a trait method signature. +pub(super) fn check_refining_return_position_impl_trait_in_trait<'tcx>( + tcx: TyCtxt<'tcx>, + impl_m: ty::AssocItem, + trait_m: ty::AssocItem, + impl_trait_ref: ty::TraitRef<'tcx>, +) { + if !tcx.impl_method_has_trait_impl_trait_tys(impl_m.def_id) { + return; + } + + let impl_def_id = impl_m.container_id(tcx); + let impl_m_args = ty::GenericArgs::identity_for_item(tcx, impl_m.def_id); + let trait_m_to_impl_m_args = impl_m_args.rebase_onto(tcx, impl_def_id, impl_trait_ref.args); + let bound_trait_m_sig = tcx.fn_sig(trait_m.def_id).instantiate(tcx, trait_m_to_impl_m_args); + let trait_m_sig = tcx.liberate_late_bound_regions(impl_m.def_id, bound_trait_m_sig); + + let Ok(hidden_tys) = tcx.collect_return_position_impl_trait_in_trait_tys(impl_m.def_id) else { + // Error already emitted, no need to delay another. + return; + }; + + let mut collector = ImplTraitInTraitCollector { tcx, types: FxIndexSet::default() }; + trait_m_sig.visit_with(&mut collector); + + // Bound that we find on RPITITs in the trait signature. + let mut trait_bounds = vec![]; + // Bounds that we find on the RPITITs in the impl signature. + let mut impl_bounds = vec![]; + + for trait_projection in collector.types.into_iter().rev() { + let impl_opaque_args = trait_projection.args.rebase_onto(tcx, trait_m.def_id, impl_m_args); + let hidden_ty = hidden_tys[&trait_projection.def_id].instantiate(tcx, impl_opaque_args); + + // If the hidden type is not an opaque, then we have "refined" the trait signature. + let ty::Alias(ty::Opaque, impl_opaque) = *hidden_ty.kind() else { + report_mismatched_rpitit_signature( + tcx, + trait_m_sig, + trait_m.def_id, + impl_m.def_id, + None, + ); + return; + }; + + // This opaque also needs to be from the impl method -- otherwise, + // it's a refinement to a TAIT. + if !tcx.hir().get_if_local(impl_opaque.def_id).map_or(false, |node| { + matches!( + node.expect_item().expect_opaque_ty().origin, + hir::OpaqueTyOrigin::AsyncFn(def_id) | hir::OpaqueTyOrigin::FnReturn(def_id) + if def_id == impl_m.def_id.expect_local() + ) + }) { + report_mismatched_rpitit_signature( + tcx, + trait_m_sig, + trait_m.def_id, + impl_m.def_id, + None, + ); + return; + } + + trait_bounds.extend( + tcx.item_bounds(trait_projection.def_id).iter_instantiated(tcx, trait_projection.args), + ); + impl_bounds.extend(elaborate( + tcx, + tcx.explicit_item_bounds(impl_opaque.def_id) + .iter_instantiated_copied(tcx, impl_opaque.args), + )); + } + + let hybrid_preds = tcx + .predicates_of(impl_def_id) + .instantiate_identity(tcx) + .into_iter() + .chain(tcx.predicates_of(trait_m.def_id).instantiate_own(tcx, trait_m_to_impl_m_args)) + .map(|(clause, _)| clause); + let param_env = ty::ParamEnv::new(tcx.mk_clauses_from_iter(hybrid_preds), Reveal::UserFacing); + let param_env = normalize_param_env_or_error(tcx, param_env, ObligationCause::dummy()); + + let ref infcx = tcx.infer_ctxt().build(); + let ocx = ObligationCtxt::new(infcx); + + // Normalize the bounds. This has two purposes: + // + // 1. Project the RPITIT projections from the trait to the opaques on the impl, + // which means that they don't need to be mapped manually. + // + // 2. Project any other projections that show up in the bound. That makes sure that + // we don't consider `tests/ui/async-await/in-trait/async-associated-types.rs` + // to be refining. + let (trait_bounds, impl_bounds) = + ocx.normalize(&ObligationCause::dummy(), param_env, (trait_bounds, impl_bounds)); + + // Since we've normalized things, we need to resolve regions, since we'll + // possibly have introduced region vars during projection. We don't expect + // this resolution to have incurred any region errors -- but if we do, then + // just delay a bug. + let mut implied_wf_types = FxIndexSet::default(); + implied_wf_types.extend(trait_m_sig.inputs_and_output); + implied_wf_types.extend(ocx.normalize( + &ObligationCause::dummy(), + param_env, + trait_m_sig.inputs_and_output, + )); + if !ocx.select_all_or_error().is_empty() { + tcx.sess.delay_span_bug( + DUMMY_SP, + "encountered errors when checking RPITIT refinement (selection)", + ); + return; + } + let outlives_env = OutlivesEnvironment::with_bounds( + param_env, + infcx.implied_bounds_tys(param_env, impl_m.def_id.expect_local(), implied_wf_types), + ); + let errors = infcx.resolve_regions(&outlives_env); + if !errors.is_empty() { + tcx.sess.delay_span_bug( + DUMMY_SP, + "encountered errors when checking RPITIT refinement (regions)", + ); + return; + } + // Resolve any lifetime variables that may have been introduced during normalization. + let Ok((trait_bounds, impl_bounds)) = infcx.fully_resolve((trait_bounds, impl_bounds)) else { + tcx.sess.delay_span_bug( + DUMMY_SP, + "encountered errors when checking RPITIT refinement (resolution)", + ); + return; + }; + + // For quicker lookup, use an `IndexSet` + // (we don't use one earlier because it's not foldable..) + let trait_bounds = FxIndexSet::from_iter(trait_bounds); + + // Find any clauses that are present in the impl's RPITITs that are not + // present in the trait's RPITITs. This will trigger on trivial predicates, + // too, since we *do not* use the trait solver to prove that the RPITIT's + // bounds are not stronger -- we're doing a simple, syntactic compatibility + // check between bounds. This is strictly forwards compatible, though. + for (clause, span) in impl_bounds { + if !trait_bounds.contains(&clause) { + report_mismatched_rpitit_signature( + tcx, + trait_m_sig, + trait_m.def_id, + impl_m.def_id, + Some(span), + ); + return; + } + } +} + +struct ImplTraitInTraitCollector<'tcx> { + tcx: TyCtxt<'tcx>, + types: FxIndexSet>, +} + +impl<'tcx> TypeVisitor> for ImplTraitInTraitCollector<'tcx> { + type BreakTy = !; + + fn visit_ty(&mut self, ty: Ty<'tcx>) -> std::ops::ControlFlow { + if let ty::Alias(ty::Projection, proj) = *ty.kind() + && self.tcx.is_impl_trait_in_trait(proj.def_id) + { + if self.types.insert(proj) { + for (pred, _) in self + .tcx + .explicit_item_bounds(proj.def_id) + .iter_instantiated_copied(self.tcx, proj.args) + { + pred.visit_with(self)?; + } + } + ControlFlow::Continue(()) + } else { + ty.super_visit_with(self) + } + } +} + +fn report_mismatched_rpitit_signature<'tcx>( + tcx: TyCtxt<'tcx>, + trait_m_sig: ty::FnSig<'tcx>, + trait_m_def_id: DefId, + impl_m_def_id: DefId, + unmatched_bound: Option, +) { + let mapping = std::iter::zip( + tcx.fn_sig(trait_m_def_id).skip_binder().bound_vars(), + tcx.fn_sig(impl_m_def_id).skip_binder().bound_vars(), + ) + .filter_map(|(impl_bv, trait_bv)| { + if let ty::BoundVariableKind::Region(impl_bv) = impl_bv + && let ty::BoundVariableKind::Region(trait_bv) = trait_bv + { + Some((impl_bv, trait_bv)) + } else { + None + } + }) + .collect(); + + let mut return_ty = + trait_m_sig.output().fold_with(&mut super::RemapLateBound { tcx, mapping: &mapping }); + + if tcx.asyncness(impl_m_def_id).is_async() && tcx.asyncness(trait_m_def_id).is_async() { + let ty::Alias(ty::Projection, future_ty) = return_ty.kind() else { + bug!(); + }; + let Some(future_output_ty) = tcx + .explicit_item_bounds(future_ty.def_id) + .iter_instantiated_copied(tcx, future_ty.args) + .find_map(|(clause, _)| match clause.kind().no_bound_vars()? { + ty::ClauseKind::Projection(proj) => proj.term.ty(), + _ => None, + }) + else { + bug!() + }; + return_ty = future_output_ty; + } + + let (span, impl_return_span, pre, post) = + match tcx.hir().get_by_def_id(impl_m_def_id.expect_local()).fn_decl().unwrap().output { + hir::FnRetTy::DefaultReturn(span) => (tcx.def_span(impl_m_def_id), span, "-> ", " "), + hir::FnRetTy::Return(ty) => (ty.span, ty.span, "", ""), + }; + let trait_return_span = + tcx.hir().get_if_local(trait_m_def_id).map(|node| match node.fn_decl().unwrap().output { + hir::FnRetTy::DefaultReturn(_) => tcx.def_span(trait_m_def_id), + hir::FnRetTy::Return(ty) => ty.span, + }); + + let span = unmatched_bound.unwrap_or(span); + tcx.emit_spanned_lint( + REFINING_IMPL_TRAIT, + tcx.local_def_id_to_hir_id(impl_m_def_id.expect_local()), + span, + crate::errors::ReturnPositionImplTraitInTraitRefined { + impl_return_span, + trait_return_span, + pre, + post, + return_ty, + unmatched_bound, + }, + ); +} diff --git a/compiler/rustc_hir_analysis/src/errors.rs b/compiler/rustc_hir_analysis/src/errors.rs index 9471ad9ca9063..a1b306edd64fc 100644 --- a/compiler/rustc_hir_analysis/src/errors.rs +++ b/compiler/rustc_hir_analysis/src/errors.rs @@ -919,6 +919,21 @@ pub struct UnusedAssociatedTypeBounds { pub span: Span, } +#[derive(LintDiagnostic)] +#[diag(hir_analysis_rpitit_refined)] +pub(crate) struct ReturnPositionImplTraitInTraitRefined<'tcx> { + #[suggestion(applicability = "maybe-incorrect", code = "{pre}{return_ty}{post}")] + pub impl_return_span: Span, + #[label] + pub trait_return_span: Option, + #[label(hir_analysis_unmatched_bound_label)] + pub unmatched_bound: Option, + + pub pre: &'static str, + pub post: &'static str, + pub return_ty: Ty<'tcx>, +} + #[derive(Diagnostic)] #[diag(hir_analysis_assoc_bound_on_const)] #[note] diff --git a/compiler/rustc_lint_defs/src/builtin.rs b/compiler/rustc_lint_defs/src/builtin.rs index 394b6aa1a06b4..df0151dc4997d 100644 --- a/compiler/rustc_lint_defs/src/builtin.rs +++ b/compiler/rustc_lint_defs/src/builtin.rs @@ -3381,6 +3381,7 @@ declare_lint_pass! { PROC_MACRO_BACK_COMPAT, PROC_MACRO_DERIVE_RESOLUTION_FALLBACK, PUB_USE_OF_PRIVATE_EXTERN_CRATE, + REFINING_IMPL_TRAIT, RENAMED_AND_REMOVED_LINTS, REPR_TRANSPARENT_EXTERNAL_PRIVATE_FIELDS, RUST_2021_INCOMPATIBLE_CLOSURE_CAPTURES, @@ -4448,7 +4449,6 @@ declare_lint! { /// ### Example /// /// ```rust,compile_fail - /// /// #![deny(ambiguous_glob_imports)] /// pub fn foo() -> u32 { /// use sub::*; @@ -4484,6 +4484,50 @@ declare_lint! { }; } +declare_lint! { + /// The `refining_impl_trait` lint detects usages of return-position impl + /// traits in trait signatures which are refined by implementations. + /// + /// ### Example + /// + /// ```rust,compile_fail + /// #![feature(return_position_impl_trait_in_trait)] + /// #![deny(refining_impl_trait)] + /// + /// use std::fmt::Display; + /// + /// trait AsDisplay { + /// fn as_display(&self) -> impl Display; + /// } + /// + /// impl<'s> AsDisplay for &'s str { + /// fn as_display(&self) -> Self { + /// *self + /// } + /// } + /// + /// fn main() { + /// // users can observe that the return type of + /// // `<&str as AsDisplay>::as_display()` is `&str`. + /// let x: &str = "".as_display(); + /// } + /// ``` + /// + /// {{produces}} + /// + /// ### Explanation + /// + /// Return-position impl trait in traits (RPITITs) desugar to associated types, + /// and callers of methods for types where the implementation is known are + /// able to observe the types written in the impl signature. This may be + /// intended behavior, but may also pose a semver hazard for authors of libraries + /// who do not wish to make stronger guarantees about the types than what is + /// written in the trait signature. + pub REFINING_IMPL_TRAIT, + Warn, + "impl trait in impl method signature does not match trait method signature", +} + declare_lint! { /// The `elided_lifetimes_in_associated_constant` lint detects elided lifetimes /// that were erroneously allowed in associated constants. diff --git a/compiler/rustc_middle/src/ty/generic_args.rs b/compiler/rustc_middle/src/ty/generic_args.rs index 97dab5cb47e8b..53a3d5fc6be7f 100644 --- a/compiler/rustc_middle/src/ty/generic_args.rs +++ b/compiler/rustc_middle/src/ty/generic_args.rs @@ -440,7 +440,7 @@ impl<'tcx> GenericArgs<'tcx> { target_args: GenericArgsRef<'tcx>, ) -> GenericArgsRef<'tcx> { let defs = tcx.generics_of(source_ancestor); - tcx.mk_args_from_iter(target_args.iter().chain(self.iter().skip(defs.params.len()))) + tcx.mk_args_from_iter(target_args.iter().chain(self.iter().skip(defs.count()))) } pub fn truncate_to(&self, tcx: TyCtxt<'tcx>, generics: &ty::Generics) -> GenericArgsRef<'tcx> { diff --git a/tests/ui/async-await/in-trait/async-example-desugared-extra.rs b/tests/ui/async-await/in-trait/async-example-desugared-extra.rs index 81e1e59a36249..1bbf4836a2abb 100644 --- a/tests/ui/async-await/in-trait/async-example-desugared-extra.rs +++ b/tests/ui/async-await/in-trait/async-example-desugared-extra.rs @@ -27,8 +27,7 @@ impl Future for MyFuture { } impl MyTrait for i32 { - // FIXME: this should eventually require `#[refine]` to compile, because it also provides - // `Clone`. + #[allow(refining_impl_trait)] fn foo(&self) -> impl Future + Clone { MyFuture(*self) } diff --git a/tests/ui/async-await/in-trait/async-example-desugared.rs b/tests/ui/async-await/in-trait/async-example-desugared.rs index fb92ec786746f..0a5023176fef7 100644 --- a/tests/ui/async-await/in-trait/async-example-desugared.rs +++ b/tests/ui/async-await/in-trait/async-example-desugared.rs @@ -12,7 +12,7 @@ trait MyTrait { } impl MyTrait for i32 { - fn foo(&self) -> impl Future + '_ { + fn foo(&self) -> impl Future { async { *self } } } diff --git a/tests/ui/impl-trait/in-trait/auxiliary/rpitit.rs b/tests/ui/impl-trait/in-trait/auxiliary/rpitit.rs index cfc2193f633e0..5bd5c48ca69e4 100644 --- a/tests/ui/impl-trait/in-trait/auxiliary/rpitit.rs +++ b/tests/ui/impl-trait/in-trait/auxiliary/rpitit.rs @@ -8,6 +8,7 @@ pub trait Foo { pub struct Foreign; impl Foo for Foreign { + #[allow(refining_impl_trait)] fn bar(self) -> &'static () { &() } diff --git a/tests/ui/impl-trait/in-trait/bad-item-bound-within-rpitit.rs b/tests/ui/impl-trait/in-trait/bad-item-bound-within-rpitit.rs index ff7ad4bf38944..063adb7c378a8 100644 --- a/tests/ui/impl-trait/in-trait/bad-item-bound-within-rpitit.rs +++ b/tests/ui/impl-trait/in-trait/bad-item-bound-within-rpitit.rs @@ -17,6 +17,7 @@ impl<'a, I: 'a + Iterable> Iterable for &'a I { //~^ ERROR impl has stricter requirements than trait fn iter(&self) -> impl 'a + Iterator> { + //~^ WARN impl trait in impl method signature does not match trait method signature (*self).iter() } } diff --git a/tests/ui/impl-trait/in-trait/bad-item-bound-within-rpitit.stderr b/tests/ui/impl-trait/in-trait/bad-item-bound-within-rpitit.stderr index 106b8a7c80474..8d08d5e521e8b 100644 --- a/tests/ui/impl-trait/in-trait/bad-item-bound-within-rpitit.stderr +++ b/tests/ui/impl-trait/in-trait/bad-item-bound-within-rpitit.stderr @@ -12,6 +12,21 @@ help: copy the `where` clause predicates from the trait LL | where Self: 'b; | ~~~~~~~~~~~~~~ -error: aborting due to previous error +warning: impl trait in impl method signature does not match trait method signature + --> $DIR/bad-item-bound-within-rpitit.rs:19:28 + | +LL | fn iter(&self) -> impl '_ + Iterator>; + | ----------------------------------------- return type from trait method defined here +... +LL | fn iter(&self) -> impl 'a + Iterator> { + | ^^ this bound is stronger than that defined on the trait + | + = note: `#[warn(refining_impl_trait)]` on by default +help: replace the return type so that it matches the trait + | +LL | fn iter(&self) -> impl Iterator::Item<'_>> + '_ { + | ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ + +error: aborting due to previous error; 1 warning emitted For more information about this error, try `rustc --explain E0276`. diff --git a/tests/ui/impl-trait/in-trait/deep-match-works.rs b/tests/ui/impl-trait/in-trait/deep-match-works.rs index 78cff97c61654..ffd10d9b1ff8c 100644 --- a/tests/ui/impl-trait/in-trait/deep-match-works.rs +++ b/tests/ui/impl-trait/in-trait/deep-match-works.rs @@ -10,6 +10,7 @@ trait Foo { } impl Foo for () { + #[allow(refining_impl_trait)] fn bar() -> Wrapper { Wrapper(0) } diff --git a/tests/ui/impl-trait/in-trait/foreign.rs b/tests/ui/impl-trait/in-trait/foreign.rs index b0c93a0293512..035d91c187627 100644 --- a/tests/ui/impl-trait/in-trait/foreign.rs +++ b/tests/ui/impl-trait/in-trait/foreign.rs @@ -9,6 +9,7 @@ use std::sync::Arc; // Implement an RPITIT from another crate. struct Local; impl Foo for Local { + #[allow(refining_impl_trait)] fn bar(self) -> Arc { Arc::new(String::new()) } diff --git a/tests/ui/impl-trait/in-trait/issue-102571.rs b/tests/ui/impl-trait/in-trait/issue-102571.rs index 61c91e64417b3..ccb53031c44bc 100644 --- a/tests/ui/impl-trait/in-trait/issue-102571.rs +++ b/tests/ui/impl-trait/in-trait/issue-102571.rs @@ -8,14 +8,6 @@ trait Foo { fn bar(self) -> impl Deref; } -struct A; - -impl Foo for A { - fn bar(self) -> &'static str { - "Hello, world" - } -} - fn foo(t: T) { let () = t.bar(); //~^ ERROR mismatched types diff --git a/tests/ui/impl-trait/in-trait/issue-102571.stderr b/tests/ui/impl-trait/in-trait/issue-102571.stderr index 87219941d9161..594b9ae9cd6bc 100644 --- a/tests/ui/impl-trait/in-trait/issue-102571.stderr +++ b/tests/ui/impl-trait/in-trait/issue-102571.stderr @@ -1,5 +1,5 @@ error[E0308]: mismatched types - --> $DIR/issue-102571.rs:20:9 + --> $DIR/issue-102571.rs:12:9 | LL | let () = t.bar(); | ^^ ------- this expression has type `impl Deref` diff --git a/tests/ui/impl-trait/in-trait/nested-rpitit.rs b/tests/ui/impl-trait/in-trait/nested-rpitit.rs index 65285e3a3ccaf..cb943eb5af99f 100644 --- a/tests/ui/impl-trait/in-trait/nested-rpitit.rs +++ b/tests/ui/impl-trait/in-trait/nested-rpitit.rs @@ -13,6 +13,7 @@ trait Foo { struct A; impl Foo for A { + #[allow(refining_impl_trait)] fn bar(self) -> &'static str { "Hello, world" } @@ -21,6 +22,7 @@ impl Foo for A { struct B; impl Foo for B { + #[allow(refining_impl_trait)] fn bar(self) -> Box { Box::new(42) } diff --git a/tests/ui/impl-trait/in-trait/object-safety.rs b/tests/ui/impl-trait/in-trait/object-safety.rs index 9a231e59b09bf..d1c9fba4e99f3 100644 --- a/tests/ui/impl-trait/in-trait/object-safety.rs +++ b/tests/ui/impl-trait/in-trait/object-safety.rs @@ -8,7 +8,7 @@ trait Foo { } impl Foo for u32 { - fn baz(&self) -> u32 { + fn baz(&self) -> impl Debug { 32 } } diff --git a/tests/ui/impl-trait/in-trait/refine.rs b/tests/ui/impl-trait/in-trait/refine.rs new file mode 100644 index 0000000000000..e9b5e669fde1f --- /dev/null +++ b/tests/ui/impl-trait/in-trait/refine.rs @@ -0,0 +1,36 @@ +#![feature(return_position_impl_trait_in_trait, async_fn_in_trait)] +#![deny(refining_impl_trait)] + +trait Foo { + fn bar() -> impl Sized; +} + +struct A; +impl Foo for A { + fn bar() -> impl Copy {} + //~^ ERROR impl method signature does not match trait method signature +} + +struct B; +impl Foo for B { + fn bar() {} + //~^ ERROR impl method signature does not match trait method signature +} + +struct C; +impl Foo for C { + fn bar() -> () {} + //~^ ERROR impl method signature does not match trait method signature +} + +trait Late { + fn bar<'a>(&'a self) -> impl Sized + 'a; +} + +struct D; +impl Late for D { + fn bar(&self) -> impl Copy + '_ {} + //~^ ERROR impl method signature does not match trait method signature +} + +fn main() {} diff --git a/tests/ui/impl-trait/in-trait/refine.stderr b/tests/ui/impl-trait/in-trait/refine.stderr new file mode 100644 index 0000000000000..28d53270da3d7 --- /dev/null +++ b/tests/ui/impl-trait/in-trait/refine.stderr @@ -0,0 +1,63 @@ +error: impl trait in impl method signature does not match trait method signature + --> $DIR/refine.rs:10:22 + | +LL | fn bar() -> impl Sized; + | ---------- return type from trait method defined here +... +LL | fn bar() -> impl Copy {} + | ^^^^ this bound is stronger than that defined on the trait + | +note: the lint level is defined here + --> $DIR/refine.rs:2:9 + | +LL | #![deny(refining_impl_trait)] + | ^^^^^^^^^^^^^^^^^^^ +help: replace the return type so that it matches the trait + | +LL | fn bar() -> impl Sized {} + | ~~~~~~~~~~ + +error: impl trait in impl method signature does not match trait method signature + --> $DIR/refine.rs:16:5 + | +LL | fn bar() -> impl Sized; + | ---------- return type from trait method defined here +... +LL | fn bar() {} + | ^^^^^^^^ + | +help: replace the return type so that it matches the trait + | +LL | fn bar() -> impl Sized {} + | +++++++++++++ + +error: impl trait in impl method signature does not match trait method signature + --> $DIR/refine.rs:22:17 + | +LL | fn bar() -> impl Sized; + | ---------- return type from trait method defined here +... +LL | fn bar() -> () {} + | ^^ + | +help: replace the return type so that it matches the trait + | +LL | fn bar() -> impl Sized {} + | ~~~~~~~~~~ + +error: impl trait in impl method signature does not match trait method signature + --> $DIR/refine.rs:32:27 + | +LL | fn bar<'a>(&'a self) -> impl Sized + 'a; + | --------------- return type from trait method defined here +... +LL | fn bar(&self) -> impl Copy + '_ {} + | ^^^^ this bound is stronger than that defined on the trait + | +help: replace the return type so that it matches the trait + | +LL | fn bar(&self) -> impl Sized + '_ {} + | ~~~~~~~~~~~~~~~ + +error: aborting due to 4 previous errors + diff --git a/tests/ui/impl-trait/in-trait/reveal.rs b/tests/ui/impl-trait/in-trait/reveal.rs index d6ede1cc495c6..65425ba1a84f8 100644 --- a/tests/ui/impl-trait/in-trait/reveal.rs +++ b/tests/ui/impl-trait/in-trait/reveal.rs @@ -8,6 +8,7 @@ trait Foo { } impl Foo for () { + #[allow(refining_impl_trait)] fn f() -> Box { Box::new(String::new()) } diff --git a/tests/ui/impl-trait/in-trait/rpitit-shadowed-by-missing-adt.rs b/tests/ui/impl-trait/in-trait/rpitit-shadowed-by-missing-adt.rs index 7682884f87931..747f7ed9e727f 100644 --- a/tests/ui/impl-trait/in-trait/rpitit-shadowed-by-missing-adt.rs +++ b/tests/ui/impl-trait/in-trait/rpitit-shadowed-by-missing-adt.rs @@ -10,6 +10,7 @@ pub trait Tr { } impl Tr for () { + #[allow(refining_impl_trait)] fn w() -> &'static () { &() } diff --git a/tests/ui/impl-trait/in-trait/signature-mismatch.failure.stderr b/tests/ui/impl-trait/in-trait/signature-mismatch.failure.stderr index 186580f57566d..468cf12f1bc03 100644 --- a/tests/ui/impl-trait/in-trait/signature-mismatch.failure.stderr +++ b/tests/ui/impl-trait/in-trait/signature-mismatch.failure.stderr @@ -1,5 +1,5 @@ error[E0623]: lifetime mismatch - --> $DIR/signature-mismatch.rs:77:10 + --> $DIR/signature-mismatch.rs:79:10 | LL | &'a self, | -------- this parameter and the return type are declared with different lifetimes... diff --git a/tests/ui/impl-trait/in-trait/signature-mismatch.rs b/tests/ui/impl-trait/in-trait/signature-mismatch.rs index c84a3b8f46b5e..135467d57eed7 100644 --- a/tests/ui/impl-trait/in-trait/signature-mismatch.rs +++ b/tests/ui/impl-trait/in-trait/signature-mismatch.rs @@ -3,7 +3,6 @@ //[success] check-pass #![feature(return_position_impl_trait_in_trait)] -#![allow(incomplete_features)] use std::future::Future; @@ -45,6 +44,7 @@ impl AsyncTrait for Struct { // Does not capture more lifetimes that trait def'n, since trait def'n // implicitly captures all in-scope lifetimes. #[cfg(success)] + #[allow(refining_impl_trait)] fn async_fn<'a>(&self, buff: &'a [u8]) -> impl Future> + 'a { async move { buff.to_vec() } } @@ -52,6 +52,7 @@ impl AsyncTrait for Struct { // Does not capture more lifetimes that trait def'n, since trait def'n // implicitly captures all in-scope lifetimes. #[cfg(success)] + #[allow(refining_impl_trait)] fn async_fn_early<'a: 'a>(&self, buff: &'a [u8]) -> impl Future> + 'a { async move { buff.to_vec() } } @@ -59,6 +60,7 @@ impl AsyncTrait for Struct { // Does not capture more lifetimes that trait def'n, since trait def'n // implicitly captures all in-scope lifetimes. #[cfg(success)] + #[allow(refining_impl_trait)] fn async_fn_multiple<'a, 'b>( &'a self, buff: &'b [u8], diff --git a/tests/ui/impl-trait/in-trait/specialization-substs-remap.rs b/tests/ui/impl-trait/in-trait/specialization-substs-remap.rs index c9ee877db8ec5..83812d6cd8aca 100644 --- a/tests/ui/impl-trait/in-trait/specialization-substs-remap.rs +++ b/tests/ui/impl-trait/in-trait/specialization-substs-remap.rs @@ -12,6 +12,7 @@ impl Foo for U where U: Copy, { + #[allow(refining_impl_trait)] fn bar(&self) -> U { *self } diff --git a/tests/ui/impl-trait/in-trait/success.rs b/tests/ui/impl-trait/in-trait/success.rs index 4cbe682b46f73..f583604a021cf 100644 --- a/tests/ui/impl-trait/in-trait/success.rs +++ b/tests/ui/impl-trait/in-trait/success.rs @@ -10,12 +10,14 @@ trait Foo { } impl Foo for i32 { + #[allow(refining_impl_trait)] fn bar(&self) -> i32 { *self } } impl Foo for &'static str { + #[allow(refining_impl_trait)] fn bar(&self) -> &'static str { *self } @@ -24,6 +26,7 @@ impl Foo for &'static str { struct Yay; impl Foo for Yay { + #[allow(refining_impl_trait)] fn bar(&self) -> String { String::from(":^)") } From 4745d34bc3ac377f8444d5dac94e0f206b7cded7 Mon Sep 17 00:00:00 2001 From: Michael Goulet Date: Thu, 7 Sep 2023 00:20:31 +0000 Subject: [PATCH 2/4] Use self instead of the actual self ty --- .../src/check/compare_impl_item/refine.rs | 18 +++++++++++++++--- .../bad-item-bound-within-rpitit.stderr | 4 ++-- 2 files changed, 17 insertions(+), 5 deletions(-) diff --git a/compiler/rustc_hir_analysis/src/check/compare_impl_item/refine.rs b/compiler/rustc_hir_analysis/src/check/compare_impl_item/refine.rs index bc80cd62057f7..b013f292e52e0 100644 --- a/compiler/rustc_hir_analysis/src/check/compare_impl_item/refine.rs +++ b/compiler/rustc_hir_analysis/src/check/compare_impl_item/refine.rs @@ -29,6 +29,18 @@ pub(super) fn check_refining_return_position_impl_trait_in_trait<'tcx>( let trait_m_to_impl_m_args = impl_m_args.rebase_onto(tcx, impl_def_id, impl_trait_ref.args); let bound_trait_m_sig = tcx.fn_sig(trait_m.def_id).instantiate(tcx, trait_m_to_impl_m_args); let trait_m_sig = tcx.liberate_late_bound_regions(impl_m.def_id, bound_trait_m_sig); + // replace the self type of the trait ref with `Self` so that diagnostics render better. + let trait_m_sig_with_self_for_diag = tcx.liberate_late_bound_regions( + impl_m.def_id, + tcx.fn_sig(trait_m.def_id).instantiate( + tcx, + tcx.mk_args_from_iter( + [tcx.types.self_param.into()] + .into_iter() + .chain(trait_m_to_impl_m_args.iter().skip(1)), + ), + ), + ); let Ok(hidden_tys) = tcx.collect_return_position_impl_trait_in_trait_tys(impl_m.def_id) else { // Error already emitted, no need to delay another. @@ -51,7 +63,7 @@ pub(super) fn check_refining_return_position_impl_trait_in_trait<'tcx>( let ty::Alias(ty::Opaque, impl_opaque) = *hidden_ty.kind() else { report_mismatched_rpitit_signature( tcx, - trait_m_sig, + trait_m_sig_with_self_for_diag, trait_m.def_id, impl_m.def_id, None, @@ -70,7 +82,7 @@ pub(super) fn check_refining_return_position_impl_trait_in_trait<'tcx>( }) { report_mismatched_rpitit_signature( tcx, - trait_m_sig, + trait_m_sig_with_self_for_diag, trait_m.def_id, impl_m.def_id, None, @@ -163,7 +175,7 @@ pub(super) fn check_refining_return_position_impl_trait_in_trait<'tcx>( if !trait_bounds.contains(&clause) { report_mismatched_rpitit_signature( tcx, - trait_m_sig, + trait_m_sig_with_self_for_diag, trait_m.def_id, impl_m.def_id, Some(span), diff --git a/tests/ui/impl-trait/in-trait/bad-item-bound-within-rpitit.stderr b/tests/ui/impl-trait/in-trait/bad-item-bound-within-rpitit.stderr index 8d08d5e521e8b..c0e07e94dabbe 100644 --- a/tests/ui/impl-trait/in-trait/bad-item-bound-within-rpitit.stderr +++ b/tests/ui/impl-trait/in-trait/bad-item-bound-within-rpitit.stderr @@ -24,8 +24,8 @@ LL | fn iter(&self) -> impl 'a + Iterator> { = note: `#[warn(refining_impl_trait)]` on by default help: replace the return type so that it matches the trait | -LL | fn iter(&self) -> impl Iterator::Item<'_>> + '_ { - | ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ +LL | fn iter(&self) -> impl Iterator::Item<'_>> + '_ { + | ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ error: aborting due to previous error; 1 warning emitted From 4d05da46e7154a6fc31bed3b217892d9a98cc0e3 Mon Sep 17 00:00:00 2001 From: Michael Goulet Date: Thu, 7 Sep 2023 00:45:12 +0000 Subject: [PATCH 3/4] Don't emit refining_impl_trait for private items --- .../src/check/compare_impl_item/refine.rs | 28 +++++++++++++++++++ compiler/rustc_lint_defs/src/builtin.rs | 2 +- .../in-trait/async-example-desugared-extra.rs | 6 ++-- .../impl-trait/in-trait/auxiliary/rpitit.rs | 4 +-- .../in-trait/bad-item-bound-within-rpitit.rs | 2 +- .../impl-trait/in-trait/deep-match-works.rs | 8 +++--- tests/ui/impl-trait/in-trait/foreign.rs | 15 ++++++++-- tests/ui/impl-trait/in-trait/nested-rpitit.rs | 12 ++++---- tests/ui/impl-trait/in-trait/refine.rs | 24 ++++++++++++---- tests/ui/impl-trait/in-trait/refine.stderr | 2 +- tests/ui/impl-trait/in-trait/reveal.rs | 6 ++-- .../rpitit-shadowed-by-missing-adt.rs | 4 +-- .../impl-trait/in-trait/signature-mismatch.rs | 14 +++++----- .../in-trait/specialization-substs-remap.rs | 6 ++-- tests/ui/impl-trait/in-trait/success.rs | 12 ++++---- 15 files changed, 98 insertions(+), 47 deletions(-) diff --git a/compiler/rustc_hir_analysis/src/check/compare_impl_item/refine.rs b/compiler/rustc_hir_analysis/src/check/compare_impl_item/refine.rs index b013f292e52e0..a8149b634ef7c 100644 --- a/compiler/rustc_hir_analysis/src/check/compare_impl_item/refine.rs +++ b/compiler/rustc_hir_analysis/src/check/compare_impl_item/refine.rs @@ -23,8 +23,22 @@ pub(super) fn check_refining_return_position_impl_trait_in_trait<'tcx>( if !tcx.impl_method_has_trait_impl_trait_tys(impl_m.def_id) { return; } + // crate-private traits don't have any library guarantees, there's no need to do this check. + if !tcx.visibility(trait_m.container_id(tcx)).is_public() { + return; + } + // If a type in the trait ref is private, then there's also no reason to to do this check. let impl_def_id = impl_m.container_id(tcx); + for arg in impl_trait_ref.args { + if let Some(ty) = arg.as_type() + && let Some(self_visibility) = type_visibility(tcx, ty) + && !self_visibility.is_public() + { + return; + } + } + let impl_m_args = ty::GenericArgs::identity_for_item(tcx, impl_m.def_id); let trait_m_to_impl_m_args = impl_m_args.rebase_onto(tcx, impl_def_id, impl_trait_ref.args); let bound_trait_m_sig = tcx.fn_sig(trait_m.def_id).instantiate(tcx, trait_m_to_impl_m_args); @@ -281,3 +295,17 @@ fn report_mismatched_rpitit_signature<'tcx>( }, ); } + +fn type_visibility<'tcx>(tcx: TyCtxt<'tcx>, ty: Ty<'tcx>) -> Option> { + match *ty.kind() { + ty::Ref(_, ty, _) => type_visibility(tcx, ty), + ty::Adt(def, args) => { + if def.is_fundamental() { + type_visibility(tcx, args.type_at(0)) + } else { + Some(tcx.visibility(def.did())) + } + } + _ => None, + } +} diff --git a/compiler/rustc_lint_defs/src/builtin.rs b/compiler/rustc_lint_defs/src/builtin.rs index df0151dc4997d..2567e273e1131 100644 --- a/compiler/rustc_lint_defs/src/builtin.rs +++ b/compiler/rustc_lint_defs/src/builtin.rs @@ -4496,7 +4496,7 @@ declare_lint! { /// /// use std::fmt::Display; /// - /// trait AsDisplay { + /// pub trait AsDisplay { /// fn as_display(&self) -> impl Display; /// } /// diff --git a/tests/ui/async-await/in-trait/async-example-desugared-extra.rs b/tests/ui/async-await/in-trait/async-example-desugared-extra.rs index 1bbf4836a2abb..3505690f1ec7a 100644 --- a/tests/ui/async-await/in-trait/async-example-desugared-extra.rs +++ b/tests/ui/async-await/in-trait/async-example-desugared-extra.rs @@ -2,14 +2,14 @@ // edition: 2021 #![feature(async_fn_in_trait)] -#![feature(return_position_impl_trait_in_trait)] +#![feature(return_position_impl_trait_in_trait, lint_reasons)] #![allow(incomplete_features)] use std::future::Future; use std::pin::Pin; use std::task::Poll; -trait MyTrait { +pub trait MyTrait { async fn foo(&self) -> i32; } @@ -27,7 +27,7 @@ impl Future for MyFuture { } impl MyTrait for i32 { - #[allow(refining_impl_trait)] + #[expect(refining_impl_trait)] fn foo(&self) -> impl Future + Clone { MyFuture(*self) } diff --git a/tests/ui/impl-trait/in-trait/auxiliary/rpitit.rs b/tests/ui/impl-trait/in-trait/auxiliary/rpitit.rs index 5bd5c48ca69e4..6e99402113ad0 100644 --- a/tests/ui/impl-trait/in-trait/auxiliary/rpitit.rs +++ b/tests/ui/impl-trait/in-trait/auxiliary/rpitit.rs @@ -1,4 +1,4 @@ -#![feature(return_position_impl_trait_in_trait)] +#![feature(return_position_impl_trait_in_trait, lint_reasons)] use std::ops::Deref; @@ -8,7 +8,7 @@ pub trait Foo { pub struct Foreign; impl Foo for Foreign { - #[allow(refining_impl_trait)] + #[expect(refining_impl_trait)] fn bar(self) -> &'static () { &() } diff --git a/tests/ui/impl-trait/in-trait/bad-item-bound-within-rpitit.rs b/tests/ui/impl-trait/in-trait/bad-item-bound-within-rpitit.rs index 063adb7c378a8..fbbbb8585d174 100644 --- a/tests/ui/impl-trait/in-trait/bad-item-bound-within-rpitit.rs +++ b/tests/ui/impl-trait/in-trait/bad-item-bound-within-rpitit.rs @@ -2,7 +2,7 @@ #![feature(return_position_impl_trait_in_trait)] -trait Iterable { +pub trait Iterable { type Item<'a> where Self: 'a; diff --git a/tests/ui/impl-trait/in-trait/deep-match-works.rs b/tests/ui/impl-trait/in-trait/deep-match-works.rs index ffd10d9b1ff8c..fc290f11f9d9c 100644 --- a/tests/ui/impl-trait/in-trait/deep-match-works.rs +++ b/tests/ui/impl-trait/in-trait/deep-match-works.rs @@ -1,16 +1,16 @@ // check-pass -#![feature(return_position_impl_trait_in_trait)] +#![feature(return_position_impl_trait_in_trait, lint_reasons)] #![allow(incomplete_features)] -struct Wrapper(T); +pub struct Wrapper(T); -trait Foo { +pub trait Foo { fn bar() -> Wrapper; } impl Foo for () { - #[allow(refining_impl_trait)] + #[expect(refining_impl_trait)] fn bar() -> Wrapper { Wrapper(0) } diff --git a/tests/ui/impl-trait/in-trait/foreign.rs b/tests/ui/impl-trait/in-trait/foreign.rs index 035d91c187627..6285d7786d56a 100644 --- a/tests/ui/impl-trait/in-trait/foreign.rs +++ b/tests/ui/impl-trait/in-trait/foreign.rs @@ -1,15 +1,25 @@ // check-pass // aux-build: rpitit.rs +#![feature(lint_reasons)] + extern crate rpitit; use rpitit::{Foo, Foreign}; use std::sync::Arc; // Implement an RPITIT from another crate. -struct Local; +pub struct Local; impl Foo for Local { - #[allow(refining_impl_trait)] + #[expect(refining_impl_trait)] + fn bar(self) -> Arc { + Arc::new(String::new()) + } +} + +struct LocalIgnoreRefining; +impl Foo for LocalIgnoreRefining { + #[deny(refining_impl_trait)] fn bar(self) -> Arc { Arc::new(String::new()) } @@ -24,4 +34,5 @@ fn main() { let &() = Foreign.bar(); let x: Arc = Local.bar(); + let x: Arc = LocalIgnoreRefining.bar(); } diff --git a/tests/ui/impl-trait/in-trait/nested-rpitit.rs b/tests/ui/impl-trait/in-trait/nested-rpitit.rs index cb943eb5af99f..58ba1acaf14b0 100644 --- a/tests/ui/impl-trait/in-trait/nested-rpitit.rs +++ b/tests/ui/impl-trait/in-trait/nested-rpitit.rs @@ -1,28 +1,28 @@ // check-pass -#![feature(return_position_impl_trait_in_trait)] +#![feature(return_position_impl_trait_in_trait, lint_reasons)] #![allow(incomplete_features)] use std::fmt::Display; use std::ops::Deref; -trait Foo { +pub trait Foo { fn bar(self) -> impl Deref; } -struct A; +pub struct A; impl Foo for A { - #[allow(refining_impl_trait)] + #[expect(refining_impl_trait)] fn bar(self) -> &'static str { "Hello, world" } } -struct B; +pub struct B; impl Foo for B { - #[allow(refining_impl_trait)] + #[expect(refining_impl_trait)] fn bar(self) -> Box { Box::new(42) } diff --git a/tests/ui/impl-trait/in-trait/refine.rs b/tests/ui/impl-trait/in-trait/refine.rs index e9b5e669fde1f..a91f9b3e72258 100644 --- a/tests/ui/impl-trait/in-trait/refine.rs +++ b/tests/ui/impl-trait/in-trait/refine.rs @@ -1,33 +1,45 @@ #![feature(return_position_impl_trait_in_trait, async_fn_in_trait)] #![deny(refining_impl_trait)] -trait Foo { +pub trait Foo { fn bar() -> impl Sized; } -struct A; +pub struct A; impl Foo for A { fn bar() -> impl Copy {} //~^ ERROR impl method signature does not match trait method signature } -struct B; +pub struct B; impl Foo for B { fn bar() {} //~^ ERROR impl method signature does not match trait method signature } -struct C; +pub struct C; impl Foo for C { fn bar() -> () {} //~^ ERROR impl method signature does not match trait method signature } -trait Late { +struct Private; +impl Foo for Private { + fn bar() -> () {} +} + +pub trait Arg { + fn bar() -> impl Sized; +} +impl Arg for A { + fn bar() -> () {} +} + +pub trait Late { fn bar<'a>(&'a self) -> impl Sized + 'a; } -struct D; +pub struct D; impl Late for D { fn bar(&self) -> impl Copy + '_ {} //~^ ERROR impl method signature does not match trait method signature diff --git a/tests/ui/impl-trait/in-trait/refine.stderr b/tests/ui/impl-trait/in-trait/refine.stderr index 28d53270da3d7..65dfdf869e4b1 100644 --- a/tests/ui/impl-trait/in-trait/refine.stderr +++ b/tests/ui/impl-trait/in-trait/refine.stderr @@ -46,7 +46,7 @@ LL | fn bar() -> impl Sized {} | ~~~~~~~~~~ error: impl trait in impl method signature does not match trait method signature - --> $DIR/refine.rs:32:27 + --> $DIR/refine.rs:44:27 | LL | fn bar<'a>(&'a self) -> impl Sized + 'a; | --------------- return type from trait method defined here diff --git a/tests/ui/impl-trait/in-trait/reveal.rs b/tests/ui/impl-trait/in-trait/reveal.rs index 65425ba1a84f8..b1b46d75b8f37 100644 --- a/tests/ui/impl-trait/in-trait/reveal.rs +++ b/tests/ui/impl-trait/in-trait/reveal.rs @@ -1,14 +1,14 @@ // check-pass -#![feature(return_position_impl_trait_in_trait)] +#![feature(return_position_impl_trait_in_trait, lint_reasons)] #![allow(incomplete_features)] -trait Foo { +pub trait Foo { fn f() -> Box; } impl Foo for () { - #[allow(refining_impl_trait)] + #[expect(refining_impl_trait)] fn f() -> Box { Box::new(String::new()) } diff --git a/tests/ui/impl-trait/in-trait/rpitit-shadowed-by-missing-adt.rs b/tests/ui/impl-trait/in-trait/rpitit-shadowed-by-missing-adt.rs index 747f7ed9e727f..44a2b4303441e 100644 --- a/tests/ui/impl-trait/in-trait/rpitit-shadowed-by-missing-adt.rs +++ b/tests/ui/impl-trait/in-trait/rpitit-shadowed-by-missing-adt.rs @@ -1,6 +1,6 @@ // issue: 113903 -#![feature(return_position_impl_trait_in_trait)] +#![feature(return_position_impl_trait_in_trait, lint_reasons)] use std::ops::Deref; @@ -10,7 +10,7 @@ pub trait Tr { } impl Tr for () { - #[allow(refining_impl_trait)] + #[expect(refining_impl_trait)] fn w() -> &'static () { &() } diff --git a/tests/ui/impl-trait/in-trait/signature-mismatch.rs b/tests/ui/impl-trait/in-trait/signature-mismatch.rs index 135467d57eed7..685c0f06e885b 100644 --- a/tests/ui/impl-trait/in-trait/signature-mismatch.rs +++ b/tests/ui/impl-trait/in-trait/signature-mismatch.rs @@ -2,17 +2,17 @@ // revisions: success failure //[success] check-pass -#![feature(return_position_impl_trait_in_trait)] +#![feature(return_position_impl_trait_in_trait, lint_reasons)] use std::future::Future; -trait Captures<'a> {} +pub trait Captures<'a> {} impl Captures<'_> for T {} -trait Captures2<'a, 'b> {} +pub trait Captures2<'a, 'b> {} impl Captures2<'_, '_> for T {} -trait AsyncTrait { +pub trait AsyncTrait { #[cfg(success)] fn async_fn(&self, buff: &[u8]) -> impl Future>; @@ -44,7 +44,7 @@ impl AsyncTrait for Struct { // Does not capture more lifetimes that trait def'n, since trait def'n // implicitly captures all in-scope lifetimes. #[cfg(success)] - #[allow(refining_impl_trait)] + #[expect(refining_impl_trait)] fn async_fn<'a>(&self, buff: &'a [u8]) -> impl Future> + 'a { async move { buff.to_vec() } } @@ -52,7 +52,7 @@ impl AsyncTrait for Struct { // Does not capture more lifetimes that trait def'n, since trait def'n // implicitly captures all in-scope lifetimes. #[cfg(success)] - #[allow(refining_impl_trait)] + #[expect(refining_impl_trait)] fn async_fn_early<'a: 'a>(&self, buff: &'a [u8]) -> impl Future> + 'a { async move { buff.to_vec() } } @@ -60,7 +60,7 @@ impl AsyncTrait for Struct { // Does not capture more lifetimes that trait def'n, since trait def'n // implicitly captures all in-scope lifetimes. #[cfg(success)] - #[allow(refining_impl_trait)] + #[expect(refining_impl_trait)] fn async_fn_multiple<'a, 'b>( &'a self, buff: &'b [u8], diff --git a/tests/ui/impl-trait/in-trait/specialization-substs-remap.rs b/tests/ui/impl-trait/in-trait/specialization-substs-remap.rs index 83812d6cd8aca..41fc285883a84 100644 --- a/tests/ui/impl-trait/in-trait/specialization-substs-remap.rs +++ b/tests/ui/impl-trait/in-trait/specialization-substs-remap.rs @@ -1,10 +1,10 @@ // check-pass #![feature(specialization)] -#![feature(return_position_impl_trait_in_trait)] +#![feature(return_position_impl_trait_in_trait, lint_reasons)] #![allow(incomplete_features)] -trait Foo { +pub trait Foo { fn bar(&self) -> impl Sized; } @@ -12,7 +12,7 @@ impl Foo for U where U: Copy, { - #[allow(refining_impl_trait)] + #[expect(refining_impl_trait)] fn bar(&self) -> U { *self } diff --git a/tests/ui/impl-trait/in-trait/success.rs b/tests/ui/impl-trait/in-trait/success.rs index f583604a021cf..7d415ea17a46f 100644 --- a/tests/ui/impl-trait/in-trait/success.rs +++ b/tests/ui/impl-trait/in-trait/success.rs @@ -1,32 +1,32 @@ // check-pass -#![feature(return_position_impl_trait_in_trait)] +#![feature(return_position_impl_trait_in_trait, lint_reasons)] #![allow(incomplete_features)] use std::fmt::Display; -trait Foo { +pub trait Foo { fn bar(&self) -> impl Display; } impl Foo for i32 { - #[allow(refining_impl_trait)] + #[expect(refining_impl_trait)] fn bar(&self) -> i32 { *self } } impl Foo for &'static str { - #[allow(refining_impl_trait)] + #[expect(refining_impl_trait)] fn bar(&self) -> &'static str { *self } } -struct Yay; +pub struct Yay; impl Foo for Yay { - #[allow(refining_impl_trait)] + #[expect(refining_impl_trait)] fn bar(&self) -> String { String::from(":^)") } From 7714db873e3172f34bed004ac1976cf72a54c2e9 Mon Sep 17 00:00:00 2001 From: Michael Goulet Date: Thu, 7 Sep 2023 00:49:04 +0000 Subject: [PATCH 4/4] Add note --- compiler/rustc_hir_analysis/messages.ftl | 1 + compiler/rustc_hir_analysis/src/errors.rs | 1 + .../impl-trait/in-trait/bad-item-bound-within-rpitit.stderr | 1 + tests/ui/impl-trait/in-trait/refine.stderr | 4 ++++ 4 files changed, 7 insertions(+) diff --git a/compiler/rustc_hir_analysis/messages.ftl b/compiler/rustc_hir_analysis/messages.ftl index 6a08fc528d70c..50da227831200 100644 --- a/compiler/rustc_hir_analysis/messages.ftl +++ b/compiler/rustc_hir_analysis/messages.ftl @@ -226,6 +226,7 @@ hir_analysis_rpitit_refined = impl trait in impl method signature does not match .suggestion = replace the return type so that it matches the trait .label = return type from trait method defined here .unmatched_bound_label = this bound is stronger than that defined on the trait + .note = add `#[allow(refining_impl_trait)]` if it is intended for this to be part of the public API of this crate hir_analysis_self_in_impl_self = `Self` is not valid in the self type of an impl block diff --git a/compiler/rustc_hir_analysis/src/errors.rs b/compiler/rustc_hir_analysis/src/errors.rs index a1b306edd64fc..7614913f4bf51 100644 --- a/compiler/rustc_hir_analysis/src/errors.rs +++ b/compiler/rustc_hir_analysis/src/errors.rs @@ -921,6 +921,7 @@ pub struct UnusedAssociatedTypeBounds { #[derive(LintDiagnostic)] #[diag(hir_analysis_rpitit_refined)] +#[note] pub(crate) struct ReturnPositionImplTraitInTraitRefined<'tcx> { #[suggestion(applicability = "maybe-incorrect", code = "{pre}{return_ty}{post}")] pub impl_return_span: Span, diff --git a/tests/ui/impl-trait/in-trait/bad-item-bound-within-rpitit.stderr b/tests/ui/impl-trait/in-trait/bad-item-bound-within-rpitit.stderr index c0e07e94dabbe..a5fb338ea4e13 100644 --- a/tests/ui/impl-trait/in-trait/bad-item-bound-within-rpitit.stderr +++ b/tests/ui/impl-trait/in-trait/bad-item-bound-within-rpitit.stderr @@ -21,6 +21,7 @@ LL | fn iter(&self) -> impl '_ + Iterator>; LL | fn iter(&self) -> impl 'a + Iterator> { | ^^ this bound is stronger than that defined on the trait | + = note: add `#[allow(refining_impl_trait)]` if it is intended for this to be part of the public API of this crate = note: `#[warn(refining_impl_trait)]` on by default help: replace the return type so that it matches the trait | diff --git a/tests/ui/impl-trait/in-trait/refine.stderr b/tests/ui/impl-trait/in-trait/refine.stderr index 65dfdf869e4b1..29aa08e25bb91 100644 --- a/tests/ui/impl-trait/in-trait/refine.stderr +++ b/tests/ui/impl-trait/in-trait/refine.stderr @@ -7,6 +7,7 @@ LL | fn bar() -> impl Sized; LL | fn bar() -> impl Copy {} | ^^^^ this bound is stronger than that defined on the trait | + = note: add `#[allow(refining_impl_trait)]` if it is intended for this to be part of the public API of this crate note: the lint level is defined here --> $DIR/refine.rs:2:9 | @@ -26,6 +27,7 @@ LL | fn bar() -> impl Sized; LL | fn bar() {} | ^^^^^^^^ | + = note: add `#[allow(refining_impl_trait)]` if it is intended for this to be part of the public API of this crate help: replace the return type so that it matches the trait | LL | fn bar() -> impl Sized {} @@ -40,6 +42,7 @@ LL | fn bar() -> impl Sized; LL | fn bar() -> () {} | ^^ | + = note: add `#[allow(refining_impl_trait)]` if it is intended for this to be part of the public API of this crate help: replace the return type so that it matches the trait | LL | fn bar() -> impl Sized {} @@ -54,6 +57,7 @@ LL | fn bar<'a>(&'a self) -> impl Sized + 'a; LL | fn bar(&self) -> impl Copy + '_ {} | ^^^^ this bound is stronger than that defined on the trait | + = note: add `#[allow(refining_impl_trait)]` if it is intended for this to be part of the public API of this crate help: replace the return type so that it matches the trait | LL | fn bar(&self) -> impl Sized + '_ {}