From b8f98662577f5423e2a2c4ffdfafbad9e4526058 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Esteban=20K=C3=BCber?= Date: Mon, 2 Mar 2020 15:26:00 -0800 Subject: [PATCH] Tweak output for invalid negative impl errors --- src/librustc_ast_lowering/item.rs | 34 ++++++++++------- src/librustc_hir/hir.rs | 5 ++- src/librustc_hir/intravisit.rs | 1 + src/librustc_hir/print.rs | 1 + src/librustc_typeck/check/wfcheck.rs | 38 +++++++++++++------ src/librustc_typeck/collect.rs | 5 ++- src/librustdoc/visit_ast.rs | 1 + src/test/ui/error-codes/E0192.stderr | 8 ++-- .../specialization/defaultimpl/validation.rs | 2 +- .../defaultimpl/validation.stderr | 20 ++++++---- src/test/ui/syntax-trait-polarity.rs | 4 +- src/test/ui/syntax-trait-polarity.stderr | 16 +++++--- .../typeck/typeck-negative-impls-builtin.rs | 2 +- .../typeck-negative-impls-builtin.stderr | 8 ++-- 14 files changed, 95 insertions(+), 50 deletions(-) diff --git a/src/librustc_ast_lowering/item.rs b/src/librustc_ast_lowering/item.rs index c22b2812a9e3c..a029c2957b2d6 100644 --- a/src/librustc_ast_lowering/item.rs +++ b/src/librustc_ast_lowering/item.rs @@ -403,10 +403,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, @@ -435,9 +440,6 @@ impl<'hir> LoweringContext<'_, 'hir> { bug!("`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( @@ -868,27 +870,31 @@ impl<'hir> LoweringContext<'_, 'hir> { AssocItemKind::MacCall(..) => bug!("`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) => { @@ -903,8 +909,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 @@ -939,12 +943,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) { 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) } } } diff --git a/src/librustc_hir/hir.rs b/src/librustc_hir/hir.rs index 0ef35b184e17f..bb864edc99969 100644 --- a/src/librustc_hir/hir.rs +++ b/src/librustc_hir/hir.rs @@ -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, } } @@ -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, constness: Constness, generics: Generics<'hir>, diff --git a/src/librustc_hir/intravisit.rs b/src/librustc_hir/intravisit.rs index 316b31f069855..11749cf996b44 100644 --- a/src/librustc_hir/intravisit.rs +++ b/src/librustc_hir/intravisit.rs @@ -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, diff --git a/src/librustc_hir/print.rs b/src/librustc_hir/print.rs index 4e9618b7676e8..cd16e451f1db8 100644 --- a/src/librustc_hir/print.rs +++ b/src/librustc_hir/print.rs @@ -632,6 +632,7 @@ impl<'a> State<'a> { polarity, defaultness, constness, + defaultness_span: _, ref generics, ref of_trait, ref self_ty, diff --git a/src/librustc_typeck/check/wfcheck.rs b/src/librustc_typeck/check/wfcheck.rs index 72c58af7912fb..27be0faade82f 100644 --- a/src/librustc_typeck/check/wfcheck.rs +++ b/src/librustc_typeck/check/wfcheck.rs @@ -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(..) => { diff --git a/src/librustc_typeck/collect.rs b/src/librustc_typeck/collect.rs index a79c065307796..052a2b3d4e870 100644 --- a/src/librustc_typeck/collect.rs +++ b/src/librustc_typeck/collect.rs @@ -1580,9 +1580,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 } diff --git a/src/librustdoc/visit_ast.rs b/src/librustdoc/visit_ast.rs index feaf391c95afb..e49395c6fd438 100644 --- a/src/librustdoc/visit_ast.rs +++ b/src/librustdoc/visit_ast.rs @@ -563,6 +563,7 @@ impl<'a, 'tcx> RustdocVisitor<'a, 'tcx> { polarity, defaultness, constness, + defaultness_span: _, ref generics, ref of_trait, self_ty, diff --git a/src/test/ui/error-codes/E0192.stderr b/src/test/ui/error-codes/E0192.stderr index 8faa550a50935..da706dea167f6 100644 --- a/src/test/ui/error-codes/E0192.stderr +++ b/src/test/ui/error-codes/E0192.stderr @@ -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 diff --git a/src/test/ui/specialization/defaultimpl/validation.rs b/src/test/ui/specialization/defaultimpl/validation.rs index 26b3f1ec41491..5b8a72104e31a 100644 --- a/src/test/ui/specialization/defaultimpl/validation.rs +++ b/src/test/ui/specialization/defaultimpl/validation.rs @@ -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() {} diff --git a/src/test/ui/specialization/defaultimpl/validation.stderr b/src/test/ui/specialization/defaultimpl/validation.stderr index 6e19d79e48f6b..e03153f343b8e 100644 --- a/src/test/ui/specialization/defaultimpl/validation.stderr +++ b/src/test/ui/specialization/defaultimpl/validation.stderr @@ -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 diff --git a/src/test/ui/syntax-trait-polarity.rs b/src/test/ui/syntax-trait-polarity.rs index 1b7fc1587e6d2..bfbe6394c1694 100644 --- a/src/test/ui/syntax-trait-polarity.rs +++ b/src/test/ui/syntax-trait-polarity.rs @@ -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); @@ -22,6 +22,6 @@ impl !TestType2 {} unsafe impl !Send for TestType2 {} //~^ ERROR negative impls cannot be unsafe impl !TestTrait for TestType2 {} -//~^ ERROR negative impls are only allowed for auto traits +//~^ ERROR invalid negative impl fn main() {} diff --git a/src/test/ui/syntax-trait-polarity.stderr b/src/test/ui/syntax-trait-polarity.stderr index 5777e0ade908e..f70d67ec7dc98 100644 --- a/src/test/ui/syntax-trait-polarity.stderr +++ b/src/test/ui/syntax-trait-polarity.stderr @@ -32,17 +32,21 @@ LL | unsafe impl !Send for TestType2 {} | | 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 !TestTrait for TestType2 {} - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | ^^^^^^^^^^ + | + = note: negative impls are only allowed for auto traits, like `Send` and `Sync` error: aborting due to 6 previous errors diff --git a/src/test/ui/typeck/typeck-negative-impls-builtin.rs b/src/test/ui/typeck/typeck-negative-impls-builtin.rs index 7bdd1035a1bf2..fef98977cc435 100644 --- a/src/test/ui/typeck/typeck-negative-impls-builtin.rs +++ b/src/test/ui/typeck/typeck-negative-impls-builtin.rs @@ -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() {} diff --git a/src/test/ui/typeck/typeck-negative-impls-builtin.stderr b/src/test/ui/typeck/typeck-negative-impls-builtin.stderr index 4e3d054ff6fad..c90655086acda 100644 --- a/src/test/ui/typeck/typeck-negative-impls-builtin.stderr +++ b/src/test/ui/typeck/typeck-negative-impls-builtin.stderr @@ -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