Skip to content

Commit ea45edf

Browse files
authored
Auto merge of #35745 - jroesch:soundness-fix-29859, r=nikomatsakis
Fix soundness bug described in #29859 This is an attempt at fixing the problems described in #29859 based on an IRC conversation between @nikomatsakis and I today. I'm waiting on a full build to come back, otherwise both tests trigger the correct error.
2 parents c615b21 + a254282 commit ea45edf

13 files changed

+151
-153
lines changed

src/librustc_typeck/check/wfcheck.rs

+72-4
Original file line numberDiff line numberDiff line change
@@ -256,16 +256,84 @@ impl<'ccx, 'gcx> CheckTypeWellFormedVisitor<'ccx, 'gcx> {
256256
});
257257
}
258258

259+
fn check_auto_trait(&mut self,
260+
trait_def_id: DefId,
261+
items: &[hir::TraitItem],
262+
span: Span)
263+
{
264+
// We want to ensure:
265+
//
266+
// 1) that there are no items contained within
267+
// the trait defintion
268+
//
269+
// 2) that the definition doesn't violate the no-super trait rule
270+
// for auto traits.
271+
//
272+
// 3) that the trait definition does not have any type parameters
273+
274+
let predicates = self.tcx().lookup_predicates(trait_def_id);
275+
276+
// We must exclude the Self : Trait predicate contained by all
277+
// traits.
278+
let has_predicates =
279+
predicates.predicates.iter().any(|predicate| {
280+
match predicate {
281+
&ty::Predicate::Trait(ref poly_trait_ref) => {
282+
let self_ty = poly_trait_ref.0.self_ty();
283+
!(self_ty.is_self() && poly_trait_ref.def_id() == trait_def_id)
284+
},
285+
_ => true,
286+
}
287+
});
288+
289+
let trait_def = self.tcx().lookup_trait_def(trait_def_id);
290+
291+
let has_ty_params =
292+
trait_def.generics
293+
.types
294+
.len() > 1;
295+
296+
// We use an if-else here, since the generics will also trigger
297+
// an extraneous error message when we find predicates like
298+
// `T : Sized` for a trait like: `trait Magic<T>`.
299+
//
300+
// We also put the check on the number of items here,
301+
// as it seems confusing to report an error about
302+
// extraneous predicates created by things like
303+
// an associated type inside the trait.
304+
let mut err = None;
305+
if !items.is_empty() {
306+
error_380(self.ccx, span);
307+
} else if has_ty_params {
308+
err = Some(struct_span_err!(self.tcx().sess, span, E0567,
309+
"traits with auto impls (`e.g. impl \
310+
Trait for ..`) can not have type parameters"));
311+
} else if has_predicates {
312+
err = Some(struct_span_err!(self.tcx().sess, span, E0568,
313+
"traits with auto impls (`e.g. impl \
314+
Trait for ..`) cannot have predicates"));
315+
}
316+
317+
// Finally if either of the above conditions apply we should add a note
318+
// indicating that this error is the result of a recent soundness fix.
319+
match err {
320+
None => {},
321+
Some(mut e) => {
322+
e.note("the new auto trait rules are the result of a \
323+
recent soundness fix; see #29859 for more details");
324+
e.emit();
325+
}
326+
}
327+
}
328+
259329
fn check_trait(&mut self,
260330
item: &hir::Item,
261331
items: &[hir::TraitItem])
262332
{
263333
let trait_def_id = self.tcx().map.local_def_id(item.id);
264334

265335
if self.tcx().trait_has_default_impl(trait_def_id) {
266-
if !items.is_empty() {
267-
error_380(self.ccx, item.span);
268-
}
336+
self.check_auto_trait(trait_def_id, items, item.span);
269337
}
270338

271339
self.for_item(item).with_fcx(|fcx, this| {
@@ -626,7 +694,7 @@ fn error_192(ccx: &CrateCtxt, span: Span) {
626694

627695
fn error_380(ccx: &CrateCtxt, span: Span) {
628696
span_err!(ccx.tcx.sess, span, E0380,
629-
"traits with default impls (`e.g. unsafe impl \
697+
"traits with default impls (`e.g. impl \
630698
Trait for ..`) must have no methods or associated items")
631699
}
632700

src/librustc_typeck/diagnostics.rs

+2
Original file line numberDiff line numberDiff line change
@@ -4072,4 +4072,6 @@ register_diagnostics! {
40724072
E0563, // cannot determine a type for this `impl Trait`: {}
40734073
E0564, // only named lifetimes are allowed in `impl Trait`,
40744074
// but `{}` was found in the type `{}`
4075+
E0567, // auto traits can not have type parameters
4076+
E0568, // auto-traits can not have predicates,
40754077
}

src/test/compile-fail/issue-23080-2.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,7 @@
1313
#![feature(optin_builtin_traits)]
1414

1515
unsafe trait Trait {
16-
//~^ error: traits with default impls (`e.g. unsafe impl Trait for ..`) must have no methods or associated items
16+
//~^ ERROR E0380
1717
type Output;
1818
}
1919

src/test/compile-fail/issue-23080.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,7 @@
1313
#![feature(optin_builtin_traits)]
1414

1515
unsafe trait Trait {
16-
//~^ error: traits with default impls (`e.g. unsafe impl Trait for ..`) must have no methods or associated items
16+
//~^ ERROR E0380
1717
fn method(&self) {
1818
println!("Hello");
1919
}

src/test/compile-fail/traits-inductive-overflow-auto-normal-auto.rs

-32
This file was deleted.

src/test/compile-fail/traits-inductive-overflow-supertrait-oibit.rs

+2-2
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,7 @@
1414

1515
#![feature(optin_builtin_traits)]
1616

17-
trait Magic: Copy {}
17+
trait Magic: Copy {} //~ ERROR E0568
1818
impl Magic for .. {}
1919

2020
fn copy<T: Magic>(x: T) -> (T, T) { (x, x) }
@@ -23,6 +23,6 @@ fn copy<T: Magic>(x: T) -> (T, T) { (x, x) }
2323
struct NoClone;
2424

2525
fn main() {
26-
let (a, b) = copy(NoClone); //~ ERROR E0277
26+
let (a, b) = copy(NoClone);
2727
println!("{:?} {:?}", a, b);
2828
}
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
// Copyright 2015 The Rust Project Developers. See the COPYRIGHT
1+
// Copyright 2016 The Rust Project Developers. See the COPYRIGHT
22
// file at the top-level directory of this distribution and at
33
// http://rust-lang.org/COPYRIGHT.
44
//
@@ -8,20 +8,18 @@
88
// option. This file may not be copied, modified, or distributed
99
// except according to those terms.
1010

11-
// Test that when a `..` impl applies, we also check that any
12-
// supertrait conditions are met.
13-
1411
#![feature(optin_builtin_traits)]
1512

16-
trait MyTrait : 'static {}
17-
18-
impl MyTrait for .. {}
13+
trait Magic : Sized where Option<Self> : Magic {} //~ ERROR E0568
14+
impl Magic for .. {}
15+
impl<T:Magic> Magic for T {}
1916

20-
fn foo<T:MyTrait>() { }
17+
fn copy<T: Magic>(x: T) -> (T, T) { (x, x) }
2118

22-
fn bar<'a>() {
23-
foo::<&'a ()>(); //~ ERROR does not fulfill the required lifetime
24-
}
19+
#[derive(Debug)]
20+
struct NoClone;
2521

2622
fn main() {
23+
let (a, b) = copy(NoClone);
24+
println!("{:?} {:?}", a, b);
2725
}
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,49 @@
1+
// Copyright 2016 The Rust Project Developers. See the COPYRIGHT
2+
// file at the top-level directory of this distribution and at
3+
// http://rust-lang.org/COPYRIGHT.
4+
//
5+
// Licensed under the Apache License, Version 2.0 <LICENSE-APACHE or
6+
// http://www.apache.org/licenses/LICENSE-2.0> or the MIT license
7+
// <LICENSE-MIT or http://opensource.org/licenses/MIT>, at your
8+
// option. This file may not be copied, modified, or distributed
9+
// except according to those terms.
10+
11+
// This test is for #29859, we need to ensure auto traits,
12+
// (also known previously as default traits), do not have
13+
// supertraits. Since the compiler synthesizes these
14+
// instances on demand, we are essentially enabling
15+
// users to write axioms if we view trait selection,
16+
// as a proof system.
17+
//
18+
// For example the below test allows us to add the rule:
19+
// forall (T : Type), T : Copy
20+
//
21+
// Providing a copy instance for *any* type, which
22+
// is most definitely unsound. Imagine copying a
23+
// type that contains a mutable reference, enabling
24+
// mutable aliasing.
25+
//
26+
// You can imagine an even more dangerous test,
27+
// which currently compiles on nightly.
28+
//
29+
// fn main() {
30+
// let mut i = 10;
31+
// let (a, b) = copy(&mut i);
32+
// println!("{:?} {:?}", a, b);
33+
// }
34+
35+
#![feature(optin_builtin_traits)]
36+
37+
trait Magic: Copy {} //~ ERROR E0568
38+
impl Magic for .. {}
39+
impl<T:Magic> Magic for T {}
40+
41+
fn copy<T: Magic>(x: T) -> (T, T) { (x, x) }
42+
43+
#[derive(Debug)]
44+
struct NoClone;
45+
46+
fn main() {
47+
let (a, b) = copy(NoClone);
48+
println!("{:?} {:?}", a, b);
49+
}
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,14 @@
1+
// Copyright 2016 The Rust Project Developers. See the COPYRIGHT
2+
// file at the top-level directory of this distribution and at
3+
// http://rust-lang.org/COPYRIGHT.
4+
//
5+
// Licensed under the Apache License, Version 2.0 <LICENSE-APACHE or
6+
// http://www.apache.org/licenses/LICENSE-2.0> or the MIT license
7+
// <LICENSE-MIT or http://opensource.org/licenses/MIT>, at your
8+
// option. This file may not be copied, modified, or distributed
9+
// except according to those terms.
10+
11+
#![feature(optin_builtin_traits)]
12+
13+
trait Magic<T> {} //~ ERROR E0567
14+
impl Magic<isize> for .. {}

src/test/compile-fail/typeck-default-trait-impl-supertrait.rs

-29
This file was deleted.

src/test/compile-fail/typeck-default-trait-impl-trait-where-clause-2.rs

-36
This file was deleted.

src/test/compile-fail/typeck-default-trait-impl-trait-where-clause.rs

-36
This file was deleted.

src/test/rustdoc/auxiliary/rustdoc-default-impl.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,7 @@
1414
pub mod bar {
1515
use std::marker;
1616

17-
pub trait Bar: 'static {}
17+
pub trait Bar {}
1818

1919
impl Bar for .. {}
2020

0 commit comments

Comments
 (0)