From 09c6c54715e4586d87f65f71a68ada24581a2aef Mon Sep 17 00:00:00 2001 From: Lukas Markeffsky <@> Date: Mon, 13 Jun 2022 22:46:44 +0200 Subject: [PATCH 1/4] Improve diagnostic for E0691 --- compiler/rustc_typeck/src/check/check.rs | 51 ++++++++++++++++----- src/test/ui/repr/repr-transparent.rs | 8 ++++ src/test/ui/repr/repr-transparent.stderr | 58 ++++++++++++++++++++---- 3 files changed, 97 insertions(+), 20 deletions(-) diff --git a/compiler/rustc_typeck/src/check/check.rs b/compiler/rustc_typeck/src/check/check.rs index 80abb28ee58c1..4888d38d49daf 100644 --- a/compiler/rustc_typeck/src/check/check.rs +++ b/compiler/rustc_typeck/src/check/check.rs @@ -20,7 +20,9 @@ use rustc_middle::hir::nested_filter; use rustc_middle::ty::layout::{LayoutError, MAX_SIMD_LANES}; use rustc_middle::ty::subst::GenericArgKind; use rustc_middle::ty::util::{Discr, IntTypeExt}; -use rustc_middle::ty::{self, ParamEnv, ToPredicate, Ty, TyCtxt, TypeFoldable, TypeSuperFoldable}; +use rustc_middle::ty::{ + self, ParamEnv, ToPredicate, Ty, TyCtxt, TyKind, TypeFoldable, TypeSuperFoldable, +}; use rustc_session::lint::builtin::{UNINHABITED_STATIC, UNSUPPORTED_CALLING_CONVENTIONS}; use rustc_span::symbol::sym; use rustc_span::{self, Span}; @@ -1327,28 +1329,53 @@ pub(super) fn check_transparent<'tcx>(tcx: TyCtxt<'tcx>, sp: Span, adt: ty::AdtD let layout = tcx.layout_of(param_env.and(ty)); // We are currently checking the type this field came from, so it must be local let span = tcx.hir().span_if_local(field.did).unwrap(); - let zst = layout.map_or(false, |layout| layout.is_zst()); - let align1 = layout.map_or(false, |layout| layout.align.abi.bytes() == 1); - (span, zst, align1) + let array_len = match ty.kind() { + TyKind::Array(_, len) => len.try_eval_usize(tcx, param_env), + _ => None, + }; + let zst = array_len == Some(0) || layout.map_or(false, |layout| layout.is_zst()); + let align = layout.ok().map(|layout| layout.align.abi.bytes()); + (span, zst, align) }); let non_zst_fields = - field_infos.clone().filter_map(|(span, zst, _align1)| if !zst { Some(span) } else { None }); + field_infos.clone().filter_map(|(span, zst, _align)| if !zst { Some(span) } else { None }); let non_zst_count = non_zst_fields.clone().count(); if non_zst_count >= 2 { bad_non_zero_sized_fields(tcx, adt, non_zst_count, non_zst_fields, sp); } - for (span, zst, align1) in field_infos { - if zst && !align1 { - struct_span_err!( + for (span, zst, align) in field_infos { + if zst && align != Some(1) { + let mut err = 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(); + ); + + if align.is_none() { + err.span_label(span, "may have alignment larger than 1"); + } else { + err.span_label(span, "has alignment larger than 1"); + }; + + if let Some(item_list) = + tcx.get_attr(adt.did(), sym::repr).and_then(|attr| attr.meta_item_list()) + { + for item in item_list { + if item.name_or_empty() == sym::transparent { + err.span_suggestion_verbose( + item.span(), + "Try using `#[repc(C)]` instead", + "C", + Applicability::MachineApplicable, + ); + } + } + } + + err.emit(); } } } @@ -1475,7 +1502,7 @@ fn format_discriminant_overflow<'tcx>( && let rustc_ast::LitKind::Int(lit_value, _int_kind) = &lit.node && dis.val != *lit_value { - return format!("`{dis}` (overflowed from `{lit_value}`)"); + return format!("`{dis}` (overflowed from `{lit_value}`)"); } } diff --git a/src/test/ui/repr/repr-transparent.rs b/src/test/ui/repr/repr-transparent.rs index 8c9d1639c0a51..63573b816b5fd 100644 --- a/src/test/ui/repr/repr-transparent.rs +++ b/src/test/ui/repr/repr-transparent.rs @@ -41,6 +41,9 @@ struct ZstAlign32(PhantomData); #[repr(transparent)] struct GenericAlign(ZstAlign32, u32); //~ ERROR alignment larger than 1 +#[repr(transparent)] +struct GenericAlignZeroArray([T; 0], u32); //~ ERROR alignment larger than 1 + #[repr(transparent)] //~ ERROR unsupported representation for zero-variant enum enum Void {} //~ ERROR transparent enum needs exactly one variant, but has 0 @@ -76,6 +79,11 @@ enum GenericAlignEnum { Foo { bar: ZstAlign32, baz: u32 } //~ ERROR alignment larger than 1 } +#[repr(transparent)] +enum GenericAlignEnumZeroArray { + Foo { bar: [T; 0], baz: u32 } //~ ERROR alignment larger than 1 +} + #[repr(transparent)] union UnitUnion { u: (), diff --git a/src/test/ui/repr/repr-transparent.stderr b/src/test/ui/repr/repr-transparent.stderr index 001a181881f14..d1e6b68f4c7e4 100644 --- a/src/test/ui/repr/repr-transparent.stderr +++ b/src/test/ui/repr/repr-transparent.stderr @@ -23,15 +23,36 @@ error[E0691]: zero-sized field in transparent struct has alignment larger than 1 | LL | struct NontrivialAlignZst(u32, [u16; 0]); | ^^^^^^^^ has alignment larger than 1 + | +help: Try using `#[repc(C)]` instead + | +LL | #[repr(C)] + | ~ error[E0691]: zero-sized field in transparent struct has alignment larger than 1 --> $DIR/repr-transparent.rs:42:24 | LL | struct GenericAlign(ZstAlign32, u32); | ^^^^^^^^^^^^^ has alignment larger than 1 + | +help: Try using `#[repc(C)]` instead + | +LL | #[repr(C)] + | ~ + +error[E0691]: zero-sized field in transparent struct has alignment larger than 1 + --> $DIR/repr-transparent.rs:45:33 + | +LL | struct GenericAlignZeroArray([T; 0], u32); + | ^^^^^^ may have alignment larger than 1 + | +help: Try using `#[repc(C)]` instead + | +LL | #[repr(C)] + | ~ error[E0084]: unsupported representation for zero-variant enum - --> $DIR/repr-transparent.rs:44:1 + --> $DIR/repr-transparent.rs:47:1 | LL | #[repr(transparent)] | ^^^^^^^^^^^^^^^^^^^^ @@ -39,13 +60,13 @@ LL | enum Void {} | ------------ zero-variant enum error[E0731]: transparent enum needs exactly one variant, but has 0 - --> $DIR/repr-transparent.rs:45:1 + --> $DIR/repr-transparent.rs:48:1 | LL | enum Void {} | ^^^^^^^^^ needs exactly one variant, but has 0 error[E0690]: the variant of a transparent enum needs at most one non-zero-sized field, but has 2 - --> $DIR/repr-transparent.rs:58:1 + --> $DIR/repr-transparent.rs:61:1 | LL | enum TooManyFieldsEnum { | ^^^^^^^^^^^^^^^^^^^^^^ needs at most one non-zero-sized field, but has 2 @@ -55,7 +76,7 @@ LL | Foo(u32, String), | this field is non-zero-sized error[E0731]: transparent enum needs exactly one variant, but has 2 - --> $DIR/repr-transparent.rs:64:1 + --> $DIR/repr-transparent.rs:67:1 | LL | enum MultipleVariants { | ^^^^^^^^^^^^^^^^^^^^^ needs exactly one variant, but has 2 @@ -65,19 +86,40 @@ LL | Bar, | --- too many variants in `MultipleVariants` error[E0691]: zero-sized field in transparent enum has alignment larger than 1 - --> $DIR/repr-transparent.rs:71:14 + --> $DIR/repr-transparent.rs:74:14 | LL | Foo(u32, [u16; 0]), | ^^^^^^^^ has alignment larger than 1 + | +help: Try using `#[repc(C)]` instead + | +LL | #[repr(C)] + | ~ error[E0691]: zero-sized field in transparent enum has alignment larger than 1 - --> $DIR/repr-transparent.rs:76:11 + --> $DIR/repr-transparent.rs:79:11 | LL | Foo { bar: ZstAlign32, baz: u32 } | ^^^^^^^^^^^^^^^^^^ has alignment larger than 1 + | +help: Try using `#[repc(C)]` instead + | +LL | #[repr(C)] + | ~ + +error[E0691]: zero-sized field in transparent enum has alignment larger than 1 + --> $DIR/repr-transparent.rs:84:11 + | +LL | Foo { bar: [T; 0], baz: u32 } + | ^^^^^^^^^^^ may have alignment larger than 1 + | +help: Try using `#[repc(C)]` instead + | +LL | #[repr(C)] + | ~ error[E0690]: transparent union needs at most one non-zero-sized field, but has 2 - --> $DIR/repr-transparent.rs:85:1 + --> $DIR/repr-transparent.rs:93:1 | LL | union TooManyFields { | ^^^^^^^^^^^^^^^^^^^ needs at most one non-zero-sized field, but has 2 @@ -86,7 +128,7 @@ LL | u: u32, LL | s: i32 | ------ this field is non-zero-sized -error: aborting due to 11 previous errors +error: aborting due to 13 previous errors Some errors have detailed explanations: E0084, E0690, E0691, E0731. For more information about an error, try `rustc --explain E0084`. From bd3d1ae1ebbd417c8a702911100af8a8ca309aae Mon Sep 17 00:00:00 2001 From: Lukas Markeffsky <@> Date: Mon, 13 Jun 2022 23:32:04 +0200 Subject: [PATCH 2/4] fix error (maybe), use short/inline suggestions --- compiler/rustc_typeck/src/check/check.rs | 30 ++++++++-------- src/test/ui/repr/repr-transparent.stderr | 45 ++++++++---------------- 2 files changed, 30 insertions(+), 45 deletions(-) diff --git a/compiler/rustc_typeck/src/check/check.rs b/compiler/rustc_typeck/src/check/check.rs index 4888d38d49daf..97c0afa960041 100644 --- a/compiler/rustc_typeck/src/check/check.rs +++ b/compiler/rustc_typeck/src/check/check.rs @@ -21,7 +21,7 @@ use rustc_middle::ty::layout::{LayoutError, MAX_SIMD_LANES}; use rustc_middle::ty::subst::GenericArgKind; use rustc_middle::ty::util::{Discr, IntTypeExt}; use rustc_middle::ty::{ - self, ParamEnv, ToPredicate, Ty, TyCtxt, TyKind, TypeFoldable, TypeSuperFoldable, + self, Array, ParamEnv, ToPredicate, Ty, TyCtxt, TypeFoldable, TypeSuperFoldable, }; use rustc_session::lint::builtin::{UNINHABITED_STATIC, UNSUPPORTED_CALLING_CONVENTIONS}; use rustc_span::symbol::sym; @@ -1206,17 +1206,17 @@ pub(super) fn check_packed(tcx: TyCtxt<'_>, sp: Span, def: ty::AdtDef<'_>) { for attr in tcx.get_attrs(def.did(), sym::repr) { for r in attr::parse_repr_attr(&tcx.sess, attr) { if let attr::ReprPacked(pack) = r - && let Some(repr_pack) = repr.pack - && pack as u64 != repr_pack.bytes() - { - struct_span_err!( - tcx.sess, - sp, - E0634, - "type has conflicting packed representation hints" - ) - .emit(); - } + && let Some(repr_pack) = repr.pack + && pack as u64 != repr_pack.bytes() + { + struct_span_err!( + tcx.sess, + sp, + E0634, + "type has conflicting packed representation hints" + ) + .emit(); + } } } if repr.align.is_some() { @@ -1330,7 +1330,7 @@ pub(super) fn check_transparent<'tcx>(tcx: TyCtxt<'tcx>, sp: Span, adt: ty::AdtD // We are currently checking the type this field came from, so it must be local let span = tcx.hir().span_if_local(field.did).unwrap(); let array_len = match ty.kind() { - TyKind::Array(_, len) => len.try_eval_usize(tcx, param_env), + Array(_, len) => len.try_eval_usize(tcx, param_env), _ => None, }; let zst = array_len == Some(0) || layout.map_or(false, |layout| layout.is_zst()); @@ -1365,9 +1365,9 @@ pub(super) fn check_transparent<'tcx>(tcx: TyCtxt<'tcx>, sp: Span, adt: ty::AdtD { for item in item_list { if item.name_or_empty() == sym::transparent { - err.span_suggestion_verbose( + err.span_suggestion_short( item.span(), - "Try using `#[repc(C)]` instead", + "Try using `repr(C)` instead", "C", Applicability::MachineApplicable, ); diff --git a/src/test/ui/repr/repr-transparent.stderr b/src/test/ui/repr/repr-transparent.stderr index d1e6b68f4c7e4..af95f173a7b98 100644 --- a/src/test/ui/repr/repr-transparent.stderr +++ b/src/test/ui/repr/repr-transparent.stderr @@ -21,35 +21,26 @@ LL | pub struct StructWithProjection(f32, ::It); error[E0691]: zero-sized field in transparent struct has alignment larger than 1 --> $DIR/repr-transparent.rs:36:32 | +LL | #[repr(transparent)] + | ----------- help: Try using `repr(C)` instead LL | struct NontrivialAlignZst(u32, [u16; 0]); | ^^^^^^^^ has alignment larger than 1 - | -help: Try using `#[repc(C)]` instead - | -LL | #[repr(C)] - | ~ error[E0691]: zero-sized field in transparent struct has alignment larger than 1 --> $DIR/repr-transparent.rs:42:24 | +LL | #[repr(transparent)] + | ----------- help: Try using `repr(C)` instead LL | struct GenericAlign(ZstAlign32, u32); | ^^^^^^^^^^^^^ has alignment larger than 1 - | -help: Try using `#[repc(C)]` instead - | -LL | #[repr(C)] - | ~ error[E0691]: zero-sized field in transparent struct has alignment larger than 1 --> $DIR/repr-transparent.rs:45:33 | +LL | #[repr(transparent)] + | ----------- help: Try using `repr(C)` instead LL | struct GenericAlignZeroArray([T; 0], u32); | ^^^^^^ may have alignment larger than 1 - | -help: Try using `#[repc(C)]` instead - | -LL | #[repr(C)] - | ~ error[E0084]: unsupported representation for zero-variant enum --> $DIR/repr-transparent.rs:47:1 @@ -88,35 +79,29 @@ LL | Bar, error[E0691]: zero-sized field in transparent enum has alignment larger than 1 --> $DIR/repr-transparent.rs:74:14 | +LL | #[repr(transparent)] + | ----------- help: Try using `repr(C)` instead +LL | enum NontrivialAlignZstEnum { LL | Foo(u32, [u16; 0]), | ^^^^^^^^ has alignment larger than 1 - | -help: Try using `#[repc(C)]` instead - | -LL | #[repr(C)] - | ~ error[E0691]: zero-sized field in transparent enum has alignment larger than 1 --> $DIR/repr-transparent.rs:79:11 | +LL | #[repr(transparent)] + | ----------- help: Try using `repr(C)` instead +LL | enum GenericAlignEnum { LL | Foo { bar: ZstAlign32, baz: u32 } | ^^^^^^^^^^^^^^^^^^ has alignment larger than 1 - | -help: Try using `#[repc(C)]` instead - | -LL | #[repr(C)] - | ~ error[E0691]: zero-sized field in transparent enum has alignment larger than 1 --> $DIR/repr-transparent.rs:84:11 | +LL | #[repr(transparent)] + | ----------- help: Try using `repr(C)` instead +LL | enum GenericAlignEnumZeroArray { LL | Foo { bar: [T; 0], baz: u32 } | ^^^^^^^^^^^ may have alignment larger than 1 - | -help: Try using `#[repc(C)]` instead - | -LL | #[repr(C)] - | ~ error[E0690]: transparent union needs at most one non-zero-sized field, but has 2 --> $DIR/repr-transparent.rs:93:1 From 5c2d5dc5d4dcc8c85d121e17510acb3f213d5b6f Mon Sep 17 00:00:00 2001 From: Lukas Markeffsky <@> Date: Tue, 14 Jun 2022 00:07:28 +0200 Subject: [PATCH 3/4] print alignment in error message, if it is known --- compiler/rustc_typeck/src/check/check.rs | 6 +++--- src/test/ui/repr/repr-transparent.stderr | 8 ++++---- 2 files changed, 7 insertions(+), 7 deletions(-) diff --git a/compiler/rustc_typeck/src/check/check.rs b/compiler/rustc_typeck/src/check/check.rs index 97c0afa960041..fc3f8d027d31f 100644 --- a/compiler/rustc_typeck/src/check/check.rs +++ b/compiler/rustc_typeck/src/check/check.rs @@ -1354,10 +1354,10 @@ pub(super) fn check_transparent<'tcx>(tcx: TyCtxt<'tcx>, sp: Span, adt: ty::AdtD adt.descr(), ); - if align.is_none() { - err.span_label(span, "may have alignment larger than 1"); + if let Some(align) = align { + err.span_label(span, format!("has alignment of {align}, which is larger than 1")); } else { - err.span_label(span, "has alignment larger than 1"); + err.span_label(span, "may have alignment larger than 1"); }; if let Some(item_list) = diff --git a/src/test/ui/repr/repr-transparent.stderr b/src/test/ui/repr/repr-transparent.stderr index af95f173a7b98..bbdbbe05690e5 100644 --- a/src/test/ui/repr/repr-transparent.stderr +++ b/src/test/ui/repr/repr-transparent.stderr @@ -24,7 +24,7 @@ error[E0691]: zero-sized field in transparent struct has alignment larger than 1 LL | #[repr(transparent)] | ----------- help: Try using `repr(C)` instead LL | struct NontrivialAlignZst(u32, [u16; 0]); - | ^^^^^^^^ has alignment larger than 1 + | ^^^^^^^^ has alignment of 2, which is larger than 1 error[E0691]: zero-sized field in transparent struct has alignment larger than 1 --> $DIR/repr-transparent.rs:42:24 @@ -32,7 +32,7 @@ error[E0691]: zero-sized field in transparent struct has alignment larger than 1 LL | #[repr(transparent)] | ----------- help: Try using `repr(C)` instead LL | struct GenericAlign(ZstAlign32, u32); - | ^^^^^^^^^^^^^ has alignment larger than 1 + | ^^^^^^^^^^^^^ has alignment of 32, which is larger than 1 error[E0691]: zero-sized field in transparent struct has alignment larger than 1 --> $DIR/repr-transparent.rs:45:33 @@ -83,7 +83,7 @@ LL | #[repr(transparent)] | ----------- help: Try using `repr(C)` instead LL | enum NontrivialAlignZstEnum { LL | Foo(u32, [u16; 0]), - | ^^^^^^^^ has alignment larger than 1 + | ^^^^^^^^ has alignment of 2, which is larger than 1 error[E0691]: zero-sized field in transparent enum has alignment larger than 1 --> $DIR/repr-transparent.rs:79:11 @@ -92,7 +92,7 @@ LL | #[repr(transparent)] | ----------- help: Try using `repr(C)` instead LL | enum GenericAlignEnum { LL | Foo { bar: ZstAlign32, baz: u32 } - | ^^^^^^^^^^^^^^^^^^ has alignment larger than 1 + | ^^^^^^^^^^^^^^^^^^ has alignment of 32, which is larger than 1 error[E0691]: zero-sized field in transparent enum has alignment larger than 1 --> $DIR/repr-transparent.rs:84:11 From ebd42e5a05586841e8237c67e613e44090cd8a31 Mon Sep 17 00:00:00 2001 From: Lukas Markeffsky <@> Date: Tue, 14 Jun 2022 11:07:22 +0200 Subject: [PATCH 4/4] Split `zero_sized_array` from `zst` --- compiler/rustc_typeck/src/check/check.rs | 21 +++++++++++++-------- 1 file changed, 13 insertions(+), 8 deletions(-) diff --git a/compiler/rustc_typeck/src/check/check.rs b/compiler/rustc_typeck/src/check/check.rs index fc3f8d027d31f..91629edb3b23b 100644 --- a/compiler/rustc_typeck/src/check/check.rs +++ b/compiler/rustc_typeck/src/check/check.rs @@ -1329,23 +1329,28 @@ pub(super) fn check_transparent<'tcx>(tcx: TyCtxt<'tcx>, sp: Span, adt: ty::AdtD let layout = tcx.layout_of(param_env.and(ty)); // We are currently checking the type this field came from, so it must be local let span = tcx.hir().span_if_local(field.did).unwrap(); - let array_len = match ty.kind() { - Array(_, len) => len.try_eval_usize(tcx, param_env), - _ => None, + let zero_sized_array = match ty.kind() { + Array(_, len) => len.try_eval_usize(tcx, param_env) == Some(0), + _ => false, }; - let zst = array_len == Some(0) || layout.map_or(false, |layout| layout.is_zst()); + let zst = layout.map_or(false, |layout| layout.is_zst()); let align = layout.ok().map(|layout| layout.align.abi.bytes()); - (span, zst, align) + (span, zst, zero_sized_array, align) }); let non_zst_fields = - field_infos.clone().filter_map(|(span, zst, _align)| if !zst { Some(span) } else { None }); + field_infos.clone().filter_map( + |(span, zst, zero_sized_array, _align)| { + if !zst && !zero_sized_array { Some(span) } else { None } + }, + ); let non_zst_count = non_zst_fields.clone().count(); if non_zst_count >= 2 { bad_non_zero_sized_fields(tcx, adt, non_zst_count, non_zst_fields, sp); } - for (span, zst, align) in field_infos { - if zst && align != Some(1) { + + for (span, zst, zero_sized_array, align) in field_infos { + if (zst || zero_sized_array) && align != Some(1) { let mut err = struct_span_err!( tcx.sess, span,