Skip to content

Commit e24252a

Browse files
committed
Auto merge of #68970 - matthewjasper:min-spec, r=nikomatsakis
Implement a feature for a sound specialization subset This implements a new feature (`min_specialization`) that restricts specialization to a subset that is reasonable for the standard library to use. The plan is to then: * Update `libcore` and `liballoc` to compile with `min_specialization`. * Add a lint to forbid use of `feature(specialization)` (and other unsound, type system extending features) in the standard library. * Fix the soundness issues around `specialization`. * Remove `min_specialization` The rest of this is an overview from a comment in this PR ## 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<T> SpecExtend<T> for std::vec::IntoIter<T> { /* specialized impl */ } impl<T, I: Iterator<Item=T>> SpecExtend<T> for I { /* default impl */ } ``` We get that the subst for `impl2` are `[T, std::vec::IntoIter<T>]`. `T` is constrained to be `<I as Iterator>::Item`, so we check only `std::vec::IntoIter<T>` 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. ### 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_specialization_marker There are also some specialization on traits with no methods, including the `FusedIterator` trait which is advertised as allowing optimizations. We allow marking marker traits with an unstable attribute that means we ignore them in point 3 of the checks above. This is unsound but we allow it in the short term because it can't cause use after frees with purely safe code in the same way as specializing on traits methods can. r? @nikomatsakis cc #31844 #67194
2 parents dd67187 + 39ee66a commit e24252a

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

48 files changed

+1095
-73
lines changed

src/libcore/marker.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -90,6 +90,7 @@ impl<T: ?Sized> !Send for *mut T {}
9090
ch19-04-advanced-types.html#dynamically-sized-types-and-the-sized-trait>"
9191
)]
9292
#[fundamental] // for Default, for example, which requires that `[T]: !Default` be evaluatable
93+
#[cfg_attr(not(bootstrap), rustc_specialization_trait)]
9394
pub trait Sized {
9495
// Empty.
9596
}

src/libproc_macro/lib.rs

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,8 @@
2626
#![feature(in_band_lifetimes)]
2727
#![feature(optin_builtin_traits)]
2828
#![feature(rustc_attrs)]
29-
#![feature(specialization)]
29+
#![cfg_attr(bootstrap, feature(specialization))]
30+
#![cfg_attr(not(bootstrap), feature(min_specialization))]
3031
#![recursion_limit = "256"]
3132

3233
#[unstable(feature = "proc_macro_internals", issue = "27812")]

src/librustc/traits/specialization_graph.rs

Lines changed: 19 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@ use crate::ty::{self, TyCtxt};
44
use rustc_ast::ast::Ident;
55
use rustc_data_structures::fx::FxHashMap;
66
use rustc_data_structures::stable_hasher::{HashStable, StableHasher};
7+
use rustc_errors::ErrorReported;
78
use rustc_hir::def_id::{DefId, DefIdMap};
89

910
/// A per-trait graph of impls in specialization order. At the moment, this
@@ -23,17 +24,20 @@ use rustc_hir::def_id::{DefId, DefIdMap};
2324
/// has at most one parent.
2425
#[derive(RustcEncodable, RustcDecodable, HashStable)]
2526
pub struct Graph {
26-
// All impls have a parent; the "root" impls have as their parent the `def_id`
27-
// of the trait.
27+
/// All impls have a parent; the "root" impls have as their parent the `def_id`
28+
/// of the trait.
2829
pub parent: DefIdMap<DefId>,
2930

30-
// The "root" impls are found by looking up the trait's def_id.
31+
/// The "root" impls are found by looking up the trait's def_id.
3132
pub children: DefIdMap<Children>,
33+
34+
/// Whether an error was emitted while constructing the graph.
35+
pub has_errored: bool,
3236
}
3337

3438
impl Graph {
3539
pub fn new() -> Graph {
36-
Graph { parent: Default::default(), children: Default::default() }
40+
Graph { parent: Default::default(), children: Default::default(), has_errored: false }
3741
}
3842

3943
/// The parent of a given impl, which is the `DefId` of the trait when the
@@ -179,17 +183,22 @@ impl<'tcx> Ancestors<'tcx> {
179183
}
180184

181185
/// Walk up the specialization ancestors of a given impl, starting with that
182-
/// impl itself.
186+
/// impl itself. Returns `None` if an error was reported while building the
187+
/// specialization graph.
183188
pub fn ancestors(
184189
tcx: TyCtxt<'tcx>,
185190
trait_def_id: DefId,
186191
start_from_impl: DefId,
187-
) -> Ancestors<'tcx> {
192+
) -> Result<Ancestors<'tcx>, ErrorReported> {
188193
let specialization_graph = tcx.specialization_graph_of(trait_def_id);
189-
Ancestors {
190-
trait_def_id,
191-
specialization_graph,
192-
current_source: Some(Node::Impl(start_from_impl)),
194+
if specialization_graph.has_errored {
195+
Err(ErrorReported)
196+
} else {
197+
Ok(Ancestors {
198+
trait_def_id,
199+
specialization_graph,
200+
current_source: Some(Node::Impl(start_from_impl)),
201+
})
193202
}
194203
}
195204

