Skip to content

Commit

Permalink
Auto merge of #69649 - estebank:negative-impl-span, r=Centril
Browse files Browse the repository at this point in the history
Tweak output for invalid negative impl errors

Follow up to #69722. Tweak negative impl errors emitted in the HIR:

```
error[E0192]: invalid negative impl
  --> $DIR/E0192.rs:9:6
   |
LL | impl !Trait for Foo { }
   |      ^^^^^^
   |
   = note: negative impls are only allowed for auto traits, like `Send` and `Sync`
```
  • Loading branch information
bors committed Mar 23, 2020
2 parents 5aa8f19 + b8f9866 commit 8549cfe
Show file tree
Hide file tree
Showing 14 changed files with 95 additions and 50 deletions.
34 changes: 21 additions & 13 deletions src/librustc_ast_lowering/item.rs
Original file line number Diff line number Diff line change
Expand Up @@ -402,10 +402,15 @@ impl<'hir> LoweringContext<'_, 'hir> {
)
});

// `defaultness.has_value()` is never called for an `impl`, always `true` in order
// to not cause an assertion failure inside the `lower_defaultness` function.
let has_val = true;
let (defaultness, defaultness_span) = self.lower_defaultness(defaultness, has_val);
hir::ItemKind::Impl {
unsafety: self.lower_unsafety(unsafety),
polarity,
defaultness: self.lower_defaultness(defaultness, true /* [1] */),
defaultness,
defaultness_span,
constness: self.lower_constness(constness),
generics,
of_trait: trait_ref,
Expand Down Expand Up @@ -434,9 +439,6 @@ impl<'hir> LoweringContext<'_, 'hir> {
panic!("`TyMac` should have been expanded by now")
}
}

// [1] `defaultness.has_value()` is never called for an `impl`, always `true` in order to
// not cause an assertion failure inside the `lower_defaultness` function.
}

fn lower_const_item(
Expand Down Expand Up @@ -867,27 +869,31 @@ impl<'hir> LoweringContext<'_, 'hir> {
AssocItemKind::MacCall(..) => panic!("`TyMac` should have been expanded by now"),
};

// Since `default impl` is not yet implemented, this is always true in impls.
let has_value = true;
let (defaultness, _) = self.lower_defaultness(i.kind.defaultness(), has_value);
hir::ImplItem {
hir_id: self.lower_node_id(i.id),
ident: i.ident,
attrs: self.lower_attrs(&i.attrs),
generics,
vis: self.lower_visibility(&i.vis, None),
defaultness: self.lower_defaultness(i.kind.defaultness(), true /* [1] */),
defaultness,
kind,
span: i.span,
}

// [1] since `default impl` is not yet implemented, this is always true in impls
}

fn lower_impl_item_ref(&mut self, i: &AssocItem) -> hir::ImplItemRef<'hir> {
// Since `default impl` is not yet implemented, this is always true in impls.
let has_value = true;
let (defaultness, _) = self.lower_defaultness(i.kind.defaultness(), has_value);
hir::ImplItemRef {
id: hir::ImplItemId { hir_id: self.lower_node_id(i.id) },
ident: i.ident,
span: i.span,
vis: self.lower_visibility(&i.vis, Some(i.id)),
defaultness: self.lower_defaultness(i.kind.defaultness(), true /* [1] */),
defaultness,
kind: match &i.kind {
AssocItemKind::Const(..) => hir::AssocItemKind::Const,
AssocItemKind::TyAlias(.., ty) => {
Expand All @@ -902,8 +908,6 @@ impl<'hir> LoweringContext<'_, 'hir> {
AssocItemKind::MacCall(..) => unimplemented!(),
},
}

// [1] since `default impl` is not yet implemented, this is always true in impls
}

/// If an `explicit_owner` is given, this method allocates the `HirId` in
Expand Down Expand Up @@ -938,12 +942,16 @@ impl<'hir> LoweringContext<'_, 'hir> {
respan(v.span, node)
}

