Skip to content

Commit b8f9866

Browse files
estebankestebank
committed
Tweak output for invalid negative impl errors
1 parent 5ae85f4 commit b8f9866

File tree

14 files changed

+95
-50
lines changed

14 files changed

+95
-50
lines changed

‎src/librustc_ast_lowering/item.rs‎

Lines changed: 21 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -403,10 +403,15 @@ impl<'hir> LoweringContext<'_, 'hir> {
403403
)
404404
});
405405

406+
// `defaultness.has_value()` is never called for an `impl`, always `true` in order
407+
// to not cause an assertion failure inside the `lower_defaultness` function.
408+
let has_val = true;
409+
let (defaultness, defaultness_span) = self.lower_defaultness(defaultness, has_val);
406410
hir::ItemKind::Impl {
407411
unsafety: self.lower_unsafety(unsafety),
408412
polarity,
409-
defaultness: self.lower_defaultness(defaultness, true /* [1] */),
413+
defaultness,
414+
defaultness_span,
410415
constness: self.lower_constness(constness),
411416
generics,
412417
of_trait: trait_ref,
@@ -435,9 +440,6 @@ impl<'hir> LoweringContext<'_, 'hir> {
435440
bug!("`TyMac` should have been expanded by now")
436441
}
437442
}
438-
439-
// [1] `defaultness.has_value()` is never called for an `impl`, always `true` in order to
440-
// not cause an assertion failure inside the `lower_defaultness` function.
441443
}
442444

443445
fn lower_const_item(
@@ -868,27 +870,31 @@ impl<'hir> LoweringContext<'_, 'hir> {
868870
AssocItemKind::MacCall(..) => bug!("`TyMac` should have been expanded by now"),
869871
};
870872

873+
// Since `default impl` is not yet implemented, this is always true in impls.
874+
let has_value = true;
875+
let (defaultness, _) = self.lower_defaultness(i.kind.defaultness(), has_value);
871876
hir::ImplItem {
872877
hir_id: self.lower_node_id(i.id),
873878
ident: i.ident,
874879
attrs: self.lower_attrs(&i.attrs),
875880
generics,
876881
vis: self.lower_visibility(&i.vis, None),
877-
defaultness: self.lower_defaultness(i.kind.defaultness(), true /* [1] */),
882+
defaultness,
878883
kind,
879884
span: i.span,
880885
}
881-
882-
// [1] since `default impl` is not yet implemented, this is always true in impls
883886
}
884887

885888
fn lower_impl_item_ref(&mut self, i: &AssocItem) -> hir::ImplItemRef<'hir> {
889+
// Since `default impl` is not yet implemented, this is always true in impls.
890+
let has_value = true;
891+
let (defaultness, _) = self.lower_defaultness(i.kind.defaultness(), has_value);
886892
hir::ImplItemRef {
887893
id: hir::ImplItemId { hir_id: self.lower_node_id(i.id) },
888894
ident: i.ident,
889895
span: i.span,
890896
vis: self.lower_visibility(&i.vis, Some(i.id)),
891-
defaultness: self.lower_defaultness(i.kind.defaultness(), true /* [1] */),
897+
defaultness,
892898
kind: match &i.kind {
893899
AssocItemKind::Const(..) => hir::AssocItemKind::Const,
894900
AssocItemKind::TyAlias(.., ty) => {
@@ -903,8 +909,6 @@ impl<'hir> LoweringContext<'_, 'hir> {
903909
AssocItemKind::MacCall(..) => unimplemented!(),
904910
},
905911
}
906-
907-
// [1] since `default impl` is not yet implemented, this is always true in impls
908912
}
909913

910914
/// If an `explicit_owner` is given, this method allocates the `HirId` in
@@ -939,12 +943,16 @@ impl<'hir> LoweringContext<'_, 'hir> {
939943
respan(v.span, node)
940944
}
941945

