Skip to content

syntax: Rewrite parsing of impls #46455

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Jan 14, 2018
Merged
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions src/librustc/hir/lowering.rs
Original file line number Diff line number Diff line change
@@ -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
16 changes: 14 additions & 2 deletions src/librustc_passes/ast_validation.rs
Original file line number Diff line number Diff line change
@@ -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,
46 changes: 46 additions & 0 deletions src/librustc_passes/diagnostics.rs
Original file line number Diff line number Diff line change
@@ -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)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this ignore can be removed

#![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.
31 changes: 15 additions & 16 deletions src/librustc_typeck/check/wfcheck.rs
Original file line number Diff line number Diff line change
@@ -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(..) => {
@@ -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,
16 changes: 2 additions & 14 deletions src/librustc_typeck/coherence/inherent_impls.rs
Original file line number Diff line number Diff line change
@@ -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();
7 changes: 3 additions & 4 deletions src/librustc_typeck/coherence/orphan.rs
Original file line number Diff line number Diff line change
@@ -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)>(..)
// }
13 changes: 5 additions & 8 deletions src/librustc_typeck/coherence/unsafety.rs
Original file line number Diff line number Diff line change
@@ -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,
@@ -69,6 +62,10 @@ impl<'cx, 'tcx, 'v> UnsafetyChecker<'cx, 'tcx> {
g.attr_name());
}

(_, _, Unsafety::Unsafe, hir::ImplPolarity::Negative) => {
// Reported in AST validation
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

delay_span_bug please

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) |
48 changes: 1 addition & 47 deletions src/librustc_typeck/diagnostics.rs
Original file line number Diff line number Diff line change
@@ -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
@@ -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
323 changes: 156 additions & 167 deletions src/libsyntax/parse/parser.rs

Large diffs are not rendered by default.

2 changes: 1 addition & 1 deletion src/test/compile-fail/E0198.rs
Original file line number Diff line number Diff line change
@@ -12,7 +12,7 @@

struct Foo;

unsafe impl !Clone for Foo { } //~ ERROR negative implementations are not unsafe [E0198]
unsafe impl !Send for Foo { } //~ ERROR E0198

fn main() {
}
2 changes: 1 addition & 1 deletion src/test/compile-fail/coherence-negative-impls-safe.rs
Original file line number Diff line number Diff line change
@@ -15,6 +15,6 @@ use std::marker::Send;
struct TestType;

unsafe impl !Send for TestType {}
//~^ ERROR negative implementations are not unsafe
//~^ ERROR negative impls cannot be unsafe

fn main() {}
23 changes: 23 additions & 0 deletions src/test/compile-fail/issue-46438.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
// Copyright 2017 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.

macro_rules! m {
($my_type: ty) => {
impl $my_type for u8 {}
}
}

trait Trait {}

m!(Tr);

