Skip to content

Commit

Permalink
Auto merge of #35745 - jroesch:soundness-fix-29859, r=nikomatsakis
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
bors authored Aug 29, 2016
2 parents 86dde9b + dba5cbe commit b58dddd
Show file tree
Hide file tree
Showing 13 changed files with 151 additions and 153 deletions.
76 changes: 72 additions & 4 deletions src/librustc_typeck/check/wfcheck.rs
Original file line number Diff line number Diff line change
Expand Up @@ -251,16 +251,84 @@ 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, E0566,
"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, E0565,
"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])
{
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| {
Expand Down Expand Up @@ -618,7 +686,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")
}

Expand Down
2 changes: 2 additions & 0 deletions src/librustc_typeck/diagnostics.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4079,4 +4079,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 `{}`
E0565, // auto-traits can not have predicates,
E0566, // auto traits can not have type parameters
}
2 changes: 1 addition & 1 deletion src/test/compile-fail/issue-23080-2.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}

Expand Down
2 changes: 1 addition & 1 deletion src/test/compile-fail/issue-23080.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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");
}
Expand Down

This file was deleted.

Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@

#![feature(optin_builtin_traits)]

trait Magic: Copy {}
trait Magic: Copy {} //~ ERROR E0565
impl Magic for .. {}

fn copy<T: Magic>(x: T) -> (T, T) { (x, x) }
Expand All @@ -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);
}
Original file line number Diff line number Diff line change
@@ -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.
//
Expand All @@ -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 {}

impl MyTrait for .. {}
trait Magic : Sized where Option<Self> : Magic {} //~ ERROR E0565
impl Magic for .. {}
impl<T:Magic> Magic for T {}

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

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);
}
49 changes: 49 additions & 0 deletions src/test/compile-fail/typeck-auto-trait-no-supertraits.rs
Original file line number Diff line number Diff line change
@@ -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 E0565
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);
}
14 changes: 14 additions & 0 deletions src/test/compile-fail/typeck-auto-trait-no-typeparams.rs
Original file line number Diff line number Diff line change
@@ -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 E0566
impl Magic<isize> for .. {}
29 changes: 0 additions & 29 deletions src/test/compile-fail/typeck-default-trait-impl-supertrait.rs

This file was deleted.

This file was deleted.

This file was deleted.

2 changes: 1 addition & 1 deletion src/test/rustdoc/auxiliary/rustdoc-default-impl.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@
pub mod bar {
use std::marker;

pub trait Bar: 'static {}
pub trait Bar {}

impl Bar for .. {}

Expand Down

0 comments on commit b58dddd

Please sign in to comment.