Skip to content

Commit

Permalink
Address feedback, and remove invalid tests
Browse files Browse the repository at this point in the history
  • Loading branch information
jroesch committed Sep 2, 2016
1 parent 352fac9 commit 90d1a53
Show file tree
Hide file tree
Showing 11 changed files with 108 additions and 214 deletions.
10 changes: 0 additions & 10 deletions src/librustc/ty/mod.rs
Expand Up @@ -762,16 +762,6 @@ pub struct GenericPredicates<'tcx> {
}

impl<'a, 'gcx, 'tcx> GenericPredicates<'tcx> {
pub fn empty() -> GenericPredicates<'tcx> {
GenericPredicates {
predicates: VecPerParamSpace::empty(),
}
}

pub fn is_empty(&self) -> bool {
self.predicates.is_empty()
}

pub fn instantiate(&self, tcx: TyCtxt<'a, 'gcx, 'tcx>, substs: &Substs<'tcx>)
-> InstantiatedPredicates<'tcx> {
let mut instantiated = InstantiatedPredicates::empty();
Expand Down
95 changes: 55 additions & 40 deletions src/librustc_typeck/check/wfcheck.rs
Expand Up @@ -252,33 +252,71 @@ impl<'ccx, 'gcx> CheckTypeWellFormedVisitor<'ccx, 'gcx> {
}

fn check_auto_trait(&mut self,
trait_def_id: DefId,
span: Span)
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);

// If we must exclude the Self : Trait predicate contained by all
// We must exclude the Self : Trait predicate contained by all
// traits.
let no_refl_predicates : Vec<_> =
predicates.predicates.iter().filter(|predicate| {
match *predicate {
&ty::Predicate::Trait(ref poly_trait_ref) =>
poly_trait_ref.def_id() != trait_def_id,
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,
}
}).collect();
}
});

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>`.
if !trait_def.generics.types.get_slice(ParamSpace::TypeSpace).is_empty() {
error_566(self.ccx, span);
} else if !no_refl_predicates.is_empty() {
error_565(self.ccx, span);
//
// 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.

if !items.is_empty() {
error_380(self.ccx, span);
} else if has_ty_params {
span_err!(self.tcx().sess, span, E0566,
"traits with auto impls (`e.g. unsafe impl \
Trait for ..`) can not have type parameters")
} else if has_predicates {
span_err!(self.tcx().sess, span, E0565,
"traits with auto impls (`e.g. unsafe impl \
Trait for ..`) can not 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.
if has_ty_params || has_predicates {
self.tcx().sess.span_note_without_error(
span,
"the new auto trait rules are the result of a \
recent soundness fix; see #29859 for more details")
}
}

fn check_trait(&mut self,
Expand All @@ -287,19 +325,10 @@ impl<'ccx, 'gcx> CheckTypeWellFormedVisitor<'ccx, 'gcx> {
{
let trait_def_id = self.tcx().map.local_def_id(item.id);

// TODO: in a second pass, globally rename to auto_trait,
// from default_impl.
if self.tcx().trait_has_default_impl(trait_def_id) {
// We want to both 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.

if !items.is_empty() {
error_380(self.ccx, item.span);
}

self.check_auto_trait(trait_def_id, item.span);
self.check_auto_trait(trait_def_id, items, item.span);
}

self.for_item(item).with_fcx(|fcx, this| {
Expand All @@ -311,8 +340,6 @@ impl<'ccx, 'gcx> CheckTypeWellFormedVisitor<'ccx, 'gcx> {
});
}



fn check_item_fn(&mut self,
item: &hir::Item,
body: &hir::Block)
Expand Down Expand Up @@ -678,18 +705,6 @@ fn error_380(ccx: &CrateCtxt, span: Span) {
Trait for ..`) must have no methods or associated items")
}

fn error_565(ccx: &CrateCtxt, span: Span) {
span_err!(ccx.tcx.sess, span, E0565,
"traits with default impls (`e.g. unsafe impl \
Trait for ..`) can not have predicates")
}

fn error_566(ccx: &CrateCtxt, span: Span) {
span_err!(ccx.tcx.sess, span, E0566,
"traits with default impls (`e.g. unsafe impl \
Trait for ..`) can not have type parameters")
}

fn error_392<'a, 'tcx>(ccx: &CrateCtxt<'a, 'tcx>, span: Span, param_name: ast::Name)
-> DiagnosticBuilder<'tcx> {
let mut err = struct_span_err!(ccx.tcx.sess, span, E0392,
Expand Down

This file was deleted.

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);
}
Expand Up @@ -10,7 +10,7 @@

#![feature(optin_builtin_traits)]

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

Expand Down
49 changes: 49 additions & 0 deletions src/test/compile-fail/typeck-auto-trait-no-supertraits.rs
@@ -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);
}
Expand Up @@ -10,5 +10,5 @@

#![feature(optin_builtin_traits)]

trait Magic<T> {} //~ E0566
trait Magic<T> {} //~ ERROR E0566
impl Magic<isize> for .. {}
27 changes: 0 additions & 27 deletions src/test/compile-fail/typeck-default-trait-impl-superregion.rs

This file was deleted.

29 changes: 0 additions & 29 deletions src/test/compile-fail/typeck-default-trait-impl-supertrait.rs

This file was deleted.

This file was deleted.

0 comments on commit 90d1a53

Please sign in to comment.