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.
This commit is contained in:
bors 2016-09-08 10:40:31 -07:00 committed by GitHub
commit ea45edf0ee
13 changed files with 151 additions and 153 deletions

View File

@ -256,6 +256,76 @@ impl<'ccx, 'gcx> CheckTypeWellFormedVisitor<'ccx, 'gcx> {
});
}
fn check_auto_trait(&mut self,
trait_def_id: DefId,
items: &[hir::TraitItem],
span: Span)
{
// We want to ensure:
//
// 1) that there are no items contained within
// the trait defintion
//
// 2) that the definition doesn't violate the no-super trait rule
// for auto traits.
//
// 3) that the trait definition does not have any type parameters
let predicates = self.tcx().lookup_predicates(trait_def_id);
// We must exclude the Self : Trait predicate contained by all
// traits.
let has_predicates =
predicates.predicates.iter().any(|predicate| {
match predicate {
&ty::Predicate::Trait(ref poly_trait_ref) => {
let self_ty = poly_trait_ref.0.self_ty();
!(self_ty.is_self() && poly_trait_ref.def_id() == trait_def_id)
},
_ => true,
}
});
let trait_def = self.tcx().lookup_trait_def(trait_def_id);
let has_ty_params =
trait_def.generics
.types
.len() > 1;
// We use an if-else here, since the generics will also trigger
// an extraneous error message when we find predicates like
// `T : Sized` for a trait like: `trait Magic<T>`.
//
// We also put the check on the number of items here,
// as it seems confusing to report an error about
// extraneous predicates created by things like
// an associated type inside the trait.
let mut err = None;
if !items.is_empty() {
error_380(self.ccx, span);
} else if has_ty_params {
err = Some(struct_span_err!(self.tcx().sess, span, E0567,
"traits with auto impls (`e.g. impl \
Trait for ..`) can not have type parameters"));
} else if has_predicates {
err = Some(struct_span_err!(self.tcx().sess, span, E0568,
"traits with auto impls (`e.g. impl \
Trait for ..`) cannot have predicates"));
}
// Finally if either of the above conditions apply we should add a note
// indicating that this error is the result of a recent soundness fix.
match err {
None => {},
Some(mut e) => {
e.note("the new auto trait rules are the result of a \
recent soundness fix; see #29859 for more details");
e.emit();
}
}
}
fn check_trait(&mut self,
item: &hir::Item,
items: &[hir::TraitItem])
@ -263,9 +333,7 @@ impl<'ccx, 'gcx> CheckTypeWellFormedVisitor<'ccx, 'gcx> {
let trait_def_id = self.tcx().map.local_def_id(item.id);
if self.tcx().trait_has_default_impl(trait_def_id) {
if !items.is_empty() {
error_380(self.ccx, item.span);
}
self.check_auto_trait(trait_def_id, items, item.span);
}
self.for_item(item).with_fcx(|fcx, this| {
@ -626,7 +694,7 @@ fn error_192(ccx: &CrateCtxt, span: Span) {
fn error_380(ccx: &CrateCtxt, span: Span) {
span_err!(ccx.tcx.sess, span, E0380,
"traits with default impls (`e.g. unsafe impl \
"traits with default impls (`e.g. impl \
Trait for ..`) must have no methods or associated items")
}

View File

@ -4072,4 +4072,6 @@ register_diagnostics! {
E0563, // cannot determine a type for this `impl Trait`: {}
E0564, // only named lifetimes are allowed in `impl Trait`,
// but `{}` was found in the type `{}`
E0567, // auto traits can not have type parameters
E0568, // auto-traits can not have predicates,
}

View File

@ -13,7 +13,7 @@
#![feature(optin_builtin_traits)]
unsafe trait Trait {
//~^ error: traits with default impls (`e.g. unsafe impl Trait for ..`) must have no methods or associated items
//~^ ERROR E0380
type Output;
}

View File

@ -13,7 +13,7 @@
#![feature(optin_builtin_traits)]
unsafe trait Trait {
//~^ error: traits with default impls (`e.g. unsafe impl Trait for ..`) must have no methods or associated items
//~^ ERROR E0380
fn method(&self) {
println!("Hello");
}

View File

@ -1,32 +0,0 @@
// Copyright 2015 The Rust Project Developers. See the COPYRIGHT
// file at the top-level directory of this distribution and at
// http://rust-lang.org/COPYRIGHT.
//
// Licensed under the Apache License, Version 2.0 <LICENSE-APACHE or
// http://www.apache.org/licenses/LICENSE-2.0> or the MIT license
// <LICENSE-MIT or http://opensource.org/licenses/MIT>, at your
// option. This file may not be copied, modified, or distributed
// except according to those terms.
// Test for a potential corner case in current impl where you have an
// auto trait (Magic1) that depends on a normal trait (Magic2) which
// in turn depends on the auto trait (Magic1). This was incorrectly
// being considered coinductive, but because of the normal trait
// interfering, it should not be.
#![feature(optin_builtin_traits)]
trait Magic1: Magic2 { }
impl Magic1 for .. {}
trait Magic2 { }
impl<T: Magic1> Magic2 for T { }
fn is_magic1<T: Magic1>() { }
#[derive(Debug)]
struct NoClone;
fn main() {
is_magic1::<NoClone>(); //~ ERROR E0275
}

View File

@ -14,7 +14,7 @@
#![feature(optin_builtin_traits)]
trait Magic: Copy {}
trait Magic: Copy {} //~ ERROR E0568
impl Magic for .. {}
fn copy<T: Magic>(x: T) -> (T, T) { (x, x) }
@ -23,6 +23,6 @@ fn copy<T: Magic>(x: T) -> (T, T) { (x, x) }
struct NoClone;
fn main() {
let (a, b) = copy(NoClone); //~ ERROR E0277
let (a, b) = copy(NoClone);
println!("{:?} {:?}", a, b);
}

View File

@ -1,4 +1,4 @@
// Copyright 2015 The Rust Project Developers. See the COPYRIGHT
// Copyright 2016 The Rust Project Developers. See the COPYRIGHT
// file at the top-level directory of this distribution and at
// http://rust-lang.org/COPYRIGHT.
//
@ -8,20 +8,18 @@
// option. This file may not be copied, modified, or distributed
// except according to those terms.
// Test that when a `..` impl applies, we also check that any
// supertrait conditions are met.
#![feature(optin_builtin_traits)]
trait MyTrait : 'static {}
trait Magic : Sized where Option<Self> : Magic {} //~ ERROR E0568
impl Magic for .. {}
impl<T:Magic> Magic for T {}
impl MyTrait for .. {}
fn copy<T: Magic>(x: T) -> (T, T) { (x, x) }
fn foo<T:MyTrait>() { }
fn bar<'a>() {
foo::<&'a ()>(); //~ ERROR does not fulfill the required lifetime
}
#[derive(Debug)]
struct NoClone;
fn main() {
let (a, b) = copy(NoClone);
println!("{:?} {:?}", a, b);
}

View File

@ -0,0 +1,49 @@
// Copyright 2016 The Rust Project Developers. See the COPYRIGHT
// file at the top-level directory of this distribution and at
// http://rust-lang.org/COPYRIGHT.
//
// Licensed under the Apache License, Version 2.0 <LICENSE-APACHE or
// http://www.apache.org/licenses/LICENSE-2.0> or the MIT license
// <LICENSE-MIT or http://opensource.org/licenses/MIT>, at your
// option. This file may not be copied, modified, or distributed
// except according to those terms.
// This test is for #29859, we need to ensure auto traits,
// (also known previously as default traits), do not have
// supertraits. Since the compiler synthesizes these
// instances on demand, we are essentially enabling
// users to write axioms if we view trait selection,
// as a proof system.
//
// For example the below test allows us to add the rule:
// forall (T : Type), T : Copy
//
// Providing a copy instance for *any* type, which
// is most definitely unsound. Imagine copying a
// type that contains a mutable reference, enabling
// mutable aliasing.
//
// You can imagine an even more dangerous test,
// which currently compiles on nightly.
//
// fn main() {
// let mut i = 10;
// let (a, b) = copy(&mut i);
// println!("{:?} {:?}", a, b);
// }
#![feature(optin_builtin_traits)]
trait Magic: Copy {} //~ ERROR E0568
impl Magic for .. {}
impl<T:Magic> Magic for T {}
fn copy<T: Magic>(x: T) -> (T, T) { (x, x) }
#[derive(Debug)]
struct NoClone;
fn main() {
let (a, b) = copy(NoClone);
println!("{:?} {:?}", a, b);
}

View File

@ -0,0 +1,14 @@
// Copyright 2016 The Rust Project Developers. See the COPYRIGHT
// file at the top-level directory of this distribution and at
// http://rust-lang.org/COPYRIGHT.
//
// Licensed under the Apache License, Version 2.0 <LICENSE-APACHE or
// http://www.apache.org/licenses/LICENSE-2.0> or the MIT license
// <LICENSE-MIT or http://opensource.org/licenses/MIT>, at your
// option. This file may not be copied, modified, or distributed
// except according to those terms.
#![feature(optin_builtin_traits)]
trait Magic<T> {} //~ ERROR E0567
impl Magic<isize> for .. {}

View File

@ -1,29 +0,0 @@
// Copyright 2015 The Rust Project Developers. See the COPYRIGHT
// file at the top-level directory of this distribution and at
// http://rust-lang.org/COPYRIGHT.
//
// Licensed under the Apache License, Version 2.0 <LICENSE-APACHE or
// http://www.apache.org/licenses/LICENSE-2.0> or the MIT license
// <LICENSE-MIT or http://opensource.org/licenses/MIT>, at your
// option. This file may not be copied, modified, or distributed
// except according to those terms.
// Test that when a `..` impl applies, we also check that any
// supertrait conditions are met.
#![feature(optin_builtin_traits)]
trait NotImplemented { }
trait MyTrait : NotImplemented {}
impl MyTrait for .. {}
fn foo<T:MyTrait>() { bar::<T>() }
fn bar<T:NotImplemented>() { }
fn main() {
foo::<i32>(); //~ ERROR `i32: NotImplemented` is not satisfied
bar::<i64>(); //~ ERROR `i64: NotImplemented` is not satisfied
}

View File

@ -1,36 +0,0 @@
// Copyright 2015 The Rust Project Developers. See the COPYRIGHT
// file at the top-level directory of this distribution and at
// http://rust-lang.org/COPYRIGHT.
//
// Licensed under the Apache License, Version 2.0 <LICENSE-APACHE or
// http://www.apache.org/licenses/LICENSE-2.0> or the MIT license
// <LICENSE-MIT or http://opensource.org/licenses/MIT>, at your
// option. This file may not be copied, modified, or distributed
// except according to those terms.
// ignore-tidy-linelength
// Test that when a `..` impl applies, we also check that any
// supertrait conditions are met.
#![feature(optin_builtin_traits)]
trait NotImplemented { }
trait MyTrait: Sized
where Option<Self> : NotImplemented
{}
impl NotImplemented for i32 {}
impl MyTrait for .. {}
fn bar<T:NotImplemented>() { }
fn test() {
bar::<Option<i32>>();
//~^ ERROR `std::option::Option<i32>: NotImplemented` is not satisfied
}
fn main() {
}

View File

@ -1,36 +0,0 @@
// Copyright 2015 The Rust Project Developers. See the COPYRIGHT
// file at the top-level directory of this distribution and at
// http://rust-lang.org/COPYRIGHT.
//
// Licensed under the Apache License, Version 2.0 <LICENSE-APACHE or
// http://www.apache.org/licenses/LICENSE-2.0> or the MIT license
// <LICENSE-MIT or http://opensource.org/licenses/MIT>, at your
// option. This file may not be copied, modified, or distributed
// except according to those terms.
// ignore-tidy-linelength
// Test that when a `..` impl applies, we also check that any
// supertrait conditions are met.
#![feature(optin_builtin_traits)]
trait NotImplemented { }
trait MyTrait: Sized
where Option<Self> : NotImplemented
{}
impl NotImplemented for i32 {}
impl MyTrait for .. {}
fn foo<T:MyTrait>() {
//~^ ERROR `std::option::Option<T>: NotImplemented` is not satisfied
// This should probably typecheck. This is #20671.
}
fn bar<T:NotImplemented>() { }
fn main() {
}

View File

@ -14,7 +14,7 @@
pub mod bar {
use std::marker;
pub trait Bar: 'static {}
pub trait Bar {}
impl Bar for .. {}