From a62dd0e3bab098a4dd389a7942c2f4861f83443f Mon Sep 17 00:00:00 2001 From: Matthew Jasper Date: Sat, 8 Feb 2020 17:07:36 +0000 Subject: [PATCH 1/6] Add `min_specialization` feature Currently the only difference between it and `specialization` is that it only allows specializing functions. --- src/librustc_ast_passes/feature_gate.rs | 20 +++++++++++++------ src/librustc_feature/active.rs | 5 +++++ .../traits/specialize/mod.rs | 4 +++- 3 files changed, 22 insertions(+), 7 deletions(-) diff --git a/src/librustc_ast_passes/feature_gate.rs b/src/librustc_ast_passes/feature_gate.rs index 5f36149451dd2..f9f4c3efeb644 100644 --- a/src/librustc_ast_passes/feature_gate.rs +++ b/src/librustc_ast_passes/feature_gate.rs @@ -542,15 +542,12 @@ impl<'a> Visitor<'a> for PostExpansionVisitor<'a> { } fn visit_assoc_item(&mut self, i: &'a ast::AssocItem, ctxt: AssocCtxt) { - if let ast::Defaultness::Default(_) = i.kind.defaultness() { - gate_feature_post!(&self, specialization, i.span, "specialization is unstable"); - } - - match i.kind { + let is_fn = match i.kind { ast::AssocItemKind::Fn(_, ref sig, _, _) => { if let (ast::Const::Yes(_), AssocCtxt::Trait) = (sig.header.constness, ctxt) { gate_feature_post!(&self, const_fn, i.span, "const fn is unstable"); } + true } ast::AssocItemKind::TyAlias(_, ref generics, _, ref ty) => { if let (Some(_), AssocCtxt::Trait) = (ty, ctxt) { @@ -565,8 +562,19 @@ impl<'a> Visitor<'a> for PostExpansionVisitor<'a> { self.check_impl_trait(ty); } self.check_gat(generics, i.span); + false } - _ => {} + _ => false, + }; + if let ast::Defaultness::Default(_) = i.kind.defaultness() { + // Limit `min_specialization` to only specializing functions. + gate_feature_fn!( + &self, + |x: &Features| x.specialization || (is_fn && x.min_specialization), + i.span, + sym::specialization, + "specialization is unstable" + ); } visit::walk_assoc_item(self, i, ctxt) } diff --git a/src/librustc_feature/active.rs b/src/librustc_feature/active.rs index 3a0fc6f8da143..9f1fee8fc09d8 100644 --- a/src/librustc_feature/active.rs +++ b/src/librustc_feature/active.rs @@ -301,6 +301,11 @@ declare_features! ( /// Allows specialization of implementations (RFC 1210). (active, specialization, "1.7.0", Some(31844), None), + /// A minimal, sound subset of specialization intended to be used by the + /// standard library until the soundness issues with specialization + /// are fixed. + (active, min_specialization, "1.7.0", Some(31844), None), + /// Allows using `#[naked]` on functions. (active, naked_functions, "1.9.0", Some(32408), None), diff --git a/src/librustc_trait_selection/traits/specialize/mod.rs b/src/librustc_trait_selection/traits/specialize/mod.rs index d1d4a58fdf297..770253e635eec 100644 --- a/src/librustc_trait_selection/traits/specialize/mod.rs +++ b/src/librustc_trait_selection/traits/specialize/mod.rs @@ -161,7 +161,9 @@ pub(super) fn specializes(tcx: TyCtxt<'_>, (impl1_def_id, impl2_def_id): (DefId, // The feature gate should prevent introducing new specializations, but not // taking advantage of upstream ones. - if !tcx.features().specialization && (impl1_def_id.is_local() || impl2_def_id.is_local()) { + let features = tcx.features(); + let specialization_enabled = features.specialization || features.min_specialization; + if !specialization_enabled && (impl1_def_id.is_local() || impl2_def_id.is_local()) { return false; } From c24b4bf41098edd860d03463e00f2590293196dc Mon Sep 17 00:00:00 2001 From: Matthew Jasper Date: Sat, 8 Feb 2020 17:56:25 +0000 Subject: [PATCH 2/6] Add attributes to allow specializing on traits --- src/libcore/marker.rs | 1 + src/librustc/ty/trait_def.rs | 33 ++++++++++++++++++- src/librustc_feature/builtin_attrs.rs | 8 +++++ src/librustc_metadata/rmeta/decoder.rs | 2 ++ src/librustc_metadata/rmeta/encoder.rs | 1 + src/librustc_metadata/rmeta/mod.rs | 1 + src/librustc_span/symbol.rs | 3 ++ src/librustc_typeck/check/wfcheck.rs | 5 ++- src/librustc_typeck/coherence/mod.rs | 16 +++++++++ src/librustc_typeck/collect.rs | 17 +++++++++- .../auxiliary/specialization-trait.rs | 6 ++++ .../impl_specialization_trait.rs | 16 +++++++++ .../impl_specialization_trait.stderr | 10 ++++++ 13 files changed, 116 insertions(+), 3 deletions(-) create mode 100644 src/test/ui/specialization/min_specialization/auxiliary/specialization-trait.rs create mode 100644 src/test/ui/specialization/min_specialization/impl_specialization_trait.rs create mode 100644 src/test/ui/specialization/min_specialization/impl_specialization_trait.stderr diff --git a/src/libcore/marker.rs b/src/libcore/marker.rs index 2800f11cc01b1..2b908f07af8da 100644 --- a/src/libcore/marker.rs +++ b/src/libcore/marker.rs @@ -90,6 +90,7 @@ impl !Send for *mut T {} ch19-04-advanced-types.html#dynamically-sized-types-and-the-sized-trait>" )] #[fundamental] // for Default, for example, which requires that `[T]: !Default` be evaluatable +#[cfg_attr(not(bootstrap), rustc_specialization_trait)] pub trait Sized { // Empty. } diff --git a/src/librustc/ty/trait_def.rs b/src/librustc/ty/trait_def.rs index 0cf1c397648ed..2b5d72de3586f 100644 --- a/src/librustc/ty/trait_def.rs +++ b/src/librustc/ty/trait_def.rs @@ -33,11 +33,33 @@ pub struct TraitDef { /// and thus `impl`s of it are allowed to overlap. pub is_marker: bool, + /// Used to determine whether the standard library is allowed to specialize + /// on this trait. + pub specialization_kind: TraitSpecializationKind, + /// The ICH of this trait's DefPath, cached here so it doesn't have to be /// recomputed all the time. pub def_path_hash: DefPathHash, } +/// Whether this trait is treated specially by the standard library +/// specialization lint. +#[derive(HashStable, PartialEq, Clone, Copy, RustcEncodable, RustcDecodable)] +pub enum TraitSpecializationKind { + /// The default. Specializing on this trait is not allowed. + None, + /// Specializing on this trait is allowed because it doesn't have any + /// methods. For example `Sized` or `FusedIterator`. + /// Applies to traits with the `rustc_unsafe_specialization_marker` + /// attribute. + Marker, + /// Specializing on this trait is allowed because all of the impls of this + /// trait are "always applicable". Always applicable means that if + /// `X<'x>: T<'y>` for any lifetimes, then `for<'a, 'b> X<'a>: T<'b>`. + /// Applies to traits with the `rustc_specialization_trait` attribute. + AlwaysApplicable, +} + #[derive(Default)] pub struct TraitImpls { blanket_impls: Vec, @@ -52,9 +74,18 @@ impl<'tcx> TraitDef { paren_sugar: bool, has_auto_impl: bool, is_marker: bool, + specialization_kind: TraitSpecializationKind, def_path_hash: DefPathHash, ) -> TraitDef { - TraitDef { def_id, unsafety, paren_sugar, has_auto_impl, is_marker, def_path_hash } + TraitDef { + def_id, + unsafety, + paren_sugar, + has_auto_impl, + is_marker, + specialization_kind, + def_path_hash, + } } pub fn ancestors( diff --git a/src/librustc_feature/builtin_attrs.rs b/src/librustc_feature/builtin_attrs.rs index e9a5364c65838..83830b9d78ad4 100644 --- a/src/librustc_feature/builtin_attrs.rs +++ b/src/librustc_feature/builtin_attrs.rs @@ -530,6 +530,14 @@ pub const BUILTIN_ATTRIBUTES: &[BuiltinAttribute] = &[ rustc_test_marker, Normal, template!(Word), "the `#[rustc_test_marker]` attribute is used internally to track tests", ), + rustc_attr!( + rustc_unsafe_specialization_marker, Normal, template!(Word), + "the `#[rustc_unsafe_specialization_marker]` attribute is used to check specializations" + ), + rustc_attr!( + rustc_specialization_trait, Normal, template!(Word), + "the `#[rustc_specialization_trait]` attribute is used to check specializations" + ), // ========================================================================== // Internal attributes, Testing: diff --git a/src/librustc_metadata/rmeta/decoder.rs b/src/librustc_metadata/rmeta/decoder.rs index 1d8eb0cde468a..0fab64978ee00 100644 --- a/src/librustc_metadata/rmeta/decoder.rs +++ b/src/librustc_metadata/rmeta/decoder.rs @@ -651,6 +651,7 @@ impl<'a, 'tcx> CrateMetadataRef<'a> { data.paren_sugar, data.has_auto_impl, data.is_marker, + data.specialization_kind, self.def_path_table.def_path_hash(item_id), ) } @@ -660,6 +661,7 @@ impl<'a, 'tcx> CrateMetadataRef<'a> { false, false, false, + ty::trait_def::TraitSpecializationKind::None, self.def_path_table.def_path_hash(item_id), ), _ => bug!("def-index does not refer to trait or trait alias"), diff --git a/src/librustc_metadata/rmeta/encoder.rs b/src/librustc_metadata/rmeta/encoder.rs index ce62f15f85d93..1d8b2c0f6f553 100644 --- a/src/librustc_metadata/rmeta/encoder.rs +++ b/src/librustc_metadata/rmeta/encoder.rs @@ -1114,6 +1114,7 @@ impl EncodeContext<'tcx> { paren_sugar: trait_def.paren_sugar, has_auto_impl: self.tcx.trait_is_auto(def_id), is_marker: trait_def.is_marker, + specialization_kind: trait_def.specialization_kind, }; EntryKind::Trait(self.lazy(data)) diff --git a/src/librustc_metadata/rmeta/mod.rs b/src/librustc_metadata/rmeta/mod.rs index 89e26b15d502b..2672d772b1cf1 100644 --- a/src/librustc_metadata/rmeta/mod.rs +++ b/src/librustc_metadata/rmeta/mod.rs @@ -350,6 +350,7 @@ struct TraitData { paren_sugar: bool, has_auto_impl: bool, is_marker: bool, + specialization_kind: ty::trait_def::TraitSpecializationKind, } #[derive(RustcEncodable, RustcDecodable)] diff --git a/src/librustc_span/symbol.rs b/src/librustc_span/symbol.rs index bca4bfee85ad8..389a8bb992082 100644 --- a/src/librustc_span/symbol.rs +++ b/src/librustc_span/symbol.rs @@ -453,6 +453,7 @@ symbols! { min_align_of, min_const_fn, min_const_unsafe_fn, + min_specialization, mips_target_feature, mmx_target_feature, module, @@ -654,6 +655,8 @@ symbols! { rustc_proc_macro_decls, rustc_promotable, rustc_regions, + rustc_unsafe_specialization_marker, + rustc_specialization_trait, rustc_stable, rustc_std_internal_symbol, rustc_symbol_name, diff --git a/src/librustc_typeck/check/wfcheck.rs b/src/librustc_typeck/check/wfcheck.rs index 026e68e10e04d..2f1654c18d91c 100644 --- a/src/librustc_typeck/check/wfcheck.rs +++ b/src/librustc_typeck/check/wfcheck.rs @@ -4,6 +4,7 @@ use crate::constrained_generic_params::{identify_constrained_generic_params, Par use rustc::middle::lang_items; use rustc::session::parse::feature_err; use rustc::ty::subst::{InternalSubsts, Subst}; +use rustc::ty::trait_def::TraitSpecializationKind; use rustc::ty::{ self, AdtKind, GenericParamDefKind, ToPredicate, Ty, TyCtxt, TypeFoldable, WithConstness, }; @@ -412,7 +413,9 @@ fn check_trait(tcx: TyCtxt<'_>, item: &hir::Item<'_>) { let trait_def_id = tcx.hir().local_def_id(item.hir_id); let trait_def = tcx.trait_def(trait_def_id); - if trait_def.is_marker { + if trait_def.is_marker + || matches!(trait_def.specialization_kind, TraitSpecializationKind::Marker) + { for associated_def_id in &*tcx.associated_item_def_ids(trait_def_id) { struct_span_err!( tcx.sess, diff --git a/src/librustc_typeck/coherence/mod.rs b/src/librustc_typeck/coherence/mod.rs index 0d0149f967358..27b2c19499ccd 100644 --- a/src/librustc_typeck/coherence/mod.rs +++ b/src/librustc_typeck/coherence/mod.rs @@ -76,6 +76,22 @@ fn enforce_trait_manually_implementable(tcx: TyCtxt<'_>, impl_def_id: DefId, tra return; } + if let ty::trait_def::TraitSpecializationKind::AlwaysApplicable = + tcx.trait_def(trait_def_id).specialization_kind + { + if !tcx.features().specialization && !tcx.features().min_specialization { + let span = impl_header_span(tcx, impl_def_id); + tcx.sess + .struct_span_err( + span, + "implementing `rustc_specialization_trait` traits is unstable", + ) + .help("add `#![feature(min_specialization)]` to the crate attributes to enable") + .emit(); + return; + } + } + let trait_name = if did == li.fn_trait() { "Fn" } else if did == li.fn_mut_trait() { diff --git a/src/librustc_typeck/collect.rs b/src/librustc_typeck/collect.rs index cd63dacdcda0e..6597cc476892c 100644 --- a/src/librustc_typeck/collect.rs +++ b/src/librustc_typeck/collect.rs @@ -1032,8 +1032,23 @@ fn trait_def(tcx: TyCtxt<'_>, def_id: DefId) -> &ty::TraitDef { } let is_marker = tcx.has_attr(def_id, sym::marker); + let spec_kind = if tcx.has_attr(def_id, sym::rustc_unsafe_specialization_marker) { + ty::trait_def::TraitSpecializationKind::Marker + } else if tcx.has_attr(def_id, sym::rustc_specialization_trait) { + ty::trait_def::TraitSpecializationKind::AlwaysApplicable + } else { + ty::trait_def::TraitSpecializationKind::None + }; let def_path_hash = tcx.def_path_hash(def_id); - let def = ty::TraitDef::new(def_id, unsafety, paren_sugar, is_auto, is_marker, def_path_hash); + let def = ty::TraitDef::new( + def_id, + unsafety, + paren_sugar, + is_auto, + is_marker, + spec_kind, + def_path_hash, + ); tcx.arena.alloc(def) } diff --git a/src/test/ui/specialization/min_specialization/auxiliary/specialization-trait.rs b/src/test/ui/specialization/min_specialization/auxiliary/specialization-trait.rs new file mode 100644 index 0000000000000..6ec0d261d518a --- /dev/null +++ b/src/test/ui/specialization/min_specialization/auxiliary/specialization-trait.rs @@ -0,0 +1,6 @@ +#![feature(rustc_attrs)] + +#[rustc_specialization_trait] +pub trait SpecTrait { + fn method(&self); +} diff --git a/src/test/ui/specialization/min_specialization/impl_specialization_trait.rs b/src/test/ui/specialization/min_specialization/impl_specialization_trait.rs new file mode 100644 index 0000000000000..723ed71c3e95d --- /dev/null +++ b/src/test/ui/specialization/min_specialization/impl_specialization_trait.rs @@ -0,0 +1,16 @@ +// Check that specialization traits can't be implemented without a feature. + +// gate-test-min_specialization + +// aux-build:specialization-trait.rs + +extern crate specialization_trait; + +struct A {} + +impl specialization_trait::SpecTrait for A { + //~^ ERROR implementing `rustc_specialization_trait` traits is unstable + fn method(&self) {} +} + +fn main() {} diff --git a/src/test/ui/specialization/min_specialization/impl_specialization_trait.stderr b/src/test/ui/specialization/min_specialization/impl_specialization_trait.stderr new file mode 100644 index 0000000000000..934103d49dc0e --- /dev/null +++ b/src/test/ui/specialization/min_specialization/impl_specialization_trait.stderr @@ -0,0 +1,10 @@ +error: implementing `rustc_specialization_trait` traits is unstable + --> $DIR/impl_specialization_trait.rs:11:1 + | +LL | impl specialization_trait::SpecTrait for A { + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | + = help: add `#![feature(min_specialization)]` to the crate attributes to enable + +error: aborting due to previous error + From 32d330df30c079c8d0bcdeff9049152a65c36b31 Mon Sep 17 00:00:00 2001 From: Matthew Jasper Date: Sat, 8 Feb 2020 20:10:06 +0000 Subject: [PATCH 3/6] Avoid ICEs when we emit errors constructing the specialization graph --- src/librustc/traits/specialization_graph.rs | 29 +++++++++----- src/librustc/ty/trait_def.rs | 3 +- src/librustc_metadata/rmeta/encoder.rs | 13 ++++--- .../traits/project.rs | 24 +++++++----- .../traits/specialize/mod.rs | 38 ++++++++++--------- src/librustc_typeck/check/mod.rs | 28 ++++++++------ 6 files changed, 79 insertions(+), 56 deletions(-) diff --git a/src/librustc/traits/specialization_graph.rs b/src/librustc/traits/specialization_graph.rs index d481e578fc1dd..1847326a742eb 100644 --- a/src/librustc/traits/specialization_graph.rs +++ b/src/librustc/traits/specialization_graph.rs @@ -4,6 +4,7 @@ use crate::ty::{self, TyCtxt}; use rustc_ast::ast::Ident; use rustc_data_structures::fx::FxHashMap; use rustc_data_structures::stable_hasher::{HashStable, StableHasher}; +use rustc_errors::ErrorReported; use rustc_hir::def_id::{DefId, DefIdMap}; /// A per-trait graph of impls in specialization order. At the moment, this @@ -23,17 +24,20 @@ use rustc_hir::def_id::{DefId, DefIdMap}; /// has at most one parent. #[derive(RustcEncodable, RustcDecodable, HashStable)] pub struct Graph { - // All impls have a parent; the "root" impls have as their parent the `def_id` - // of the trait. + /// All impls have a parent; the "root" impls have as their parent the `def_id` + /// of the trait. pub parent: DefIdMap, - // The "root" impls are found by looking up the trait's def_id. + /// The "root" impls are found by looking up the trait's def_id. pub children: DefIdMap, + + /// Whether an error was emitted while constructing the graph. + pub has_errored: bool, } impl Graph { pub fn new() -> Graph { - Graph { parent: Default::default(), children: Default::default() } + Graph { parent: Default::default(), children: Default::default(), has_errored: false } } /// The parent of a given impl, which is the `DefId` of the trait when the @@ -179,17 +183,22 @@ impl<'tcx> Ancestors<'tcx> { } /// Walk up the specialization ancestors of a given impl, starting with that -/// impl itself. +/// impl itself. Returns `None` if an error was reported while building the +/// specialization graph. pub fn ancestors( tcx: TyCtxt<'tcx>, trait_def_id: DefId, start_from_impl: DefId, -) -> Ancestors<'tcx> { +) -> Result, ErrorReported> { let specialization_graph = tcx.specialization_graph_of(trait_def_id); - Ancestors { - trait_def_id, - specialization_graph, - current_source: Some(Node::Impl(start_from_impl)), + if specialization_graph.has_errored { + Err(ErrorReported) + } else { + Ok(Ancestors { + trait_def_id, + specialization_graph, + current_source: Some(Node::Impl(start_from_impl)), + }) } } diff --git a/src/librustc/ty/trait_def.rs b/src/librustc/ty/trait_def.rs index 2b5d72de3586f..948f1b8501d53 100644 --- a/src/librustc/ty/trait_def.rs +++ b/src/librustc/ty/trait_def.rs @@ -9,6 +9,7 @@ use rustc_hir::def_id::DefId; use rustc_data_structures::fx::FxHashMap; use rustc_data_structures::stable_hasher::{HashStable, StableHasher}; +use rustc_errors::ErrorReported; use rustc_macros::HashStable; /// A trait's definition with type information. @@ -92,7 +93,7 @@ impl<'tcx> TraitDef { &self, tcx: TyCtxt<'tcx>, of_impl: DefId, - ) -> specialization_graph::Ancestors<'tcx> { + ) -> Result, ErrorReported> { specialization_graph::ancestors(tcx, self.def_id, of_impl) } } diff --git a/src/librustc_metadata/rmeta/encoder.rs b/src/librustc_metadata/rmeta/encoder.rs index 1d8b2c0f6f553..3f079b286b474 100644 --- a/src/librustc_metadata/rmeta/encoder.rs +++ b/src/librustc_metadata/rmeta/encoder.rs @@ -1077,12 +1077,13 @@ impl EncodeContext<'tcx> { let polarity = self.tcx.impl_polarity(def_id); let parent = if let Some(trait_ref) = trait_ref { let trait_def = self.tcx.trait_def(trait_ref.def_id); - trait_def.ancestors(self.tcx, def_id).nth(1).and_then(|node| { - match node { - specialization_graph::Node::Impl(parent) => Some(parent), - _ => None, - } - }) + trait_def.ancestors(self.tcx, def_id).ok() + .and_then(|mut an| an.nth(1).and_then(|node| { + match node { + specialization_graph::Node::Impl(parent) => Some(parent), + _ => None, + } + })) } else { None }; diff --git a/src/librustc_trait_selection/traits/project.rs b/src/librustc_trait_selection/traits/project.rs index dde78aa4357e9..1ad915742127e 100644 --- a/src/librustc_trait_selection/traits/project.rs +++ b/src/librustc_trait_selection/traits/project.rs @@ -21,6 +21,7 @@ use rustc::ty::fold::{TypeFoldable, TypeFolder}; use rustc::ty::subst::{InternalSubsts, Subst}; use rustc::ty::{self, ToPolyTraitRef, ToPredicate, Ty, TyCtxt, WithConstness}; use rustc_ast::ast::Ident; +use rustc_errors::ErrorReported; use rustc_hir::def_id::DefId; use rustc_span::symbol::sym; use rustc_span::DUMMY_SP; @@ -1010,7 +1011,8 @@ fn assemble_candidates_from_impls<'cx, 'tcx>( // NOTE: This should be kept in sync with the similar code in // `rustc::ty::instance::resolve_associated_item()`. let node_item = - assoc_ty_def(selcx, impl_data.impl_def_id, obligation.predicate.item_def_id); + assoc_ty_def(selcx, impl_data.impl_def_id, obligation.predicate.item_def_id) + .map_err(|ErrorReported| ())?; let is_default = if node_item.node.is_from_trait() { // If true, the impl inherited a `type Foo = Bar` @@ -1405,7 +1407,10 @@ fn confirm_impl_candidate<'cx, 'tcx>( let trait_def_id = tcx.trait_id_of_impl(impl_def_id).unwrap(); let param_env = obligation.param_env; - let assoc_ty = assoc_ty_def(selcx, impl_def_id, assoc_item_id); + let assoc_ty = match assoc_ty_def(selcx, impl_def_id, assoc_item_id) { + Ok(assoc_ty) => assoc_ty, + Err(ErrorReported) => return Progress { ty: tcx.types.err, obligations: nested }, + }; if !assoc_ty.item.defaultness.has_value() { // This means that the impl is missing a definition for the @@ -1444,14 +1449,14 @@ fn assoc_ty_def( selcx: &SelectionContext<'_, '_>, impl_def_id: DefId, assoc_ty_def_id: DefId, -) -> specialization_graph::NodeItem { +) -> Result, ErrorReported> { let tcx = selcx.tcx(); let assoc_ty_name = tcx.associated_item(assoc_ty_def_id).ident; let trait_def_id = tcx.impl_trait_ref(impl_def_id).unwrap().def_id; let trait_def = tcx.trait_def(trait_def_id); // This function may be called while we are still building the - // specialization graph that is queried below (via TraidDef::ancestors()), + // specialization graph that is queried below (via TraitDef::ancestors()), // so, in order to avoid unnecessary infinite recursion, we manually look // for the associated item at the given impl. // If there is no such item in that impl, this function will fail with a @@ -1461,17 +1466,16 @@ fn assoc_ty_def( if matches!(item.kind, ty::AssocKind::Type | ty::AssocKind::OpaqueTy) && tcx.hygienic_eq(item.ident, assoc_ty_name, trait_def_id) { - return specialization_graph::NodeItem { + return Ok(specialization_graph::NodeItem { node: specialization_graph::Node::Impl(impl_def_id), item: *item, - }; + }); } } - if let Some(assoc_item) = - trait_def.ancestors(tcx, impl_def_id).leaf_def(tcx, assoc_ty_name, ty::AssocKind::Type) - { - assoc_item + let ancestors = trait_def.ancestors(tcx, impl_def_id)?; + if let Some(assoc_item) = ancestors.leaf_def(tcx, assoc_ty_name, ty::AssocKind::Type) { + Ok(assoc_item) } else { // This is saying that neither the trait nor // the impl contain a definition for this diff --git a/src/librustc_trait_selection/traits/specialize/mod.rs b/src/librustc_trait_selection/traits/specialize/mod.rs index 770253e635eec..b763851b86ef6 100644 --- a/src/librustc_trait_selection/traits/specialize/mod.rs +++ b/src/librustc_trait_selection/traits/specialize/mod.rs @@ -130,24 +130,27 @@ pub fn find_associated_item<'tcx>( let trait_def_id = tcx.trait_id_of_impl(impl_data.impl_def_id).unwrap(); let trait_def = tcx.trait_def(trait_def_id); - let ancestors = trait_def.ancestors(tcx, impl_data.impl_def_id); - match ancestors.leaf_def(tcx, item.ident, item.kind) { - Some(node_item) => { - let substs = tcx.infer_ctxt().enter(|infcx| { - let param_env = param_env.with_reveal_all(); - let substs = substs.rebase_onto(tcx, trait_def_id, impl_data.substs); - let substs = translate_substs( - &infcx, - param_env, - impl_data.impl_def_id, - substs, - node_item.node, - ); - infcx.tcx.erase_regions(&substs) - }); - (node_item.item.def_id, substs) + if let Ok(ancestors) = trait_def.ancestors(tcx, impl_data.impl_def_id) { + match ancestors.leaf_def(tcx, item.ident, item.kind) { + Some(node_item) => { + let substs = tcx.infer_ctxt().enter(|infcx| { + let param_env = param_env.with_reveal_all(); + let substs = substs.rebase_onto(tcx, trait_def_id, impl_data.substs); + let substs = translate_substs( + &infcx, + param_env, + impl_data.impl_def_id, + substs, + node_item.node, + ); + infcx.tcx.erase_regions(&substs) + }); + (node_item.item.def_id, substs) + } + None => bug!("{:?} not found in {:?}", item, impl_data.impl_def_id), } - None => bug!("{:?} not found in {:?}", item, impl_data.impl_def_id), + } else { + (item.def_id, substs) } } @@ -382,6 +385,7 @@ pub(super) fn specialization_graph_provider( match used_to_be_allowed { None => { + sg.has_errored = true; let err = struct_span_err!(tcx.sess, impl_span, E0119, ""); decorate(LintDiagnosticBuilder::new(err)); } diff --git a/src/librustc_typeck/check/mod.rs b/src/librustc_typeck/check/mod.rs index 1975b24899960..9e20dcec1af91 100644 --- a/src/librustc_typeck/check/mod.rs +++ b/src/librustc_typeck/check/mod.rs @@ -1901,8 +1901,11 @@ fn check_specialization_validity<'tcx>( hir::ImplItemKind::TyAlias(_) => ty::AssocKind::Type, }; - let mut ancestor_impls = trait_def - .ancestors(tcx, impl_id) + let ancestors = match trait_def.ancestors(tcx, impl_id) { + Ok(ancestors) => ancestors, + Err(_) => return, + }; + let mut ancestor_impls = ancestors .skip(1) .filter_map(|parent| { if parent.is_from_trait() { @@ -2083,16 +2086,17 @@ fn check_impl_items_against_trait<'tcx>( // Check for missing items from trait let mut missing_items = Vec::new(); - for trait_item in tcx.associated_items(impl_trait_ref.def_id).in_definition_order() { - let is_implemented = trait_def - .ancestors(tcx, impl_id) - .leaf_def(tcx, trait_item.ident, trait_item.kind) - .map(|node_item| !node_item.node.is_from_trait()) - .unwrap_or(false); - - if !is_implemented && !traits::impl_is_default(tcx, impl_id) { - if !trait_item.defaultness.has_value() { - missing_items.push(*trait_item); + if let Ok(ancestors) = trait_def.ancestors(tcx, impl_id) { + for trait_item in tcx.associated_items(impl_trait_ref.def_id).in_definition_order() { + let is_implemented = ancestors + .leaf_def(tcx, trait_item.ident, trait_item.kind) + .map(|node_item| !node_item.node.is_from_trait()) + .unwrap_or(false); + + if !is_implemented && !traits::impl_is_default(tcx, impl_id) { + if !trait_item.defaultness.has_value() { + missing_items.push(*trait_item); + } } } } From 0bbbe719e831c585e0e7bdd33b2432ac5ecb1b31 Mon Sep 17 00:00:00 2001 From: Matthew Jasper Date: Sat, 8 Feb 2020 20:14:02 +0000 Subject: [PATCH 4/6] Implement soundness check for min_specialization --- .../constrained_generic_params.rs | 14 +- src/librustc_typeck/impl_wf_check.rs | 14 +- .../impl_wf_check/min_specialization.rs | 390 ++++++++++++++++++ src/librustc_typeck/lib.rs | 1 + .../dyn-trait-assoc-types.rs | 32 ++ .../dyn-trait-assoc-types.stderr | 20 + .../repeated_projection_type.rs | 24 ++ .../repeated_projection_type.stderr | 11 + .../min_specialization/repeating_lifetimes.rs | 19 + .../repeating_lifetimes.stderr | 11 + .../min_specialization/repeating_param.rs | 17 + .../min_specialization/repeating_param.stderr | 11 + .../min_specialization/spec-iter.rs | 20 + .../min_specialization/spec-reference.rs | 19 + .../specialization_marker.rs | 17 + .../specialization_marker.stderr | 15 + .../specialization_trait.rs | 26 ++ .../specialization_trait.stderr | 29 ++ .../specialize_on_marker.rs | 24 ++ .../specialize_on_spec_trait.rs | 27 ++ .../specialize_on_static.rs | 18 + .../specialize_on_static.stderr | 11 + .../min_specialization/specialize_on_trait.rs | 20 + .../specialize_on_trait.stderr | 11 + 24 files changed, 796 insertions(+), 5 deletions(-) create mode 100644 src/librustc_typeck/impl_wf_check/min_specialization.rs create mode 100644 src/test/ui/specialization/min_specialization/dyn-trait-assoc-types.rs create mode 100644 src/test/ui/specialization/min_specialization/dyn-trait-assoc-types.stderr create mode 100644 src/test/ui/specialization/min_specialization/repeated_projection_type.rs create mode 100644 src/test/ui/specialization/min_specialization/repeated_projection_type.stderr create mode 100644 src/test/ui/specialization/min_specialization/repeating_lifetimes.rs create mode 100644 src/test/ui/specialization/min_specialization/repeating_lifetimes.stderr create mode 100644 src/test/ui/specialization/min_specialization/repeating_param.rs create mode 100644 src/test/ui/specialization/min_specialization/repeating_param.stderr create mode 100644 src/test/ui/specialization/min_specialization/spec-iter.rs create mode 100644 src/test/ui/specialization/min_specialization/spec-reference.rs create mode 100644 src/test/ui/specialization/min_specialization/specialization_marker.rs create mode 100644 src/test/ui/specialization/min_specialization/specialization_marker.stderr create mode 100644 src/test/ui/specialization/min_specialization/specialization_trait.rs create mode 100644 src/test/ui/specialization/min_specialization/specialization_trait.stderr create mode 100644 src/test/ui/specialization/min_specialization/specialize_on_marker.rs create mode 100644 src/test/ui/specialization/min_specialization/specialize_on_spec_trait.rs create mode 100644 src/test/ui/specialization/min_specialization/specialize_on_static.rs create mode 100644 src/test/ui/specialization/min_specialization/specialize_on_static.stderr create mode 100644 src/test/ui/specialization/min_specialization/specialize_on_trait.rs create mode 100644 src/test/ui/specialization/min_specialization/specialize_on_trait.stderr diff --git a/src/librustc_typeck/constrained_generic_params.rs b/src/librustc_typeck/constrained_generic_params.rs index aff3768e35c55..b16aa6ff3b2e4 100644 --- a/src/librustc_typeck/constrained_generic_params.rs +++ b/src/librustc_typeck/constrained_generic_params.rs @@ -79,10 +79,18 @@ impl<'tcx> TypeVisitor<'tcx> for ParameterCollector { } fn visit_const(&mut self, c: &'tcx ty::Const<'tcx>) -> bool { - if let ty::ConstKind::Param(data) = c.val { - self.parameters.push(Parameter::from(data)); + match c.val { + ty::ConstKind::Unevaluated(..) if !self.include_nonconstraining => { + // Constant expressions are not injective + return c.ty.visit_with(self); + } + ty::ConstKind::Param(data) => { + self.parameters.push(Parameter::from(data)); + } + _ => {} } - false + + c.super_visit_with(self) } } diff --git a/src/librustc_typeck/impl_wf_check.rs b/src/librustc_typeck/impl_wf_check.rs index 0a765a1f9c93c..7ee25bbede2c5 100644 --- a/src/librustc_typeck/impl_wf_check.rs +++ b/src/librustc_typeck/impl_wf_check.rs @@ -9,6 +9,8 @@ //! fixed, but for the moment it's easier to do these checks early. use crate::constrained_generic_params as cgp; +use min_specialization::check_min_specialization; + use rustc::ty::query::Providers; use rustc::ty::{self, TyCtxt, TypeFoldable}; use rustc_data_structures::fx::{FxHashMap, FxHashSet}; @@ -16,9 +18,11 @@ use rustc_errors::struct_span_err; use rustc_hir as hir; use rustc_hir::def_id::DefId; use rustc_hir::itemlikevisit::ItemLikeVisitor; +use rustc_span::Span; + use std::collections::hash_map::Entry::{Occupied, Vacant}; -use rustc_span::Span; +mod min_specialization; /// Checks that all the type/lifetime parameters on an impl also /// appear in the trait ref or self type (or are constrained by a @@ -60,7 +64,9 @@ pub fn impl_wf_check(tcx: TyCtxt<'_>) { } fn check_mod_impl_wf(tcx: TyCtxt<'_>, module_def_id: DefId) { - tcx.hir().visit_item_likes_in_module(module_def_id, &mut ImplWfCheck { tcx }); + let min_specialization = tcx.features().min_specialization; + tcx.hir() + .visit_item_likes_in_module(module_def_id, &mut ImplWfCheck { tcx, min_specialization }); } pub fn provide(providers: &mut Providers<'_>) { @@ -69,6 +75,7 @@ pub fn provide(providers: &mut Providers<'_>) { struct ImplWfCheck<'tcx> { tcx: TyCtxt<'tcx>, + min_specialization: bool, } impl ItemLikeVisitor<'tcx> for ImplWfCheck<'tcx> { @@ -77,6 +84,9 @@ impl ItemLikeVisitor<'tcx> for ImplWfCheck<'tcx> { let impl_def_id = self.tcx.hir().local_def_id(item.hir_id); enforce_impl_params_are_constrained(self.tcx, impl_def_id, items); enforce_impl_items_are_distinct(self.tcx, items); + if self.min_specialization { + check_min_specialization(self.tcx, impl_def_id, item.span); + } } } diff --git a/src/librustc_typeck/impl_wf_check/min_specialization.rs b/src/librustc_typeck/impl_wf_check/min_specialization.rs new file mode 100644 index 0000000000000..cd52dae5f9ca2 --- /dev/null +++ b/src/librustc_typeck/impl_wf_check/min_specialization.rs @@ -0,0 +1,390 @@ +//! # Minimal Specialization +//! +//! This module contains the checks for sound specialization used when the +//! `min_specialization` feature is enabled. This requires that the impl is +//! *always applicable*. +//! +//! If `impl1` specializes `impl2` then `impl1` is always applicable if we know +//! that all the bounds of `impl2` are satisfied, and all of the bounds of +//! `impl1` are satisfied for some choice of lifetimes then we know that +//! `impl1` applies for any choice of lifetimes. +//! +//! ## Basic approach +//! +//! To enforce this requirement on specializations we take the following +//! approach: +//! +//! 1. Match up the substs for `impl2` so that the implemented trait and +//! self-type match those for `impl1`. +//! 2. Check for any direct use of `'static` in the substs of `impl2`. +//! 3. Check that all of the generic parameters of `impl1` occur at most once +//! in the *unconstrained* substs for `impl2`. A parameter is constrained if +//! its value is completely determined by an associated type projection +//! predicate. +//! 4. Check that all predicates on `impl1` also exist on `impl2` (after +//! matching substs). +//! +//! ## Example +//! +//! Suppose we have the following always applicable impl: +//! +//! ```rust +//! impl SpecExtend for std::vec::IntoIter { /* specialized impl */ } +//! impl> SpecExtend for I { /* default impl */ } +//! ``` +//! +//! We get that the subst for `impl2` are `[T, std::vec::IntoIter]`. `T` is +//! constrained to be `::Item`, so we check only +//! `std::vec::IntoIter` for repeated parameters, which it doesn't have. The +//! predicates of `impl1` are only `T: Sized`, which is also a predicate of +//! `impl2`. So this specialization is sound. +//! +//! ## Extensions +//! +//! Unfortunately not all specializations in the standard library are allowed +//! by this. So there are two extensions to these rules that allow specializing +//! on some traits: that is, using them as bounds on the specializing impl, +//! even when they don't occur in the base impl. +//! +//! ### rustc_specialization_trait +//! +//! If a trait is always applicable, then it's sound to specialize on it. We +//! check trait is always applicable in the same way as impls, except that step +//! 4 is now "all predicates on `impl1` are always applicable". We require that +//! `specialization` or `min_specialization` is enabled to implement these +//! traits. +//! +//! ### rustc_unsafe_specialization_marker +//! +//! There are also some specialization on traits with no methods, including the +//! stable `FusedIterator` trait. We allow marking marker traits with an +//! unstable attribute that means we ignore them in point 3 of the checks +//! above. This is unsound, in the sense that the specialized impl may be used +//! when it doesn't apply, but we allow it in the short term since it can't +//! cause use after frees with purely safe code in the same way as specializing +//! on traits with methods can. + +use crate::constrained_generic_params as cgp; + +use rustc::middle::region::ScopeTree; +use rustc::ty::subst::{GenericArg, InternalSubsts, SubstsRef}; +use rustc::ty::trait_def::TraitSpecializationKind; +use rustc::ty::{self, InstantiatedPredicates, TyCtxt, TypeFoldable}; +use rustc_data_structures::fx::FxHashSet; +use rustc_hir as hir; +use rustc_hir::def_id::DefId; +use rustc_infer::infer::outlives::env::OutlivesEnvironment; +use rustc_infer::infer::{InferCtxt, SuppressRegionErrors, TyCtxtInferExt}; +use rustc_infer::traits::specialization_graph::Node; +use rustc_span::Span; +use rustc_trait_selection::traits::{self, translate_substs, wf}; + +pub(super) fn check_min_specialization(tcx: TyCtxt<'_>, impl_def_id: DefId, span: Span) { + if let Some(node) = parent_specialization_node(tcx, impl_def_id) { + tcx.infer_ctxt().enter(|infcx| { + check_always_applicable(&infcx, impl_def_id, node, span); + }); + } +} + +fn parent_specialization_node(tcx: TyCtxt<'_>, impl1_def_id: DefId) -> Option { + let trait_ref = tcx.impl_trait_ref(impl1_def_id)?; + let trait_def = tcx.trait_def(trait_ref.def_id); + + let impl2_node = trait_def.ancestors(tcx, impl1_def_id).ok()?.nth(1)?; + + let always_applicable_trait = + matches!(trait_def.specialization_kind, TraitSpecializationKind::AlwaysApplicable); + if impl2_node.is_from_trait() && !always_applicable_trait { + // Implementing a normal trait isn't a specialization. + return None; + } + Some(impl2_node) +} + +/// Check that `impl1` is a sound specialization +fn check_always_applicable( + infcx: &InferCtxt<'_, '_>, + impl1_def_id: DefId, + impl2_node: Node, + span: Span, +) { + if let Some((impl1_substs, impl2_substs)) = + get_impl_substs(infcx, impl1_def_id, impl2_node, span) + { + let impl2_def_id = impl2_node.def_id(); + debug!( + "check_always_applicable(\nimpl1_def_id={:?},\nimpl2_def_id={:?},\nimpl2_substs={:?}\n)", + impl1_def_id, impl2_def_id, impl2_substs + ); + + let tcx = infcx.tcx; + + let parent_substs = if impl2_node.is_from_trait() { + impl2_substs.to_vec() + } else { + unconstrained_parent_impl_substs(tcx, impl2_def_id, impl2_substs) + }; + + check_static_lifetimes(tcx, &parent_substs, span); + check_duplicate_params(tcx, impl1_substs, &parent_substs, span); + + check_predicates(tcx, impl1_def_id, impl1_substs, impl2_node, impl2_substs, span); + } +} + +/// Given a specializing impl `impl1`, and the base impl `impl2`, returns two +/// substitutions `(S1, S2)` that equate their trait references. The returned +/// types are expressed in terms of the generics of `impl1`. +/// +/// Example +/// +/// impl Foo for B { /* impl2 */ } +/// impl Foo> for C { /* impl1 */ } +/// +/// Would return `S1 = [C]` and `S2 = [Vec, C]`. +fn get_impl_substs<'tcx>( + infcx: &InferCtxt<'_, 'tcx>, + impl1_def_id: DefId, + impl2_node: Node, + span: Span, +) -> Option<(SubstsRef<'tcx>, SubstsRef<'tcx>)> { + let tcx = infcx.tcx; + let param_env = tcx.param_env(impl1_def_id); + + let impl1_substs = InternalSubsts::identity_for_item(tcx, impl1_def_id); + let impl2_substs = translate_substs(infcx, param_env, impl1_def_id, impl1_substs, impl2_node); + + // Conservatively use an empty `ParamEnv`. + let outlives_env = OutlivesEnvironment::new(ty::ParamEnv::empty()); + infcx.resolve_regions_and_report_errors( + impl1_def_id, + &ScopeTree::default(), + &outlives_env, + SuppressRegionErrors::default(), + ); + let impl2_substs = match infcx.fully_resolve(&impl2_substs) { + Ok(s) => s, + Err(_) => { + tcx.sess.struct_span_err(span, "could not resolve substs on overridden impl").emit(); + return None; + } + }; + Some((impl1_substs, impl2_substs)) +} + +/// Returns a list of all of the unconstrained subst of the given impl. +/// +/// For example given the impl: +/// +/// impl<'a, T, I> ... where &'a I: IntoIterator +/// +/// This would return the substs corresponding to `['a, I]`, because knowing +/// `'a` and `I` determines the value of `T`. +fn unconstrained_parent_impl_substs<'tcx>( + tcx: TyCtxt<'tcx>, + impl_def_id: DefId, + impl_substs: SubstsRef<'tcx>, +) -> Vec> { + let impl_generic_predicates = tcx.predicates_of(impl_def_id); + let mut unconstrained_parameters = FxHashSet::default(); + let mut constrained_params = FxHashSet::default(); + let impl_trait_ref = tcx.impl_trait_ref(impl_def_id); + + // Unfortunately the functions in `constrained_generic_parameters` don't do + // what we want here. We want only a list of constrained parameters while + // the functions in `cgp` add the constrained parameters to a list of + // unconstrained parameters. + for (predicate, _) in impl_generic_predicates.predicates.iter() { + if let ty::Predicate::Projection(proj) = predicate { + let projection_ty = proj.skip_binder().projection_ty; + let projected_ty = proj.skip_binder().ty; + + let unbound_trait_ref = projection_ty.trait_ref(tcx); + if Some(unbound_trait_ref) == impl_trait_ref { + continue; + } + + unconstrained_parameters.extend(cgp::parameters_for(&projection_ty, true)); + + for param in cgp::parameters_for(&projected_ty, false) { + if !unconstrained_parameters.contains(¶m) { + constrained_params.insert(param.0); + } + } + + unconstrained_parameters.extend(cgp::parameters_for(&projected_ty, true)); + } + } + + impl_substs + .iter() + .enumerate() + .filter(|&(idx, _)| !constrained_params.contains(&(idx as u32))) + .map(|(_, arg)| *arg) + .collect() +} + +/// Check that parameters of the derived impl don't occur more than once in the +/// equated substs of the base impl. +/// +/// For example forbid the following: +/// +/// impl Tr for A { } +/// impl Tr for (B, B) { } +/// +/// Note that only consider the unconstrained parameters of the base impl: +/// +/// impl> Tr for I { } +/// impl Tr for Vec { } +/// +/// The substs for the parent impl here are `[T, Vec]`, which repeats `T`, +/// but `S` is constrained in the parent impl, so `parent_substs` is only +/// `[Vec]`. This means we allow this impl. +fn check_duplicate_params<'tcx>( + tcx: TyCtxt<'tcx>, + impl1_substs: SubstsRef<'tcx>, + parent_substs: &Vec>, + span: Span, +) { + let mut base_params = cgp::parameters_for(parent_substs, true); + base_params.sort_by_key(|param| param.0); + if let (_, [duplicate, ..]) = base_params.partition_dedup() { + let param = impl1_substs[duplicate.0 as usize]; + tcx.sess + .struct_span_err(span, &format!("specializing impl repeats parameter `{}`", param)) + .emit(); + } +} + +/// Check that `'static` lifetimes are not introduced by the specializing impl. +/// +/// For example forbid the following: +/// +/// impl Tr for A { } +/// impl Tr for &'static i32 { } +fn check_static_lifetimes<'tcx>( + tcx: TyCtxt<'tcx>, + parent_substs: &Vec>, + span: Span, +) { + if tcx.any_free_region_meets(parent_substs, |r| *r == ty::ReStatic) { + tcx.sess.struct_span_err(span, &format!("cannot specialize on `'static` lifetime")).emit(); + } +} + +/// Check whether predicates on the specializing impl (`impl1`) are allowed. +/// +/// Each predicate `P` must be: +/// +/// * global (not reference any parameters) +/// * `T: Tr` predicate where `Tr` is an always-applicable trait +/// * on the base `impl impl2` +/// * Currently this check is done using syntactic equality, which is +/// conservative but generally sufficient. +fn check_predicates<'tcx>( + tcx: TyCtxt<'tcx>, + impl1_def_id: DefId, + impl1_substs: SubstsRef<'tcx>, + impl2_node: Node, + impl2_substs: SubstsRef<'tcx>, + span: Span, +) { + let impl1_predicates = tcx.predicates_of(impl1_def_id).instantiate(tcx, impl1_substs); + let mut impl2_predicates = if impl2_node.is_from_trait() { + // Always applicable traits have to be always applicable without any + // assumptions. + InstantiatedPredicates::empty() + } else { + tcx.predicates_of(impl2_node.def_id()).instantiate(tcx, impl2_substs) + }; + debug!( + "check_always_applicable(\nimpl1_predicates={:?},\nimpl2_predicates={:?}\n)", + impl1_predicates, impl2_predicates, + ); + + // Since impls of always applicable traits don't get to assume anything, we + // can also assume their supertraits apply. + // + // For example, we allow: + // + // #[rustc_specialization_trait] + // trait AlwaysApplicable: Debug { } + // + // impl Tr for T { } + // impl Tr for T { } + // + // Specializing on `AlwaysApplicable` allows also specializing on `Debug` + // which is sound because we forbid impls like the following + // + // impl AlwaysApplicable for D { } + let always_applicable_traits: Vec<_> = impl1_predicates + .predicates + .iter() + .filter(|predicate| { + matches!( + trait_predicate_kind(tcx, predicate), + Some(TraitSpecializationKind::AlwaysApplicable) + ) + }) + .copied() + .collect(); + impl2_predicates.predicates.extend(traits::elaborate_predicates(tcx, always_applicable_traits)); + + for predicate in impl1_predicates.predicates { + if !impl2_predicates.predicates.contains(&predicate) { + check_specialization_on(tcx, &predicate, span) + } + } +} + +fn check_specialization_on<'tcx>(tcx: TyCtxt<'tcx>, predicate: &ty::Predicate<'tcx>, span: Span) { + debug!("can_specialize_on(predicate = {:?})", predicate); + match predicate { + // Global predicates are either always true or always false, so we + // are fine to specialize on. + _ if predicate.is_global() => (), + // We allow specializing on explicitly marked traits with no associated + // items. + ty::Predicate::Trait(pred, hir::Constness::NotConst) => { + if !matches!( + trait_predicate_kind(tcx, predicate), + Some(TraitSpecializationKind::Marker) + ) { + tcx.sess + .struct_span_err( + span, + &format!( + "cannot specialize on trait `{}`", + tcx.def_path_str(pred.def_id()), + ), + ) + .emit() + } + } + _ => tcx + .sess + .struct_span_err(span, &format!("cannot specialize on `{:?}`", predicate)) + .emit(), + } +} + +fn trait_predicate_kind<'tcx>( + tcx: TyCtxt<'tcx>, + predicate: &ty::Predicate<'tcx>, +) -> Option { + match predicate { + ty::Predicate::Trait(pred, hir::Constness::NotConst) => { + Some(tcx.trait_def(pred.def_id()).specialization_kind) + } + ty::Predicate::Trait(_, hir::Constness::Const) + | ty::Predicate::RegionOutlives(_) + | ty::Predicate::TypeOutlives(_) + | ty::Predicate::Projection(_) + | ty::Predicate::WellFormed(_) + | ty::Predicate::Subtype(_) + | ty::Predicate::ObjectSafe(_) + | ty::Predicate::ClosureKind(..) + | ty::Predicate::ConstEvaluatable(..) => None, + } +} diff --git a/src/librustc_typeck/lib.rs b/src/librustc_typeck/lib.rs index 4e7985dd98812..bab354cff7c9c 100644 --- a/src/librustc_typeck/lib.rs +++ b/src/librustc_typeck/lib.rs @@ -64,6 +64,7 @@ This API is completely unstable and subject to change. #![feature(nll)] #![feature(try_blocks)] #![feature(never_type)] +#![feature(slice_partition_dedup)] #![recursion_limit = "256"] #[macro_use] diff --git a/src/test/ui/specialization/min_specialization/dyn-trait-assoc-types.rs b/src/test/ui/specialization/min_specialization/dyn-trait-assoc-types.rs new file mode 100644 index 0000000000000..03cab00b0fb96 --- /dev/null +++ b/src/test/ui/specialization/min_specialization/dyn-trait-assoc-types.rs @@ -0,0 +1,32 @@ +// Test that associated types in trait objects are not considered to be +// constrained. + +#![feature(min_specialization)] + +trait Specializable { + fn f(); +} + +trait B { + type Y; +} + +trait C { + type Y; +} + +impl Specializable for A { + default fn f() {} +} + +impl<'a, T> Specializable for dyn B + 'a { + //~^ ERROR specializing impl repeats parameter `T` + fn f() {} +} + +impl<'a, T> Specializable for dyn C + 'a { + //~^ ERROR specializing impl repeats parameter `T` + fn f() {} +} + +fn main() {} diff --git a/src/test/ui/specialization/min_specialization/dyn-trait-assoc-types.stderr b/src/test/ui/specialization/min_specialization/dyn-trait-assoc-types.stderr new file mode 100644 index 0000000000000..6345cee2c3781 --- /dev/null +++ b/src/test/ui/specialization/min_specialization/dyn-trait-assoc-types.stderr @@ -0,0 +1,20 @@ +error: specializing impl repeats parameter `T` + --> $DIR/dyn-trait-assoc-types.rs:22:1 + | +LL | / impl<'a, T> Specializable for dyn B + 'a { +LL | | +LL | | fn f() {} +LL | | } + | |_^ + +error: specializing impl repeats parameter `T` + --> $DIR/dyn-trait-assoc-types.rs:27:1 + | +LL | / impl<'a, T> Specializable for dyn C + 'a { +LL | | +LL | | fn f() {} +LL | | } + | |_^ + +error: aborting due to 2 previous errors + diff --git a/src/test/ui/specialization/min_specialization/repeated_projection_type.rs b/src/test/ui/specialization/min_specialization/repeated_projection_type.rs new file mode 100644 index 0000000000000..f21f39f066981 --- /dev/null +++ b/src/test/ui/specialization/min_specialization/repeated_projection_type.rs @@ -0,0 +1,24 @@ +// Test that projection bounds can't be specialized on. + +#![feature(min_specialization)] + +trait X { + fn f(); +} +trait Id { + type This; +} +impl Id for T { + type This = T; +} + +impl X for T { + default fn f() {} +} + +impl> X for V { + //~^ ERROR cannot specialize on + fn f() {} +} + +fn main() {} diff --git a/src/test/ui/specialization/min_specialization/repeated_projection_type.stderr b/src/test/ui/specialization/min_specialization/repeated_projection_type.stderr new file mode 100644 index 0000000000000..7cc4357a704c0 --- /dev/null +++ b/src/test/ui/specialization/min_specialization/repeated_projection_type.stderr @@ -0,0 +1,11 @@ +error: cannot specialize on `Binder(ProjectionPredicate(ProjectionTy { substs: [V], item_def_id: DefId(0:6 ~ repeated_projection_type[317d]::Id[0]::This[0]) }, (I,)))` + --> $DIR/repeated_projection_type.rs:19:1 + | +LL | / impl> X for V { +LL | | +LL | | fn f() {} +LL | | } + | |_^ + +error: aborting due to previous error + diff --git a/src/test/ui/specialization/min_specialization/repeating_lifetimes.rs b/src/test/ui/specialization/min_specialization/repeating_lifetimes.rs new file mode 100644 index 0000000000000..49bfacec0ae12 --- /dev/null +++ b/src/test/ui/specialization/min_specialization/repeating_lifetimes.rs @@ -0,0 +1,19 @@ +// Test that directly specializing on repeated lifetime parameters is not +// allowed. + +#![feature(min_specialization)] + +trait X { + fn f(); +} + +impl X for T { + default fn f() {} +} + +impl<'a> X for (&'a u8, &'a u8) { + //~^ ERROR specializing impl repeats parameter `'a` + fn f() {} +} + +fn main() {} diff --git a/src/test/ui/specialization/min_specialization/repeating_lifetimes.stderr b/src/test/ui/specialization/min_specialization/repeating_lifetimes.stderr new file mode 100644 index 0000000000000..ce9309f70122b --- /dev/null +++ b/src/test/ui/specialization/min_specialization/repeating_lifetimes.stderr @@ -0,0 +1,11 @@ +error: specializing impl repeats parameter `'a` + --> $DIR/repeating_lifetimes.rs:14:1 + | +LL | / impl<'a> X for (&'a u8, &'a u8) { +LL | | +LL | | fn f() {} +LL | | } + | |_^ + +error: aborting due to previous error + diff --git a/src/test/ui/specialization/min_specialization/repeating_param.rs b/src/test/ui/specialization/min_specialization/repeating_param.rs new file mode 100644 index 0000000000000..5a1c97fd321a9 --- /dev/null +++ b/src/test/ui/specialization/min_specialization/repeating_param.rs @@ -0,0 +1,17 @@ +// Test that specializing on two type parameters being equal is not allowed. + +#![feature(min_specialization)] + +trait X { + fn f(); +} + +impl X for T { + default fn f() {} +} +impl X for (T, T) { + //~^ ERROR specializing impl repeats parameter `T` + fn f() {} +} + +fn main() {} diff --git a/src/test/ui/specialization/min_specialization/repeating_param.stderr b/src/test/ui/specialization/min_specialization/repeating_param.stderr new file mode 100644 index 0000000000000..8b4be1c499537 --- /dev/null +++ b/src/test/ui/specialization/min_specialization/repeating_param.stderr @@ -0,0 +1,11 @@ +error: specializing impl repeats parameter `T` + --> $DIR/repeating_param.rs:12:1 + | +LL | / impl X for (T, T) { +LL | | +LL | | fn f() {} +LL | | } + | |_^ + +error: aborting due to previous error + diff --git a/src/test/ui/specialization/min_specialization/spec-iter.rs b/src/test/ui/specialization/min_specialization/spec-iter.rs new file mode 100644 index 0000000000000..e17e9dd5f133c --- /dev/null +++ b/src/test/ui/specialization/min_specialization/spec-iter.rs @@ -0,0 +1,20 @@ +// Check that we can specialize on a concrete iterator type. This requires us +// to consider which parameters in the parent impl are constrained. + +// check-pass + +#![feature(min_specialization)] + +trait SpecFromIter { + fn f(&self); +} + +impl<'a, T: 'a, I: Iterator> SpecFromIter for I { + default fn f(&self) {} +} + +impl<'a, T> SpecFromIter for std::slice::Iter<'a, T> { + fn f(&self) {} +} + +fn main() {} diff --git a/src/test/ui/specialization/min_specialization/spec-reference.rs b/src/test/ui/specialization/min_specialization/spec-reference.rs new file mode 100644 index 0000000000000..377889e2ccad2 --- /dev/null +++ b/src/test/ui/specialization/min_specialization/spec-reference.rs @@ -0,0 +1,19 @@ +// Check that lifetime parameters are allowed in specializing impls. + +// check-pass + +#![feature(min_specialization)] + +trait MySpecTrait { + fn f(); +} + +impl MySpecTrait for T { + default fn f() {} +} + +impl<'a, T: ?Sized> MySpecTrait for &'a T { + fn f() {} +} + +fn main() {} diff --git a/src/test/ui/specialization/min_specialization/specialization_marker.rs b/src/test/ui/specialization/min_specialization/specialization_marker.rs new file mode 100644 index 0000000000000..93462d02ea578 --- /dev/null +++ b/src/test/ui/specialization/min_specialization/specialization_marker.rs @@ -0,0 +1,17 @@ +// Test that `rustc_unsafe_specialization_marker` is only allowed on marker traits. + +#![feature(rustc_attrs)] + +#[rustc_unsafe_specialization_marker] +trait SpecMarker { + fn f(); + //~^ ERROR marker traits +} + +#[rustc_unsafe_specialization_marker] +trait SpecMarker2 { + type X; + //~^ ERROR marker traits +} + +fn main() {} diff --git a/src/test/ui/specialization/min_specialization/specialization_marker.stderr b/src/test/ui/specialization/min_specialization/specialization_marker.stderr new file mode 100644 index 0000000000000..ffeced198211f --- /dev/null +++ b/src/test/ui/specialization/min_specialization/specialization_marker.stderr @@ -0,0 +1,15 @@ +error[E0714]: marker traits cannot have associated items + --> $DIR/specialization_marker.rs:7:5 + | +LL | fn f(); + | ^^^^^^^ + +error[E0714]: marker traits cannot have associated items + --> $DIR/specialization_marker.rs:13:5 + | +LL | type X; + | ^^^^^^^ + +error: aborting due to 2 previous errors + +For more information about this error, try `rustc --explain E0714`. diff --git a/src/test/ui/specialization/min_specialization/specialization_trait.rs b/src/test/ui/specialization/min_specialization/specialization_trait.rs new file mode 100644 index 0000000000000..d597278d29693 --- /dev/null +++ b/src/test/ui/specialization/min_specialization/specialization_trait.rs @@ -0,0 +1,26 @@ +// Test that `rustc_specialization_trait` requires always applicable impls. + +#![feature(min_specialization)] +#![feature(rustc_attrs)] + +#[rustc_specialization_trait] +trait SpecMarker { + fn f(); +} + +impl SpecMarker for &'static u8 { + //~^ ERROR cannot specialize + fn f() {} +} + +impl SpecMarker for (T, T) { + //~^ ERROR specializing impl + fn f() {} +} + +impl SpecMarker for [T] { + //~^ ERROR cannot specialize + fn f() {} +} + +fn main() {} diff --git a/src/test/ui/specialization/min_specialization/specialization_trait.stderr b/src/test/ui/specialization/min_specialization/specialization_trait.stderr new file mode 100644 index 0000000000000..4357d2318fc5d --- /dev/null +++ b/src/test/ui/specialization/min_specialization/specialization_trait.stderr @@ -0,0 +1,29 @@ +error: cannot specialize on `'static` lifetime + --> $DIR/specialization_trait.rs:11:1 + | +LL | / impl SpecMarker for &'static u8 { +LL | | +LL | | fn f() {} +LL | | } + | |_^ + +error: specializing impl repeats parameter `T` + --> $DIR/specialization_trait.rs:16:1 + | +LL | / impl SpecMarker for (T, T) { +LL | | +LL | | fn f() {} +LL | | } + | |_^ + +error: cannot specialize on trait `std::clone::Clone` + --> $DIR/specialization_trait.rs:21:1 + | +LL | / impl SpecMarker for [T] { +LL | | +LL | | fn f() {} +LL | | } + | |_^ + +error: aborting due to 3 previous errors + diff --git a/src/test/ui/specialization/min_specialization/specialize_on_marker.rs b/src/test/ui/specialization/min_specialization/specialize_on_marker.rs new file mode 100644 index 0000000000000..4219bd13b1816 --- /dev/null +++ b/src/test/ui/specialization/min_specialization/specialize_on_marker.rs @@ -0,0 +1,24 @@ +// Test that specializing on a `rustc_unsafe_specialization_marker` trait is +// allowed. + +// check-pass + +#![feature(min_specialization)] +#![feature(rustc_attrs)] + +#[rustc_unsafe_specialization_marker] +trait SpecMarker {} + +trait X { + fn f(); +} + +impl X for T { + default fn f() {} +} + +impl X for T { + fn f() {} +} + +fn main() {} diff --git a/src/test/ui/specialization/min_specialization/specialize_on_spec_trait.rs b/src/test/ui/specialization/min_specialization/specialize_on_spec_trait.rs new file mode 100644 index 0000000000000..abbab5c23dbb7 --- /dev/null +++ b/src/test/ui/specialization/min_specialization/specialize_on_spec_trait.rs @@ -0,0 +1,27 @@ +// Test that specializing on a `rustc_specialization_trait` trait is allowed. + +// check-pass + +#![feature(min_specialization)] +#![feature(rustc_attrs)] + +#[rustc_specialization_trait] +trait SpecTrait { + fn g(&self); +} + +trait X { + fn f(&self); +} + +impl X for T { + default fn f(&self) {} +} + +impl X for T { + fn f(&self) { + self.g(); + } +} + +fn main() {} diff --git a/src/test/ui/specialization/min_specialization/specialize_on_static.rs b/src/test/ui/specialization/min_specialization/specialize_on_static.rs new file mode 100644 index 0000000000000..dd1b05401e6e7 --- /dev/null +++ b/src/test/ui/specialization/min_specialization/specialize_on_static.rs @@ -0,0 +1,18 @@ +// Test that directly specializing on `'static` is not allowed. + +#![feature(min_specialization)] + +trait X { + fn f(); +} + +impl X for &'_ T { + default fn f() {} +} + +impl X for &'static u8 { + //~^ ERROR cannot specialize on `'static` lifetime + fn f() {} +} + +fn main() {} diff --git a/src/test/ui/specialization/min_specialization/specialize_on_static.stderr b/src/test/ui/specialization/min_specialization/specialize_on_static.stderr new file mode 100644 index 0000000000000..d1809d6dfbb52 --- /dev/null +++ b/src/test/ui/specialization/min_specialization/specialize_on_static.stderr @@ -0,0 +1,11 @@ +error: cannot specialize on `'static` lifetime + --> $DIR/specialize_on_static.rs:13:1 + | +LL | / impl X for &'static u8 { +LL | | +LL | | fn f() {} +LL | | } + | |_^ + +error: aborting due to previous error + diff --git a/src/test/ui/specialization/min_specialization/specialize_on_trait.rs b/src/test/ui/specialization/min_specialization/specialize_on_trait.rs new file mode 100644 index 0000000000000..0588442c32080 --- /dev/null +++ b/src/test/ui/specialization/min_specialization/specialize_on_trait.rs @@ -0,0 +1,20 @@ +// Test that specializing on a trait is not allowed in general. + +#![feature(min_specialization)] + +trait SpecMarker {} + +trait X { + fn f(); +} + +impl X for T { + default fn f() {} +} + +impl X for T { + //~^ ERROR cannot specialize on trait `SpecMarker` + fn f() {} +} + +fn main() {} diff --git a/src/test/ui/specialization/min_specialization/specialize_on_trait.stderr b/src/test/ui/specialization/min_specialization/specialize_on_trait.stderr new file mode 100644 index 0000000000000..35445fd09b949 --- /dev/null +++ b/src/test/ui/specialization/min_specialization/specialize_on_trait.stderr @@ -0,0 +1,11 @@ +error: cannot specialize on trait `SpecMarker` + --> $DIR/specialize_on_trait.rs:15:1 + | +LL | / impl X for T { +LL | | +LL | | fn f() {} +LL | | } + | |_^ + +error: aborting due to previous error + From 4377ac3e2f58f923062e6e945f06a9a93fcc70df Mon Sep 17 00:00:00 2001 From: Matthew Jasper Date: Sat, 8 Feb 2020 20:54:33 +0000 Subject: [PATCH 5/6] Use min_specialization in libstd and libproc_macro --- src/libproc_macro/lib.rs | 3 ++- src/libstd/lib.rs | 3 ++- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/src/libproc_macro/lib.rs b/src/libproc_macro/lib.rs index 59ce14c97c0e6..8f16e3c08ec5b 100644 --- a/src/libproc_macro/lib.rs +++ b/src/libproc_macro/lib.rs @@ -26,7 +26,8 @@ #![feature(in_band_lifetimes)] #![feature(optin_builtin_traits)] #![feature(rustc_attrs)] -#![feature(specialization)] +#![cfg_attr(bootstrap, feature(specialization))] +#![cfg_attr(not(bootstrap), feature(min_specialization))] #![recursion_limit = "256"] #[unstable(feature = "proc_macro_internals", issue = "27812")] diff --git a/src/libstd/lib.rs b/src/libstd/lib.rs index 231908ddda0ae..703f60e5250a2 100644 --- a/src/libstd/lib.rs +++ b/src/libstd/lib.rs @@ -292,7 +292,8 @@ #![feature(shrink_to)] #![feature(slice_concat_ext)] #![feature(slice_internals)] -#![feature(specialization)] +#![cfg_attr(bootstrap, feature(specialization))] +#![cfg_attr(not(bootstrap), feature(min_specialization))] #![feature(staged_api)] #![feature(std_internals)] #![feature(stdsimd)] From 39ee66ab82fc38a13d046ac1caa1eb55edfa8901 Mon Sep 17 00:00:00 2001 From: Matthew Jasper Date: Mon, 9 Mar 2020 21:59:13 +0000 Subject: [PATCH 6/6] Consider well-formed predicates in min-specialization --- .../impl_wf_check/min_specialization.rs | 27 ++++++++++++++--- .../implcit-well-formed-bounds.rs | 30 +++++++++++++++++++ .../specialization_super_trait.rs | 18 +++++++++++ .../specialization_super_trait.stderr | 11 +++++++ 4 files changed, 82 insertions(+), 4 deletions(-) create mode 100644 src/test/ui/specialization/min_specialization/implcit-well-formed-bounds.rs create mode 100644 src/test/ui/specialization/min_specialization/specialization_super_trait.rs create mode 100644 src/test/ui/specialization/min_specialization/specialization_super_trait.stderr diff --git a/src/librustc_typeck/impl_wf_check/min_specialization.rs b/src/librustc_typeck/impl_wf_check/min_specialization.rs index cd52dae5f9ca2..e96a8c454b8c7 100644 --- a/src/librustc_typeck/impl_wf_check/min_specialization.rs +++ b/src/librustc_typeck/impl_wf_check/min_specialization.rs @@ -21,8 +21,9 @@ //! in the *unconstrained* substs for `impl2`. A parameter is constrained if //! its value is completely determined by an associated type projection //! predicate. -//! 4. Check that all predicates on `impl1` also exist on `impl2` (after -//! matching substs). +//! 4. Check that all predicates on `impl1` either exist on `impl2` (after +//! matching substs), or are well-formed predicates for the trait's type +//! arguments. //! //! ## Example //! @@ -129,7 +130,7 @@ fn check_always_applicable( check_static_lifetimes(tcx, &parent_substs, span); check_duplicate_params(tcx, impl1_substs, &parent_substs, span); - check_predicates(tcx, impl1_def_id, impl1_substs, impl2_node, impl2_substs, span); + check_predicates(infcx, impl1_def_id, impl1_substs, impl2_node, impl2_substs, span); } } @@ -282,14 +283,17 @@ fn check_static_lifetimes<'tcx>( /// * on the base `impl impl2` /// * Currently this check is done using syntactic equality, which is /// conservative but generally sufficient. +/// * a well-formed predicate of a type argument of the trait being implemented, +/// including the `Self`-type. fn check_predicates<'tcx>( - tcx: TyCtxt<'tcx>, + infcx: &InferCtxt<'_, 'tcx>, impl1_def_id: DefId, impl1_substs: SubstsRef<'tcx>, impl2_node: Node, impl2_substs: SubstsRef<'tcx>, span: Span, ) { + let tcx = infcx.tcx; let impl1_predicates = tcx.predicates_of(impl1_def_id).instantiate(tcx, impl1_substs); let mut impl2_predicates = if impl2_node.is_from_trait() { // Always applicable traits have to be always applicable without any @@ -329,6 +333,21 @@ fn check_predicates<'tcx>( }) .copied() .collect(); + + // Include the well-formed predicates of the type parameters of the impl. + for ty in tcx.impl_trait_ref(impl1_def_id).unwrap().substs.types() { + if let Some(obligations) = wf::obligations( + infcx, + tcx.param_env(impl1_def_id), + tcx.hir().as_local_hir_id(impl1_def_id).unwrap(), + ty, + span, + ) { + impl2_predicates + .predicates + .extend(obligations.into_iter().map(|obligation| obligation.predicate)) + } + } impl2_predicates.predicates.extend(traits::elaborate_predicates(tcx, always_applicable_traits)); for predicate in impl1_predicates.predicates { diff --git a/src/test/ui/specialization/min_specialization/implcit-well-formed-bounds.rs b/src/test/ui/specialization/min_specialization/implcit-well-formed-bounds.rs new file mode 100644 index 0000000000000..98d7f9194351c --- /dev/null +++ b/src/test/ui/specialization/min_specialization/implcit-well-formed-bounds.rs @@ -0,0 +1,30 @@ +// Test that specializing on the well-formed predicates of the trait and +// self-type of an impl is allowed. + +// check-pass + +#![feature(min_specialization)] + +struct OrdOnly(T); + +trait SpecTrait { + fn f(); +} + +impl SpecTrait for T { + default fn f() {} +} + +impl SpecTrait<()> for OrdOnly { + fn f() {} +} + +impl SpecTrait> for () { + fn f() {} +} + +impl SpecTrait<(OrdOnly, OrdOnly)> for &[OrdOnly] { + fn f() {} +} + +fn main() {} diff --git a/src/test/ui/specialization/min_specialization/specialization_super_trait.rs b/src/test/ui/specialization/min_specialization/specialization_super_trait.rs new file mode 100644 index 0000000000000..145f376edf938 --- /dev/null +++ b/src/test/ui/specialization/min_specialization/specialization_super_trait.rs @@ -0,0 +1,18 @@ +// Test that supertraits can't be assumed in impls of +// `rustc_specialization_trait`, as such impls would +// allow specializing on the supertrait. + +#![feature(min_specialization)] +#![feature(rustc_attrs)] + +#[rustc_specialization_trait] +trait SpecMarker: Default { + fn f(); +} + +impl SpecMarker for T { + //~^ ERROR cannot specialize + fn f() {} +} + +fn main() {} diff --git a/src/test/ui/specialization/min_specialization/specialization_super_trait.stderr b/src/test/ui/specialization/min_specialization/specialization_super_trait.stderr new file mode 100644 index 0000000000000..154c839c6da6e --- /dev/null +++ b/src/test/ui/specialization/min_specialization/specialization_super_trait.stderr @@ -0,0 +1,11 @@ +error: cannot specialize on trait `std::default::Default` + --> $DIR/specialization_super_trait.rs:13:1 + | +LL | / impl SpecMarker for T { +LL | | +LL | | fn f() {} +LL | | } + | |_^ + +error: aborting due to previous error +