From 56e30e1f3f953f3b8b88f46e32a2ec1f5573943c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Esteban=20K=C3=BCber?= Date: Tue, 11 Jun 2019 13:28:22 -0700 Subject: [PATCH 1/2] Tweak transparent enums and unions diagnostic spans --- src/librustc_typeck/check/mod.rs | 68 +++++++++----- .../feature-gate-transparent_enums.stderr | 6 +- .../feature-gate-transparent_unions.stderr | 7 +- src/test/ui/repr/repr-transparent.stderr | 94 +++++++------------ 4 files changed, 83 insertions(+), 92 deletions(-) diff --git a/src/librustc_typeck/check/mod.rs b/src/librustc_typeck/check/mod.rs index c857eac5d3c18..5b51c02b812e0 100644 --- a/src/librustc_typeck/check/mod.rs +++ b/src/librustc_typeck/check/mod.rs @@ -1794,25 +1794,39 @@ fn check_transparent<'tcx>(tcx: TyCtxt<'tcx>, sp: Span, def_id: DefId) { if !adt.repr.transparent() { return; } + let sp = tcx.sess.source_map().def_span(sp); if adt.is_enum() { if !tcx.features().transparent_enums { - emit_feature_err(&tcx.sess.parse_sess, - sym::transparent_enums, - sp, - GateIssue::Language, - "transparent enums are unstable"); + emit_feature_err( + &tcx.sess.parse_sess, + sym::transparent_enums, + sp, + GateIssue::Language, + "transparent enums are unstable", + ); } if adt.variants.len() != 1 { let variant_spans: Vec<_> = adt.variants.iter().map(|variant| { tcx.hir().span_if_local(variant.def_id).unwrap() }).collect(); - let mut err = struct_span_err!(tcx.sess, sp, E0731, - "transparent enum needs exactly one variant, but has {}", - adt.variants.len()); - if !variant_spans.is_empty() { - err.span_note(variant_spans, &format!("the following variants exist on `{}`", - tcx.def_path_str(def_id))); + let msg = format!( + "needs exactly one variant, but has {}", + adt.variants.len(), + ); + let mut err = struct_span_err!(tcx.sess, sp, E0731, "transparent enum {}", msg); + err.span_label(sp, &msg); + match &variant_spans[..] { + &[] => {}, + &[ref start.., ref end] => { + for variant_span in start { + err.span_label(*variant_span, ""); + } + err.span_label(*end, &format!( + "too many variants in `{}`", + tcx.def_path_str(def_id), + )); + }, } err.emit(); if adt.variants.is_empty() { @@ -1847,23 +1861,31 @@ fn check_transparent<'tcx>(tcx: TyCtxt<'tcx>, sp: Span, def_id: DefId) { if non_zst_count != 1 { let field_spans: Vec<_> = non_zst_fields.map(|(span, _zst, _align1)| span).collect(); - let mut err = struct_span_err!(tcx.sess, sp, E0690, - "{}transparent {} needs exactly one non-zero-sized field, but has {}", - if adt.is_enum() { "the variant of a " } else { "" }, - adt.descr(), - non_zst_count); - if !field_spans.is_empty() { - err.span_note(field_spans, - &format!("the following non-zero-sized fields exist on `{}`:", - tcx.def_path_str(def_id))); + let msg = format!("needs exactly one non-zero-sized field, but has {}", non_zst_count); + let mut err = struct_span_err!( + tcx.sess, + sp, + E0690, + "{}transparent {} {}", + if adt.is_enum() { "the variant of a " } else { "" }, + adt.descr(), + msg, + ); + err.span_label(sp, &msg); + for sp in &field_spans { + err.span_label(*sp, "this field is non-zero-sized"); } err.emit(); } for (span, zst, align1) in field_infos { if zst && !align1 { - span_err!(tcx.sess, span, E0691, - "zero-sized field in transparent {} has alignment larger than 1", - adt.descr()); + struct_span_err!( + tcx.sess, + span, + E0691, + "zero-sized field in transparent {} has alignment larger than 1", + adt.descr(), + ).span_label(span, "has alignment larger than 1").emit(); } } } diff --git a/src/test/ui/feature-gates/feature-gate-transparent_enums.stderr b/src/test/ui/feature-gates/feature-gate-transparent_enums.stderr index 4b22654e9e411..8ba079b89f509 100644 --- a/src/test/ui/feature-gates/feature-gate-transparent_enums.stderr +++ b/src/test/ui/feature-gates/feature-gate-transparent_enums.stderr @@ -1,10 +1,8 @@ error[E0658]: transparent enums are unstable --> $DIR/feature-gate-transparent_enums.rs:2:1 | -LL | / enum OkButUnstableEnum { -LL | | Foo((), String, ()), -LL | | } - | |_^ +LL | enum OkButUnstableEnum { + | ^^^^^^^^^^^^^^^^^^^^^^ | = note: for more information, see https://github.com/rust-lang/rust/issues/60405 = help: add #![feature(transparent_enums)] to the crate attributes to enable diff --git a/src/test/ui/feature-gates/feature-gate-transparent_unions.stderr b/src/test/ui/feature-gates/feature-gate-transparent_unions.stderr index 933b227de63b9..341324c3d6764 100644 --- a/src/test/ui/feature-gates/feature-gate-transparent_unions.stderr +++ b/src/test/ui/feature-gates/feature-gate-transparent_unions.stderr @@ -1,11 +1,8 @@ error[E0658]: transparent unions are unstable --> $DIR/feature-gate-transparent_unions.rs:2:1 | -LL | / union OkButUnstableUnion { -LL | | field: u8, -LL | | zst: (), -LL | | } - | |_^ +LL | union OkButUnstableUnion { + | ^^^^^^^^^^^^^^^^^^^^^^^^ | = note: for more information, see https://github.com/rust-lang/rust/issues/60405 = help: add #![feature(transparent_unions)] to the crate attributes to enable diff --git a/src/test/ui/repr/repr-transparent.stderr b/src/test/ui/repr/repr-transparent.stderr index ea16bdf53783d..f0c1fbe8ac9e1 100644 --- a/src/test/ui/repr/repr-transparent.stderr +++ b/src/test/ui/repr/repr-transparent.stderr @@ -2,61 +2,57 @@ error[E0690]: transparent struct needs exactly one non-zero-sized field, but has --> $DIR/repr-transparent.rs:11:1 | LL | struct NoFields; - | ^^^^^^^^^^^^^^^^ + | ^^^^^^^^^^^^^^^^ needs exactly one non-zero-sized field, but has 0 error[E0690]: transparent struct needs exactly one non-zero-sized field, but has 0 --> $DIR/repr-transparent.rs:14:1 | LL | struct ContainsOnlyZst(()); - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^ needs exactly one non-zero-sized field, but has 0 error[E0690]: transparent struct needs exactly one non-zero-sized field, but has 0 --> $DIR/repr-transparent.rs:17:1 | LL | struct ContainsOnlyZstArray([bool; 0]); - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ needs exactly one non-zero-sized field, but has 0 error[E0690]: transparent struct needs exactly one non-zero-sized field, but has 0 --> $DIR/repr-transparent.rs:20:1 | LL | struct ContainsMultipleZst(PhantomData<*const i32>, NoFields); - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ needs exactly one non-zero-sized field, but has 0 error[E0690]: transparent struct needs exactly one non-zero-sized field, but has 2 --> $DIR/repr-transparent.rs:24:1 | LL | struct MultipleNonZst(u8, u8); - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ - | -note: the following non-zero-sized fields exist on `MultipleNonZst`: - --> $DIR/repr-transparent.rs:24:23 - | -LL | struct MultipleNonZst(u8, u8); - | ^^ ^^ + | ^^^^^^^^^^^^^^^^^^^^^^--^^--^^ + | | | | + | | | this field is non-zero-sized + | | this field is non-zero-sized + | needs exactly one non-zero-sized field, but has 2 error[E0690]: transparent struct needs exactly one non-zero-sized field, but has 2 --> $DIR/repr-transparent.rs:30:1 | LL | pub struct StructWithProjection(f32, ::It); - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ - | -note: the following non-zero-sized fields exist on `StructWithProjection`: - --> $DIR/repr-transparent.rs:30:33 - | -LL | pub struct StructWithProjection(f32, ::It); - | ^^^ ^^^^^^^^^^^^^^^^^^^ + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^---^^-------------------^^ + | | | | + | | | this field is non-zero-sized + | | this field is non-zero-sized + | needs exactly one non-zero-sized field, but has 2 error[E0691]: zero-sized field in transparent struct has alignment larger than 1 --> $DIR/repr-transparent.rs:34:32 | LL | struct NontrivialAlignZst(u32, [u16; 0]); - | ^^^^^^^^ + | ^^^^^^^^ has alignment larger than 1 error[E0691]: zero-sized field in transparent struct has alignment larger than 1 --> $DIR/repr-transparent.rs:40:24 | LL | struct GenericAlign(ZstAlign32, u32); - | ^^^^^^^^^^^^^ + | ^^^^^^^^^^^^^ has alignment larger than 1 error[E0084]: unsupported representation for zero-variant enum --> $DIR/repr-transparent.rs:42:1 @@ -70,71 +66,49 @@ error[E0731]: transparent enum needs exactly one variant, but has 0 --> $DIR/repr-transparent.rs:43:1 | LL | enum Void {} - | ^^^^^^^^^^^^ + | ^^^^^^^^^ needs exactly one variant, but has 0 error[E0690]: the variant of a transparent enum needs exactly one non-zero-sized field, but has 0 --> $DIR/repr-transparent.rs:47:1 | -LL | / enum FieldlessEnum { -LL | | Foo, -LL | | } - | |_^ +LL | enum FieldlessEnum { + | ^^^^^^^^^^^^^^^^^^ needs exactly one non-zero-sized field, but has 0 error[E0690]: the variant of a transparent enum needs exactly one non-zero-sized field, but has 2 --> $DIR/repr-transparent.rs:52:1 | -LL | / enum TooManyFieldsEnum { -LL | | Foo(u32, String), -LL | | } - | |_^ - | -note: the following non-zero-sized fields exist on `TooManyFieldsEnum`: - --> $DIR/repr-transparent.rs:53:9 - | +LL | enum TooManyFieldsEnum { + | ^^^^^^^^^^^^^^^^^^^^^^ needs exactly one non-zero-sized field, but has 2 LL | Foo(u32, String), - | ^^^ ^^^^^^ + | --- ------ this field is non-zero-sized + | | + | this field is non-zero-sized error[E0731]: transparent enum needs exactly one variant, but has 2 --> $DIR/repr-transparent.rs:58:1 | -LL | / enum TooManyVariants { -LL | | Foo(String), -LL | | Bar, -LL | | } - | |_^ - | -note: the following variants exist on `TooManyVariants` - --> $DIR/repr-transparent.rs:59:5 - | +LL | enum TooManyVariants { + | ^^^^^^^^^^^^^^^^^^^^ needs exactly one variant, but has 2 LL | Foo(String), - | ^^^^^^^^^^^ + | ----------- LL | Bar, - | ^^^ + | --- too many variants in `TooManyVariants` error[E0690]: transparent union needs exactly one non-zero-sized field, but has 0 --> $DIR/repr-transparent.rs:64:1 | -LL | / union UnitUnion { -LL | | u: (), -LL | | } - | |_^ +LL | union UnitUnion { + | ^^^^^^^^^^^^^^^ needs exactly one non-zero-sized field, but has 0 error[E0690]: transparent union needs exactly one non-zero-sized field, but has 2 --> $DIR/repr-transparent.rs:69:1 | -LL | / union TooManyFields { -LL | | u: u32, -LL | | s: i32 -LL | | } - | |_^ - | -note: the following non-zero-sized fields exist on `TooManyFields`: - --> $DIR/repr-transparent.rs:70:5 - | +LL | union TooManyFields { + | ^^^^^^^^^^^^^^^^^^^ needs exactly one non-zero-sized field, but has 2 LL | u: u32, - | ^^^^^^ + | ------ this field is non-zero-sized LL | s: i32 - | ^^^^^^ + | ------ this field is non-zero-sized error: aborting due to 15 previous errors From f06b76122cde7ddd4f6778d77952b63baa70bdfe Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Esteban=20K=C3=BCber?= Date: Tue, 11 Jun 2019 18:21:53 -0700 Subject: [PATCH 2/2] review comments: move diagnostic code out of happy path --- src/librustc_typeck/check/mod.rs | 93 ++++++++++++++++++-------------- 1 file changed, 53 insertions(+), 40 deletions(-) diff --git a/src/librustc_typeck/check/mod.rs b/src/librustc_typeck/check/mod.rs index 5b51c02b812e0..2cb955c7c2ef0 100644 --- a/src/librustc_typeck/check/mod.rs +++ b/src/librustc_typeck/check/mod.rs @@ -1789,6 +1789,52 @@ fn check_packed_inner<'tcx>(tcx: TyCtxt<'tcx>, def_id: DefId, stack: &mut Vec(tcx: TyCtxt<'tcx>, adt: &'tcx ty::AdtDef, sp: Span, did: DefId) { + let variant_spans: Vec<_> = adt.variants.iter().map(|variant| { + tcx.hir().span_if_local(variant.def_id).unwrap() + }).collect(); + let msg = format!( + "needs exactly one variant, but has {}", + adt.variants.len(), + ); + let mut err = struct_span_err!(tcx.sess, sp, E0731, "transparent enum {}", msg); + err.span_label(sp, &msg); + if let &[ref start.., ref end] = &variant_spans[..] { + for variant_span in start { + err.span_label(*variant_span, ""); + } + err.span_label(*end, &format!("too many variants in `{}`", tcx.def_path_str(did))); + } + err.emit(); +} + +/// Emit an error when encountering more or less than one non-zero-sized field in a transparent +/// enum. +fn bad_non_zero_sized_fields<'tcx>( + tcx: TyCtxt<'tcx>, + adt: &'tcx ty::AdtDef, + field_count: usize, + field_spans: impl Iterator, + sp: Span, +) { + let msg = format!("needs exactly one non-zero-sized field, but has {}", field_count); + let mut err = struct_span_err!( + tcx.sess, + sp, + E0690, + "{}transparent {} {}", + if adt.is_enum() { "the variant of a " } else { "" }, + adt.descr(), + msg, + ); + err.span_label(sp, &msg); + for sp in field_spans { + err.span_label(sp, "this field is non-zero-sized"); + } + err.emit(); +} + fn check_transparent<'tcx>(tcx: TyCtxt<'tcx>, sp: Span, def_id: DefId) { let adt = tcx.adt_def(def_id); if !adt.repr.transparent() { @@ -1807,28 +1853,7 @@ fn check_transparent<'tcx>(tcx: TyCtxt<'tcx>, sp: Span, def_id: DefId) { ); } if adt.variants.len() != 1 { - let variant_spans: Vec<_> = adt.variants.iter().map(|variant| { - tcx.hir().span_if_local(variant.def_id).unwrap() - }).collect(); - let msg = format!( - "needs exactly one variant, but has {}", - adt.variants.len(), - ); - let mut err = struct_span_err!(tcx.sess, sp, E0731, "transparent enum {}", msg); - err.span_label(sp, &msg); - match &variant_spans[..] { - &[] => {}, - &[ref start.., ref end] => { - for variant_span in start { - err.span_label(*variant_span, ""); - } - err.span_label(*end, &format!( - "too many variants in `{}`", - tcx.def_path_str(def_id), - )); - }, - } - err.emit(); + bad_variant_count(tcx, adt, sp, def_id); if adt.variants.is_empty() { // Don't bother checking the fields. No variants (and thus no fields) exist. return; @@ -1856,26 +1881,14 @@ fn check_transparent<'tcx>(tcx: TyCtxt<'tcx>, sp: Span, def_id: DefId) { (span, zst, align1) }); - let non_zst_fields = field_infos.clone().filter(|(_span, zst, _align1)| !*zst); + let non_zst_fields = field_infos.clone().filter_map(|(span, zst, _align1)| if !zst { + Some(span) + } else { + None + }); let non_zst_count = non_zst_fields.clone().count(); if non_zst_count != 1 { - let field_spans: Vec<_> = non_zst_fields.map(|(span, _zst, _align1)| span).collect(); - - let msg = format!("needs exactly one non-zero-sized field, but has {}", non_zst_count); - let mut err = struct_span_err!( - tcx.sess, - sp, - E0690, - "{}transparent {} {}", - if adt.is_enum() { "the variant of a " } else { "" }, - adt.descr(), - msg, - ); - err.span_label(sp, &msg); - for sp in &field_spans { - err.span_label(*sp, "this field is non-zero-sized"); - } - err.emit(); + bad_non_zero_sized_fields(tcx, adt, non_zst_count, non_zst_fields, sp); } for (span, zst, align1) in field_infos { if zst && !align1 {