From ca581f91614dacabe1f6629435b9c74720e469cd Mon Sep 17 00:00:00 2001 From: Oli Scherer Date: Mon, 5 Jun 2023 15:57:21 +0000 Subject: [PATCH 1/9] Don't require associated types with `Self: Sized` bounds in `dyn Trait` objects --- compiler/rustc_hir_analysis/src/astconv/mod.rs | 10 ++++++++++ compiler/rustc_middle/src/query/mod.rs | 4 ++++ .../src/traits/object_safety.rs | 7 ++++++- tests/ui/object-safety/assoc_type_bounds_sized.rs | 4 +++- .../ui/object-safety/assoc_type_bounds_sized.stderr | 12 ------------ 5 files changed, 23 insertions(+), 14 deletions(-) delete mode 100644 tests/ui/object-safety/assoc_type_bounds_sized.stderr diff --git a/compiler/rustc_hir_analysis/src/astconv/mod.rs b/compiler/rustc_hir_analysis/src/astconv/mod.rs index 752c5ad535a97..6291d124ab49f 100644 --- a/compiler/rustc_hir_analysis/src/astconv/mod.rs +++ b/compiler/rustc_hir_analysis/src/astconv/mod.rs @@ -1131,6 +1131,16 @@ impl<'o, 'tcx> dyn AstConv<'tcx> + 'o { } } + for def_ids in associated_types.values_mut() { + for def_id in def_ids.clone() { + // If the associated type has a `where Self: Sized` bound, we do not need to constrain the associated + // type in the `dyn Trait`. + if tcx.generics_require_sized_self(def_id) { + def_ids.remove(&def_id); + } + } + } + self.complain_about_missing_associated_types( associated_types, potential_assoc_types, diff --git a/compiler/rustc_middle/src/query/mod.rs b/compiler/rustc_middle/src/query/mod.rs index 6f942e0bc8697..0e1c6d19e3189 100644 --- a/compiler/rustc_middle/src/query/mod.rs +++ b/compiler/rustc_middle/src/query/mod.rs @@ -2197,6 +2197,10 @@ rustc_queries! { desc { "getting cfg-ed out item names" } separate_provide_extern } + + query generics_require_sized_self(def_id: DefId) -> bool { + desc { "check whether the item has a `where Self: Sized` bound" } + } } rustc_query_append! { define_callbacks! } diff --git a/compiler/rustc_trait_selection/src/traits/object_safety.rs b/compiler/rustc_trait_selection/src/traits/object_safety.rs index 08be60c65d457..6a9f0ffc425a3 100644 --- a/compiler/rustc_trait_selection/src/traits/object_safety.rs +++ b/compiler/rustc_trait_selection/src/traits/object_safety.rs @@ -922,5 +922,10 @@ pub fn contains_illegal_impl_trait_in_trait<'tcx>( } pub fn provide(providers: &mut Providers) { - *providers = Providers { object_safety_violations, check_is_object_safe, ..*providers }; + *providers = Providers { + object_safety_violations, + check_is_object_safe, + generics_require_sized_self, + ..*providers + }; } diff --git a/tests/ui/object-safety/assoc_type_bounds_sized.rs b/tests/ui/object-safety/assoc_type_bounds_sized.rs index 61ad3cf9dc6dc..546cde37f63fb 100644 --- a/tests/ui/object-safety/assoc_type_bounds_sized.rs +++ b/tests/ui/object-safety/assoc_type_bounds_sized.rs @@ -1,9 +1,11 @@ +// check-pass + trait Foo { type Bar where Self: Sized; } -fn foo(_: &dyn Foo) {} //~ ERROR: the value of the associated type `Bar` (from trait `Foo`) must be specified +fn foo(_: &dyn Foo) {} fn main() {} diff --git a/tests/ui/object-safety/assoc_type_bounds_sized.stderr b/tests/ui/object-safety/assoc_type_bounds_sized.stderr deleted file mode 100644 index 49d624f9b1d26..0000000000000 --- a/tests/ui/object-safety/assoc_type_bounds_sized.stderr +++ /dev/null @@ -1,12 +0,0 @@ -error[E0191]: the value of the associated type `Bar` (from trait `Foo`) must be specified - --> $DIR/assoc_type_bounds_sized.rs:7:16 - | -LL | type Bar - | -------- `Bar` defined here -... -LL | fn foo(_: &dyn Foo) {} - | ^^^ help: specify the associated type: `Foo` - -error: aborting due to previous error - -For more information about this error, try `rustc --explain E0191`. From a49b73656880f8d2379dad52088195add197be2c Mon Sep 17 00:00:00 2001 From: Oli Scherer Date: Mon, 5 Jun 2023 17:08:27 +0000 Subject: [PATCH 2/9] Lint now-unnecessary associated type bounds --- compiler/rustc_hir_analysis/messages.ftl | 5 ++++ .../rustc_hir_analysis/src/astconv/mod.rs | 24 ++++++++++++++--- compiler/rustc_hir_analysis/src/errors.rs | 10 ++++++- compiler/rustc_lint_defs/src/builtin.rs | 26 ++++++++++++++++++ .../assoc_type_bounds_sized_unnecessary.rs | 14 ++++++++++ ...assoc_type_bounds_sized_unnecessary.stderr | 27 +++++++++++++++++++ 6 files changed, 102 insertions(+), 4 deletions(-) create mode 100644 tests/ui/object-safety/assoc_type_bounds_sized_unnecessary.rs create mode 100644 tests/ui/object-safety/assoc_type_bounds_sized_unnecessary.stderr diff --git a/compiler/rustc_hir_analysis/messages.ftl b/compiler/rustc_hir_analysis/messages.ftl index ad26c495c02d5..0738961d6ce48 100644 --- a/compiler/rustc_hir_analysis/messages.ftl +++ b/compiler/rustc_hir_analysis/messages.ftl @@ -288,6 +288,11 @@ hir_analysis_unrecognized_intrinsic_function = unrecognized intrinsic function: `{$name}` .label = unrecognized intrinsic +hir_analysis_unused_associated_type_bounds = + unnecessary associated type bound for not object safe associated type + .note = this associated type has a `where Self: Sized` bound. Thus, while the associated type can be specified, it cannot be used in any way, because trait objects are not `Sized`. + .suggestion = remove this bound + hir_analysis_value_of_associated_struct_already_specified = the value of the associated type `{$item_name}` (from trait `{$def_path}`) is already specified .label = re-bound here diff --git a/compiler/rustc_hir_analysis/src/astconv/mod.rs b/compiler/rustc_hir_analysis/src/astconv/mod.rs index 6291d124ab49f..1c906da28c089 100644 --- a/compiler/rustc_hir_analysis/src/astconv/mod.rs +++ b/compiler/rustc_hir_analysis/src/astconv/mod.rs @@ -29,6 +29,7 @@ use rustc_hir::intravisit::{walk_generics, Visitor as _}; use rustc_hir::{GenericArg, GenericArgs, OpaqueTyOrigin}; use rustc_infer::infer::{InferCtxt, InferOk, TyCtxtInferExt}; use rustc_infer::traits::ObligationCause; +use rustc_lint_defs::builtin::UNUSED_ASSOCIATED_TYPE_BOUNDS; use rustc_middle::middle::stability::AllowUnstable; use rustc_middle::ty::subst::{self, GenericArgKind, InternalSubsts, SubstsRef}; use rustc_middle::ty::DynKind; @@ -929,6 +930,7 @@ impl<'o, 'tcx> dyn AstConv<'tcx> + 'o { fn conv_object_ty_poly_trait_ref( &self, span: Span, + hir_id: hir::HirId, hir_trait_bounds: &[hir::PolyTraitRef<'_>], lifetime: &hir::Lifetime, borrowed: bool, @@ -1125,9 +1127,18 @@ impl<'o, 'tcx> dyn AstConv<'tcx> + 'o { // So every `Projection` clause is an `Assoc = Foo` bound. `associated_types` contains all associated // types's `DefId`, so the following loop removes all the `DefIds` of the associated types that have a // corresponding `Projection` clause - for (projection_bound, _) in &projection_bounds { + for (projection_bound, span) in &projection_bounds { for def_ids in associated_types.values_mut() { - def_ids.remove(&projection_bound.projection_def_id()); + let def_id = projection_bound.projection_def_id(); + def_ids.remove(&def_id); + if tcx.generics_require_sized_self(def_id) { + tcx.emit_spanned_lint( + UNUSED_ASSOCIATED_TYPE_BOUNDS, + hir_id, + *span, + crate::errors::UnusedAssociatedTypeBounds { span: *span }, + ); + } } } @@ -2812,7 +2823,14 @@ impl<'o, 'tcx> dyn AstConv<'tcx> + 'o { TraitObjectSyntax::DynStar => ty::DynStar, }; - self.conv_object_ty_poly_trait_ref(ast_ty.span, bounds, lifetime, borrowed, repr) + self.conv_object_ty_poly_trait_ref( + ast_ty.span, + ast_ty.hir_id, + bounds, + lifetime, + borrowed, + repr, + ) } hir::TyKind::Path(hir::QPath::Resolved(maybe_qself, path)) => { debug!(?maybe_qself, ?path); diff --git a/compiler/rustc_hir_analysis/src/errors.rs b/compiler/rustc_hir_analysis/src/errors.rs index cb840592edd22..205e26d0edaac 100644 --- a/compiler/rustc_hir_analysis/src/errors.rs +++ b/compiler/rustc_hir_analysis/src/errors.rs @@ -5,7 +5,7 @@ use rustc_errors::{ error_code, Applicability, DiagnosticBuilder, ErrorGuaranteed, Handler, IntoDiagnostic, MultiSpan, }; -use rustc_macros::{Diagnostic, Subdiagnostic}; +use rustc_macros::{Diagnostic, LintDiagnostic, Subdiagnostic}; use rustc_middle::ty::{self, print::TraitRefPrintOnlyTraitPath, Ty}; use rustc_span::{symbol::Ident, Span, Symbol}; @@ -900,3 +900,11 @@ pub(crate) enum LateBoundInApit { param_span: Span, }, } + +#[derive(LintDiagnostic)] +#[diag(hir_analysis_unused_associated_type_bounds)] +#[note] +pub struct UnusedAssociatedTypeBounds { + #[suggestion(code = "")] + pub span: Span, +} diff --git a/compiler/rustc_lint_defs/src/builtin.rs b/compiler/rustc_lint_defs/src/builtin.rs index ef82a6c17eeea..d2d0d58784e30 100644 --- a/compiler/rustc_lint_defs/src/builtin.rs +++ b/compiler/rustc_lint_defs/src/builtin.rs @@ -3468,6 +3468,32 @@ declare_lint! { report_in_external_macro } +declare_lint! { + /// The `unused_associated_type_bounds` lint is emitted when an + /// associated type bound is added to a trait object, but the associated + /// type has a `where Self: Sized` bound, and is thus unavailable on the + /// trait object anyway. + /// + /// ### Example + /// + /// ```rust + /// trait Foo { + /// type Bar where Self: Sized; + /// } + /// type Mop = dyn Foo; + /// ``` + /// + /// {{produces}} + /// + /// ### Explanation + /// + /// Just like methods with `Self: Sized` bounds are unavailable on trait + /// objects, associated types can be removed from the trait object. + pub UNUSED_ASSOCIATED_TYPE_BOUNDS, + Warn, + "detects unused `Foo = Bar` bounds in `dyn Trait`" +} + declare_lint! { /// The `unused_doc_comments` lint detects doc comments that aren't used /// by `rustdoc`. diff --git a/tests/ui/object-safety/assoc_type_bounds_sized_unnecessary.rs b/tests/ui/object-safety/assoc_type_bounds_sized_unnecessary.rs new file mode 100644 index 0000000000000..564081e4ef6d5 --- /dev/null +++ b/tests/ui/object-safety/assoc_type_bounds_sized_unnecessary.rs @@ -0,0 +1,14 @@ +// check-pass + +trait Foo { + type Bar + where + Self: Sized; +} + +fn foo(_: &dyn Foo) {} +//~^ WARN: unnecessary associated type bound for not object safe associated type +//~| WARN: unnecessary associated type bound for not object safe associated type +//~| WARN: unnecessary associated type bound for not object safe associated type + +fn main() {} diff --git a/tests/ui/object-safety/assoc_type_bounds_sized_unnecessary.stderr b/tests/ui/object-safety/assoc_type_bounds_sized_unnecessary.stderr new file mode 100644 index 0000000000000..d0a4179fe3e1c --- /dev/null +++ b/tests/ui/object-safety/assoc_type_bounds_sized_unnecessary.stderr @@ -0,0 +1,27 @@ +warning: unnecessary associated type bound for not object safe associated type + --> $DIR/assoc_type_bounds_sized_unnecessary.rs:9:20 + | +LL | fn foo(_: &dyn Foo) {} + | ^^^^^^^^ help: remove this bound + | + = note: this associated type has a `where Self: Sized` bound. Thus, while the associated type can be specified, it cannot be used in any way, because trait objects are not `Sized`. + = note: `#[warn(unused_associated_type_bounds)]` on by default + +warning: unnecessary associated type bound for not object safe associated type + --> $DIR/assoc_type_bounds_sized_unnecessary.rs:9:20 + | +LL | fn foo(_: &dyn Foo) {} + | ^^^^^^^^ help: remove this bound + | + = note: this associated type has a `where Self: Sized` bound. Thus, while the associated type can be specified, it cannot be used in any way, because trait objects are not `Sized`. + +warning: unnecessary associated type bound for not object safe associated type + --> $DIR/assoc_type_bounds_sized_unnecessary.rs:9:20 + | +LL | fn foo(_: &dyn Foo) {} + | ^^^^^^^^ help: remove this bound + | + = note: this associated type has a `where Self: Sized` bound. Thus, while the associated type can be specified, it cannot be used in any way, because trait objects are not `Sized`. + +warning: 3 warnings emitted + From 62fbbac2d98eb4cc1804f92a7d31713cd676b497 Mon Sep 17 00:00:00 2001 From: Oli Scherer Date: Mon, 5 Jun 2023 17:47:41 +0000 Subject: [PATCH 3/9] tidy: move a large function out of an even larger file --- .../rustc_hir_analysis/src/astconv/mod.rs | 409 +---------------- .../src/astconv/object_safety.rs | 415 ++++++++++++++++++ 2 files changed, 418 insertions(+), 406 deletions(-) create mode 100644 compiler/rustc_hir_analysis/src/astconv/object_safety.rs diff --git a/compiler/rustc_hir_analysis/src/astconv/mod.rs b/compiler/rustc_hir_analysis/src/astconv/mod.rs index 1c906da28c089..a6503e032c747 100644 --- a/compiler/rustc_hir_analysis/src/astconv/mod.rs +++ b/compiler/rustc_hir_analysis/src/astconv/mod.rs @@ -6,14 +6,13 @@ mod bounds; mod errors; pub mod generics; mod lint; +mod object_safety; use crate::astconv::errors::prohibit_assoc_ty_binding; use crate::astconv::generics::{check_generic_arg_count, create_substs_for_generic_args}; use crate::bounds::Bounds; use crate::collect::HirPlaceholderCollector; -use crate::errors::{ - AmbiguousLifetimeBound, TraitObjectDeclaredWithNoTraits, TypeofReservedKeywordUsed, -}; +use crate::errors::{AmbiguousLifetimeBound, TypeofReservedKeywordUsed}; use crate::middle::resolve_bound_vars as rbv; use crate::require_c_abi_if_c_variadic; use rustc_ast::TraitObjectSyntax; @@ -29,27 +28,19 @@ use rustc_hir::intravisit::{walk_generics, Visitor as _}; use rustc_hir::{GenericArg, GenericArgs, OpaqueTyOrigin}; use rustc_infer::infer::{InferCtxt, InferOk, TyCtxtInferExt}; use rustc_infer::traits::ObligationCause; -use rustc_lint_defs::builtin::UNUSED_ASSOCIATED_TYPE_BOUNDS; use rustc_middle::middle::stability::AllowUnstable; use rustc_middle::ty::subst::{self, GenericArgKind, InternalSubsts, SubstsRef}; -use rustc_middle::ty::DynKind; use rustc_middle::ty::GenericParamDefKind; -use rustc_middle::ty::ToPredicate; use rustc_middle::ty::{self, Const, IsSuggestable, Ty, TyCtxt, TypeVisitableExt}; use rustc_session::lint::builtin::AMBIGUOUS_ASSOCIATED_ITEMS; use rustc_span::edit_distance::find_best_match_for_name; use rustc_span::symbol::{kw, Ident, Symbol}; use rustc_span::{sym, Span, DUMMY_SP}; use rustc_target::spec::abi; -use rustc_trait_selection::traits::error_reporting::report_object_safety_error; use rustc_trait_selection::traits::wf::object_region_bounds; -use rustc_trait_selection::traits::{ - self, astconv_object_safety_violations, NormalizeExt, ObligationCtxt, -}; +use rustc_trait_selection::traits::{self, NormalizeExt, ObligationCtxt}; use rustc_type_ir::fold::{TypeFoldable, TypeFolder, TypeSuperFoldable}; -use smallvec::{smallvec, SmallVec}; -use std::collections::BTreeSet; use std::fmt::Display; use std::slice; @@ -927,400 +918,6 @@ impl<'o, 'tcx> dyn AstConv<'tcx> + 'o { } } - fn conv_object_ty_poly_trait_ref( - &self, - span: Span, - hir_id: hir::HirId, - hir_trait_bounds: &[hir::PolyTraitRef<'_>], - lifetime: &hir::Lifetime, - borrowed: bool, - representation: DynKind, - ) -> Ty<'tcx> { - let tcx = self.tcx(); - - let mut bounds = Bounds::default(); - let mut potential_assoc_types = Vec::new(); - let dummy_self = self.tcx().types.trait_object_dummy_self; - for trait_bound in hir_trait_bounds.iter().rev() { - if let GenericArgCountResult { - correct: - Err(GenericArgCountMismatch { invalid_args: cur_potential_assoc_types, .. }), - .. - } = self.instantiate_poly_trait_ref( - &trait_bound.trait_ref, - trait_bound.span, - ty::BoundConstness::NotConst, - ty::ImplPolarity::Positive, - dummy_self, - &mut bounds, - false, - // FIXME: This should be `true`, but we don't really handle - // associated type bounds or type aliases in objects in a way - // that makes this meaningful, I think. - OnlySelfBounds(false), - ) { - potential_assoc_types.extend(cur_potential_assoc_types); - } - } - - let mut trait_bounds = vec![]; - let mut projection_bounds = vec![]; - for (pred, span) in bounds.clauses() { - let bound_pred = pred.kind(); - match bound_pred.skip_binder() { - ty::ClauseKind::Trait(trait_pred) => { - assert_eq!(trait_pred.polarity, ty::ImplPolarity::Positive); - trait_bounds.push(( - bound_pred.rebind(trait_pred.trait_ref), - span, - trait_pred.constness, - )); - } - ty::ClauseKind::Projection(proj) => { - projection_bounds.push((bound_pred.rebind(proj), span)); - } - ty::ClauseKind::TypeOutlives(_) => { - // Do nothing, we deal with regions separately - } - ty::ClauseKind::RegionOutlives(_) - | ty::ClauseKind::ConstArgHasType(..) - | ty::ClauseKind::WellFormed(_) - | ty::ClauseKind::ConstEvaluatable(_) => { - bug!() - } - } - } - - // Expand trait aliases recursively and check that only one regular (non-auto) trait - // is used and no 'maybe' bounds are used. - let expanded_traits = - traits::expand_trait_aliases(tcx, trait_bounds.iter().map(|&(a, b, _)| (a, b))); - - let (mut auto_traits, regular_traits): (Vec<_>, Vec<_>) = expanded_traits - .filter(|i| i.trait_ref().self_ty().skip_binder() == dummy_self) - .partition(|i| tcx.trait_is_auto(i.trait_ref().def_id())); - if regular_traits.len() > 1 { - let first_trait = ®ular_traits[0]; - let additional_trait = ®ular_traits[1]; - let mut err = struct_span_err!( - tcx.sess, - additional_trait.bottom().1, - E0225, - "only auto traits can be used as additional traits in a trait object" - ); - additional_trait.label_with_exp_info( - &mut err, - "additional non-auto trait", - "additional use", - ); - first_trait.label_with_exp_info(&mut err, "first non-auto trait", "first use"); - err.help(format!( - "consider creating a new trait with all of these as supertraits and using that \ - trait here instead: `trait NewTrait: {} {{}}`", - regular_traits - .iter() - .map(|t| t.trait_ref().print_only_trait_path().to_string()) - .collect::>() - .join(" + "), - )); - err.note( - "auto-traits like `Send` and `Sync` are traits that have special properties; \ - for more information on them, visit \ - ", - ); - err.emit(); - } - - if regular_traits.is_empty() && auto_traits.is_empty() { - let trait_alias_span = trait_bounds - .iter() - .map(|&(trait_ref, _, _)| trait_ref.def_id()) - .find(|&trait_ref| tcx.is_trait_alias(trait_ref)) - .map(|trait_ref| tcx.def_span(trait_ref)); - let reported = - tcx.sess.emit_err(TraitObjectDeclaredWithNoTraits { span, trait_alias_span }); - return tcx.ty_error(reported); - } - - // Check that there are no gross object safety violations; - // most importantly, that the supertraits don't contain `Self`, - // to avoid ICEs. - for item in ®ular_traits { - let object_safety_violations = - astconv_object_safety_violations(tcx, item.trait_ref().def_id()); - if !object_safety_violations.is_empty() { - let reported = report_object_safety_error( - tcx, - span, - item.trait_ref().def_id(), - &object_safety_violations, - ) - .emit(); - return tcx.ty_error(reported); - } - } - - // Use a `BTreeSet` to keep output in a more consistent order. - let mut associated_types: FxHashMap> = FxHashMap::default(); - - let regular_traits_refs_spans = trait_bounds - .into_iter() - .filter(|(trait_ref, _, _)| !tcx.trait_is_auto(trait_ref.def_id())); - - for (base_trait_ref, span, constness) in regular_traits_refs_spans { - assert_eq!(constness, ty::BoundConstness::NotConst); - let base_pred: ty::Predicate<'tcx> = base_trait_ref.to_predicate(tcx); - for pred in traits::elaborate(tcx, [base_pred]) { - debug!("conv_object_ty_poly_trait_ref: observing object predicate `{:?}`", pred); - - let bound_predicate = pred.kind(); - match bound_predicate.skip_binder() { - ty::PredicateKind::Clause(ty::ClauseKind::Trait(pred)) => { - let pred = bound_predicate.rebind(pred); - associated_types.entry(span).or_default().extend( - tcx.associated_items(pred.def_id()) - .in_definition_order() - .filter(|item| item.kind == ty::AssocKind::Type) - .filter(|item| item.opt_rpitit_info.is_none()) - .map(|item| item.def_id), - ); - } - ty::PredicateKind::Clause(ty::ClauseKind::Projection(pred)) => { - let pred = bound_predicate.rebind(pred); - // A `Self` within the original bound will be substituted with a - // `trait_object_dummy_self`, so check for that. - let references_self = match pred.skip_binder().term.unpack() { - ty::TermKind::Ty(ty) => ty.walk().any(|arg| arg == dummy_self.into()), - ty::TermKind::Const(c) => { - c.ty().walk().any(|arg| arg == dummy_self.into()) - } - }; - - // If the projection output contains `Self`, force the user to - // elaborate it explicitly to avoid a lot of complexity. - // - // The "classically useful" case is the following: - // ``` - // trait MyTrait: FnMut() -> ::MyOutput { - // type MyOutput; - // } - // ``` - // - // Here, the user could theoretically write `dyn MyTrait`, - // but actually supporting that would "expand" to an infinitely-long type - // `fix $ τ → dyn MyTrait::MyOutput`. - // - // Instead, we force the user to write - // `dyn MyTrait`, which is uglier but works. See - // the discussion in #56288 for alternatives. - if !references_self { - // Include projections defined on supertraits. - projection_bounds.push((pred, span)); - } - } - _ => (), - } - } - } - - // `dyn Trait` desugars to (not Rust syntax) `dyn Trait where ::Assoc = Foo`. - // So every `Projection` clause is an `Assoc = Foo` bound. `associated_types` contains all associated - // types's `DefId`, so the following loop removes all the `DefIds` of the associated types that have a - // corresponding `Projection` clause - for (projection_bound, span) in &projection_bounds { - for def_ids in associated_types.values_mut() { - let def_id = projection_bound.projection_def_id(); - def_ids.remove(&def_id); - if tcx.generics_require_sized_self(def_id) { - tcx.emit_spanned_lint( - UNUSED_ASSOCIATED_TYPE_BOUNDS, - hir_id, - *span, - crate::errors::UnusedAssociatedTypeBounds { span: *span }, - ); - } - } - } - - for def_ids in associated_types.values_mut() { - for def_id in def_ids.clone() { - // If the associated type has a `where Self: Sized` bound, we do not need to constrain the associated - // type in the `dyn Trait`. - if tcx.generics_require_sized_self(def_id) { - def_ids.remove(&def_id); - } - } - } - - self.complain_about_missing_associated_types( - associated_types, - potential_assoc_types, - hir_trait_bounds, - ); - - // De-duplicate auto traits so that, e.g., `dyn Trait + Send + Send` is the same as - // `dyn Trait + Send`. - // We remove duplicates by inserting into a `FxHashSet` to avoid re-ordering - // the bounds - let mut duplicates = FxHashSet::default(); - auto_traits.retain(|i| duplicates.insert(i.trait_ref().def_id())); - debug!("regular_traits: {:?}", regular_traits); - debug!("auto_traits: {:?}", auto_traits); - - // Erase the `dummy_self` (`trait_object_dummy_self`) used above. - let existential_trait_refs = regular_traits.iter().map(|i| { - i.trait_ref().map_bound(|trait_ref: ty::TraitRef<'tcx>| { - assert_eq!(trait_ref.self_ty(), dummy_self); - - // Verify that `dummy_self` did not leak inside default type parameters. This - // could not be done at path creation, since we need to see through trait aliases. - let mut missing_type_params = vec![]; - let mut references_self = false; - let generics = tcx.generics_of(trait_ref.def_id); - let substs: Vec<_> = trait_ref - .substs - .iter() - .enumerate() - .skip(1) // Remove `Self` for `ExistentialPredicate`. - .map(|(index, arg)| { - if arg == dummy_self.into() { - let param = &generics.params[index]; - missing_type_params.push(param.name); - return tcx.ty_error_misc().into(); - } else if arg.walk().any(|arg| arg == dummy_self.into()) { - references_self = true; - return tcx.ty_error_misc().into(); - } - arg - }) - .collect(); - let substs = tcx.mk_substs(&substs); - - let span = i.bottom().1; - let empty_generic_args = hir_trait_bounds.iter().any(|hir_bound| { - hir_bound.trait_ref.path.res == Res::Def(DefKind::Trait, trait_ref.def_id) - && hir_bound.span.contains(span) - }); - self.complain_about_missing_type_params( - missing_type_params, - trait_ref.def_id, - span, - empty_generic_args, - ); - - if references_self { - let def_id = i.bottom().0.def_id(); - let mut err = struct_span_err!( - tcx.sess, - i.bottom().1, - E0038, - "the {} `{}` cannot be made into an object", - tcx.def_descr(def_id), - tcx.item_name(def_id), - ); - err.note( - rustc_middle::traits::ObjectSafetyViolation::SupertraitSelf(smallvec![]) - .error_msg(), - ); - err.emit(); - } - - ty::ExistentialTraitRef { def_id: trait_ref.def_id, substs } - }) - }); - - let existential_projections = projection_bounds - .iter() - // We filter out traits that don't have `Self` as their self type above, - // we need to do the same for projections. - .filter(|(bound, _)| bound.skip_binder().self_ty() == dummy_self) - .map(|(bound, _)| { - bound.map_bound(|mut b| { - assert_eq!(b.projection_ty.self_ty(), dummy_self); - - // Like for trait refs, verify that `dummy_self` did not leak inside default type - // parameters. - let references_self = b.projection_ty.substs.iter().skip(1).any(|arg| { - if arg.walk().any(|arg| arg == dummy_self.into()) { - return true; - } - false - }); - if references_self { - let guar = tcx.sess.delay_span_bug( - span, - "trait object projection bounds reference `Self`", - ); - let substs: Vec<_> = b - .projection_ty - .substs - .iter() - .map(|arg| { - if arg.walk().any(|arg| arg == dummy_self.into()) { - return tcx.ty_error(guar).into(); - } - arg - }) - .collect(); - b.projection_ty.substs = tcx.mk_substs(&substs); - } - - ty::ExistentialProjection::erase_self_ty(tcx, b) - }) - }); - - let regular_trait_predicates = existential_trait_refs - .map(|trait_ref| trait_ref.map_bound(ty::ExistentialPredicate::Trait)); - let auto_trait_predicates = auto_traits.into_iter().map(|trait_ref| { - ty::Binder::dummy(ty::ExistentialPredicate::AutoTrait(trait_ref.trait_ref().def_id())) - }); - // N.b. principal, projections, auto traits - // FIXME: This is actually wrong with multiple principals in regards to symbol mangling - let mut v = regular_trait_predicates - .chain( - existential_projections.map(|x| x.map_bound(ty::ExistentialPredicate::Projection)), - ) - .chain(auto_trait_predicates) - .collect::>(); - v.sort_by(|a, b| a.skip_binder().stable_cmp(tcx, &b.skip_binder())); - v.dedup(); - let existential_predicates = tcx.mk_poly_existential_predicates(&v); - - // Use explicitly-specified region bound. - let region_bound = if !lifetime.is_elided() { - self.ast_region_to_region(lifetime, None) - } else { - self.compute_object_lifetime_bound(span, existential_predicates).unwrap_or_else(|| { - if tcx.named_bound_var(lifetime.hir_id).is_some() { - self.ast_region_to_region(lifetime, None) - } else { - self.re_infer(None, span).unwrap_or_else(|| { - let mut err = struct_span_err!( - tcx.sess, - span, - E0228, - "the lifetime bound for this object type cannot be deduced \ - from context; please supply an explicit bound" - ); - let e = if borrowed { - // We will have already emitted an error E0106 complaining about a - // missing named lifetime in `&dyn Trait`, so we elide this one. - err.delay_as_bug() - } else { - err.emit() - }; - ty::Region::new_error(tcx, e) - }) - } - }) - }; - debug!("region_bound: {:?}", region_bound); - - let ty = tcx.mk_dynamic(existential_predicates, region_bound, representation); - debug!("trait_object_type: {:?}", ty); - ty - } - fn report_ambiguous_associated_type( &self, span: Span, diff --git a/compiler/rustc_hir_analysis/src/astconv/object_safety.rs b/compiler/rustc_hir_analysis/src/astconv/object_safety.rs new file mode 100644 index 0000000000000..4bdc33a1620c3 --- /dev/null +++ b/compiler/rustc_hir_analysis/src/astconv/object_safety.rs @@ -0,0 +1,415 @@ +use crate::astconv::{GenericArgCountMismatch, GenericArgCountResult, OnlySelfBounds}; +use crate::bounds::Bounds; +use crate::errors::TraitObjectDeclaredWithNoTraits; +use rustc_data_structures::fx::{FxHashMap, FxHashSet}; +use rustc_errors::struct_span_err; +use rustc_hir as hir; +use rustc_hir::def::{DefKind, Res}; +use rustc_hir::def_id::DefId; +use rustc_lint_defs::builtin::UNUSED_ASSOCIATED_TYPE_BOUNDS; +use rustc_middle::ty::{self, Ty}; +use rustc_middle::ty::{DynKind, ToPredicate}; +use rustc_span::Span; +use rustc_trait_selection::traits::error_reporting::report_object_safety_error; +use rustc_trait_selection::traits::{self, astconv_object_safety_violations}; + +use smallvec::{smallvec, SmallVec}; +use std::collections::BTreeSet; + +use super::AstConv; + +impl<'o, 'tcx> dyn AstConv<'tcx> + 'o { + pub(super) fn conv_object_ty_poly_trait_ref( + &self, + span: Span, + hir_id: hir::HirId, + hir_trait_bounds: &[hir::PolyTraitRef<'_>], + lifetime: &hir::Lifetime, + borrowed: bool, + representation: DynKind, + ) -> Ty<'tcx> { + let tcx = self.tcx(); + + let mut bounds = Bounds::default(); + let mut potential_assoc_types = Vec::new(); + let dummy_self = self.tcx().types.trait_object_dummy_self; + for trait_bound in hir_trait_bounds.iter().rev() { + if let GenericArgCountResult { + correct: + Err(GenericArgCountMismatch { invalid_args: cur_potential_assoc_types, .. }), + .. + } = self.instantiate_poly_trait_ref( + &trait_bound.trait_ref, + trait_bound.span, + ty::BoundConstness::NotConst, + ty::ImplPolarity::Positive, + dummy_self, + &mut bounds, + false, + // FIXME: This should be `true`, but we don't really handle + // associated type bounds or type aliases in objects in a way + // that makes this meaningful, I think. + OnlySelfBounds(false), + ) { + potential_assoc_types.extend(cur_potential_assoc_types); + } + } + + let mut trait_bounds = vec![]; + let mut projection_bounds = vec![]; + for (pred, span) in bounds.clauses() { + let bound_pred = pred.kind(); + match bound_pred.skip_binder() { + ty::ClauseKind::Trait(trait_pred) => { + assert_eq!(trait_pred.polarity, ty::ImplPolarity::Positive); + trait_bounds.push(( + bound_pred.rebind(trait_pred.trait_ref), + span, + trait_pred.constness, + )); + } + ty::ClauseKind::Projection(proj) => { + projection_bounds.push((bound_pred.rebind(proj), span)); + } + ty::ClauseKind::TypeOutlives(_) => { + // Do nothing, we deal with regions separately + } + ty::ClauseKind::RegionOutlives(_) + | ty::ClauseKind::ConstArgHasType(..) + | ty::ClauseKind::WellFormed(_) + | ty::ClauseKind::ConstEvaluatable(_) => { + bug!() + } + } + } + + // Expand trait aliases recursively and check that only one regular (non-auto) trait + // is used and no 'maybe' bounds are used. + let expanded_traits = + traits::expand_trait_aliases(tcx, trait_bounds.iter().map(|&(a, b, _)| (a, b))); + + let (mut auto_traits, regular_traits): (Vec<_>, Vec<_>) = expanded_traits + .filter(|i| i.trait_ref().self_ty().skip_binder() == dummy_self) + .partition(|i| tcx.trait_is_auto(i.trait_ref().def_id())); + if regular_traits.len() > 1 { + let first_trait = ®ular_traits[0]; + let additional_trait = ®ular_traits[1]; + let mut err = struct_span_err!( + tcx.sess, + additional_trait.bottom().1, + E0225, + "only auto traits can be used as additional traits in a trait object" + ); + additional_trait.label_with_exp_info( + &mut err, + "additional non-auto trait", + "additional use", + ); + first_trait.label_with_exp_info(&mut err, "first non-auto trait", "first use"); + err.help(format!( + "consider creating a new trait with all of these as supertraits and using that \ + trait here instead: `trait NewTrait: {} {{}}`", + regular_traits + .iter() + .map(|t| t.trait_ref().print_only_trait_path().to_string()) + .collect::>() + .join(" + "), + )); + err.note( + "auto-traits like `Send` and `Sync` are traits that have special properties; \ + for more information on them, visit \ + ", + ); + err.emit(); + } + + if regular_traits.is_empty() && auto_traits.is_empty() { + let trait_alias_span = trait_bounds + .iter() + .map(|&(trait_ref, _, _)| trait_ref.def_id()) + .find(|&trait_ref| tcx.is_trait_alias(trait_ref)) + .map(|trait_ref| tcx.def_span(trait_ref)); + let reported = + tcx.sess.emit_err(TraitObjectDeclaredWithNoTraits { span, trait_alias_span }); + return tcx.ty_error(reported); + } + + // Check that there are no gross object safety violations; + // most importantly, that the supertraits don't contain `Self`, + // to avoid ICEs. + for item in ®ular_traits { + let object_safety_violations = + astconv_object_safety_violations(tcx, item.trait_ref().def_id()); + if !object_safety_violations.is_empty() { + let reported = report_object_safety_error( + tcx, + span, + item.trait_ref().def_id(), + &object_safety_violations, + ) + .emit(); + return tcx.ty_error(reported); + } + } + + // Use a `BTreeSet` to keep output in a more consistent order. + let mut associated_types: FxHashMap> = FxHashMap::default(); + + let regular_traits_refs_spans = trait_bounds + .into_iter() + .filter(|(trait_ref, _, _)| !tcx.trait_is_auto(trait_ref.def_id())); + + for (base_trait_ref, span, constness) in regular_traits_refs_spans { + assert_eq!(constness, ty::BoundConstness::NotConst); + let base_pred: ty::Predicate<'tcx> = base_trait_ref.to_predicate(tcx); + for pred in traits::elaborate(tcx, [base_pred]) { + debug!("conv_object_ty_poly_trait_ref: observing object predicate `{:?}`", pred); + + let bound_predicate = pred.kind(); + match bound_predicate.skip_binder() { + ty::PredicateKind::Clause(ty::ClauseKind::Trait(pred)) => { + let pred = bound_predicate.rebind(pred); + associated_types.entry(span).or_default().extend( + tcx.associated_items(pred.def_id()) + .in_definition_order() + .filter(|item| item.kind == ty::AssocKind::Type) + .filter(|item| item.opt_rpitit_info.is_none()) + .map(|item| item.def_id), + ); + } + ty::PredicateKind::Clause(ty::ClauseKind::Projection(pred)) => { + let pred = bound_predicate.rebind(pred); + // A `Self` within the original bound will be substituted with a + // `trait_object_dummy_self`, so check for that. + let references_self = match pred.skip_binder().term.unpack() { + ty::TermKind::Ty(ty) => ty.walk().any(|arg| arg == dummy_self.into()), + ty::TermKind::Const(c) => { + c.ty().walk().any(|arg| arg == dummy_self.into()) + } + }; + + // If the projection output contains `Self`, force the user to + // elaborate it explicitly to avoid a lot of complexity. + // + // The "classically useful" case is the following: + // ``` + // trait MyTrait: FnMut() -> ::MyOutput { + // type MyOutput; + // } + // ``` + // + // Here, the user could theoretically write `dyn MyTrait`, + // but actually supporting that would "expand" to an infinitely-long type + // `fix $ τ → dyn MyTrait::MyOutput`. + // + // Instead, we force the user to write + // `dyn MyTrait`, which is uglier but works. See + // the discussion in #56288 for alternatives. + if !references_self { + // Include projections defined on supertraits. + projection_bounds.push((pred, span)); + } + } + _ => (), + } + } + } + + // `dyn Trait` desugars to (not Rust syntax) `dyn Trait where ::Assoc = Foo`. + // So every `Projection` clause is an `Assoc = Foo` bound. `associated_types` contains all associated + // types's `DefId`, so the following loop removes all the `DefIds` of the associated types that have a + // corresponding `Projection` clause + for (projection_bound, span) in &projection_bounds { + for def_ids in associated_types.values_mut() { + let def_id = projection_bound.projection_def_id(); + def_ids.remove(&def_id); + if tcx.generics_require_sized_self(def_id) { + tcx.emit_spanned_lint( + UNUSED_ASSOCIATED_TYPE_BOUNDS, + hir_id, + *span, + crate::errors::UnusedAssociatedTypeBounds { span: *span }, + ); + } + } + } + + for def_ids in associated_types.values_mut() { + for def_id in def_ids.clone() { + // If the associated type has a `where Self: Sized` bound, we do not need to constrain the associated + // type in the `dyn Trait`. + if tcx.generics_require_sized_self(def_id) { + def_ids.remove(&def_id); + } + } + } + + self.complain_about_missing_associated_types( + associated_types, + potential_assoc_types, + hir_trait_bounds, + ); + + // De-duplicate auto traits so that, e.g., `dyn Trait + Send + Send` is the same as + // `dyn Trait + Send`. + // We remove duplicates by inserting into a `FxHashSet` to avoid re-ordering + // the bounds + let mut duplicates = FxHashSet::default(); + auto_traits.retain(|i| duplicates.insert(i.trait_ref().def_id())); + debug!("regular_traits: {:?}", regular_traits); + debug!("auto_traits: {:?}", auto_traits); + + // Erase the `dummy_self` (`trait_object_dummy_self`) used above. + let existential_trait_refs = regular_traits.iter().map(|i| { + i.trait_ref().map_bound(|trait_ref: ty::TraitRef<'tcx>| { + assert_eq!(trait_ref.self_ty(), dummy_self); + + // Verify that `dummy_self` did not leak inside default type parameters. This + // could not be done at path creation, since we need to see through trait aliases. + let mut missing_type_params = vec![]; + let mut references_self = false; + let generics = tcx.generics_of(trait_ref.def_id); + let substs: Vec<_> = trait_ref + .substs + .iter() + .enumerate() + .skip(1) // Remove `Self` for `ExistentialPredicate`. + .map(|(index, arg)| { + if arg == dummy_self.into() { + let param = &generics.params[index]; + missing_type_params.push(param.name); + return tcx.ty_error_misc().into(); + } else if arg.walk().any(|arg| arg == dummy_self.into()) { + references_self = true; + return tcx.ty_error_misc().into(); + } + arg + }) + .collect(); + let substs = tcx.mk_substs(&substs); + + let span = i.bottom().1; + let empty_generic_args = hir_trait_bounds.iter().any(|hir_bound| { + hir_bound.trait_ref.path.res == Res::Def(DefKind::Trait, trait_ref.def_id) + && hir_bound.span.contains(span) + }); + self.complain_about_missing_type_params( + missing_type_params, + trait_ref.def_id, + span, + empty_generic_args, + ); + + if references_self { + let def_id = i.bottom().0.def_id(); + let mut err = struct_span_err!( + tcx.sess, + i.bottom().1, + E0038, + "the {} `{}` cannot be made into an object", + tcx.def_descr(def_id), + tcx.item_name(def_id), + ); + err.note( + rustc_middle::traits::ObjectSafetyViolation::SupertraitSelf(smallvec![]) + .error_msg(), + ); + err.emit(); + } + + ty::ExistentialTraitRef { def_id: trait_ref.def_id, substs } + }) + }); + + let existential_projections = projection_bounds + .iter() + // We filter out traits that don't have `Self` as their self type above, + // we need to do the same for projections. + .filter(|(bound, _)| bound.skip_binder().self_ty() == dummy_self) + .map(|(bound, _)| { + bound.map_bound(|mut b| { + assert_eq!(b.projection_ty.self_ty(), dummy_self); + + // Like for trait refs, verify that `dummy_self` did not leak inside default type + // parameters. + let references_self = b.projection_ty.substs.iter().skip(1).any(|arg| { + if arg.walk().any(|arg| arg == dummy_self.into()) { + return true; + } + false + }); + if references_self { + let guar = tcx.sess.delay_span_bug( + span, + "trait object projection bounds reference `Self`", + ); + let substs: Vec<_> = b + .projection_ty + .substs + .iter() + .map(|arg| { + if arg.walk().any(|arg| arg == dummy_self.into()) { + return tcx.ty_error(guar).into(); + } + arg + }) + .collect(); + b.projection_ty.substs = tcx.mk_substs(&substs); + } + + ty::ExistentialProjection::erase_self_ty(tcx, b) + }) + }); + + let regular_trait_predicates = existential_trait_refs + .map(|trait_ref| trait_ref.map_bound(ty::ExistentialPredicate::Trait)); + let auto_trait_predicates = auto_traits.into_iter().map(|trait_ref| { + ty::Binder::dummy(ty::ExistentialPredicate::AutoTrait(trait_ref.trait_ref().def_id())) + }); + // N.b. principal, projections, auto traits + // FIXME: This is actually wrong with multiple principals in regards to symbol mangling + let mut v = regular_trait_predicates + .chain( + existential_projections.map(|x| x.map_bound(ty::ExistentialPredicate::Projection)), + ) + .chain(auto_trait_predicates) + .collect::>(); + v.sort_by(|a, b| a.skip_binder().stable_cmp(tcx, &b.skip_binder())); + v.dedup(); + let existential_predicates = tcx.mk_poly_existential_predicates(&v); + + // Use explicitly-specified region bound. + let region_bound = if !lifetime.is_elided() { + self.ast_region_to_region(lifetime, None) + } else { + self.compute_object_lifetime_bound(span, existential_predicates).unwrap_or_else(|| { + if tcx.named_bound_var(lifetime.hir_id).is_some() { + self.ast_region_to_region(lifetime, None) + } else { + self.re_infer(None, span).unwrap_or_else(|| { + let mut err = struct_span_err!( + tcx.sess, + span, + E0228, + "the lifetime bound for this object type cannot be deduced \ + from context; please supply an explicit bound" + ); + let e = if borrowed { + // We will have already emitted an error E0106 complaining about a + // missing named lifetime in `&dyn Trait`, so we elide this one. + err.delay_as_bug() + } else { + err.emit() + }; + ty::Region::new_error(tcx, e) + }) + } + }) + }; + debug!("region_bound: {:?}", region_bound); + + let ty = tcx.mk_dynamic(existential_predicates, region_bound, representation); + debug!("trait_object_type: {:?}", ty); + ty + } +} From ce3cff47e0216e6fed4fb87e31d9d24307e6c49a Mon Sep 17 00:00:00 2001 From: Oli Scherer Date: Thu, 15 Jun 2023 13:29:13 +0000 Subject: [PATCH 4/9] Add more tests --- .../object-safety/assoc_type_bounds_sized.rs | 13 ++++++++++ .../assoc_type_bounds_sized_others.rs | 25 +++++++++++++++++++ .../assoc_type_bounds_sized_others.stderr | 21 ++++++++++++++++ 3 files changed, 59 insertions(+) create mode 100644 tests/ui/object-safety/assoc_type_bounds_sized_others.rs create mode 100644 tests/ui/object-safety/assoc_type_bounds_sized_others.stderr diff --git a/tests/ui/object-safety/assoc_type_bounds_sized.rs b/tests/ui/object-safety/assoc_type_bounds_sized.rs index 546cde37f63fb..6d10ceeb1b4c3 100644 --- a/tests/ui/object-safety/assoc_type_bounds_sized.rs +++ b/tests/ui/object-safety/assoc_type_bounds_sized.rs @@ -1,3 +1,6 @@ +//! This test checks that associated types only need to be +//! mentioned in trait objects, if they don't require `Self: Sized`. + // check-pass trait Foo { @@ -8,4 +11,14 @@ trait Foo { fn foo(_: &dyn Foo) {} +trait Other: Sized {} + +trait Boo { + type Assoc + where + Self: Other; +} + +fn boo(_: &dyn Boo) {} + fn main() {} diff --git a/tests/ui/object-safety/assoc_type_bounds_sized_others.rs b/tests/ui/object-safety/assoc_type_bounds_sized_others.rs new file mode 100644 index 0000000000000..647b72a759fa5 --- /dev/null +++ b/tests/ui/object-safety/assoc_type_bounds_sized_others.rs @@ -0,0 +1,25 @@ +//! This test checks that even if some associated types have +//! `where Self: Sized` bounds, those without still need to be +//! mentioned in trait objects. + +trait Foo { + type Bar + where + Self: Sized; + type Bop; +} + +fn foo(_: &dyn Foo) {} +//~^ ERROR the value of the associated type `Bop` (from trait `Foo`) must be specified + +trait Bar { + type Bop; + type Bar + where + Self: Sized; +} + +fn bar(_: &dyn Bar) {} +//~^ ERROR the value of the associated type `Bop` (from trait `Bar`) must be specified + +fn main() {} diff --git a/tests/ui/object-safety/assoc_type_bounds_sized_others.stderr b/tests/ui/object-safety/assoc_type_bounds_sized_others.stderr new file mode 100644 index 0000000000000..e4c44334b3463 --- /dev/null +++ b/tests/ui/object-safety/assoc_type_bounds_sized_others.stderr @@ -0,0 +1,21 @@ +error[E0191]: the value of the associated type `Bop` (from trait `Foo`) must be specified + --> $DIR/assoc_type_bounds_sized_others.rs:12:16 + | +LL | type Bop; + | -------- `Bop` defined here +... +LL | fn foo(_: &dyn Foo) {} + | ^^^ help: specify the associated type: `Foo` + +error[E0191]: the value of the associated type `Bop` (from trait `Bar`) must be specified + --> $DIR/assoc_type_bounds_sized_others.rs:22:16 + | +LL | type Bop; + | -------- `Bop` defined here +... +LL | fn bar(_: &dyn Bar) {} + | ^^^ help: specify the associated type: `Bar` + +error: aborting due to 2 previous errors + +For more information about this error, try `rustc --explain E0191`. From 307b5ffff37338e943d1c2b51e10f3d26654e6c2 Mon Sep 17 00:00:00 2001 From: Oli Scherer Date: Fri, 16 Jun 2023 12:38:57 +0000 Subject: [PATCH 5/9] Make all generics_require_sized_self go through the query to get caching. --- compiler/rustc_trait_selection/src/traits/object_safety.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/compiler/rustc_trait_selection/src/traits/object_safety.rs b/compiler/rustc_trait_selection/src/traits/object_safety.rs index 6a9f0ffc425a3..0adcf33618113 100644 --- a/compiler/rustc_trait_selection/src/traits/object_safety.rs +++ b/compiler/rustc_trait_selection/src/traits/object_safety.rs @@ -101,7 +101,7 @@ pub fn is_vtable_safe_method(tcx: TyCtxt<'_>, trait_def_id: DefId, method: ty::A debug_assert!(tcx.generics_of(trait_def_id).has_self); debug!("is_vtable_safe_method({:?}, {:?})", trait_def_id, method); // Any method that has a `Self: Sized` bound cannot be called. - if generics_require_sized_self(tcx, method.def_id) { + if tcx.generics_require_sized_self(method.def_id) { return false; } @@ -331,7 +331,7 @@ fn super_predicates_have_non_lifetime_binders( } fn trait_has_sized_self(tcx: TyCtxt<'_>, trait_def_id: DefId) -> bool { - generics_require_sized_self(tcx, trait_def_id) + tcx.generics_require_sized_self(trait_def_id) } fn generics_require_sized_self(tcx: TyCtxt<'_>, def_id: DefId) -> bool { @@ -364,7 +364,7 @@ fn object_safety_violation_for_assoc_item( ) -> Option { // Any item that has a `Self : Sized` requisite is otherwise // exempt from the regulations. - if generics_require_sized_self(tcx, item.def_id) { + if tcx.generics_require_sized_self(item.def_id) { return None; } From 25e3785b8691e269e4e10bb8deeae0df205e22c1 Mon Sep 17 00:00:00 2001 From: Oli Scherer Date: Fri, 16 Jun 2023 12:46:28 +0000 Subject: [PATCH 6/9] Make `unused_associated_type_bounds`'s lint level changeable --- compiler/rustc_lint_defs/src/builtin.rs | 1 + tests/ui/object-safety/assoc_type_bounds_sized_unnecessary.rs | 3 +++ 2 files changed, 4 insertions(+) diff --git a/compiler/rustc_lint_defs/src/builtin.rs b/compiler/rustc_lint_defs/src/builtin.rs index d2d0d58784e30..87c542dc2e26a 100644 --- a/compiler/rustc_lint_defs/src/builtin.rs +++ b/compiler/rustc_lint_defs/src/builtin.rs @@ -3409,6 +3409,7 @@ declare_lint_pass! { UNSTABLE_SYNTAX_PRE_EXPANSION, UNSUPPORTED_CALLING_CONVENTIONS, UNUSED_ASSIGNMENTS, + UNUSED_ASSOCIATED_TYPE_BOUNDS, UNUSED_ATTRIBUTES, UNUSED_CRATE_DEPENDENCIES, UNUSED_EXTERN_CRATES, diff --git a/tests/ui/object-safety/assoc_type_bounds_sized_unnecessary.rs b/tests/ui/object-safety/assoc_type_bounds_sized_unnecessary.rs index 564081e4ef6d5..800624e3124b6 100644 --- a/tests/ui/object-safety/assoc_type_bounds_sized_unnecessary.rs +++ b/tests/ui/object-safety/assoc_type_bounds_sized_unnecessary.rs @@ -11,4 +11,7 @@ fn foo(_: &dyn Foo) {} //~| WARN: unnecessary associated type bound for not object safe associated type //~| WARN: unnecessary associated type bound for not object safe associated type +#[allow(unused_associated_type_bounds)] +fn bar(_: &dyn Foo) {} + fn main() {} From 243687a37c9a0c3f9f6cc0d28ad1730efe53902c Mon Sep 17 00:00:00 2001 From: Oli Scherer Date: Fri, 16 Jun 2023 15:05:39 +0000 Subject: [PATCH 7/9] Prefer `retain` over hand-rolling an inefficient version of it --- .../rustc_hir_analysis/src/astconv/object_safety.rs | 10 +++------- 1 file changed, 3 insertions(+), 7 deletions(-) diff --git a/compiler/rustc_hir_analysis/src/astconv/object_safety.rs b/compiler/rustc_hir_analysis/src/astconv/object_safety.rs index 4bdc33a1620c3..6232b1d2a2393 100644 --- a/compiler/rustc_hir_analysis/src/astconv/object_safety.rs +++ b/compiler/rustc_hir_analysis/src/astconv/object_safety.rs @@ -235,13 +235,9 @@ impl<'o, 'tcx> dyn AstConv<'tcx> + 'o { } for def_ids in associated_types.values_mut() { - for def_id in def_ids.clone() { - // If the associated type has a `where Self: Sized` bound, we do not need to constrain the associated - // type in the `dyn Trait`. - if tcx.generics_require_sized_self(def_id) { - def_ids.remove(&def_id); - } - } + // If the associated type has a `where Self: Sized` bound, we do not need to constrain the associated + // type in the `dyn Trait`. + def_ids.retain(|def_id| !tcx.generics_require_sized_self(def_id)); } self.complain_about_missing_associated_types( From 3219993fa8972b55e6959979a7ccab49b2f30e5d Mon Sep 17 00:00:00 2001 From: Oli Scherer Date: Fri, 16 Jun 2023 15:15:45 +0000 Subject: [PATCH 8/9] Only use a single loop over the associated types --- compiler/rustc_hir_analysis/src/astconv/object_safety.rs | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/compiler/rustc_hir_analysis/src/astconv/object_safety.rs b/compiler/rustc_hir_analysis/src/astconv/object_safety.rs index 6232b1d2a2393..918724e047788 100644 --- a/compiler/rustc_hir_analysis/src/astconv/object_safety.rs +++ b/compiler/rustc_hir_analysis/src/astconv/object_safety.rs @@ -219,8 +219,8 @@ impl<'o, 'tcx> dyn AstConv<'tcx> + 'o { // So every `Projection` clause is an `Assoc = Foo` bound. `associated_types` contains all associated // types's `DefId`, so the following loop removes all the `DefIds` of the associated types that have a // corresponding `Projection` clause - for (projection_bound, span) in &projection_bounds { - for def_ids in associated_types.values_mut() { + for def_ids in associated_types.values_mut() { + for (projection_bound, span) in &projection_bounds { let def_id = projection_bound.projection_def_id(); def_ids.remove(&def_id); if tcx.generics_require_sized_self(def_id) { @@ -232,9 +232,6 @@ impl<'o, 'tcx> dyn AstConv<'tcx> + 'o { ); } } - } - - for def_ids in associated_types.values_mut() { // If the associated type has a `where Self: Sized` bound, we do not need to constrain the associated // type in the `dyn Trait`. def_ids.retain(|def_id| !tcx.generics_require_sized_self(def_id)); From f80aec7429e43b7ef5f508e3c41819b2552f6f26 Mon Sep 17 00:00:00 2001 From: Oli Scherer Date: Fri, 16 Jun 2023 15:21:53 +0000 Subject: [PATCH 9/9] Test that you can't circumvent the `Sized` bound check --- .../assoc_type_bounds_sized_used.rs | 20 +++++++ .../assoc_type_bounds_sized_used.stderr | 53 +++++++++++++++++++ 2 files changed, 73 insertions(+) create mode 100644 tests/ui/object-safety/assoc_type_bounds_sized_used.rs create mode 100644 tests/ui/object-safety/assoc_type_bounds_sized_used.stderr diff --git a/tests/ui/object-safety/assoc_type_bounds_sized_used.rs b/tests/ui/object-safety/assoc_type_bounds_sized_used.rs new file mode 100644 index 0000000000000..cf5345b1c1dca --- /dev/null +++ b/tests/ui/object-safety/assoc_type_bounds_sized_used.rs @@ -0,0 +1,20 @@ +//! This test checks that even if some associated types have +//! `where Self: Sized` bounds, those without still need to be +//! mentioned in trait objects. + +trait Bop { + type Bar: Default + where + Self: Sized; +} + +fn bop() { + let _ = ::Bar::default(); + //~^ ERROR: trait bounds were not satisfied + //~| ERROR: the size for values of type `T` cannot be known at compilation time +} + +fn main() { + bop::(); + //~^ ERROR: the size for values of type `dyn Bop` cannot be known at compilation time +} diff --git a/tests/ui/object-safety/assoc_type_bounds_sized_used.stderr b/tests/ui/object-safety/assoc_type_bounds_sized_used.stderr new file mode 100644 index 0000000000000..f8488d842e2f1 --- /dev/null +++ b/tests/ui/object-safety/assoc_type_bounds_sized_used.stderr @@ -0,0 +1,53 @@ +error[E0599]: the function or associated item `default` exists for associated type `::Bar`, but its trait bounds were not satisfied + --> $DIR/assoc_type_bounds_sized_used.rs:12:30 + | +LL | let _ = ::Bar::default(); + | ^^^^^^^ function or associated item cannot be called on `::Bar` due to unsatisfied trait bounds + | + = note: the following trait bounds were not satisfied: + `T: Sized` + which is required by `::Bar: Default` +help: consider restricting the type parameter to satisfy the trait bound + | +LL | fn bop() where T: Sized { + | ++++++++++++++ + +error[E0277]: the size for values of type `T` cannot be known at compilation time + --> $DIR/assoc_type_bounds_sized_used.rs:12:13 + | +LL | fn bop() { + | - this type parameter needs to be `Sized` +LL | let _ = ::Bar::default(); + | ^^^^^^^^^^^^^^^ doesn't have a size known at compile-time + | +note: required by a bound in `Bop::Bar` + --> $DIR/assoc_type_bounds_sized_used.rs:8:15 + | +LL | type Bar: Default + | --- required by a bound in this associated type +LL | where +LL | Self: Sized; + | ^^^^^ required by this bound in `Bop::Bar` +help: consider removing the `?Sized` bound to make the type parameter `Sized` + | +LL - fn bop() { +LL + fn bop() { + | + +error[E0277]: the size for values of type `dyn Bop` cannot be known at compilation time + --> $DIR/assoc_type_bounds_sized_used.rs:18:11 + | +LL | bop::(); + | ^^^^^^^ doesn't have a size known at compile-time + | + = help: the trait `Sized` is not implemented for `dyn Bop` +note: required by a bound in `bop` + --> $DIR/assoc_type_bounds_sized_used.rs:11:11 + | +LL | fn bop() { + | ^^^ required by this bound in `bop` + +error: aborting due to 3 previous errors + +Some errors have detailed explanations: E0277, E0599. +For more information about an error, try `rustc --explain E0277`.