Skip to content

Commit

Permalink
syntax: Rewrite parsing of impls
Browse files Browse the repository at this point in the history
Properly parse impls for the never type `!`
Recover from missing `for` in `impl Trait for Type`
Prohibit inherent default impls and default impls of auto traits
Change wording in more diagnostics to use "auto traits"
Some minor code cleanups in the parser
  • Loading branch information
petrochenkov committed Jan 14, 2018
1 parent 48ab4cd commit d19e4c4
Show file tree
Hide file tree
Showing 23 changed files with 330 additions and 274 deletions.
4 changes: 2 additions & 2 deletions src/librustc/hir/lowering.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1502,8 +1502,8 @@ impl<'a> LoweringContext<'a> {
fn_def_id: Option<DefId>,
impl_trait_return_allow: bool)
-> P<hir::FnDecl> {
// NOTE: The two last paramters here have to do with impl Trait. If fn_def_id is Some,
// then impl Trait arguments are lowered into generic paramters on the given
// NOTE: The two last parameters here have to do with impl Trait. If fn_def_id is Some,
// then impl Trait arguments are lowered into generic parameters on the given
// fn_def_id, otherwise impl Trait is disallowed. (for now)
//
// Furthermore, if impl_trait_return_allow is true, then impl Trait may be used in
Expand Down
16 changes: 14 additions & 2 deletions src/librustc_passes/ast_validation.rs
Original file line number Diff line number Diff line change
Expand Up @@ -215,24 +215,36 @@ impl<'a> Visitor<'a> for AstValidator<'a> {

fn visit_item(&mut self, item: &'a Item) {
match item.node {
ItemKind::Impl(.., Some(..), ref ty, ref impl_items) => {
ItemKind::Impl(unsafety, polarity, _, _, Some(..), ref ty, ref impl_items) => {
self.invalid_visibility(&item.vis, item.span, None);
if ty.node == TyKind::Err {
self.err_handler()
.struct_span_err(item.span, "`impl Trait for .. {}` is an obsolete syntax")
.help("use `auto trait Trait {}` instead").emit();
}
if unsafety == Unsafety::Unsafe && polarity == ImplPolarity::Negative {
span_err!(self.session, item.span, E0198, "negative impls cannot be unsafe");
}
for impl_item in impl_items {
self.invalid_visibility(&impl_item.vis, impl_item.span, None);
if let ImplItemKind::Method(ref sig, _) = impl_item.node {
self.check_trait_fn_not_const(sig.constness);
}
}
}
ItemKind::Impl(.., None, _, _) => {
ItemKind::Impl(unsafety, polarity, defaultness, _, None, _, _) => {
self.invalid_visibility(&item.vis,
item.span,
Some("place qualifiers on individual impl items instead"));
if unsafety == Unsafety::Unsafe {
span_err!(self.session, item.span, E0197, "inherent impls cannot be unsafe");
}
if polarity == ImplPolarity::Negative {
self.err_handler().span_err(item.span, "inherent impls cannot be negative");
}
if defaultness == Defaultness::Default {
self.err_handler().span_err(item.span, "inherent impls cannot be default");
}
}
ItemKind::ForeignMod(..) => {
self.invalid_visibility(&item.vis,
Expand Down
46 changes: 46 additions & 0 deletions src/librustc_passes/diagnostics.rs
Original file line number Diff line number Diff line change
Expand Up @@ -82,6 +82,52 @@ extern {
```
"##,

E0197: r##"
Inherent implementations (one that do not implement a trait but provide
methods associated with a type) are always safe because they are not
implementing an unsafe trait. Removing the `unsafe` keyword from the inherent
implementation will resolve this error.
```compile_fail,E0197
struct Foo;
// this will cause this error
unsafe impl Foo { }
// converting it to this will fix it
impl Foo { }
```
"##,

E0198: r##"
A negative implementation is one that excludes a type from implementing a
particular trait. Not being able to use a trait is always a safe operation,
so negative implementations are always safe and never need to be marked as
unsafe.
```compile_fail
#![feature(optin_builtin_traits)]
struct Foo;
// unsafe is unnecessary
unsafe impl !Clone for Foo { }
```
This will compile:
```ignore (ignore auto_trait future compatibility warning)
#![feature(optin_builtin_traits)]
struct Foo;
auto trait Enterprise {}
impl !Enterprise for Foo { }
```
Please note that negative impls are only allowed for auto traits.
"##,

E0265: r##"
This error indicates that a static or constant references itself.
All statics and constants need to resolve to a value in an acyclic manner.
Expand Down
31 changes: 15 additions & 16 deletions src/librustc_typeck/check/wfcheck.rs
Original file line number Diff line number Diff line change
Expand Up @@ -107,16 +107,21 @@ impl<'a, 'gcx> CheckTypeWellFormedVisitor<'a, 'gcx> {
//
// won't be allowed unless there's an *explicit* implementation of `Send`
// for `T`
hir::ItemImpl(_, hir::ImplPolarity::Positive, _, _,
ref trait_ref, ref self_ty, _) => {
self.check_impl(item, self_ty, trait_ref);
}
hir::ItemImpl(_, hir::ImplPolarity::Negative, _, _, Some(_), ..) => {
// FIXME(#27579) what amount of WF checking do we need for neg impls?

let trait_ref = tcx.impl_trait_ref(tcx.hir.local_def_id(item.id)).unwrap();
if !tcx.trait_is_auto(trait_ref.def_id) {
error_192(tcx, item.span);
hir::ItemImpl(_, polarity, defaultness, _, ref trait_ref, ref self_ty, _) => {
let is_auto = tcx.impl_trait_ref(tcx.hir.local_def_id(item.id))
.map_or(false, |trait_ref| tcx.trait_is_auto(trait_ref.def_id));
if let (hir::Defaultness::Default { .. }, true) = (defaultness, is_auto) {
tcx.sess.span_err(item.span, "impls of auto traits cannot be default");
}
if polarity == hir::ImplPolarity::Positive {
self.check_impl(item, self_ty, trait_ref);
} else {
// FIXME(#27579) what amount of WF checking do we need for neg impls?
if trait_ref.is_some() && !is_auto {
span_err!(tcx.sess, item.span, E0192,
"negative impls are only allowed for \
auto traits (e.g., `Send` and `Sync`)")
}
}
}
hir::ItemFn(..) => {
Expand Down Expand Up @@ -661,12 +666,6 @@ impl<'a, 'gcx, 'tcx> FnCtxt<'a, 'gcx, 'tcx> {
}
}

fn error_192(tcx: TyCtxt, span: Span) {
span_err!(tcx.sess, span, E0192,
"negative impls are only allowed for traits with \
default impls (e.g., `Send` and `Sync`)")
}

fn error_392<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>, span: Span, param_name: ast::Name)
-> DiagnosticBuilder<'tcx> {
let mut err = struct_span_err!(tcx.sess, span, E0392,
Expand Down
16 changes: 2 additions & 14 deletions src/librustc_typeck/coherence/inherent_impls.rs
Original file line number Diff line number Diff line change
Expand Up @@ -93,23 +93,11 @@ struct InherentCollect<'a, 'tcx: 'a> {

impl<'a, 'tcx, 'v> ItemLikeVisitor<'v> for InherentCollect<'a, 'tcx> {
fn visit_item(&mut self, item: &hir::Item) {
let (unsafety, ty) = match item.node {
hir::ItemImpl(unsafety, .., None, ref ty, _) => (unsafety, ty),
let ty = match item.node {
hir::ItemImpl(.., None, ref ty, _) => ty,
_ => return
};

match unsafety {
hir::Unsafety::Normal => {
// OK
}
hir::Unsafety::Unsafe => {
span_err!(self.tcx.sess,
item.span,
E0197,
"inherent impls cannot be declared as unsafe");
}
}

let def_id = self.tcx.hir.local_def_id(item.id);
let self_ty = self.tcx.type_of(def_id);
let lang_items = self.tcx.lang_items();
Expand Down
7 changes: 3 additions & 4 deletions src/librustc_typeck/coherence/orphan.rs
Original file line number Diff line number Diff line change
Expand Up @@ -67,16 +67,15 @@ impl<'cx, 'tcx, 'v> ItemLikeVisitor<'v> for OrphanChecker<'cx, 'tcx> {
}
}

// In addition to the above rules, we restrict impls of defaulted traits
// In addition to the above rules, we restrict impls of auto traits
// so that they can only be implemented on nominal types, such as structs,
// enums or foreign types. To see why this restriction exists, consider the
// following example (#22978). Imagine that crate A defines a defaulted trait
// following example (#22978). Imagine that crate A defines an auto trait
// `Foo` and a fn that operates on pairs of types:
//
// ```
// // Crate A
// trait Foo { }
// impl Foo for .. { }
// auto trait Foo { }
// fn two_foos<A:Foo,B:Foo>(..) {
// one_foo::<(A,B)>(..)
// }
Expand Down
13 changes: 5 additions & 8 deletions src/librustc_typeck/coherence/unsafety.rs
Original file line number Diff line number Diff line change
Expand Up @@ -37,14 +37,7 @@ impl<'cx, 'tcx, 'v> UnsafetyChecker<'cx, 'tcx> {
let trait_def = self.tcx.trait_def(trait_ref.def_id);
let unsafe_attr = impl_generics.and_then(|g| g.carries_unsafe_attr());
match (trait_def.unsafety, unsafe_attr, unsafety, polarity) {
(_, _, Unsafety::Unsafe, hir::ImplPolarity::Negative) => {
span_err!(self.tcx.sess,
item.span,
E0198,
"negative implementations are not unsafe");
}

(Unsafety::Normal, None, Unsafety::Unsafe, _) => {
(Unsafety::Normal, None, Unsafety::Unsafe, hir::ImplPolarity::Positive) => {
span_err!(self.tcx.sess,
item.span,
E0199,
Expand All @@ -69,6 +62,10 @@ impl<'cx, 'tcx, 'v> UnsafetyChecker<'cx, 'tcx> {
g.attr_name());
}

(_, _, Unsafety::Unsafe, hir::ImplPolarity::Negative) => {
// Reported in AST validation
self.tcx.sess.delay_span_bug(item.span, "unsafe negative impl");
}
(_, _, Unsafety::Normal, hir::ImplPolarity::Negative) |
(Unsafety::Unsafe, _, Unsafety::Unsafe, hir::ImplPolarity::Positive) |
(Unsafety::Normal, Some(_), Unsafety::Unsafe, hir::ImplPolarity::Positive) |
Expand Down
48 changes: 1 addition & 47 deletions src/librustc_typeck/diagnostics.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1715,7 +1715,7 @@ type Foo = Trait<Bar=i32>; // ok!
"##,

E0192: r##"
Negative impls are only allowed for traits with default impls. For more
Negative impls are only allowed for auto traits. For more
information see the [opt-in builtin traits RFC][RFC 19].
[RFC 19]: https://github.com/rust-lang/rfcs/blob/master/text/0019-opt-in-builtin-traits.md
Expand Down Expand Up @@ -1821,52 +1821,6 @@ impl Trait for Foo {
```
"##,

E0197: r##"
Inherent implementations (one that do not implement a trait but provide
methods associated with a type) are always safe because they are not
implementing an unsafe trait. Removing the `unsafe` keyword from the inherent
implementation will resolve this error.
```compile_fail,E0197
struct Foo;
// this will cause this error
unsafe impl Foo { }
// converting it to this will fix it
impl Foo { }
```
"##,

E0198: r##"
A negative implementation is one that excludes a type from implementing a
particular trait. Not being able to use a trait is always a safe operation,
so negative implementations are always safe and never need to be marked as
unsafe.
```compile_fail
#![feature(optin_builtin_traits)]
struct Foo;
// unsafe is unnecessary
unsafe impl !Clone for Foo { }
```
This will compile:
```
#![feature(optin_builtin_traits)]
struct Foo;
auto trait Enterprise {}
impl !Enterprise for Foo { }
```
Please note that negative impls are only allowed for traits with default impls.
"##,

E0199: r##"
Safe traits should not have unsafe implementations, therefore marking an
implementation for a safe trait unsafe will cause a compiler error. Removing
Expand Down
Loading

0 comments on commit d19e4c4

Please sign in to comment.