src/librustc/ty/trait_def.rs

Lines changed: 34 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@ use rustc_hir::HirId;
1010

1111
use rustc_data_structures::fx::FxHashMap;
1212
use rustc_data_structures::stable_hasher::{HashStable, StableHasher};
13+
use rustc_errors::ErrorReported;
1314
use rustc_macros::HashStable;
1415
use std::collections::BTreeMap;
1516

@@ -35,11 +36,33 @@ pub struct TraitDef {
3536
/// and thus `impl`s of it are allowed to overlap.
3637
pub is_marker: bool,
3738

39+
/// Used to determine whether the standard library is allowed to specialize
40+
/// on this trait.
41+
pub specialization_kind: TraitSpecializationKind,
42+
3843
/// The ICH of this trait's DefPath, cached here so it doesn't have to be
3944
/// recomputed all the time.
4045
pub def_path_hash: DefPathHash,
4146
}
4247

48+
/// Whether this trait is treated specially by the standard library
49+
/// specialization lint.
50+
#[derive(HashStable, PartialEq, Clone, Copy, RustcEncodable, RustcDecodable)]
51+
pub enum TraitSpecializationKind {
52+
/// The default. Specializing on this trait is not allowed.
53+
None,
54+
/// Specializing on this trait is allowed because it doesn't have any
55+
/// methods. For example `Sized` or `FusedIterator`.
56+
/// Applies to traits with the `rustc_unsafe_specialization_marker`
57+
/// attribute.
58+
Marker,
59+
/// Specializing on this trait is allowed because all of the impls of this
60+
/// trait are "always applicable". Always applicable means that if
61+
/// `X<'x>: T<'y>` for any lifetimes, then `for<'a, 'b> X<'a>: T<'b>`.
62+
/// Applies to traits with the `rustc_specialization_trait` attribute.
63+
AlwaysApplicable,
64+
}
65+
4366
#[derive(Default)]
4467
pub struct TraitImpls {
4568
blanket_impls: Vec<DefId>,
@@ -54,16 +77,25 @@ impl<'tcx> TraitDef {
5477
paren_sugar: bool,
5578
has_auto_impl: bool,
5679
is_marker: bool,
80+
specialization_kind: TraitSpecializationKind,
5781
def_path_hash: DefPathHash,
5882
) -> TraitDef {
59-
TraitDef { def_id, unsafety, paren_sugar, has_auto_impl, is_marker, def_path_hash }
83+
TraitDef {
84+
def_id,
85+
unsafety,
86+
paren_sugar,
87+
has_auto_impl,
88+
is_marker,
89+
specialization_kind,
90+
def_path_hash,
91+
}
6092
}
6193

6294
pub fn ancestors(
6395
&self,
6496
tcx: TyCtxt<'tcx>,
6597
of_impl: DefId,
66-
) -> specialization_graph::Ancestors<'tcx> {
98+
) -> Result<specialization_graph::Ancestors<'tcx>, ErrorReported> {
6799
specialization_graph::ancestors(tcx, self.def_id, of_impl)
68100
}
69101
}

src/librustc_ast_passes/feature_gate.rs

Lines changed: 14 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -542,15 +542,12 @@ impl<'a> Visitor<'a> for PostExpansionVisitor<'a> {
542542
}
543543

