From c8f86cad2dda3181bfedc165d3dd4bf452770228 Mon Sep 17 00:00:00 2001 From: Matthew Jasper Date: Thu, 30 Sep 2021 21:42:09 +0100 Subject: [PATCH 1/2] Elaborate predicates in min_specialization checks Supertraits of specialization markers could circumvent checks for min_specialization. Elaborating predicates prevents this. --- .../rustc_trait_selection/src/traits/mod.rs | 4 +- .../src/impl_wf_check/min_specialization.rs | 48 ++++++++++++------- .../spec-marker-supertraits.rs | 29 +++++++++++ .../spec-marker-supertraits.stderr | 13 +++++ 4 files changed, 76 insertions(+), 18 deletions(-) create mode 100644 src/test/ui/specialization/min_specialization/spec-marker-supertraits.rs create mode 100644 src/test/ui/specialization/min_specialization/spec-marker-supertraits.stderr diff --git a/compiler/rustc_trait_selection/src/traits/mod.rs b/compiler/rustc_trait_selection/src/traits/mod.rs index b3c9cf4c173ec..d777f586e23c5 100644 --- a/compiler/rustc_trait_selection/src/traits/mod.rs +++ b/compiler/rustc_trait_selection/src/traits/mod.rs @@ -64,7 +64,9 @@ pub use self::specialize::specialization_graph::FutureCompatOverlapErrorKind; pub use self::specialize::{specialization_graph, translate_substs, OverlapError}; pub use self::structural_match::search_for_structural_match_violation; pub use self::structural_match::NonStructuralMatchTy; -pub use self::util::{elaborate_predicates, elaborate_trait_ref, elaborate_trait_refs}; +pub use self::util::{ + elaborate_obligations, elaborate_predicates, elaborate_trait_ref, elaborate_trait_refs, +}; pub use self::util::{expand_trait_aliases, TraitAliasExpander}; pub use self::util::{ get_vtable_index_of_object_method, impl_item_is_final, predicate_for_trait_def, upcast_choices, diff --git a/compiler/rustc_typeck/src/impl_wf_check/min_specialization.rs b/compiler/rustc_typeck/src/impl_wf_check/min_specialization.rs index 8ecd6034ad695..f4bb5761c19bd 100644 --- a/compiler/rustc_typeck/src/impl_wf_check/min_specialization.rs +++ b/compiler/rustc_typeck/src/impl_wf_check/min_specialization.rs @@ -74,7 +74,7 @@ use rustc_infer::infer::{InferCtxt, RegionckMode, TyCtxtInferExt}; use rustc_infer::traits::specialization_graph::Node; use rustc_middle::ty::subst::{GenericArg, InternalSubsts, SubstsRef}; use rustc_middle::ty::trait_def::TraitSpecializationKind; -use rustc_middle::ty::{self, InstantiatedPredicates, TyCtxt, TypeFoldable}; +use rustc_middle::ty::{self, TyCtxt, TypeFoldable}; use rustc_span::Span; use rustc_trait_selection::traits::{self, translate_substs, wf}; @@ -294,13 +294,27 @@ fn check_predicates<'tcx>( span: Span, ) { let tcx = infcx.tcx; - let impl1_predicates = tcx.predicates_of(impl1_def_id).instantiate(tcx, impl1_substs); + let impl1_predicates: Vec<_> = traits::elaborate_predicates( + tcx, + tcx.predicates_of(impl1_def_id).instantiate(tcx, impl1_substs).predicates.into_iter(), + ) + .map(|obligation| obligation.predicate) + .collect(); + let mut impl2_predicates = if impl2_node.is_from_trait() { // Always applicable traits have to be always applicable without any // assumptions. - InstantiatedPredicates::empty() + Vec::new() } else { - tcx.predicates_of(impl2_node.def_id()).instantiate(tcx, impl2_substs) + traits::elaborate_predicates( + tcx, + tcx.predicates_of(impl2_node.def_id()) + .instantiate(tcx, impl2_substs) + .predicates + .into_iter(), + ) + .map(|obligation| obligation.predicate) + .collect() }; debug!( "check_always_applicable(\nimpl1_predicates={:?},\nimpl2_predicates={:?}\n)", @@ -322,13 +336,12 @@ fn check_predicates<'tcx>( // which is sound because we forbid impls like the following // // impl AlwaysApplicable for D { } - let always_applicable_traits = - impl1_predicates.predicates.iter().copied().filter(|&predicate| { - matches!( - trait_predicate_kind(tcx, predicate), - Some(TraitSpecializationKind::AlwaysApplicable) - ) - }); + let always_applicable_traits = impl1_predicates.iter().copied().filter(|&predicate| { + matches!( + trait_predicate_kind(tcx, predicate), + Some(TraitSpecializationKind::AlwaysApplicable) + ) + }); // Include the well-formed predicates of the type parameters of the impl. for arg in tcx.impl_trait_ref(impl1_def_id).unwrap().substs { @@ -340,18 +353,19 @@ fn check_predicates<'tcx>( arg, span, ) { - impl2_predicates - .predicates - .extend(obligations.into_iter().map(|obligation| obligation.predicate)) + impl2_predicates.extend( + traits::elaborate_obligations(tcx, obligations) + .map(|obligation| obligation.predicate), + ) } } - impl2_predicates.predicates.extend( + impl2_predicates.extend( traits::elaborate_predicates(tcx, always_applicable_traits) .map(|obligation| obligation.predicate), ); - for predicate in impl1_predicates.predicates { - if !impl2_predicates.predicates.contains(&predicate) { + for predicate in impl1_predicates { + if !impl2_predicates.contains(&predicate) { check_specialization_on(tcx, predicate, span) } } diff --git a/src/test/ui/specialization/min_specialization/spec-marker-supertraits.rs b/src/test/ui/specialization/min_specialization/spec-marker-supertraits.rs new file mode 100644 index 0000000000000..3bb2480e9e2be --- /dev/null +++ b/src/test/ui/specialization/min_specialization/spec-marker-supertraits.rs @@ -0,0 +1,29 @@ +// Check that supertraits cannot be used to work around min_specialization +// limitations. + +#![feature(min_specialization)] +#![feature(rustc_attrs)] + +trait HasMethod { + fn method(&self); +} + +#[rustc_unsafe_specialization_marker] +trait Marker: HasMethod {} + +trait Spec { + fn spec_me(&self); +} + +impl Spec for T { + default fn spec_me(&self) {} +} + +impl Spec for T { + //~^ ERROR cannot specialize on trait `HasMethod` + fn spec_me(&self) { + self.method(); + } +} + +fn main() {} diff --git a/src/test/ui/specialization/min_specialization/spec-marker-supertraits.stderr b/src/test/ui/specialization/min_specialization/spec-marker-supertraits.stderr new file mode 100644 index 0000000000000..964109dd10f47 --- /dev/null +++ b/src/test/ui/specialization/min_specialization/spec-marker-supertraits.stderr @@ -0,0 +1,13 @@ +error: cannot specialize on trait `HasMethod` + --> $DIR/spec-marker-supertraits.rs:22:1 + | +LL | / impl Spec for T { +LL | | +LL | | fn spec_me(&self) { +LL | | self.method(); +LL | | } +LL | | } + | |_^ + +error: aborting due to previous error + From 051d5b0118186433cdb1e12c6198b877bfa0a8fc Mon Sep 17 00:00:00 2001 From: Matthew Jasper Date: Thu, 30 Sep 2021 21:42:41 +0100 Subject: [PATCH 2/2] Fix standard library for min_specialization changes --- library/alloc/src/vec/into_iter.rs | 13 ++++++++++--- library/alloc/src/vec/source_iter_marker.rs | 16 +++------------- library/core/src/iter/adapters/enumerate.rs | 8 ++++---- library/core/src/iter/adapters/filter.rs | 9 ++++----- library/core/src/iter/adapters/filter_map.rs | 9 ++++----- library/core/src/iter/adapters/inspect.rs | 9 ++++----- library/core/src/iter/adapters/map.rs | 9 ++++----- library/core/src/iter/adapters/map_while.rs | 9 ++++----- library/core/src/iter/adapters/mod.rs | 11 ++++++----- library/core/src/iter/adapters/peekable.rs | 8 ++++---- library/core/src/iter/adapters/scan.rs | 9 ++++----- library/core/src/iter/adapters/skip.rs | 8 ++++---- library/core/src/iter/adapters/skip_while.rs | 9 ++++----- library/core/src/iter/adapters/take.rs | 8 ++++---- library/core/src/iter/adapters/take_while.rs | 9 ++++----- library/core/src/iter/adapters/zip.rs | 10 ++++------ 16 files changed, 71 insertions(+), 83 deletions(-) diff --git a/library/alloc/src/vec/into_iter.rs b/library/alloc/src/vec/into_iter.rs index 4cb0a4b10bd0c..096262b41d717 100644 --- a/library/alloc/src/vec/into_iter.rs +++ b/library/alloc/src/vec/into_iter.rs @@ -220,14 +220,21 @@ unsafe impl TrustedLen for IntoIter {} #[doc(hidden)] #[unstable(issue = "none", feature = "std_internals")] +#[rustc_unsafe_specialization_marker] +pub trait NonDrop {} + // T: Copy as approximation for !Drop since get_unchecked does not advance self.ptr // and thus we can't implement drop-handling -// +#[unstable(issue = "none", feature = "std_internals")] +impl NonDrop for T {} + +#[doc(hidden)] +#[unstable(issue = "none", feature = "std_internals")] // TrustedRandomAccess (without NoCoerce) must not be implemented because -// subtypes/supertypes of `T` might not be `Copy` +// subtypes/supertypes of `T` might not be `NonDrop` unsafe impl TrustedRandomAccessNoCoerce for IntoIter where - T: Copy, + T: NonDrop, { const MAY_HAVE_SIDE_EFFECT: bool = false; } diff --git a/library/alloc/src/vec/source_iter_marker.rs b/library/alloc/src/vec/source_iter_marker.rs index e05788d99c0df..6e78534cf5b10 100644 --- a/library/alloc/src/vec/source_iter_marker.rs +++ b/library/alloc/src/vec/source_iter_marker.rs @@ -6,24 +6,14 @@ use super::{AsIntoIter, InPlaceDrop, SpecFromIter, SpecFromIterNested, Vec}; /// Specialization marker for collecting an iterator pipeline into a Vec while reusing the /// source allocation, i.e. executing the pipeline in place. -/// -/// The SourceIter parent trait is necessary for the specializing function to access the allocation -/// which is to be reused. But it is not sufficient for the specialization to be valid. See -/// additional bounds on the impl. #[rustc_unsafe_specialization_marker] -pub(super) trait SourceIterMarker: SourceIter {} +pub(super) trait InPlaceIterableMarker {} -// The std-internal SourceIter/InPlaceIterable traits are only implemented by chains of -// Adapter>> (all owned by core/std). Additional bounds -// on the adapter implementations (beyond `impl Trait for Adapter`) only depend on other -// traits already marked as specialization traits (Copy, TrustedRandomAccess, FusedIterator). -// I.e. the marker does not depend on lifetimes of user-supplied types. Modulo the Copy hole, which -// several other specializations already depend on. -impl SourceIterMarker for T where T: SourceIter + InPlaceIterable {} +impl InPlaceIterableMarker for T where T: InPlaceIterable {} impl SpecFromIter for Vec where - I: Iterator + SourceIterMarker, + I: Iterator + SourceIter + InPlaceIterableMarker, { default fn from_iter(mut iterator: I) -> Self { // Additional requirements which cannot expressed via trait bounds. We rely on const eval diff --git a/library/core/src/iter/adapters/enumerate.rs b/library/core/src/iter/adapters/enumerate.rs index 3478a0cd40832..f0143cf0c3d0f 100644 --- a/library/core/src/iter/adapters/enumerate.rs +++ b/library/core/src/iter/adapters/enumerate.rs @@ -227,14 +227,14 @@ impl FusedIterator for Enumerate where I: FusedIterator {} unsafe impl TrustedLen for Enumerate where I: TrustedLen {} #[unstable(issue = "none", feature = "inplace_iteration")] -unsafe impl SourceIter for Enumerate +unsafe impl SourceIter for Enumerate where - I: SourceIter, + I: SourceIter, { - type Source = S; + type Source = I::Source; #[inline] - unsafe fn as_inner(&mut self) -> &mut S { + unsafe fn as_inner(&mut self) -> &mut I::Source { // SAFETY: unsafe function forwarding to unsafe function with the same requirements unsafe { SourceIter::as_inner(&mut self.iter) } } diff --git a/library/core/src/iter/adapters/filter.rs b/library/core/src/iter/adapters/filter.rs index d5f19f127470e..a0afaa326ad63 100644 --- a/library/core/src/iter/adapters/filter.rs +++ b/library/core/src/iter/adapters/filter.rs @@ -135,15 +135,14 @@ where impl FusedIterator for Filter where P: FnMut(&I::Item) -> bool {} #[unstable(issue = "none", feature = "inplace_iteration")] -unsafe impl SourceIter for Filter +unsafe impl SourceIter for Filter where - P: FnMut(&I::Item) -> bool, - I: SourceIter, + I: SourceIter, { - type Source = S; + type Source = I::Source; #[inline] - unsafe fn as_inner(&mut self) -> &mut S { + unsafe fn as_inner(&mut self) -> &mut I::Source { // SAFETY: unsafe function forwarding to unsafe function with the same requirements unsafe { SourceIter::as_inner(&mut self.iter) } } diff --git a/library/core/src/iter/adapters/filter_map.rs b/library/core/src/iter/adapters/filter_map.rs index 01b7be9d52daf..e0d665c9e12ba 100644 --- a/library/core/src/iter/adapters/filter_map.rs +++ b/library/core/src/iter/adapters/filter_map.rs @@ -129,15 +129,14 @@ where impl FusedIterator for FilterMap where F: FnMut(I::Item) -> Option {} #[unstable(issue = "none", feature = "inplace_iteration")] -unsafe impl SourceIter for FilterMap +unsafe impl SourceIter for FilterMap where - F: FnMut(I::Item) -> Option, - I: SourceIter, + I: SourceIter, { - type Source = S; + type Source = I::Source; #[inline] - unsafe fn as_inner(&mut self) -> &mut S { + unsafe fn as_inner(&mut self) -> &mut I::Source { // SAFETY: unsafe function forwarding to unsafe function with the same requirements unsafe { SourceIter::as_inner(&mut self.iter) } } diff --git a/library/core/src/iter/adapters/inspect.rs b/library/core/src/iter/adapters/inspect.rs index 36835d12e5685..19839fdfe5bc3 100644 --- a/library/core/src/iter/adapters/inspect.rs +++ b/library/core/src/iter/adapters/inspect.rs @@ -149,15 +149,14 @@ where impl FusedIterator for Inspect where F: FnMut(&I::Item) {} #[unstable(issue = "none", feature = "inplace_iteration")] -unsafe impl SourceIter for Inspect +unsafe impl SourceIter for Inspect where - F: FnMut(&I::Item), - I: SourceIter, + I: SourceIter, { - type Source = S; + type Source = I::Source; #[inline] - unsafe fn as_inner(&mut self) -> &mut S { + unsafe fn as_inner(&mut self) -> &mut I::Source { // SAFETY: unsafe function forwarding to unsafe function with the same requirements unsafe { SourceIter::as_inner(&mut self.iter) } } diff --git a/library/core/src/iter/adapters/map.rs b/library/core/src/iter/adapters/map.rs index 763e253e75a51..449650a22f435 100644 --- a/library/core/src/iter/adapters/map.rs +++ b/library/core/src/iter/adapters/map.rs @@ -201,15 +201,14 @@ where } #[unstable(issue = "none", feature = "inplace_iteration")] -unsafe impl SourceIter for Map +unsafe impl SourceIter for Map where - F: FnMut(I::Item) -> B, - I: SourceIter, + I: SourceIter, { - type Source = S; + type Source = I::Source; #[inline] - unsafe fn as_inner(&mut self) -> &mut S { + unsafe fn as_inner(&mut self) -> &mut I::Source { // SAFETY: unsafe function forwarding to unsafe function with the same requirements unsafe { SourceIter::as_inner(&mut self.iter) } } diff --git a/library/core/src/iter/adapters/map_while.rs b/library/core/src/iter/adapters/map_while.rs index 793b05fcf9529..1e8d6bf3e00e1 100644 --- a/library/core/src/iter/adapters/map_while.rs +++ b/library/core/src/iter/adapters/map_while.rs @@ -80,15 +80,14 @@ where } #[unstable(issue = "none", feature = "inplace_iteration")] -unsafe impl SourceIter for MapWhile +unsafe impl SourceIter for MapWhile where - P: FnMut(I::Item) -> Option, - I: SourceIter, + I: SourceIter, { - type Source = S; + type Source = I::Source; #[inline] - unsafe fn as_inner(&mut self) -> &mut S { + unsafe fn as_inner(&mut self) -> &mut I::Source { // SAFETY: unsafe function forwarding to unsafe function with the same requirements unsafe { SourceIter::as_inner(&mut self.iter) } } diff --git a/library/core/src/iter/adapters/mod.rs b/library/core/src/iter/adapters/mod.rs index 48e7dcfa7d9a3..1e1ce866ff3f2 100644 --- a/library/core/src/iter/adapters/mod.rs +++ b/library/core/src/iter/adapters/mod.rs @@ -92,9 +92,10 @@ pub use self::zip::zip; /// [`as_inner`]: SourceIter::as_inner #[unstable(issue = "none", feature = "inplace_iteration")] #[doc(hidden)] +#[rustc_specialization_trait] pub unsafe trait SourceIter { /// A source stage in an iterator pipeline. - type Source: Iterator; + type Source; /// Retrieve the source of an iterator pipeline. /// @@ -200,14 +201,14 @@ where } #[unstable(issue = "none", feature = "inplace_iteration")] -unsafe impl SourceIter for ResultShunt<'_, I, E> +unsafe impl SourceIter for ResultShunt<'_, I, E> where - I: SourceIter, + I: SourceIter, { - type Source = S; + type Source = I::Source; #[inline] - unsafe fn as_inner(&mut self) -> &mut S { + unsafe fn as_inner(&mut self) -> &mut Self::Source { // SAFETY: unsafe function forwarding to unsafe function with the same requirements unsafe { SourceIter::as_inner(&mut self.iter) } } diff --git a/library/core/src/iter/adapters/peekable.rs b/library/core/src/iter/adapters/peekable.rs index 91fa1a9ad351c..20aca323bab79 100644 --- a/library/core/src/iter/adapters/peekable.rs +++ b/library/core/src/iter/adapters/peekable.rs @@ -321,14 +321,14 @@ impl Peekable { unsafe impl TrustedLen for Peekable where I: TrustedLen {} #[unstable(issue = "none", feature = "inplace_iteration")] -unsafe impl SourceIter for Peekable +unsafe impl SourceIter for Peekable where - I: SourceIter, + I: SourceIter, { - type Source = S; + type Source = I::Source; #[inline] - unsafe fn as_inner(&mut self) -> &mut S { + unsafe fn as_inner(&mut self) -> &mut I::Source { // SAFETY: unsafe function forwarding to unsafe function with the same requirements unsafe { SourceIter::as_inner(&mut self.iter) } } diff --git a/library/core/src/iter/adapters/scan.rs b/library/core/src/iter/adapters/scan.rs index 96705b01f661f..80bfd2231241b 100644 --- a/library/core/src/iter/adapters/scan.rs +++ b/library/core/src/iter/adapters/scan.rs @@ -90,15 +90,14 @@ where } #[unstable(issue = "none", feature = "inplace_iteration")] -unsafe impl SourceIter for Scan +unsafe impl SourceIter for Scan where - I: SourceIter, - F: FnMut(&mut St, I::Item) -> Option, + I: SourceIter, { - type Source = S; + type Source = I::Source; #[inline] - unsafe fn as_inner(&mut self) -> &mut S { + unsafe fn as_inner(&mut self) -> &mut I::Source { // SAFETY: unsafe function forwarding to unsafe function with the same requirements unsafe { SourceIter::as_inner(&mut self.iter) } } diff --git a/library/core/src/iter/adapters/skip.rs b/library/core/src/iter/adapters/skip.rs index c358a6d12b7bc..bb3480008700a 100644 --- a/library/core/src/iter/adapters/skip.rs +++ b/library/core/src/iter/adapters/skip.rs @@ -180,14 +180,14 @@ where impl FusedIterator for Skip where I: FusedIterator {} #[unstable(issue = "none", feature = "inplace_iteration")] -unsafe impl SourceIter for Skip +unsafe impl SourceIter for Skip where - I: SourceIter, + I: SourceIter, { - type Source = S; + type Source = I::Source; #[inline] - unsafe fn as_inner(&mut self) -> &mut S { + unsafe fn as_inner(&mut self) -> &mut I::Source { // SAFETY: unsafe function forwarding to unsafe function with the same requirements unsafe { SourceIter::as_inner(&mut self.iter) } } diff --git a/library/core/src/iter/adapters/skip_while.rs b/library/core/src/iter/adapters/skip_while.rs index 93e29edc8df39..f29661779c056 100644 --- a/library/core/src/iter/adapters/skip_while.rs +++ b/library/core/src/iter/adapters/skip_while.rs @@ -105,15 +105,14 @@ where } #[unstable(issue = "none", feature = "inplace_iteration")] -unsafe impl SourceIter for SkipWhile +unsafe impl SourceIter for SkipWhile where - P: FnMut(&I::Item) -> bool, - I: SourceIter, + I: SourceIter, { - type Source = S; + type Source = I::Source; #[inline] - unsafe fn as_inner(&mut self) -> &mut S { + unsafe fn as_inner(&mut self) -> &mut I::Source { // SAFETY: unsafe function forwarding to unsafe function with the same requirements unsafe { SourceIter::as_inner(&mut self.iter) } } diff --git a/library/core/src/iter/adapters/take.rs b/library/core/src/iter/adapters/take.rs index beda8c32c6bdc..38d43e070b64f 100644 --- a/library/core/src/iter/adapters/take.rs +++ b/library/core/src/iter/adapters/take.rs @@ -114,14 +114,14 @@ where } #[unstable(issue = "none", feature = "inplace_iteration")] -unsafe impl SourceIter for Take +unsafe impl SourceIter for Take where - I: SourceIter, + I: SourceIter, { - type Source = S; + type Source = I::Source; #[inline] - unsafe fn as_inner(&mut self) -> &mut S { + unsafe fn as_inner(&mut self) -> &mut I::Source { // SAFETY: unsafe function forwarding to unsafe function with the same requirements unsafe { SourceIter::as_inner(&mut self.iter) } } diff --git a/library/core/src/iter/adapters/take_while.rs b/library/core/src/iter/adapters/take_while.rs index 93457d20f7c23..ded216da952a3 100644 --- a/library/core/src/iter/adapters/take_while.rs +++ b/library/core/src/iter/adapters/take_while.rs @@ -118,15 +118,14 @@ where } #[unstable(issue = "none", feature = "inplace_iteration")] -unsafe impl SourceIter for TakeWhile +unsafe impl SourceIter for TakeWhile where - P: FnMut(&I::Item) -> bool, - I: SourceIter, + I: SourceIter, { - type Source = S; + type Source = I::Source; #[inline] - unsafe fn as_inner(&mut self) -> &mut S { + unsafe fn as_inner(&mut self) -> &mut I::Source { // SAFETY: unsafe function forwarding to unsafe function with the same requirements unsafe { SourceIter::as_inner(&mut self.iter) } } diff --git a/library/core/src/iter/adapters/zip.rs b/library/core/src/iter/adapters/zip.rs index 17697fa0e045a..2b7287a413376 100644 --- a/library/core/src/iter/adapters/zip.rs +++ b/library/core/src/iter/adapters/zip.rs @@ -414,16 +414,14 @@ where // Arbitrarily selects the left side of the zip iteration as extractable "source" // it would require negative trait bounds to be able to try both #[unstable(issue = "none", feature = "inplace_iteration")] -unsafe impl SourceIter for Zip +unsafe impl SourceIter for Zip where - A: SourceIter, - B: Iterator, - S: Iterator, + A: SourceIter, { - type Source = S; + type Source = A::Source; #[inline] - unsafe fn as_inner(&mut self) -> &mut S { + unsafe fn as_inner(&mut self) -> &mut A::Source { // SAFETY: unsafe function forwarding to unsafe function with the same requirements unsafe { SourceIter::as_inner(&mut self.a) } }