fn lower_defaultness(&self, d: Defaultness, has_value: bool) -> hir::Defaultness {
fn lower_defaultness(
&self,
d: Defaultness,
has_value: bool,
) -> (hir::Defaultness, Option<Span>) {
match d {
Defaultness::Default(_) => hir::Defaultness::Default { has_value },
Defaultness::Default(sp) => (hir::Defaultness::Default { has_value }, Some(sp)),
Defaultness::Final => {
assert!(has_value);
hir::Defaultness::Final
(hir::Defaultness::Final, None)
}
}
}
Expand Down
5 changes: 4 additions & 1 deletion src/librustc_hir/hir.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2153,7 +2153,7 @@ pub enum Defaultness {
impl Defaultness {
pub fn has_value(&self) -> bool {
match *self {
Defaultness::Default { has_value, .. } => has_value,
Defaultness::Default { has_value } => has_value,
Defaultness::Final => true,
}
}
Expand Down Expand Up @@ -2502,6 +2502,9 @@ pub enum ItemKind<'hir> {
unsafety: Unsafety,
polarity: ImplPolarity,
defaultness: Defaultness,
// We do not put a `Span` in `Defaultness` because it breaks foreign crate metadata
// decoding as `Span`s cannot be decoded when a `Session` is not available.
defaultness_span: Option<Span>,
constness: Constness,
generics: Generics<'hir>,

Expand Down
1 change: 1 addition & 0 deletions src/librustc_hir/intravisit.rs
Original file line number Diff line number Diff line change
Expand Up @@ -590,6 +590,7 @@ pub fn walk_item<'v, V: Visitor<'v>>(visitor: &mut V, item: &'v Item<'v>) {
defaultness: _,
polarity: _,
constness: _,
defaultness_span: _,
ref generics,
ref of_trait,
ref self_ty,
Expand Down
1 change: 1 addition & 0 deletions src/librustc_hir/print.rs
Original file line number Diff line number Diff line change
Expand Up @@ -632,6 +632,7 @@ impl<'a> State<'a> {
polarity,
defaultness,
constness,
defaultness_span: _,
ref generics,
ref of_trait,
ref self_ty,
Expand Down
38 changes: 27 additions & 11 deletions src/librustc_typeck/check/wfcheck.rs
Original file line number Diff line number Diff line change
Expand Up @@ -98,34 +98,50 @@ pub fn check_item_well_formed(tcx: TyCtxt<'_>, def_id: DefId) {
//
// won't be allowed unless there's an *explicit* implementation of `Send`
// for `T`
hir::ItemKind::Impl { defaultness, ref of_trait, ref self_ty, .. } => {
hir::ItemKind::Impl {
defaultness,
defaultness_span,
polarity,
ref of_trait,
ref self_ty,
..
} => {
let is_auto = tcx
.impl_trait_ref(tcx.hir().local_def_id(item.hir_id))
.map_or(false, |trait_ref| tcx.trait_is_auto(trait_ref.def_id));
let polarity = tcx.impl_polarity(def_id);
if let (hir::Defaultness::Default { .. }, true) = (defaultness, is_auto) {
tcx.sess.span_err(item.span, "impls of auto traits cannot be default");
let sp = of_trait.as_ref().map(|t| t.path.span).unwrap_or(item.span);
let mut err =
tcx.sess.struct_span_err(sp, "impls of auto traits cannot be default");
err.span_labels(defaultness_span, "default because of this");
err.span_label(sp, "auto trait");
err.emit();
}
match polarity {
ty::ImplPolarity::Positive => {
// We match on both `ty::ImplPolarity` and `ast::ImplPolarity` just to get the `!` span.
match (tcx.impl_polarity(def_id), polarity) {
(ty::ImplPolarity::Positive, _) => {
check_impl(tcx, item, self_ty, of_trait);
}
ty::ImplPolarity::Negative => {
(ty::ImplPolarity::Negative, ast::ImplPolarity::Negative(span)) => {
// FIXME(#27579): what amount of WF checking do we need for neg impls?
if of_trait.is_some() && !is_auto {
if let (Some(of_trait), false) = (of_trait, is_auto) {
struct_span_err!(
tcx.sess,
item.span,
span.to(of_trait.path.span),
E0192,
"negative impls are only allowed for \
auto traits (e.g., `Send` and `Sync`)"
"invalid negative impl"
)
.note(
"negative impls are only allowed for auto traits, like `Send` and \
`Sync`",
)
.emit()
}
}
ty::ImplPolarity::Reservation => {
(ty::ImplPolarity::Reservation, _) => {
// FIXME: what amount of WF checking do we need for reservation impls?
}
_ => unreachable!(),
}
}
hir::ItemKind::Fn(..) => {
Expand Down
5 changes: 3 additions & 2 deletions src/librustc_typeck/collect.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1564,9 +1564,10 @@ fn impl_polarity(tcx: TyCtxt<'_>, def_id: DefId) -> ty::ImplPolarity {
let is_rustc_reservation = tcx.has_attr(def_id, sym::rustc_reservation_impl);
let item = tcx.hir().expect_item(hir_id);
match &item.kind {
hir::ItemKind::Impl { polarity: hir::ImplPolarity::Negative(_), .. } => {
hir::ItemKind::Impl { polarity: hir::ImplPolarity::Negative(span), of_trait, .. } => {
if is_rustc_reservation {
tcx.sess.span_err(item.span, "reservation impls can't be negative");
let span = span.to(of_trait.as_ref().map(|t| t.path.span).unwrap_or(*span));
tcx.sess.span_err(span, "reservation impls can't be negative");
}
ty::ImplPolarity::Negative
}
Expand Down
1 change: 1 addition & 0 deletions src/librustdoc/visit_ast.rs
Original file line number Diff line number Diff line change
Expand Up @@ -563,6 +563,7 @@ impl<'a, 'tcx> RustdocVisitor<'a, 'tcx> {
polarity,
defaultness,
constness,
defaultness_span: _,
ref generics,
ref of_trait,
self_ty,
Expand Down
8 changes: 5 additions & 3 deletions src/test/ui/error-codes/E0192.stderr
Original file line number Diff line number Diff line change
@@ -1,8 +1,10 @@
error[E0192]: negative impls are only allowed for auto traits (e.g., `Send` and `Sync`)
--> $DIR/E0192.rs:9:1
error[E0192]: invalid negative impl
--> $DIR/E0192.rs:9:6
|
LL | impl !Trait for Foo { }
| ^^^^^^^^^^^^^^^^^^^^^^^
| ^^^^^^
|
= note: negative impls are only allowed for auto traits, like `Send` and `Sync`

error: aborting due to previous error

Expand Down
2 changes: 1 addition & 1 deletion src/test/ui/specialization/defaultimpl/validation.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,6 @@ default unsafe impl Send for S {} //~ ERROR impls of auto traits cannot be defau
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
default impl !Tr for S {} //~ ERROR invalid negative impl

fn main() {}
20 changes: 13 additions & 7 deletions src/test/ui/specialization/defaultimpl/validation.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -9,22 +9,28 @@ LL | default impl S {}
= note: only trait implementations may be annotated with `default`

error: impls of auto traits cannot be default
--> $DIR/validation.rs:9:1
--> $DIR/validation.rs:9:21
|
LL | default unsafe impl Send for S {}
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
| ------- ^^^^ auto trait
| |
| default because of this

error: impls of auto traits cannot be default
--> $DIR/validation.rs:10:1
--> $DIR/validation.rs:10:15
|
LL | default impl !Send for Z {}
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^
| ------- ^^^^ auto trait
| |
| default because of this

error[E0192]: negative impls are only allowed for auto traits (e.g., `Send` and `Sync`)
--> $DIR/validation.rs:13:1
error[E0192]: invalid negative impl
--> $DIR/validation.rs:13:14
|
LL | default impl !Tr for S {}
| ^^^^^^^^^^^^^^^^^^^^^^^^^
| ^^^
|
= note: negative impls are only allowed for auto traits, like `Send` and `Sync`

error: aborting due to 4 previous errors

Expand Down
4 changes: 2 additions & 2 deletions src/test/ui/syntax-trait-polarity.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ 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
//~^ ERROR invalid negative impl

struct TestType2<T>(T);

Expand All @@ -22,6 +22,6 @@ impl<T> !TestType2<T> {}
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
//~^ ERROR invalid negative impl

fn main() {}
16 changes: 10 additions & 6 deletions src/test/ui/syntax-trait-polarity.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -32,17 +32,21 @@ LL | unsafe impl<T> !Send for TestType2<T> {}
| | negative because of this
| unsafe because of this

error[E0192]: negative impls are only allowed for auto traits (e.g., `Send` and `Sync`)
--> $DIR/syntax-trait-polarity.rs:14:1
error[E0192]: invalid negative impl
--> $DIR/syntax-trait-polarity.rs:14:6
|
LL | impl !TestTrait for TestType {}
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
| ^^^^^^^^^^
|
= note: negative impls are only allowed for auto traits, like `Send` and `Sync`

error[E0192]: negative impls are only allowed for auto traits (e.g., `Send` and `Sync`)
--> $DIR/syntax-trait-polarity.rs:24:1
error[E0192]: invalid negative impl
--> $DIR/syntax-trait-polarity.rs:24:9
|
LL | impl<T> !TestTrait for TestType2<T> {}
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
| ^^^^^^^^^^
|
= note: negative impls are only allowed for auto traits, like `Send` and `Sync`

error: aborting due to 6 previous errors

Expand Down
2 changes: 1 addition & 1 deletion src/test/ui/typeck/typeck-negative-impls-builtin.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,6 @@ trait TestTrait {
}

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

fn main() {}
8 changes: 5 additions & 3 deletions src/test/ui/typeck/typeck-negative-impls-builtin.stderr
Original file line number Diff line number Diff line change
@@ -1,8 +1,10 @@
error[E0192]: negative impls are only allowed for auto traits (e.g., `Send` and `Sync`)
--> $DIR/typeck-negative-impls-builtin.rs:9:1
error[E0192]: invalid negative impl
--> $DIR/typeck-negative-impls-builtin.rs:9:6
|
LL | impl !TestTrait for TestType {}
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
| ^^^^^^^^^^
|
= note: negative impls are only allowed for auto traits, like `Send` and `Sync`

error: aborting due to previous error

Expand Down

0 comments on commit 8549cfe

Please sign in to comment.