m!(&'static u8); //~ ERROR expected a trait, found type

fn main() {}
3 changes: 1 addition & 2 deletions src/test/compile-fail/phantom-oibit.rs
Original file line number Diff line number Diff line change
@@ -9,8 +9,7 @@
// except according to those terms.

// Ensure that OIBIT checks `T` when it encounters a `PhantomData<T>` field, instead of checking
// the `PhantomData<T>` type itself (which almost always implements a "default" trait
// (`impl Trait for ..`))
// the `PhantomData<T>` type itself (which almost always implements an auto trait)

#![feature(optin_builtin_traits)]

4 changes: 2 additions & 2 deletions src/test/compile-fail/private-in-public-ill-formed.rs
Original file line number Diff line number Diff line change
@@ -21,7 +21,7 @@ mod aliases_pub {
type AssocAlias = m::Pub3;
}

impl (<Priv as PrivTr>::AssocAlias) { //~ ERROR no base type found for inherent implementation
impl <Priv as PrivTr>::AssocAlias { //~ ERROR no base type found for inherent implementation
pub fn f(arg: Priv) {} // private type `aliases_pub::Priv` in public interface
}
}
@@ -37,7 +37,7 @@ mod aliases_priv {
type AssocAlias = Priv3;
}

impl (<Priv as PrivTr>::AssocAlias) { //~ ERROR no base type found for inherent implementation
impl <Priv as PrivTr>::AssocAlias { //~ ERROR no base type found for inherent implementation
pub fn f(arg: Priv) {} // OK
}
}
23 changes: 23 additions & 0 deletions src/test/compile-fail/specialization/defaultimpl/validation.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
// Copyright 2017 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)]
#![feature(specialization)]

struct S;
struct Z;

default impl S {} //~ ERROR inherent impls cannot be default

default unsafe impl Send for S {} //~ ERROR impls of auto traits cannot be default
default impl !Send for Z {} //~ ERROR impls of auto traits cannot be default

trait Tr {}
default impl !Tr for S {} //~ ERROR negative impls are only allowed for auto traits
Original file line number Diff line number Diff line change
@@ -14,7 +14,7 @@ struct TestType;

trait TestTrait {}

unsafe impl !Send for TestType {}
impl !Send for TestType {}
//~^ ERROR negative trait bounds

fn main() {}
Original file line number Diff line number Diff line change
@@ -8,28 +8,30 @@
// option. This file may not be copied, modified, or distributed
// except according to those terms.

// compile-flags: -Z parse-only -Z continue-parse-after-error

#![feature(optin_builtin_traits)]

use std::marker::Send;

struct TestType;

impl !TestType {}
//~^ ERROR inherent implementation can't be negated
//~^ ERROR inherent impls cannot be negative

trait TestTrait {}

unsafe impl !Send for TestType {}
//~^ ERROR negative impls cannot be unsafe
impl !TestTrait for TestType {}
//~^ ERROR negative impls are only allowed for auto traits

struct TestType2<T>;
struct TestType2<T>(T);

impl<T> !TestType2<T> {}
//~^ ERROR inherent implementation can't be negated
//~^ ERROR inherent impls cannot be negative

unsafe impl<T> !Send for TestType2<T> {}
//~^ ERROR negative impls cannot be unsafe
impl<T> !TestTrait for TestType2<T> {}
//~^ ERROR negative impls are only allowed for auto traits

fn main() {}
2 changes: 1 addition & 1 deletion src/test/compile-fail/trait-safety-inherent-impl.rs
Original file line number Diff line number Diff line change
@@ -12,7 +12,7 @@

struct SomeStruct;

unsafe impl SomeStruct { //~ ERROR inherent impls cannot be declared as unsafe
unsafe impl SomeStruct { //~ ERROR inherent impls cannot be unsafe
fn foo(self) { }
}

2 changes: 1 addition & 1 deletion src/test/compile-fail/typeck-negative-impls-builtin.rs
Original file line number Diff line number Diff line change
@@ -17,6 +17,6 @@ trait TestTrait {
}

impl !TestTrait for TestType {}
//~^ ERROR negative impls are only allowed for traits with default impls (e.g., `Send` and `Sync`)
//~^ ERROR negative impls are only allowed for auto traits (e.g., `Send` and `Sync`)

fn main() {}
18 changes: 18 additions & 0 deletions src/test/parse-fail/impl-qpath.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
// Copyright 2017 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.

// compile-flags: -Z parse-only

impl <*const u8>::AssocTy {} // OK
impl <Type as Trait>::AssocTy {} // OK
impl <'a + Trait>::AssocTy {} // OK
impl <<Type>::AssocTy>::AssocTy {} // OK

FAIL //~ ERROR
4 changes: 1 addition & 3 deletions src/test/parse-fail/trait-bounds-not-on-impl.rs
Original file line number Diff line number Diff line change
@@ -15,9 +15,7 @@ trait Foo {

struct Bar;

impl Foo + Owned for Bar {
//~^ ERROR not a trait
//~^^ ERROR expected one of `where` or `{`, found `Bar`
impl Foo + Owned for Bar { //~ ERROR expected a trait, found type
}

fn main() { }
2 changes: 1 addition & 1 deletion src/test/parse-fail/where_with_bound.rs
Original file line number Diff line number Diff line change
@@ -11,6 +11,6 @@
// compile-flags: -Z parse-only

fn foo<T>() where <T>::Item: ToString, T: Iterator { }
//~^ syntax `where<T>` is reserved for future use
//~^ ERROR generic parameters on `where` clauses are reserved for future use

fn main() {}
8 changes: 4 additions & 4 deletions src/test/run-pass/issue-25693.rs
Original file line number Diff line number Diff line change
@@ -8,16 +8,16 @@
// option. This file may not be copied, modified, or distributed
// except according to those terms.

pub trait Paramters { type SelfRef; }
pub trait Parameters { type SelfRef; }

struct RP<'a> { _marker: std::marker::PhantomData<&'a ()> }
struct BP;

impl<'a> Paramters for RP<'a> { type SelfRef = &'a X<RP<'a>>; }
impl Paramters for BP { type SelfRef = Box<X<BP>>; }
impl<'a> Parameters for RP<'a> { type SelfRef = &'a X<RP<'a>>; }
impl Parameters for BP { type SelfRef = Box<X<BP>>; }

pub struct Y;
pub enum X<P: Paramters> {
pub enum X<P: Parameters> {
Nothing,
SameAgain(P::SelfRef, Y)
}
21 changes: 21 additions & 0 deletions src/test/ui/span/impl-parsing.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
// Copyright 2017 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.

// compile-flags: -Z parse-only -Z continue-parse-after-error

impl ! {} // OK
impl ! where u8: Copy {} // OK

impl Trait Type {} //~ ERROR missing `for` in a trait impl
impl Trait .. {} //~ ERROR missing `for` in a trait impl
impl ?Sized for Type {} //~ ERROR expected a trait, found type
impl ?Sized for .. {} //~ ERROR expected a trait, found type

default unsafe FAIL //~ ERROR expected `impl`, found `FAIL`
32 changes: 32 additions & 0 deletions src/test/ui/span/impl-parsing.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
error: missing `for` in a trait impl
--> $DIR/impl-parsing.rs:16:11
|
16 | impl Trait Type {} //~ ERROR missing `for` in a trait impl
| ^

error: missing `for` in a trait impl
--> $DIR/impl-parsing.rs:17:11
|
17 | impl Trait .. {} //~ ERROR missing `for` in a trait impl
| ^

error: expected a trait, found type
--> $DIR/impl-parsing.rs:18:6
|
18 | impl ?Sized for Type {} //~ ERROR expected a trait, found type
| ^^^^^^

error: expected a trait, found type
--> $DIR/impl-parsing.rs:19:6
|
19 | impl ?Sized for .. {} //~ ERROR expected a trait, found type
| ^^^^^^

error: expected `impl`, found `FAIL`
--> $DIR/impl-parsing.rs:21:16
|
21 | default unsafe FAIL //~ ERROR expected `impl`, found `FAIL`
| ^^^^ expected `impl` here

error: aborting due to 5 previous errors

8 changes: 0 additions & 8 deletions src/test/ui/typeck-default-trait-impl-outside-crate.stderr

This file was deleted.