942-
fn lower_defaultness(&self, d: Defaultness, has_value: bool) -> hir::Defaultness {
946+
fn lower_defaultness(
947+
&self,
948+
d: Defaultness,
949+
has_value: bool,
950+
) -> (hir::Defaultness, Option<Span>) {
943951
match d {
944-
Defaultness::Default(_) => hir::Defaultness::Default { has_value },
952+
Defaultness::Default(sp) => (hir::Defaultness::Default { has_value }, Some(sp)),
945953
Defaultness::Final => {
946954
assert!(has_value);
947-
hir::Defaultness::Final
955+
(hir::Defaultness::Final, None)
948956
}
949957
}
950958
}

‎src/librustc_hir/hir.rs‎

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2153,7 +2153,7 @@ pub enum Defaultness {
21532153
impl Defaultness {
21542154
pub fn has_value(&self) -> bool {
21552155
match *self {
2156-
Defaultness::Default { has_value, .. } => has_value,
2156+
Defaultness::Default { has_value } => has_value,
21572157
Defaultness::Final => true,
21582158
}
21592159
}
@@ -2502,6 +2502,9 @@ pub enum ItemKind<'hir> {
25022502
unsafety: Unsafety,
25032503
polarity: ImplPolarity,
25042504
defaultness: Defaultness,
2505+
// We do not put a `Span` in `Defaultness` because it breaks foreign crate metadata
2506+
// decoding as `Span`s cannot be decoded when a `Session` is not available.
2507+
defaultness_span: Option<Span>,
25052508
constness: Constness,
25062509
generics: Generics<'hir>,
25072510

‎src/librustc_hir/intravisit.rs‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -590,6 +590,7 @@ pub fn walk_item<'v, V: Visitor<'v>>(visitor: &mut V, item: &'v Item<'v>) {
590590
defaultness: _,
591591
polarity: _,
592592
constness: _,
593+
defaultness_span: _,
593594
ref generics,
594595
ref of_trait,
595596
ref self_ty,

‎src/librustc_hir/print.rs‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -632,6 +632,7 @@ impl<'a> State<'a> {
632632
polarity,
633633
defaultness,
634634
constness,
635+
defaultness_span: _,
635636
ref generics,
636637
ref of_trait,
637638
ref self_ty,

‎src/librustc_typeck/check/wfcheck.rs‎

Lines changed: 27 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -98,34 +98,50 @@ pub fn check_item_well_formed(tcx: TyCtxt<'_>, def_id: DefId) {
9898
//
9999
// won't be allowed unless there's an *explicit* implementation of `Send`
100100
// for `T`
101-
hir::ItemKind::Impl { defaultness, ref of_trait, ref self_ty, .. } => {
101+
hir::ItemKind::Impl {
102+
defaultness,
103+
defaultness_span,
104+
polarity,
105+
ref of_trait,
106+
ref self_ty,
107+
..
108+
} => {
102109
let is_auto = tcx
103110
.impl_trait_ref(tcx.hir().local_def_id(item.hir_id))
104111
.map_or(false, |trait_ref| tcx.trait_is_auto(trait_ref.def_id));
105-
let polarity = tcx.impl_polarity(def_id);
106112
if let (hir::Defaultness::Default { .. }, true) = (defaultness, is_auto) {
107-
tcx.sess.span_err(item.span, "impls of auto traits cannot be default");
113+
let sp = of_trait.as_ref().map(|t| t.path.span).unwrap_or(item.span);
114+
let mut err =
115+
tcx.sess.struct_span_err(sp, "impls of auto traits cannot be default");
116+
err.span_labels(defaultness_span, "default because of this");
117+
err.span_label(sp, "auto trait");
118+
err.emit();
108119
}
109-
match polarity {
110-
ty::ImplPolarity::Positive => {
120+
// We match on both `ty::ImplPolarity` and `ast::ImplPolarity` just to get the `!` span.
121+
match (tcx.impl_polarity(def_id), polarity) {
122+
(ty::ImplPolarity::Positive, _) => {
111123
check_impl(tcx, item, self_ty, of_trait);
112124
}
113-
ty::ImplPolarity::Negative => {
125+
(ty::ImplPolarity::Negative, ast::ImplPolarity::Negative(span)) => {
114126
// FIXME(#27579): what amount of WF checking do we need for neg impls?
115-
if of_trait.is_some() && !is_auto {
127+
if let (Some(of_trait), false) = (of_trait, is_auto) {
116128
struct_span_err!(
117129
tcx.sess,
118-
item.span,
130+
span.to(of_trait.path.span),
119131
E0192,
120-
"negative impls are only allowed for \
121-
auto traits (e.g., `Send` and `Sync`)"
132+
"invalid negative impl"
133+
)
134+
.note(
135+
"negative impls are only allowed for auto traits, like `Send` and \
136+
`Sync`",
122137
)
123138
.emit()
124139
}
125140
}
126-
ty::ImplPolarity::Reservation => {
141+
(ty::ImplPolarity::Reservation, _) => {
127142
// FIXME: what amount of WF checking do we need for reservation impls?
128143
}
144+
_ => unreachable!(),
129145
}
130146
}
131147
hir::ItemKind::Fn(..) => {

‎src/librustc_typeck/collect.rs‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1580,9 +1580,10 @@ fn impl_polarity(tcx: TyCtxt<'_>, def_id: DefId) -> ty::ImplPolarity {
15801580
let is_rustc_reservation = tcx.has_attr(def_id, sym::rustc_reservation_impl);
15811581
let item = tcx.hir().expect_item(hir_id);
15821582
match &item.kind {
1583-
hir::ItemKind::Impl { polarity: hir::ImplPolarity::Negative(_), .. } => {
1583+
hir::ItemKind::Impl { polarity: hir::ImplPolarity::Negative(span), of_trait, .. } => {
15841584
if is_rustc_reservation {
1585-
tcx.sess.span_err(item.span, "reservation impls can't be negative");
1585+
let span = span.to(of_trait.as_ref().map(|t| t.path.span).unwrap_or(*span));
1586+
tcx.sess.span_err(span, "reservation impls can't be negative");
15861587
}
15871588
ty::ImplPolarity::Negative
15881589
}

‎src/librustdoc/visit_ast.rs‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -563,6 +563,7 @@ impl<'a, 'tcx> RustdocVisitor<'a, 'tcx> {
563563
polarity,
564564
defaultness,
565565
constness,
566+
defaultness_span: _,
566567
ref generics,
567568
ref of_trait,
568569
self_ty,

‎src/test/ui/error-codes/E0192.stderr‎

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,10 @@
1-
error[E0192]: negative impls are only allowed for auto traits (e.g., `Send` and `Sync`)
2-
--> $DIR/E0192.rs:9:1
1+
error[E0192]: invalid negative impl
2+
--> $DIR/E0192.rs:9:6
33
|
44
LL | impl !Trait for Foo { }
5-
| ^^^^^^^^^^^^^^^^^^^^^^^
5+
| ^^^^^^
6+
|
7+
= note: negative impls are only allowed for auto traits, like `Send` and `Sync`
68

79
error: aborting due to previous error
810

‎src/test/ui/specialization/defaultimpl/validation.rs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,6 @@ default unsafe impl Send for S {} //~ ERROR impls of auto traits cannot be defau
1010
default impl !Send for Z {} //~ ERROR impls of auto traits cannot be default
1111

1212
trait Tr {}
13-
default impl !Tr for S {} //~ ERROR negative impls are only allowed for auto traits
13+
default impl !Tr for S {} //~ ERROR invalid negative impl
1414

1515
fn main() {}

‎src/test/ui/specialization/defaultimpl/validation.stderr‎

Lines changed: 13 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -9,22 +9,28 @@ LL | default impl S {}
99
= note: only trait implementations may be annotated with `default`
1010

1111
error: impls of auto traits cannot be default
12-
--> $DIR/validation.rs:9:1
12+
--> $DIR/validation.rs:9:21
1313
|
1414
LL | default unsafe impl Send for S {}
15-
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
15+
| ------- ^^^^ auto trait
16+
| |
17+
| default because of this
1618

1719
error: impls of auto traits cannot be default
18-
--> $DIR/validation.rs:10:1
20+
--> $DIR/validation.rs:10:15
1921
|
2022
LL | default impl !Send for Z {}
21-
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^
23+
| ------- ^^^^ auto trait
24+
| |
25+
| default because of this
2226

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

2935
error: aborting due to 4 previous errors
3036

0 commit comments

Comments
 (0)