544544
fn visit_assoc_item(&mut self, i: &'a ast::AssocItem, ctxt: AssocCtxt) {
545-
if let ast::Defaultness::Default(_) = i.kind.defaultness() {
546-
gate_feature_post!(&self, specialization, i.span, "specialization is unstable");
547-
}
548-
549-
match i.kind {
545+
let is_fn = match i.kind {
550546
ast::AssocItemKind::Fn(_, ref sig, _, _) => {
551547
if let (ast::Const::Yes(_), AssocCtxt::Trait) = (sig.header.constness, ctxt) {
552548
gate_feature_post!(&self, const_fn, i.span, "const fn is unstable");
553549
}
550+
true
554551
}
555552
ast::AssocItemKind::TyAlias(_, ref generics, _, ref ty) => {
556553
if let (Some(_), AssocCtxt::Trait) = (ty, ctxt) {
@@ -565,8 +562,19 @@ impl<'a> Visitor<'a> for PostExpansionVisitor<'a> {
565562
self.check_impl_trait(ty);
566563
}
567564
self.check_gat(generics, i.span);
565+
false
568566
}
569-
_ => {}
567+
_ => false,
568+
};
569+
if let ast::Defaultness::Default(_) = i.kind.defaultness() {
570+
// Limit `min_specialization` to only specializing functions.
571+
gate_feature_fn!(
572+
&self,
573+
|x: &Features| x.specialization || (is_fn && x.min_specialization),
574+
i.span,
575+
sym::specialization,
576+
"specialization is unstable"
577+
);
570578
}
571579
visit::walk_assoc_item(self, i, ctxt)
572580
}

src/librustc_feature/active.rs

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -301,6 +301,11 @@ declare_features! (
301301
/// Allows specialization of implementations (RFC 1210).
302302
(active, specialization, "1.7.0", Some(31844), None),
303303

304+
/// A minimal, sound subset of specialization intended to be used by the
305+
/// standard library until the soundness issues with specialization
306+
/// are fixed.
307+
(active, min_specialization, "1.7.0", Some(31844), None),
308+
304309
/// Allows using `#[naked]` on functions.
305310
(active, naked_functions, "1.9.0", Some(32408), None),
306311

src/librustc_feature/builtin_attrs.rs

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -530,6 +530,14 @@ pub const BUILTIN_ATTRIBUTES: &[BuiltinAttribute] = &[
530530
rustc_test_marker, Normal, template!(Word),
531531
"the `#[rustc_test_marker]` attribute is used internally to track tests",
532532
),
533+
rustc_attr!(
534+
rustc_unsafe_specialization_marker, Normal, template!(Word),
535+
"the `#[rustc_unsafe_specialization_marker]` attribute is used to check specializations"
536+
),
537+
rustc_attr!(
538+
rustc_specialization_trait, Normal, template!(Word),
539+
"the `#[rustc_specialization_trait]` attribute is used to check specializations"
540+
),
533541

534542
// ==========================================================================
535543
// Internal attributes, Testing:

src/librustc_metadata/rmeta/decoder.rs

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -651,6 +651,7 @@ impl<'a, 'tcx> CrateMetadataRef<'a> {
651651
data.paren_sugar,
652652
data.has_auto_impl,
653653
data.is_marker,
654+
data.specialization_kind,
654655
self.def_path_table.def_path_hash(item_id),
655656
)
656657
}
@@ -660,6 +661,7 @@ impl<'a, 'tcx> CrateMetadataRef<'a> {
660661
false,
661662
false,
662663
false,
664+
ty::trait_def::TraitSpecializationKind::None,
663665
self.def_path_table.def_path_hash(item_id),
664666
),
665667
_ => bug!("def-index does not refer to trait or trait alias"),

src/librustc_metadata/rmeta/encoder.rs

Lines changed: 8 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1077,12 +1077,13 @@ impl EncodeContext<'tcx> {
10771077
let polarity = self.tcx.impl_polarity(def_id);
10781078
let parent = if let Some(trait_ref) = trait_ref {
10791079
let trait_def = self.tcx.trait_def(trait_ref.def_id);
1080-
trait_def.ancestors(self.tcx, def_id).nth(1).and_then(|node| {
1081-
match node {
1082-
specialization_graph::Node::Impl(parent) => Some(parent),
1083-
_ => None,
1084-
}
1085-
})
1080+
trait_def.ancestors(self.tcx, def_id).ok()
1081+
.and_then(|mut an| an.nth(1).and_then(|node| {
1082+
match node {
1083+
specialization_graph::Node::Impl(parent) => Some(parent),
1084+
_ => None,
1085+
}
1086+
}))
10861087
} else {
10871088
None
10881089
};
@@ -1114,6 +1115,7 @@ impl EncodeContext<'tcx> {
11141115
paren_sugar: trait_def.paren_sugar,
11151116
has_auto_impl: self.tcx.trait_is_auto(def_id),
11161117
is_marker: trait_def.is_marker,
1118+
specialization_kind: trait_def.specialization_kind,
11171119
};
11181120

11191121
EntryKind::Trait(self.lazy(data))

src/librustc_metadata/rmeta/mod.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -343,6 +343,7 @@ struct TraitData {
343343
paren_sugar: bool,
344344
has_auto_impl: bool,
345345
is_marker: bool,
346+
specialization_kind: ty::trait_def::TraitSpecializationKind,
346347
}
347348

348349
#[derive(RustcEncodable, RustcDecodable)]

0 commit comments

Comments
 (0)