From 8c0f6011057b5b2f7bd4d74ce63caf8d6630c39e Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Sat, 17 Aug 2019 12:57:44 +0200 Subject: [PATCH 01/18] fix Miri discriminant load/store when overflows are involved --- src/librustc_mir/interpret/operand.rs | 40 +++++++++++++++++++++---- src/librustc_mir/interpret/place.rs | 42 ++++++++++++++++++++------- 2 files changed, 65 insertions(+), 17 deletions(-) diff --git a/src/librustc_mir/interpret/operand.rs b/src/librustc_mir/interpret/operand.rs index 9ad1542905b68..3b07202f898ba 100644 --- a/src/librustc_mir/interpret/operand.rs +++ b/src/librustc_mir/interpret/operand.rs @@ -609,15 +609,20 @@ impl<'mir, 'tcx, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> { ) -> InterpResult<'tcx, (u128, VariantIdx)> { trace!("read_discriminant_value {:#?}", rval.layout); - let (discr_kind, discr_index) = match rval.layout.variants { + let (discr_layout, discr_kind, discr_index) = match rval.layout.variants { layout::Variants::Single { index } => { let discr_val = rval.layout.ty.discriminant_for_variant(*self.tcx, index).map_or( index.as_u32() as u128, |discr| discr.val); return Ok((discr_val, index)); } - layout::Variants::Multiple { ref discr_kind, discr_index, .. } => - (discr_kind, discr_index), + layout::Variants::Multiple { + discr: ref discr_layout, + ref discr_kind, + discr_index, + .. + } => + (discr_layout, discr_kind, discr_index), }; // read raw discriminant value @@ -634,7 +639,7 @@ impl<'mir, 'tcx, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> { .map_err(|_| err_unsup!(InvalidDiscriminant(raw_discr.erase_tag())))?; let real_discr = if discr_val.layout.ty.is_signed() { // going from layout tag type to typeck discriminant type - // requires first sign extending with the layout discriminant + // requires first sign extending with the discriminant layout let sexted = sign_extend(bits_discr, discr_val.layout.size) as i128; // and then zeroing with the typeck discriminant type let discr_ty = rval.layout.ty @@ -682,8 +687,31 @@ impl<'mir, 'tcx, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> { (dataful_variant.as_u32() as u128, dataful_variant) }, Ok(raw_discr) => { - let adjusted_discr = raw_discr.wrapping_sub(niche_start) - .wrapping_add(variants_start); + // FIXME: WTF, some discriminants don't have integer type. + use layout::Primitive; + let discr_layout = self.layout_of(match discr_layout.value { + Primitive::Int(int, signed) => int.to_ty(*self.tcx, signed), + Primitive::Pointer => self.tcx.types.usize, + Primitive::Float(..) => bug!("there are no float discriminants"), + })?; + let discr_val = ImmTy::from_uint(raw_discr, discr_layout); + // We need to use machine arithmetic. + let niche_start_val = ImmTy::from_uint(niche_start, discr_layout); + let variants_start_val = ImmTy::from_uint(variants_start, discr_layout); + let adjusted_discr = self.binary_op( + mir::BinOp::Sub, + discr_val, + niche_start_val, + )?; + let adjusted_discr = self.binary_op( + mir::BinOp::Add, + adjusted_discr, + variants_start_val, + )?; + let adjusted_discr = adjusted_discr + .to_scalar()? + .assert_bits(discr_val.layout.size); + // Check if this is in the range that indicates an actual discriminant. if variants_start <= adjusted_discr && adjusted_discr <= variants_end { let index = adjusted_discr as usize; assert_eq!(index as u128, adjusted_discr); diff --git a/src/librustc_mir/interpret/place.rs b/src/librustc_mir/interpret/place.rs index 1a28548618206..0a23c1222b4bf 100644 --- a/src/librustc_mir/interpret/place.rs +++ b/src/librustc_mir/interpret/place.rs @@ -8,7 +8,9 @@ use std::hash::Hash; use rustc::mir; use rustc::mir::interpret::truncate; use rustc::ty::{self, Ty}; -use rustc::ty::layout::{self, Size, Align, LayoutOf, TyLayout, HasDataLayout, VariantIdx}; +use rustc::ty::layout::{ + self, Size, Align, LayoutOf, TyLayout, HasDataLayout, VariantIdx, IntegerExt +}; use rustc::ty::TypeFoldable; use super::{ @@ -1027,7 +1029,7 @@ where } layout::Variants::Multiple { discr_kind: layout::DiscriminantKind::Tag, - ref discr, + discr: ref discr_layout, discr_index, .. } => { @@ -1038,7 +1040,7 @@ where // raw discriminants for enums are isize or bigger during // their computation, but the in-memory tag is the smallest possible // representation - let size = discr.value.size(self); + let size = discr_layout.value.size(self); let discr_val = truncate(discr_val, size); let discr_dest = self.place_field(dest, discr_index as u64)?; @@ -1050,6 +1052,7 @@ where ref niche_variants, niche_start, }, + discr: ref discr_layout, discr_index, .. } => { @@ -1057,15 +1060,32 @@ where variant_index.as_usize() < dest.layout.ty.ty_adt_def().unwrap().variants.len(), ); if variant_index != dataful_variant { - let niche_dest = - self.place_field(dest, discr_index as u64)?; - let niche_value = variant_index.as_u32() - niche_variants.start().as_u32(); - let niche_value = (niche_value as u128) - .wrapping_add(niche_start); - self.write_scalar( - Scalar::from_uint(niche_value, niche_dest.layout.size), - niche_dest + // FIXME: WTF, some discriminants don't have integer type. + use layout::Primitive; + let discr_layout = self.layout_of(match discr_layout.value { + Primitive::Int(int, signed) => int.to_ty(*self.tcx, signed), + Primitive::Pointer => self.tcx.types.usize, + Primitive::Float(..) => bug!("there are no float discriminants"), + })?; + + // We need to use machine arithmetic. + let variants_start = niche_variants.start().as_u32(); + let variants_start_val = ImmTy::from_uint(variants_start, discr_layout); + let niche_start_val = ImmTy::from_uint(niche_start, discr_layout); + let variant_index_val = ImmTy::from_uint(variant_index.as_u32(), discr_layout); + let niche_val = self.binary_op( + mir::BinOp::Sub, + variant_index_val, + variants_start_val, + )?; + let niche_val = self.binary_op( + mir::BinOp::Add, + niche_val, + niche_start_val, )?; + // Write result. + let niche_dest = self.place_field(dest, discr_index as u64)?; + self.write_immediate(*niche_val, niche_dest)?; } } } From b73f1a51dcd1011129dc5c5c55a7577134410dbc Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Mon, 16 Sep 2019 13:08:21 +0200 Subject: [PATCH 02/18] better and more consistent variable names --- src/librustc_mir/interpret/operand.rs | 16 ++++++++-------- src/librustc_mir/interpret/place.rs | 8 ++++---- 2 files changed, 12 insertions(+), 12 deletions(-) diff --git a/src/librustc_mir/interpret/operand.rs b/src/librustc_mir/interpret/operand.rs index 3b07202f898ba..2a0bad2fa746c 100644 --- a/src/librustc_mir/interpret/operand.rs +++ b/src/librustc_mir/interpret/operand.rs @@ -698,28 +698,28 @@ impl<'mir, 'tcx, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> { // We need to use machine arithmetic. let niche_start_val = ImmTy::from_uint(niche_start, discr_layout); let variants_start_val = ImmTy::from_uint(variants_start, discr_layout); - let adjusted_discr = self.binary_op( + let variant_index_relative_val = self.binary_op( mir::BinOp::Sub, discr_val, niche_start_val, )?; - let adjusted_discr = self.binary_op( + let variant_index_val = self.binary_op( mir::BinOp::Add, - adjusted_discr, + variant_index_relative_val, variants_start_val, )?; - let adjusted_discr = adjusted_discr + let variant_index = variant_index_val .to_scalar()? .assert_bits(discr_val.layout.size); // Check if this is in the range that indicates an actual discriminant. - if variants_start <= adjusted_discr && adjusted_discr <= variants_end { - let index = adjusted_discr as usize; - assert_eq!(index as u128, adjusted_discr); + if variants_start <= variant_index && variant_index <= variants_end { + let index = variant_index as usize; + assert_eq!(index as u128, variant_index); assert!(index < rval.layout.ty .ty_adt_def() .expect("tagged layout for non adt") .variants.len()); - (adjusted_discr, VariantIdx::from_usize(index)) + (variant_index, VariantIdx::from_usize(index)) } else { (dataful_variant.as_u32() as u128, dataful_variant) } diff --git a/src/librustc_mir/interpret/place.rs b/src/librustc_mir/interpret/place.rs index 0a23c1222b4bf..636b070a3d8f3 100644 --- a/src/librustc_mir/interpret/place.rs +++ b/src/librustc_mir/interpret/place.rs @@ -1073,19 +1073,19 @@ where let variants_start_val = ImmTy::from_uint(variants_start, discr_layout); let niche_start_val = ImmTy::from_uint(niche_start, discr_layout); let variant_index_val = ImmTy::from_uint(variant_index.as_u32(), discr_layout); - let niche_val = self.binary_op( + let variant_index_relative_val = self.binary_op( mir::BinOp::Sub, variant_index_val, variants_start_val, )?; - let niche_val = self.binary_op( + let discr_val = self.binary_op( mir::BinOp::Add, - niche_val, + variant_index_relative_val, niche_start_val, )?; // Write result. let niche_dest = self.place_field(dest, discr_index as u64)?; - self.write_immediate(*niche_val, niche_dest)?; + self.write_immediate(*discr_val, niche_dest)?; } } } From 75fe84f4fc764e0684e7fdafbe6139cc92082a44 Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Mon, 16 Sep 2019 13:15:23 +0200 Subject: [PATCH 03/18] factor getting the discriminant layout to a new method --- src/librustc/ty/layout.rs | 11 +++++++++++ src/librustc_mir/interpret/operand.rs | 10 ++-------- src/librustc_mir/interpret/place.rs | 11 ++--------- 3 files changed, 15 insertions(+), 17 deletions(-) diff --git a/src/librustc/ty/layout.rs b/src/librustc/ty/layout.rs index 5ec4754c4535b..e52feea1624c1 100644 --- a/src/librustc/ty/layout.rs +++ b/src/librustc/ty/layout.rs @@ -127,6 +127,7 @@ impl IntegerExt for Integer { pub trait PrimitiveExt { fn to_ty<'tcx>(&self, tcx: TyCtxt<'tcx>) -> Ty<'tcx>; + fn to_int_ty<'tcx>(&self, tcx: TyCtxt<'tcx>) -> Ty<'tcx>; } impl PrimitiveExt for Primitive { @@ -138,6 +139,16 @@ impl PrimitiveExt for Primitive { Pointer => tcx.mk_mut_ptr(tcx.mk_unit()), } } + + /// Return an *integer* type matching this primitive. + /// Useful in particular when dealing with enum discriminants. + fn to_int_ty(&self, tcx: TyCtxt<'tcx>) -> Ty<'tcx> { + match *self { + Int(i, signed) => i.to_ty(tcx, signed), + Pointer => tcx.types.usize, + Float(..) => bug!("floats do not have an int type"), + } + } } /// The first half of a fat pointer. diff --git a/src/librustc_mir/interpret/operand.rs b/src/librustc_mir/interpret/operand.rs index 2a0bad2fa746c..94fbd4ac7d7f3 100644 --- a/src/librustc_mir/interpret/operand.rs +++ b/src/librustc_mir/interpret/operand.rs @@ -5,7 +5,7 @@ use std::convert::TryInto; use rustc::{mir, ty}; use rustc::ty::layout::{ - self, Size, LayoutOf, TyLayout, HasDataLayout, IntegerExt, VariantIdx, + self, Size, LayoutOf, TyLayout, HasDataLayout, IntegerExt, PrimitiveExt, VariantIdx, }; use rustc::mir::interpret::{ @@ -687,13 +687,7 @@ impl<'mir, 'tcx, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> { (dataful_variant.as_u32() as u128, dataful_variant) }, Ok(raw_discr) => { - // FIXME: WTF, some discriminants don't have integer type. - use layout::Primitive; - let discr_layout = self.layout_of(match discr_layout.value { - Primitive::Int(int, signed) => int.to_ty(*self.tcx, signed), - Primitive::Pointer => self.tcx.types.usize, - Primitive::Float(..) => bug!("there are no float discriminants"), - })?; + let discr_layout = self.layout_of(discr_layout.value.to_int_ty(*self.tcx))?; let discr_val = ImmTy::from_uint(raw_discr, discr_layout); // We need to use machine arithmetic. let niche_start_val = ImmTy::from_uint(niche_start, discr_layout); diff --git a/src/librustc_mir/interpret/place.rs b/src/librustc_mir/interpret/place.rs index 636b070a3d8f3..9a3d70144a586 100644 --- a/src/librustc_mir/interpret/place.rs +++ b/src/librustc_mir/interpret/place.rs @@ -9,7 +9,7 @@ use rustc::mir; use rustc::mir::interpret::truncate; use rustc::ty::{self, Ty}; use rustc::ty::layout::{ - self, Size, Align, LayoutOf, TyLayout, HasDataLayout, VariantIdx, IntegerExt + self, Size, Align, LayoutOf, TyLayout, HasDataLayout, VariantIdx, PrimitiveExt }; use rustc::ty::TypeFoldable; @@ -1060,14 +1060,7 @@ where variant_index.as_usize() < dest.layout.ty.ty_adt_def().unwrap().variants.len(), ); if variant_index != dataful_variant { - // FIXME: WTF, some discriminants don't have integer type. - use layout::Primitive; - let discr_layout = self.layout_of(match discr_layout.value { - Primitive::Int(int, signed) => int.to_ty(*self.tcx, signed), - Primitive::Pointer => self.tcx.types.usize, - Primitive::Float(..) => bug!("there are no float discriminants"), - })?; - + let discr_layout = self.layout_of(discr_layout.value.to_int_ty(*self.tcx))?; // We need to use machine arithmetic. let variants_start = niche_variants.start().as_u32(); let variants_start_val = ImmTy::from_uint(variants_start, discr_layout); From 61a28d5c881d81cca9156527851911bb5aa5b818 Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Mon, 16 Sep 2019 15:04:33 +0200 Subject: [PATCH 04/18] do the variant idx computations on the host (non-overflowing) --- src/librustc_mir/interpret/operand.rs | 14 ++++++-------- src/librustc_mir/interpret/place.rs | 16 +++++++--------- 2 files changed, 13 insertions(+), 17 deletions(-) diff --git a/src/librustc_mir/interpret/operand.rs b/src/librustc_mir/interpret/operand.rs index 94fbd4ac7d7f3..9e7ae32f1eacf 100644 --- a/src/librustc_mir/interpret/operand.rs +++ b/src/librustc_mir/interpret/operand.rs @@ -687,24 +687,22 @@ impl<'mir, 'tcx, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> { (dataful_variant.as_u32() as u128, dataful_variant) }, Ok(raw_discr) => { + // We need to use machine arithmetic to get the relative variant idx. let discr_layout = self.layout_of(discr_layout.value.to_int_ty(*self.tcx))?; let discr_val = ImmTy::from_uint(raw_discr, discr_layout); - // We need to use machine arithmetic. let niche_start_val = ImmTy::from_uint(niche_start, discr_layout); - let variants_start_val = ImmTy::from_uint(variants_start, discr_layout); let variant_index_relative_val = self.binary_op( mir::BinOp::Sub, discr_val, niche_start_val, )?; - let variant_index_val = self.binary_op( - mir::BinOp::Add, - variant_index_relative_val, - variants_start_val, - )?; - let variant_index = variant_index_val + let variant_index_relative = variant_index_relative_val .to_scalar()? .assert_bits(discr_val.layout.size); + // Then computing the absolute variant idx should not overflow any more. + let variant_index = variants_start + .checked_add(variant_index_relative) + .expect("oveflow computing absolute variant idx"); // Check if this is in the range that indicates an actual discriminant. if variants_start <= variant_index && variant_index <= variants_end { let index = variant_index as usize; diff --git a/src/librustc_mir/interpret/place.rs b/src/librustc_mir/interpret/place.rs index 9a3d70144a586..5f4903d61e3b2 100644 --- a/src/librustc_mir/interpret/place.rs +++ b/src/librustc_mir/interpret/place.rs @@ -1060,17 +1060,15 @@ where variant_index.as_usize() < dest.layout.ty.ty_adt_def().unwrap().variants.len(), ); if variant_index != dataful_variant { - let discr_layout = self.layout_of(discr_layout.value.to_int_ty(*self.tcx))?; - // We need to use machine arithmetic. let variants_start = niche_variants.start().as_u32(); - let variants_start_val = ImmTy::from_uint(variants_start, discr_layout); + let variant_index_relative = variant_index.as_u32() + .checked_sub(variants_start) + .expect("overflow computing relative variant idx"); + // We need to use machine arithmetic when taking into account `niche_start`. + let discr_layout = self.layout_of(discr_layout.value.to_int_ty(*self.tcx))?; let niche_start_val = ImmTy::from_uint(niche_start, discr_layout); - let variant_index_val = ImmTy::from_uint(variant_index.as_u32(), discr_layout); - let variant_index_relative_val = self.binary_op( - mir::BinOp::Sub, - variant_index_val, - variants_start_val, - )?; + let variant_index_relative_val = + ImmTy::from_uint(variant_index_relative, discr_layout); let discr_val = self.binary_op( mir::BinOp::Add, variant_index_relative_val, From b21622cd6cc6c88cc6760e76ba1442aee1e2b9a8 Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Mon, 16 Sep 2019 16:25:39 +0200 Subject: [PATCH 05/18] add test --- .../miri_unleashed/enum_discriminants.rs | 110 ++++++++++++++++++ .../miri_unleashed/enum_discriminants.stderr | 72 ++++++++++++ 2 files changed, 182 insertions(+) create mode 100644 src/test/ui/consts/miri_unleashed/enum_discriminants.rs create mode 100644 src/test/ui/consts/miri_unleashed/enum_discriminants.stderr diff --git a/src/test/ui/consts/miri_unleashed/enum_discriminants.rs b/src/test/ui/consts/miri_unleashed/enum_discriminants.rs new file mode 100644 index 0000000000000..9f34fc73953a4 --- /dev/null +++ b/src/test/ui/consts/miri_unleashed/enum_discriminants.rs @@ -0,0 +1,110 @@ +// compile-flags: -Zunleash-the-miri-inside-of-you +// run-pass + +//! Make sure that we read and write enum discriminants correctly for corner cases caused +//! by layout optimizations. + +const OVERFLOW: usize = { + // Tests for https://github.com/rust-lang/rust/issues/62138. + #[repr(u8)] + #[allow(dead_code)] + enum WithWraparoundInvalidValues { + X = 1, + Y = 254, + } + + #[allow(dead_code)] + enum Foo { + A, + B, + C(WithWraparoundInvalidValues), + } + + let x = Foo::B; + match x { + Foo::B => 0, + _ => panic!(), + } +}; + +const MORE_OVERFLOW: usize = { + pub enum Infallible {} + + // The check that the `bool` field of `V1` is encoding a "niche variant" + // (i.e. not `V1`, so `V3` or `V4`) used to be mathematically incorrect, + // causing valid `V1` values to be interpreted as other variants. + #[allow(dead_code)] + pub enum E1 { + V1 { f: bool }, + V2 { f: Infallible }, + V3, + V4, + } + + // Computing the discriminant used to be done using the niche type (here `u8`, + // from the `bool` field of `V1`), overflowing for variants with large enough + // indices (`V3` and `V4`), causing them to be interpreted as other variants. + #[allow(dead_code)] + pub enum E2 { + V1 { f: bool }, + + /*_00*/ _01(X), _02(X), _03(X), _04(X), _05(X), _06(X), _07(X), + _08(X), _09(X), _0A(X), _0B(X), _0C(X), _0D(X), _0E(X), _0F(X), + _10(X), _11(X), _12(X), _13(X), _14(X), _15(X), _16(X), _17(X), + _18(X), _19(X), _1A(X), _1B(X), _1C(X), _1D(X), _1E(X), _1F(X), + _20(X), _21(X), _22(X), _23(X), _24(X), _25(X), _26(X), _27(X), + _28(X), _29(X), _2A(X), _2B(X), _2C(X), _2D(X), _2E(X), _2F(X), + _30(X), _31(X), _32(X), _33(X), _34(X), _35(X), _36(X), _37(X), + _38(X), _39(X), _3A(X), _3B(X), _3C(X), _3D(X), _3E(X), _3F(X), + _40(X), _41(X), _42(X), _43(X), _44(X), _45(X), _46(X), _47(X), + _48(X), _49(X), _4A(X), _4B(X), _4C(X), _4D(X), _4E(X), _4F(X), + _50(X), _51(X), _52(X), _53(X), _54(X), _55(X), _56(X), _57(X), + _58(X), _59(X), _5A(X), _5B(X), _5C(X), _5D(X), _5E(X), _5F(X), + _60(X), _61(X), _62(X), _63(X), _64(X), _65(X), _66(X), _67(X), + _68(X), _69(X), _6A(X), _6B(X), _6C(X), _6D(X), _6E(X), _6F(X), + _70(X), _71(X), _72(X), _73(X), _74(X), _75(X), _76(X), _77(X), + _78(X), _79(X), _7A(X), _7B(X), _7C(X), _7D(X), _7E(X), _7F(X), + _80(X), _81(X), _82(X), _83(X), _84(X), _85(X), _86(X), _87(X), + _88(X), _89(X), _8A(X), _8B(X), _8C(X), _8D(X), _8E(X), _8F(X), + _90(X), _91(X), _92(X), _93(X), _94(X), _95(X), _96(X), _97(X), + _98(X), _99(X), _9A(X), _9B(X), _9C(X), _9D(X), _9E(X), _9F(X), + _A0(X), _A1(X), _A2(X), _A3(X), _A4(X), _A5(X), _A6(X), _A7(X), + _A8(X), _A9(X), _AA(X), _AB(X), _AC(X), _AD(X), _AE(X), _AF(X), + _B0(X), _B1(X), _B2(X), _B3(X), _B4(X), _B5(X), _B6(X), _B7(X), + _B8(X), _B9(X), _BA(X), _BB(X), _BC(X), _BD(X), _BE(X), _BF(X), + _C0(X), _C1(X), _C2(X), _C3(X), _C4(X), _C5(X), _C6(X), _C7(X), + _C8(X), _C9(X), _CA(X), _CB(X), _CC(X), _CD(X), _CE(X), _CF(X), + _D0(X), _D1(X), _D2(X), _D3(X), _D4(X), _D5(X), _D6(X), _D7(X), + _D8(X), _D9(X), _DA(X), _DB(X), _DC(X), _DD(X), _DE(X), _DF(X), + _E0(X), _E1(X), _E2(X), _E3(X), _E4(X), _E5(X), _E6(X), _E7(X), + _E8(X), _E9(X), _EA(X), _EB(X), _EC(X), _ED(X), _EE(X), _EF(X), + _F0(X), _F1(X), _F2(X), _F3(X), _F4(X), _F5(X), _F6(X), _F7(X), + _F8(X), _F9(X), _FA(X), _FB(X), _FC(X), _FD(X), _FE(X), _FF(X), + + V3, + V4, + } + + if let E1::V2 { .. } = (E1::V1 { f: true }) { + unreachable!() + } + if let E1::V1 { .. } = (E1::V1 { f: true }) { + } else { + unreachable!() + } + + if let E2::V1 { .. } = E2::V3:: { + unreachable!() + } + if let E2::V3 { .. } = E2::V3:: { + } else { + unreachable!() + } + + 0 +}; + +fn main() { + assert_eq!(OVERFLOW, 0); + assert_eq!(MORE_OVERFLOW, 0); +} diff --git a/src/test/ui/consts/miri_unleashed/enum_discriminants.stderr b/src/test/ui/consts/miri_unleashed/enum_discriminants.stderr new file mode 100644 index 0000000000000..8ca81ad22b72b --- /dev/null +++ b/src/test/ui/consts/miri_unleashed/enum_discriminants.stderr @@ -0,0 +1,72 @@ +warning: skipping const checks + --> $DIR/enum_discriminants.rs:23:13 + | +LL | let x = Foo::B; + | ^^^^^^ + +warning: skipping const checks + --> $DIR/enum_discriminants.rs:25:9 + | +LL | Foo::B => 0, + | ^^^^^^ + +warning: skipping const checks + --> $DIR/enum_discriminants.rs:88:28 + | +LL | if let E1::V2 { .. } = (E1::V1 { f: true }) { + | ^^^^^^^^^^^^^^^^^^^^ + +warning: skipping const checks + --> $DIR/enum_discriminants.rs:88:12 + | +LL | if let E1::V2 { .. } = (E1::V1 { f: true }) { + | ^^^^^^^^^^^^^ + +warning: skipping const checks + --> $DIR/enum_discriminants.rs:108:5 + | +LL | assert_eq!(OVERFLOW, 0); + | ^^^^^^^^^^^^^^^^^^^^^^^^ + | + = note: this warning originates in a macro outside of the current crate (in Nightly builds, run with -Z external-macro-backtrace for more info) + +warning: skipping const checks + --> $DIR/enum_discriminants.rs:108:5 + | +LL | assert_eq!(OVERFLOW, 0); + | ^^^^^^^^^^^^^^^^^^^^^^^^ + | + = note: this warning originates in a macro outside of the current crate (in Nightly builds, run with -Z external-macro-backtrace for more info) + +warning: skipping const checks + --> $DIR/enum_discriminants.rs:108:5 + | +LL | assert_eq!(OVERFLOW, 0); + | ^^^^^^^^^^^^^^^^^^^^^^^^ + | + = note: this warning originates in a macro outside of the current crate (in Nightly builds, run with -Z external-macro-backtrace for more info) + +warning: skipping const checks + --> $DIR/enum_discriminants.rs:109:5 + | +LL | assert_eq!(MORE_OVERFLOW, 0); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | + = note: this warning originates in a macro outside of the current crate (in Nightly builds, run with -Z external-macro-backtrace for more info) + +warning: skipping const checks + --> $DIR/enum_discriminants.rs:109:5 + | +LL | assert_eq!(MORE_OVERFLOW, 0); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | + = note: this warning originates in a macro outside of the current crate (in Nightly builds, run with -Z external-macro-backtrace for more info) + +warning: skipping const checks + --> $DIR/enum_discriminants.rs:109:5 + | +LL | assert_eq!(MORE_OVERFLOW, 0); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | + = note: this warning originates in a macro outside of the current crate (in Nightly builds, run with -Z external-macro-backtrace for more info) + From 822393d690eb7c238c18c1bb0b1e7831c4776cd3 Mon Sep 17 00:00:00 2001 From: Aaron Hill Date: Wed, 18 Sep 2019 18:22:13 -0400 Subject: [PATCH 06/18] Point at original span when emitting unreachable lint Fixes #64590 When we emit an 'unreachable' lint, we now add a note pointing at the expression that actually causes the code to be unreachable (e.g. `return`, `break`, `panic`). This is especially useful when macros are involved, since a diverging expression might be hidden inside of a macro invocation. --- src/librustc_typeck/check/_match.rs | 4 +-- src/librustc_typeck/check/expr.rs | 2 +- src/librustc_typeck/check/mod.rs | 25 +++++++++++++------ src/test/ui/dead-code-ret.stderr | 5 ++++ src/test/ui/if-ret.stderr | 5 ++++ src/test/ui/issues/issue-2150.stderr | 6 +++++ src/test/ui/issues/issue-7246.stderr | 5 ++++ .../ui/lint/lint-attr-non-item-node.stderr | 5 ++++ src/test/ui/liveness/liveness-unused.stderr | 5 ++++ .../match-no-arms-unreachable-after.stderr | 5 ++++ src/test/ui/never-assign-dead-code.stderr | 12 +++++++++ src/test/ui/reachable/expr_add.stderr | 5 ++++ src/test/ui/reachable/expr_again.stderr | 5 ++++ src/test/ui/reachable/expr_array.stderr | 11 ++++++++ src/test/ui/reachable/expr_assign.stderr | 17 +++++++++++++ src/test/ui/reachable/expr_block.stderr | 10 ++++++++ src/test/ui/reachable/expr_box.stderr | 5 ++++ src/test/ui/reachable/expr_call.stderr | 11 ++++++++ src/test/ui/reachable/expr_cast.stderr | 5 ++++ src/test/ui/reachable/expr_if.stderr | 10 ++++++++ src/test/ui/reachable/expr_loop.stderr | 15 +++++++++++ src/test/ui/reachable/expr_match.stderr | 10 ++++++++ src/test/ui/reachable/expr_method.stderr | 11 ++++++++ src/test/ui/reachable/expr_repeat.stderr | 5 ++++ src/test/ui/reachable/expr_return.stderr | 5 ++++ src/test/ui/reachable/expr_return_in_macro.rs | 15 +++++++++++ .../ui/reachable/expr_return_in_macro.stderr | 22 ++++++++++++++++ src/test/ui/reachable/expr_struct.stderr | 23 +++++++++++++++++ src/test/ui/reachable/expr_tup.stderr | 11 ++++++++ src/test/ui/reachable/expr_type.stderr | 5 ++++ src/test/ui/reachable/expr_unary.stderr | 5 ++++ src/test/ui/reachable/expr_while.stderr | 11 ++++++++ .../protect-precedences.stderr | 5 ++++ .../ui/unreachable/unreachable-code.stderr | 5 ++++ .../ui/unreachable/unreachable-in-call.stderr | 11 ++++++++ .../unreachable-try-pattern.stderr | 5 ++++ .../unwarned-match-on-never.stderr | 17 +++++++++++++ 37 files changed, 328 insertions(+), 11 deletions(-) create mode 100644 src/test/ui/reachable/expr_return_in_macro.rs create mode 100644 src/test/ui/reachable/expr_return_in_macro.stderr diff --git a/src/librustc_typeck/check/_match.rs b/src/librustc_typeck/check/_match.rs index 308a3d8ebc2cf..a1c937b95ba8c 100644 --- a/src/librustc_typeck/check/_match.rs +++ b/src/librustc_typeck/check/_match.rs @@ -43,7 +43,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { // If there are no arms, that is a diverging match; a special case. if arms.is_empty() { - self.diverges.set(self.diverges.get() | Diverges::Always); + self.diverges.set(self.diverges.get() | Diverges::Always(expr.span)); return tcx.types.never; } @@ -69,7 +69,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { // warnings). match all_pats_diverge { Diverges::Maybe => Diverges::Maybe, - Diverges::Always | Diverges::WarnedAlways => Diverges::WarnedAlways, + Diverges::Always(..) | Diverges::WarnedAlways => Diverges::WarnedAlways, } }).collect(); diff --git a/src/librustc_typeck/check/expr.rs b/src/librustc_typeck/check/expr.rs index 56bd903040ab4..3ba453748ff32 100644 --- a/src/librustc_typeck/check/expr.rs +++ b/src/librustc_typeck/check/expr.rs @@ -170,7 +170,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { // Any expression that produces a value of type `!` must have diverged if ty.is_never() { - self.diverges.set(self.diverges.get() | Diverges::Always); + self.diverges.set(self.diverges.get() | Diverges::Always(expr.span)); } // Record the type, which applies it effects. diff --git a/src/librustc_typeck/check/mod.rs b/src/librustc_typeck/check/mod.rs index 1197160fa9501..0fced37394616 100644 --- a/src/librustc_typeck/check/mod.rs +++ b/src/librustc_typeck/check/mod.rs @@ -450,7 +450,10 @@ pub enum Diverges { /// Definitely known to diverge and therefore /// not reach the next sibling or its parent. - Always, + /// The `Span` points to the expression + /// that caused us to diverge + /// (e.g. `return`, `break`, etc) + Always(Span), /// Same as `Always` but with a reachability /// warning already emitted. @@ -487,7 +490,7 @@ impl ops::BitOrAssign for Diverges { impl Diverges { fn always(self) -> bool { - self >= Diverges::Always + self >= Diverges::Always(DUMMY_SP) } } @@ -2307,17 +2310,23 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { /// Produces warning on the given node, if the current point in the /// function is unreachable, and there hasn't been another warning. fn warn_if_unreachable(&self, id: hir::HirId, span: Span, kind: &str) { - if self.diverges.get() == Diverges::Always && + // FIXME: Combine these two 'if' expressions into one once + // let chains are implemented + if let Diverges::Always(orig_span) = self.diverges.get() { // If span arose from a desugaring of `if` or `while`, then it is the condition itself, // which diverges, that we are about to lint on. This gives suboptimal diagnostics. // Instead, stop here so that the `if`- or `while`-expression's block is linted instead. - !span.is_desugaring(DesugaringKind::CondTemporary) { - self.diverges.set(Diverges::WarnedAlways); + if !span.is_desugaring(DesugaringKind::CondTemporary) { + self.diverges.set(Diverges::WarnedAlways); - debug!("warn_if_unreachable: id={:?} span={:?} kind={}", id, span, kind); + debug!("warn_if_unreachable: id={:?} span={:?} kind={}", id, span, kind); - let msg = format!("unreachable {}", kind); - self.tcx().lint_hir(lint::builtin::UNREACHABLE_CODE, id, span, &msg); + let msg = format!("unreachable {}", kind); + let mut err = self.tcx().struct_span_lint_hir(lint::builtin::UNREACHABLE_CODE, + id, span, &msg); + err.span_note(orig_span, "any code following this expression is unreachable"); + err.emit(); + } } } diff --git a/src/test/ui/dead-code-ret.stderr b/src/test/ui/dead-code-ret.stderr index 092a176f443b2..0ce31ea40ddbb 100644 --- a/src/test/ui/dead-code-ret.stderr +++ b/src/test/ui/dead-code-ret.stderr @@ -9,6 +9,11 @@ note: lint level defined here | LL | #![deny(unreachable_code)] | ^^^^^^^^^^^^^^^^ +note: any code following this expression is unreachable + --> $DIR/dead-code-ret.rs:6:5 + | +LL | return; + | ^^^^^^ = note: this error originates in a macro outside of the current crate (in Nightly builds, run with -Z external-macro-backtrace for more info) error: aborting due to previous error diff --git a/src/test/ui/if-ret.stderr b/src/test/ui/if-ret.stderr index 73402e55a4fd8..2df8f22944ee8 100644 --- a/src/test/ui/if-ret.stderr +++ b/src/test/ui/if-ret.stderr @@ -5,4 +5,9 @@ LL | fn foo() { if (return) { } } | ^^^ | = note: `#[warn(unreachable_code)]` on by default +note: any code following this expression is unreachable + --> $DIR/if-ret.rs:6:15 + | +LL | fn foo() { if (return) { } } + | ^^^^^^^^ diff --git a/src/test/ui/issues/issue-2150.stderr b/src/test/ui/issues/issue-2150.stderr index 59000f3dd5338..623f098d0b337 100644 --- a/src/test/ui/issues/issue-2150.stderr +++ b/src/test/ui/issues/issue-2150.stderr @@ -9,6 +9,12 @@ note: lint level defined here | LL | #![deny(unreachable_code)] | ^^^^^^^^^^^^^^^^ +note: any code following this expression is unreachable + --> $DIR/issue-2150.rs:7:5 + | +LL | panic!(); + | ^^^^^^^^^ + = note: this error originates in a macro outside of the current crate (in Nightly builds, run with -Z external-macro-backtrace for more info) error: aborting due to previous error diff --git a/src/test/ui/issues/issue-7246.stderr b/src/test/ui/issues/issue-7246.stderr index e049295e9136f..d1b23672dc7f4 100644 --- a/src/test/ui/issues/issue-7246.stderr +++ b/src/test/ui/issues/issue-7246.stderr @@ -9,6 +9,11 @@ note: lint level defined here | LL | #![deny(unreachable_code)] | ^^^^^^^^^^^^^^^^ +note: any code following this expression is unreachable + --> $DIR/issue-7246.rs:6:5 + | +LL | return; + | ^^^^^^ error: aborting due to previous error diff --git a/src/test/ui/lint/lint-attr-non-item-node.stderr b/src/test/ui/lint/lint-attr-non-item-node.stderr index 6eb72c098df5c..e6c76c24c9110 100644 --- a/src/test/ui/lint/lint-attr-non-item-node.stderr +++ b/src/test/ui/lint/lint-attr-non-item-node.stderr @@ -9,6 +9,11 @@ note: lint level defined here | LL | #[deny(unreachable_code)] | ^^^^^^^^^^^^^^^^ +note: any code following this expression is unreachable + --> $DIR/lint-attr-non-item-node.rs:6:9 + | +LL | break; + | ^^^^^ error: aborting due to previous error diff --git a/src/test/ui/liveness/liveness-unused.stderr b/src/test/ui/liveness/liveness-unused.stderr index 40a677c08f2b0..84e9b5bab99ff 100644 --- a/src/test/ui/liveness/liveness-unused.stderr +++ b/src/test/ui/liveness/liveness-unused.stderr @@ -10,6 +10,11 @@ note: lint level defined here LL | #![warn(unused)] | ^^^^^^ = note: `#[warn(unreachable_code)]` implied by `#[warn(unused)]` +note: any code following this expression is unreachable + --> $DIR/liveness-unused.rs:91:9 + | +LL | continue; + | ^^^^^^^^ error: unused variable: `x` --> $DIR/liveness-unused.rs:8:7 diff --git a/src/test/ui/match/match-no-arms-unreachable-after.stderr b/src/test/ui/match/match-no-arms-unreachable-after.stderr index 65dcc6ee46554..6c46b2473cce6 100644 --- a/src/test/ui/match/match-no-arms-unreachable-after.stderr +++ b/src/test/ui/match/match-no-arms-unreachable-after.stderr @@ -9,6 +9,11 @@ note: lint level defined here | LL | #![deny(unreachable_code)] | ^^^^^^^^^^^^^^^^ +note: any code following this expression is unreachable + --> $DIR/match-no-arms-unreachable-after.rs:7:5 + | +LL | match v { } + | ^^^^^^^^^^^ error: aborting due to previous error diff --git a/src/test/ui/never-assign-dead-code.stderr b/src/test/ui/never-assign-dead-code.stderr index b1b5bf03fe52a..436c703e4b6b9 100644 --- a/src/test/ui/never-assign-dead-code.stderr +++ b/src/test/ui/never-assign-dead-code.stderr @@ -10,12 +10,24 @@ note: lint level defined here LL | #![warn(unused)] | ^^^^^^ = note: `#[warn(unreachable_code)]` implied by `#[warn(unused)]` +note: any code following this expression is unreachable + --> $DIR/never-assign-dead-code.rs:9:16 + | +LL | let x: ! = panic!("aah"); + | ^^^^^^^^^^^^^ + = note: this warning originates in a macro outside of the current crate (in Nightly builds, run with -Z external-macro-backtrace for more info) warning: unreachable call --> $DIR/never-assign-dead-code.rs:10:5 | LL | drop(x); | ^^^^ + | +note: any code following this expression is unreachable + --> $DIR/never-assign-dead-code.rs:10:10 + | +LL | drop(x); + | ^ warning: unused variable: `x` --> $DIR/never-assign-dead-code.rs:9:9 diff --git a/src/test/ui/reachable/expr_add.stderr b/src/test/ui/reachable/expr_add.stderr index 02b29021cb61a..47b4e467abecb 100644 --- a/src/test/ui/reachable/expr_add.stderr +++ b/src/test/ui/reachable/expr_add.stderr @@ -9,6 +9,11 @@ note: lint level defined here | LL | #![deny(unreachable_code)] | ^^^^^^^^^^^^^^^^ +note: any code following this expression is unreachable + --> $DIR/expr_add.rs:17:19 + | +LL | let x = Foo + return; + | ^^^^^^ error: aborting due to previous error diff --git a/src/test/ui/reachable/expr_again.stderr b/src/test/ui/reachable/expr_again.stderr index bdc3d143ea57e..8e246d940fd8d 100644 --- a/src/test/ui/reachable/expr_again.stderr +++ b/src/test/ui/reachable/expr_again.stderr @@ -9,6 +9,11 @@ note: lint level defined here | LL | #![deny(unreachable_code)] | ^^^^^^^^^^^^^^^^ +note: any code following this expression is unreachable + --> $DIR/expr_again.rs:7:9 + | +LL | continue; + | ^^^^^^^^ = note: this error originates in a macro outside of the current crate (in Nightly builds, run with -Z external-macro-backtrace for more info) error: aborting due to previous error diff --git a/src/test/ui/reachable/expr_array.stderr b/src/test/ui/reachable/expr_array.stderr index 18d7ffe74bd18..419a332e632f3 100644 --- a/src/test/ui/reachable/expr_array.stderr +++ b/src/test/ui/reachable/expr_array.stderr @@ -9,12 +9,23 @@ note: lint level defined here | LL | #![deny(unreachable_code)] | ^^^^^^^^^^^^^^^^ +note: any code following this expression is unreachable + --> $DIR/expr_array.rs:9:26 + | +LL | let x: [usize; 2] = [return, 22]; + | ^^^^^^ error: unreachable expression --> $DIR/expr_array.rs:14:25 | LL | let x: [usize; 2] = [22, return]; | ^^^^^^^^^^^^ + | +note: any code following this expression is unreachable + --> $DIR/expr_array.rs:14:30 + | +LL | let x: [usize; 2] = [22, return]; + | ^^^^^^ error: aborting due to 2 previous errors diff --git a/src/test/ui/reachable/expr_assign.stderr b/src/test/ui/reachable/expr_assign.stderr index def16d90a7490..7388fb4a6b9b2 100644 --- a/src/test/ui/reachable/expr_assign.stderr +++ b/src/test/ui/reachable/expr_assign.stderr @@ -9,18 +9,35 @@ note: lint level defined here | LL | #![deny(unreachable_code)] | ^^^^^^^^^^^^^^^^ +note: any code following this expression is unreachable + --> $DIR/expr_assign.rs:10:9 + | +LL | x = return; + | ^^^^^^ error: unreachable expression --> $DIR/expr_assign.rs:20:14 | LL | *p = return; | ^^^^^^ + | +note: any code following this expression is unreachable + --> $DIR/expr_assign.rs:20:9 + | +LL | *p = return; + | ^^ error: unreachable expression --> $DIR/expr_assign.rs:26:15 | LL | *{return; &mut i} = 22; | ^^^^^^ + | +note: any code following this expression is unreachable + --> $DIR/expr_assign.rs:26:7 + | +LL | *{return; &mut i} = 22; + | ^^^^^^ error: aborting due to 3 previous errors diff --git a/src/test/ui/reachable/expr_block.stderr b/src/test/ui/reachable/expr_block.stderr index a498502e6cb1a..03a6139d688bb 100644 --- a/src/test/ui/reachable/expr_block.stderr +++ b/src/test/ui/reachable/expr_block.stderr @@ -9,6 +9,11 @@ note: lint level defined here | LL | #![deny(unreachable_code)] | ^^^^^^^^^^^^^^^^ +note: any code following this expression is unreachable + --> $DIR/expr_block.rs:9:9 + | +LL | return; + | ^^^^^^ error: unreachable statement --> $DIR/expr_block.rs:25:9 @@ -16,6 +21,11 @@ error: unreachable statement LL | println!("foo"); | ^^^^^^^^^^^^^^^^ | +note: any code following this expression is unreachable + --> $DIR/expr_block.rs:24:9 + | +LL | return; + | ^^^^^^ = note: this error originates in a macro outside of the current crate (in Nightly builds, run with -Z external-macro-backtrace for more info) error: aborting due to 2 previous errors diff --git a/src/test/ui/reachable/expr_box.stderr b/src/test/ui/reachable/expr_box.stderr index 63137ce3da180..d0f666d2be44b 100644 --- a/src/test/ui/reachable/expr_box.stderr +++ b/src/test/ui/reachable/expr_box.stderr @@ -9,6 +9,11 @@ note: lint level defined here | LL | #![deny(unreachable_code)] | ^^^^^^^^^^^^^^^^ +note: any code following this expression is unreachable + --> $DIR/expr_box.rs:6:17 + | +LL | let x = box return; + | ^^^^^^ error: aborting due to previous error diff --git a/src/test/ui/reachable/expr_call.stderr b/src/test/ui/reachable/expr_call.stderr index f2db17e4dfe8e..3fcea90e7cd87 100644 --- a/src/test/ui/reachable/expr_call.stderr +++ b/src/test/ui/reachable/expr_call.stderr @@ -9,12 +9,23 @@ note: lint level defined here | LL | #![deny(unreachable_code)] | ^^^^^^^^^^^^^^^^ +note: any code following this expression is unreachable + --> $DIR/expr_call.rs:13:9 + | +LL | foo(return, 22); + | ^^^^^^ error: unreachable call --> $DIR/expr_call.rs:18:5 | LL | bar(return); | ^^^ + | +note: any code following this expression is unreachable + --> $DIR/expr_call.rs:18:9 + | +LL | bar(return); + | ^^^^^^ error: aborting due to 2 previous errors diff --git a/src/test/ui/reachable/expr_cast.stderr b/src/test/ui/reachable/expr_cast.stderr index 3086745d28ed4..d3ce0ca079f90 100644 --- a/src/test/ui/reachable/expr_cast.stderr +++ b/src/test/ui/reachable/expr_cast.stderr @@ -9,6 +9,11 @@ note: lint level defined here | LL | #![deny(unreachable_code)] | ^^^^^^^^^^^^^^^^ +note: any code following this expression is unreachable + --> $DIR/expr_cast.rs:9:14 + | +LL | let x = {return} as !; + | ^^^^^^ error: aborting due to previous error diff --git a/src/test/ui/reachable/expr_if.stderr b/src/test/ui/reachable/expr_if.stderr index f1690e595e5d1..03284576086a7 100644 --- a/src/test/ui/reachable/expr_if.stderr +++ b/src/test/ui/reachable/expr_if.stderr @@ -12,6 +12,11 @@ note: lint level defined here | LL | #![deny(unreachable_code)] | ^^^^^^^^^^^^^^^^ +note: any code following this expression is unreachable + --> $DIR/expr_if.rs:7:9 + | +LL | if {return} { + | ^^^^^^ error: unreachable statement --> $DIR/expr_if.rs:27:5 @@ -19,6 +24,11 @@ error: unreachable statement LL | println!("But I am."); | ^^^^^^^^^^^^^^^^^^^^^^ | +note: any code following this expression is unreachable + --> $DIR/expr_if.rs:21:9 + | +LL | return; + | ^^^^^^ = note: this error originates in a macro outside of the current crate (in Nightly builds, run with -Z external-macro-backtrace for more info) error: aborting due to 2 previous errors diff --git a/src/test/ui/reachable/expr_loop.stderr b/src/test/ui/reachable/expr_loop.stderr index 4d3e06c93a376..a4cf8cfcfd9e6 100644 --- a/src/test/ui/reachable/expr_loop.stderr +++ b/src/test/ui/reachable/expr_loop.stderr @@ -9,6 +9,11 @@ note: lint level defined here | LL | #![deny(unreachable_code)] | ^^^^^^^^^^^^^^^^ +note: any code following this expression is unreachable + --> $DIR/expr_loop.rs:7:12 + | +LL | loop { return; } + | ^^^^^^ = note: this error originates in a macro outside of the current crate (in Nightly builds, run with -Z external-macro-backtrace for more info) error: unreachable statement @@ -17,6 +22,11 @@ error: unreachable statement LL | println!("I am dead."); | ^^^^^^^^^^^^^^^^^^^^^^^ | +note: any code following this expression is unreachable + --> $DIR/expr_loop.rs:20:12 + | +LL | loop { return; } + | ^^^^^^ = note: this error originates in a macro outside of the current crate (in Nightly builds, run with -Z external-macro-backtrace for more info) error: unreachable statement @@ -25,6 +35,11 @@ error: unreachable statement LL | println!("I am dead."); | ^^^^^^^^^^^^^^^^^^^^^^^ | +note: any code following this expression is unreachable + --> $DIR/expr_loop.rs:31:5 + | +LL | loop { 'middle: loop { loop { break 'middle; } } } + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ = note: this error originates in a macro outside of the current crate (in Nightly builds, run with -Z external-macro-backtrace for more info) error: aborting due to 3 previous errors diff --git a/src/test/ui/reachable/expr_match.stderr b/src/test/ui/reachable/expr_match.stderr index 1aef06aec5ba6..b846b92dcec8b 100644 --- a/src/test/ui/reachable/expr_match.stderr +++ b/src/test/ui/reachable/expr_match.stderr @@ -9,6 +9,11 @@ note: lint level defined here | LL | #![deny(unreachable_code)] | ^^^^^^^^^^^^^^^^ +note: any code following this expression is unreachable + --> $DIR/expr_match.rs:7:22 + | +LL | match () { () => return } + | ^^^^^^ = note: this error originates in a macro outside of the current crate (in Nightly builds, run with -Z external-macro-backtrace for more info) error: unreachable statement @@ -17,6 +22,11 @@ error: unreachable statement LL | println!("I am dead"); | ^^^^^^^^^^^^^^^^^^^^^^ | +note: any code following this expression is unreachable + --> $DIR/expr_match.rs:18:31 + | +LL | match () { () if false => return, () => return } + | ^^^^^^ = note: this error originates in a macro outside of the current crate (in Nightly builds, run with -Z external-macro-backtrace for more info) error: aborting due to 2 previous errors diff --git a/src/test/ui/reachable/expr_method.stderr b/src/test/ui/reachable/expr_method.stderr index 947ea0fee889c..7ad279c9f487a 100644 --- a/src/test/ui/reachable/expr_method.stderr +++ b/src/test/ui/reachable/expr_method.stderr @@ -9,12 +9,23 @@ note: lint level defined here | LL | #![deny(unreachable_code)] | ^^^^^^^^^^^^^^^^ +note: any code following this expression is unreachable + --> $DIR/expr_method.rs:16:13 + | +LL | Foo.foo(return, 22); + | ^^^^^^ error: unreachable call --> $DIR/expr_method.rs:21:9 | LL | Foo.bar(return); | ^^^ + | +note: any code following this expression is unreachable + --> $DIR/expr_method.rs:21:13 + | +LL | Foo.bar(return); + | ^^^^^^ error: aborting due to 2 previous errors diff --git a/src/test/ui/reachable/expr_repeat.stderr b/src/test/ui/reachable/expr_repeat.stderr index 0536cdef72128..3ff6be76daea5 100644 --- a/src/test/ui/reachable/expr_repeat.stderr +++ b/src/test/ui/reachable/expr_repeat.stderr @@ -9,6 +9,11 @@ note: lint level defined here | LL | #![deny(unreachable_code)] | ^^^^^^^^^^^^^^^^ +note: any code following this expression is unreachable + --> $DIR/expr_repeat.rs:9:26 + | +LL | let x: [usize; 2] = [return; 2]; + | ^^^^^^ error: aborting due to previous error diff --git a/src/test/ui/reachable/expr_return.stderr b/src/test/ui/reachable/expr_return.stderr index 3317da58aba1c..31f7ebe7618ea 100644 --- a/src/test/ui/reachable/expr_return.stderr +++ b/src/test/ui/reachable/expr_return.stderr @@ -9,6 +9,11 @@ note: lint level defined here | LL | #![deny(unreachable_code)] | ^^^^^^^^^^^^^^^^ +note: any code following this expression is unreachable + --> $DIR/expr_return.rs:10:30 + | +LL | let x = {return {return {return;}}}; + | ^^^^^^ error: aborting due to previous error diff --git a/src/test/ui/reachable/expr_return_in_macro.rs b/src/test/ui/reachable/expr_return_in_macro.rs new file mode 100644 index 0000000000000..198ede6b14106 --- /dev/null +++ b/src/test/ui/reachable/expr_return_in_macro.rs @@ -0,0 +1,15 @@ +// Tests that we generate nice error messages +// when an expression is unreachble due to control +// flow inside of a macro expansion +#![deny(unreachable_code)] + +macro_rules! early_return { + () => { + return () + } +} + +fn main() { + return early_return!(); + //~^ ERROR unreachable expression +} diff --git a/src/test/ui/reachable/expr_return_in_macro.stderr b/src/test/ui/reachable/expr_return_in_macro.stderr new file mode 100644 index 0000000000000..ff3abb5551f92 --- /dev/null +++ b/src/test/ui/reachable/expr_return_in_macro.stderr @@ -0,0 +1,22 @@ +error: unreachable expression + --> $DIR/expr_return_in_macro.rs:13:5 + | +LL | return early_return!(); + | ^^^^^^^^^^^^^^^^^^^^^^ + | +note: lint level defined here + --> $DIR/expr_return_in_macro.rs:4:9 + | +LL | #![deny(unreachable_code)] + | ^^^^^^^^^^^^^^^^ +note: any code following this expression is unreachable + --> $DIR/expr_return_in_macro.rs:8:9 + | +LL | return () + | ^^^^^^^^^ +... +LL | return early_return!(); + | --------------- in this macro invocation + +error: aborting due to previous error + diff --git a/src/test/ui/reachable/expr_struct.stderr b/src/test/ui/reachable/expr_struct.stderr index dcccb7a4db307..d08bcc4f0d1ae 100644 --- a/src/test/ui/reachable/expr_struct.stderr +++ b/src/test/ui/reachable/expr_struct.stderr @@ -9,24 +9,47 @@ note: lint level defined here | LL | #![deny(unreachable_code)] | ^^^^^^^^^^^^^^^^ +note: any code following this expression is unreachable + --> $DIR/expr_struct.rs:14:35 + | +LL | let x = Foo { a: 22, b: 33, ..return }; + | ^^^^^^ error: unreachable expression --> $DIR/expr_struct.rs:19:33 | LL | let x = Foo { a: return, b: 33, ..return }; | ^^ + | +note: any code following this expression is unreachable + --> $DIR/expr_struct.rs:19:22 + | +LL | let x = Foo { a: return, b: 33, ..return }; + | ^^^^^^ error: unreachable expression --> $DIR/expr_struct.rs:24:39 | LL | let x = Foo { a: 22, b: return, ..return }; | ^^^^^^ + | +note: any code following this expression is unreachable + --> $DIR/expr_struct.rs:24:29 + | +LL | let x = Foo { a: 22, b: return, ..return }; + | ^^^^^^ error: unreachable expression --> $DIR/expr_struct.rs:29:13 | LL | let x = Foo { a: 22, b: return }; | ^^^^^^^^^^^^^^^^^^^^^^^^ + | +note: any code following this expression is unreachable + --> $DIR/expr_struct.rs:29:29 + | +LL | let x = Foo { a: 22, b: return }; + | ^^^^^^ error: aborting due to 4 previous errors diff --git a/src/test/ui/reachable/expr_tup.stderr b/src/test/ui/reachable/expr_tup.stderr index 1837031107d59..788499533db33 100644 --- a/src/test/ui/reachable/expr_tup.stderr +++ b/src/test/ui/reachable/expr_tup.stderr @@ -9,12 +9,23 @@ note: lint level defined here | LL | #![deny(unreachable_code)] | ^^^^^^^^^^^^^^^^ +note: any code following this expression is unreachable + --> $DIR/expr_tup.rs:9:30 + | +LL | let x: (usize, usize) = (return, 2); + | ^^^^^^ error: unreachable expression --> $DIR/expr_tup.rs:14:29 | LL | let x: (usize, usize) = (2, return); | ^^^^^^^^^^^ + | +note: any code following this expression is unreachable + --> $DIR/expr_tup.rs:14:33 + | +LL | let x: (usize, usize) = (2, return); + | ^^^^^^ error: aborting due to 2 previous errors diff --git a/src/test/ui/reachable/expr_type.stderr b/src/test/ui/reachable/expr_type.stderr index f867c89163415..15eb735da75d8 100644 --- a/src/test/ui/reachable/expr_type.stderr +++ b/src/test/ui/reachable/expr_type.stderr @@ -9,6 +9,11 @@ note: lint level defined here | LL | #![deny(unreachable_code)] | ^^^^^^^^^^^^^^^^ +note: any code following this expression is unreachable + --> $DIR/expr_type.rs:9:14 + | +LL | let x = {return}: !; + | ^^^^^^ error: aborting due to previous error diff --git a/src/test/ui/reachable/expr_unary.stderr b/src/test/ui/reachable/expr_unary.stderr index 61982289cdc6e..7f86519616639 100644 --- a/src/test/ui/reachable/expr_unary.stderr +++ b/src/test/ui/reachable/expr_unary.stderr @@ -15,6 +15,11 @@ note: lint level defined here | LL | #![deny(unreachable_code)] | ^^^^^^^^^^^^^^^^ +note: any code following this expression is unreachable + --> $DIR/expr_unary.rs:8:20 + | +LL | let x: ! = ! { return; }; + | ^^^^^^ error: aborting due to 2 previous errors diff --git a/src/test/ui/reachable/expr_while.stderr b/src/test/ui/reachable/expr_while.stderr index fc528926b4c97..b6d6d11ac691e 100644 --- a/src/test/ui/reachable/expr_while.stderr +++ b/src/test/ui/reachable/expr_while.stderr @@ -13,6 +13,11 @@ note: lint level defined here | LL | #![deny(unreachable_code)] | ^^^^^^^^^^^^^^^^ +note: any code following this expression is unreachable + --> $DIR/expr_while.rs:7:12 + | +LL | while {return} { + | ^^^^^^ error: unreachable block in `while` expression --> $DIR/expr_while.rs:22:20 @@ -23,6 +28,12 @@ LL | | LL | | println!("I am dead."); LL | | } | |_____^ + | +note: any code following this expression is unreachable + --> $DIR/expr_while.rs:22:12 + | +LL | while {return} { + | ^^^^^^ error: aborting due to 2 previous errors diff --git a/src/test/ui/rfc-2497-if-let-chains/protect-precedences.stderr b/src/test/ui/rfc-2497-if-let-chains/protect-precedences.stderr index be7ef658411e1..ca98a3947146e 100644 --- a/src/test/ui/rfc-2497-if-let-chains/protect-precedences.stderr +++ b/src/test/ui/rfc-2497-if-let-chains/protect-precedences.stderr @@ -5,4 +5,9 @@ LL | if let _ = return true && false {}; | ^^ | = note: `#[warn(unreachable_code)]` on by default +note: any code following this expression is unreachable + --> $DIR/protect-precedences.rs:13:20 + | +LL | if let _ = return true && false {}; + | ^^^^^^^^^^^^^^^^^^^^ diff --git a/src/test/ui/unreachable/unreachable-code.stderr b/src/test/ui/unreachable/unreachable-code.stderr index 803bb966be8fc..226f088c63a5e 100644 --- a/src/test/ui/unreachable/unreachable-code.stderr +++ b/src/test/ui/unreachable/unreachable-code.stderr @@ -9,6 +9,11 @@ note: lint level defined here | LL | #![deny(unreachable_code)] | ^^^^^^^^^^^^^^^^ +note: any code following this expression is unreachable + --> $DIR/unreachable-code.rs:5:3 + | +LL | loop{} + | ^^^^^^ error: aborting due to previous error diff --git a/src/test/ui/unreachable/unreachable-in-call.stderr b/src/test/ui/unreachable/unreachable-in-call.stderr index c740011c4a125..928f5634a1248 100644 --- a/src/test/ui/unreachable/unreachable-in-call.stderr +++ b/src/test/ui/unreachable/unreachable-in-call.stderr @@ -9,12 +9,23 @@ note: lint level defined here | LL | #![deny(unreachable_code)] | ^^^^^^^^^^^^^^^^ +note: any code following this expression is unreachable + --> $DIR/unreachable-in-call.rs:13:10 + | +LL | call(diverge(), + | ^^^^^^^^^ error: unreachable call --> $DIR/unreachable-in-call.rs:17:5 | LL | call( | ^^^^ + | +note: any code following this expression is unreachable + --> $DIR/unreachable-in-call.rs:19:9 + | +LL | diverge()); + | ^^^^^^^^^ error: aborting due to 2 previous errors diff --git a/src/test/ui/unreachable/unreachable-try-pattern.stderr b/src/test/ui/unreachable/unreachable-try-pattern.stderr index 758aa5a45bc17..889df790124da 100644 --- a/src/test/ui/unreachable/unreachable-try-pattern.stderr +++ b/src/test/ui/unreachable/unreachable-try-pattern.stderr @@ -9,6 +9,11 @@ note: lint level defined here | LL | #![warn(unreachable_code)] | ^^^^^^^^^^^^^^^^ +note: any code following this expression is unreachable + --> $DIR/unreachable-try-pattern.rs:19:36 + | +LL | let y = (match x { Ok(n) => Ok(n as u32), Err(e) => Err(e) })?; + | ^ warning: unreachable pattern --> $DIR/unreachable-try-pattern.rs:19:24 diff --git a/src/test/ui/unreachable/unwarned-match-on-never.stderr b/src/test/ui/unreachable/unwarned-match-on-never.stderr index ccb70d7431145..9ce6e3df8046e 100644 --- a/src/test/ui/unreachable/unwarned-match-on-never.stderr +++ b/src/test/ui/unreachable/unwarned-match-on-never.stderr @@ -9,12 +9,23 @@ note: lint level defined here | LL | #![deny(unreachable_code)] | ^^^^^^^^^^^^^^^^ +note: any code following this expression is unreachable + --> $DIR/unwarned-match-on-never.rs:8:11 + | +LL | match x {} + | ^ error: unreachable arm --> $DIR/unwarned-match-on-never.rs:15:15 | LL | () => () | ^^ + | +note: any code following this expression is unreachable + --> $DIR/unwarned-match-on-never.rs:14:11 + | +LL | match (return) { + | ^^^^^^^^ error: unreachable expression --> $DIR/unwarned-match-on-never.rs:21:5 @@ -23,6 +34,12 @@ LL | / match () { LL | | () => (), LL | | } | |_____^ + | +note: any code following this expression is unreachable + --> $DIR/unwarned-match-on-never.rs:20:5 + | +LL | return; + | ^^^^^^ error: aborting due to 3 previous errors From cd4b468e07ab27eef87aa8757220d6439defc699 Mon Sep 17 00:00:00 2001 From: Aaron Hill Date: Wed, 18 Sep 2019 19:07:39 -0400 Subject: [PATCH 07/18] Make note better when all arms in a `match` diverge --- src/librustc_typeck/check/_match.rs | 26 +++++++++++++++++-- src/librustc_typeck/check/expr.rs | 5 +++- src/librustc_typeck/check/mod.rs | 33 +++++++++++++++++++------ src/test/ui/reachable/expr_match.stderr | 12 ++++----- 4 files changed, 60 insertions(+), 16 deletions(-) diff --git a/src/librustc_typeck/check/_match.rs b/src/librustc_typeck/check/_match.rs index a1c937b95ba8c..86774466ba586 100644 --- a/src/librustc_typeck/check/_match.rs +++ b/src/librustc_typeck/check/_match.rs @@ -43,7 +43,10 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { // If there are no arms, that is a diverging match; a special case. if arms.is_empty() { - self.diverges.set(self.diverges.get() | Diverges::Always(expr.span)); + self.diverges.set(self.diverges.get() | Diverges::Always { + span: expr.span, + custom_note: None + }); return tcx.types.never; } @@ -69,7 +72,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { // warnings). match all_pats_diverge { Diverges::Maybe => Diverges::Maybe, - Diverges::Always(..) | Diverges::WarnedAlways => Diverges::WarnedAlways, + Diverges::Always { .. } | Diverges::WarnedAlways => Diverges::WarnedAlways, } }).collect(); @@ -167,6 +170,25 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { prior_arm_ty = Some(arm_ty); } + // If all of the arms in the 'match' diverge, + // and we're dealing with an actual 'match' block + // (as opposed to a 'match' desugared from something else'), + // we can emit a better note. Rather than pointing + // at a diverging expression in an arbitrary arm, + // we can point at the entire 'match' expression + match (all_arms_diverge, match_src) { + (Diverges::Always { .. }, hir::MatchSource::Normal) => { + all_arms_diverge = Diverges::Always { + span: expr.span, + custom_note: Some( + "any code following this `match` expression is unreachable, \ + as all arms diverge" + ) + }; + }, + _ => {} + } + // We won't diverge unless the discriminant or all arms diverge. self.diverges.set(discrim_diverges | all_arms_diverge); diff --git a/src/librustc_typeck/check/expr.rs b/src/librustc_typeck/check/expr.rs index 3ba453748ff32..5733b8d1db13e 100644 --- a/src/librustc_typeck/check/expr.rs +++ b/src/librustc_typeck/check/expr.rs @@ -170,7 +170,10 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { // Any expression that produces a value of type `!` must have diverged if ty.is_never() { - self.diverges.set(self.diverges.get() | Diverges::Always(expr.span)); + self.diverges.set(self.diverges.get() | Diverges::Always { + span: expr.span, + custom_note: None + }); } // Record the type, which applies it effects. diff --git a/src/librustc_typeck/check/mod.rs b/src/librustc_typeck/check/mod.rs index 0fced37394616..c44648bb9df85 100644 --- a/src/librustc_typeck/check/mod.rs +++ b/src/librustc_typeck/check/mod.rs @@ -450,10 +450,20 @@ pub enum Diverges { /// Definitely known to diverge and therefore /// not reach the next sibling or its parent. - /// The `Span` points to the expression - /// that caused us to diverge - /// (e.g. `return`, `break`, etc) - Always(Span), + Always { + /// The `Span` points to the expression + /// that caused us to diverge + /// (e.g. `return`, `break`, etc) + span: Span, + /// In some cases (e.g. a 'match' expression + /// where all arms diverge), we may be + /// able to provide a more informative + /// message to the user. + /// If this is None, a default messsage + /// will be generated, which is suitable + /// for most cases + custom_note: Option<&'static str> + }, /// Same as `Always` but with a reachability /// warning already emitted. @@ -490,7 +500,13 @@ impl ops::BitOrAssign for Diverges { impl Diverges { fn always(self) -> bool { - self >= Diverges::Always(DUMMY_SP) + // Enum comparison ignores the + // contents of fields, so we just + // fill them in with garbage here + self >= Diverges::Always { + span: DUMMY_SP, + custom_note: None + } } } @@ -2312,7 +2328,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { fn warn_if_unreachable(&self, id: hir::HirId, span: Span, kind: &str) { // FIXME: Combine these two 'if' expressions into one once // let chains are implemented - if let Diverges::Always(orig_span) = self.diverges.get() { + if let Diverges::Always { span: orig_span, custom_note } = self.diverges.get() { // If span arose from a desugaring of `if` or `while`, then it is the condition itself, // which diverges, that we are about to lint on. This gives suboptimal diagnostics. // Instead, stop here so that the `if`- or `while`-expression's block is linted instead. @@ -2324,7 +2340,10 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { let msg = format!("unreachable {}", kind); let mut err = self.tcx().struct_span_lint_hir(lint::builtin::UNREACHABLE_CODE, id, span, &msg); - err.span_note(orig_span, "any code following this expression is unreachable"); + err.span_note( + orig_span, + custom_note.unwrap_or("any code following this expression is unreachable") + ); err.emit(); } } diff --git a/src/test/ui/reachable/expr_match.stderr b/src/test/ui/reachable/expr_match.stderr index b846b92dcec8b..f587e524d350d 100644 --- a/src/test/ui/reachable/expr_match.stderr +++ b/src/test/ui/reachable/expr_match.stderr @@ -9,11 +9,11 @@ note: lint level defined here | LL | #![deny(unreachable_code)] | ^^^^^^^^^^^^^^^^ -note: any code following this expression is unreachable - --> $DIR/expr_match.rs:7:22 +note: any code following this `match` expression is unreachable, as all arms diverge + --> $DIR/expr_match.rs:7:5 | LL | match () { () => return } - | ^^^^^^ + | ^^^^^^^^^^^^^^^^^^^^^^^^^ = note: this error originates in a macro outside of the current crate (in Nightly builds, run with -Z external-macro-backtrace for more info) error: unreachable statement @@ -22,11 +22,11 @@ error: unreachable statement LL | println!("I am dead"); | ^^^^^^^^^^^^^^^^^^^^^^ | -note: any code following this expression is unreachable - --> $DIR/expr_match.rs:18:31 +note: any code following this `match` expression is unreachable, as all arms diverge + --> $DIR/expr_match.rs:18:5 | LL | match () { () if false => return, () => return } - | ^^^^^^ + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ = note: this error originates in a macro outside of the current crate (in Nightly builds, run with -Z external-macro-backtrace for more info) error: aborting due to 2 previous errors From 9e777eb45de2bf972bbf7f2075fa491e22d2d071 Mon Sep 17 00:00:00 2001 From: Aaron Hill Date: Wed, 18 Sep 2019 19:31:38 -0400 Subject: [PATCH 08/18] Some formatting cleanup --- src/librustc_typeck/check/mod.rs | 13 ++++++------- src/test/ui/reachable/expr_return_in_macro.rs | 2 +- 2 files changed, 7 insertions(+), 8 deletions(-) diff --git a/src/librustc_typeck/check/mod.rs b/src/librustc_typeck/check/mod.rs index c44648bb9df85..f6f3173e81c89 100644 --- a/src/librustc_typeck/check/mod.rs +++ b/src/librustc_typeck/check/mod.rs @@ -2338,13 +2338,12 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { debug!("warn_if_unreachable: id={:?} span={:?} kind={}", id, span, kind); let msg = format!("unreachable {}", kind); - let mut err = self.tcx().struct_span_lint_hir(lint::builtin::UNREACHABLE_CODE, - id, span, &msg); - err.span_note( - orig_span, - custom_note.unwrap_or("any code following this expression is unreachable") - ); - err.emit(); + self.tcx().struct_span_lint_hir(lint::builtin::UNREACHABLE_CODE, id, span, &msg) + .span_note( + orig_span, + custom_note.unwrap_or("any code following this expression is unreachable") + ) + .emit(); } } } diff --git a/src/test/ui/reachable/expr_return_in_macro.rs b/src/test/ui/reachable/expr_return_in_macro.rs index 198ede6b14106..4e57618bf5e77 100644 --- a/src/test/ui/reachable/expr_return_in_macro.rs +++ b/src/test/ui/reachable/expr_return_in_macro.rs @@ -1,6 +1,6 @@ // Tests that we generate nice error messages // when an expression is unreachble due to control -// flow inside of a macro expansion +// flow inside of a macro expansion. #![deny(unreachable_code)] macro_rules! early_return { From 6edcfbe59a2067df7aadc18de990b9a86216099e Mon Sep 17 00:00:00 2001 From: Aaron Hill Date: Wed, 18 Sep 2019 19:48:37 -0400 Subject: [PATCH 09/18] Apply formatting fixes Co-Authored-By: Mazdak Farrokhzad --- src/librustc_typeck/check/_match.rs | 8 ++++---- src/librustc_typeck/check/mod.rs | 10 +++++----- 2 files changed, 9 insertions(+), 9 deletions(-) diff --git a/src/librustc_typeck/check/_match.rs b/src/librustc_typeck/check/_match.rs index 86774466ba586..034cba0f12a62 100644 --- a/src/librustc_typeck/check/_match.rs +++ b/src/librustc_typeck/check/_match.rs @@ -170,12 +170,12 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { prior_arm_ty = Some(arm_ty); } - // If all of the arms in the 'match' diverge, - // and we're dealing with an actual 'match' block - // (as opposed to a 'match' desugared from something else'), + // If all of the arms in the `match` diverge, + // and we're dealing with an actual `match` block + // (as opposed to a `match` desugared from something else'), // we can emit a better note. Rather than pointing // at a diverging expression in an arbitrary arm, - // we can point at the entire 'match' expression + // we can point at the entire `match` expression match (all_arms_diverge, match_src) { (Diverges::Always { .. }, hir::MatchSource::Normal) => { all_arms_diverge = Diverges::Always { diff --git a/src/librustc_typeck/check/mod.rs b/src/librustc_typeck/check/mod.rs index f6f3173e81c89..e424aa679088d 100644 --- a/src/librustc_typeck/check/mod.rs +++ b/src/librustc_typeck/check/mod.rs @@ -453,15 +453,15 @@ pub enum Diverges { Always { /// The `Span` points to the expression /// that caused us to diverge - /// (e.g. `return`, `break`, etc) + /// (e.g. `return`, `break`, etc). span: Span, - /// In some cases (e.g. a 'match' expression + /// In some cases (e.g. a `match` expression /// where all arms diverge), we may be /// able to provide a more informative /// message to the user. - /// If this is None, a default messsage + /// If this is `None`, a default messsage /// will be generated, which is suitable - /// for most cases + /// for most cases. custom_note: Option<&'static str> }, @@ -502,7 +502,7 @@ impl Diverges { fn always(self) -> bool { // Enum comparison ignores the // contents of fields, so we just - // fill them in with garbage here + // fill them in with garbage here. self >= Diverges::Always { span: DUMMY_SP, custom_note: None From a8ce93e13a9f1e7d7933ca76f6cae9c8f6127c34 Mon Sep 17 00:00:00 2001 From: Aaron Hill Date: Wed, 18 Sep 2019 20:04:01 -0400 Subject: [PATCH 10/18] Introduce Diverges::always constructor Rename the existing Diverges.always method to Diverges.is_always --- src/librustc_typeck/check/_match.rs | 7 ++----- src/librustc_typeck/check/expr.rs | 5 +---- src/librustc_typeck/check/mod.rs | 14 ++++++++++++-- 3 files changed, 15 insertions(+), 11 deletions(-) diff --git a/src/librustc_typeck/check/_match.rs b/src/librustc_typeck/check/_match.rs index 034cba0f12a62..9481638fc14d8 100644 --- a/src/librustc_typeck/check/_match.rs +++ b/src/librustc_typeck/check/_match.rs @@ -43,10 +43,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { // If there are no arms, that is a diverging match; a special case. if arms.is_empty() { - self.diverges.set(self.diverges.get() | Diverges::Always { - span: expr.span, - custom_note: None - }); + self.diverges.set(self.diverges.get() | Diverges::always(expr.span)); return tcx.types.never; } @@ -198,7 +195,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { /// When the previously checked expression (the scrutinee) diverges, /// warn the user about the match arms being unreachable. fn warn_arms_when_scrutinee_diverges(&self, arms: &'tcx [hir::Arm], source: hir::MatchSource) { - if self.diverges.get().always() { + if self.diverges.get().is_always() { use hir::MatchSource::*; let msg = match source { IfDesugar { .. } | IfLetDesugar { .. } => "block in `if` expression", diff --git a/src/librustc_typeck/check/expr.rs b/src/librustc_typeck/check/expr.rs index 5733b8d1db13e..049f2eb16bb00 100644 --- a/src/librustc_typeck/check/expr.rs +++ b/src/librustc_typeck/check/expr.rs @@ -170,10 +170,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { // Any expression that produces a value of type `!` must have diverged if ty.is_never() { - self.diverges.set(self.diverges.get() | Diverges::Always { - span: expr.span, - custom_note: None - }); + self.diverges.set(self.diverges.get() | Diverges::always(expr.span)); } // Record the type, which applies it effects. diff --git a/src/librustc_typeck/check/mod.rs b/src/librustc_typeck/check/mod.rs index e424aa679088d..410eef5e092d9 100644 --- a/src/librustc_typeck/check/mod.rs +++ b/src/librustc_typeck/check/mod.rs @@ -470,6 +470,16 @@ pub enum Diverges { WarnedAlways } +impl Diverges { + /// Creates a `Diverges::Always` with the provided span and the default note message + fn always(span: Span) -> Diverges { + Diverges::Always { + span, + custom_note: None + } + } +} + // Convenience impls for combinig `Diverges`. impl ops::BitAnd for Diverges { @@ -499,7 +509,7 @@ impl ops::BitOrAssign for Diverges { } impl Diverges { - fn always(self) -> bool { + fn is_always(self) -> bool { // Enum comparison ignores the // contents of fields, so we just // fill them in with garbage here. @@ -3852,7 +3862,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { // // #41425 -- label the implicit `()` as being the // "found type" here, rather than the "expected type". - if !self.diverges.get().always() { + if !self.diverges.get().is_always() { // #50009 -- Do not point at the entire fn block span, point at the return type // span, as it is the cause of the requirement, and // `consider_hint_about_removing_semicolon` will point at the last expression From 41e1128969adc0190c946ccbd8b6b89ad197bc34 Mon Sep 17 00:00:00 2001 From: Aaron Hill Date: Wed, 18 Sep 2019 20:07:33 -0400 Subject: [PATCH 11/18] Use 'if let' instead of 'match' --- src/librustc_typeck/check/_match.rs | 18 +++++++----------- 1 file changed, 7 insertions(+), 11 deletions(-) diff --git a/src/librustc_typeck/check/_match.rs b/src/librustc_typeck/check/_match.rs index 9481638fc14d8..50fd72f061319 100644 --- a/src/librustc_typeck/check/_match.rs +++ b/src/librustc_typeck/check/_match.rs @@ -173,17 +173,13 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { // we can emit a better note. Rather than pointing // at a diverging expression in an arbitrary arm, // we can point at the entire `match` expression - match (all_arms_diverge, match_src) { - (Diverges::Always { .. }, hir::MatchSource::Normal) => { - all_arms_diverge = Diverges::Always { - span: expr.span, - custom_note: Some( - "any code following this `match` expression is unreachable, \ - as all arms diverge" - ) - }; - }, - _ => {} + if let (Diverges::Always { .. }, hir::MatchSource::Normal) = (all_arms_diverge, match_src) { + all_arms_diverge = Diverges::Always { + span: expr.span, + custom_note: Some( + "any code following this `match` expression is unreachable, as all arms diverge" + ) + }; } // We won't diverge unless the discriminant or all arms diverge. From 034a8fd1dff9bff585a6152bf5c52e882a2bb28b Mon Sep 17 00:00:00 2001 From: Aaron Hill Date: Wed, 18 Sep 2019 20:14:56 -0400 Subject: [PATCH 12/18] Another formatting fix Co-Authored-By: Mazdak Farrokhzad --- src/librustc_typeck/check/mod.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/librustc_typeck/check/mod.rs b/src/librustc_typeck/check/mod.rs index 410eef5e092d9..51381447fedca 100644 --- a/src/librustc_typeck/check/mod.rs +++ b/src/librustc_typeck/check/mod.rs @@ -471,7 +471,7 @@ pub enum Diverges { } impl Diverges { - /// Creates a `Diverges::Always` with the provided span and the default note message + /// Creates a `Diverges::Always` with the provided `span` and the default note message. fn always(span: Span) -> Diverges { Diverges::Always { span, From d67528ff7dbbe226fa583b9585cee2138533770e Mon Sep 17 00:00:00 2001 From: Aaron Hill Date: Wed, 18 Sep 2019 21:57:50 -0400 Subject: [PATCH 13/18] Merge inherent impl blocks for `Diverges --- src/librustc_typeck/check/mod.rs | 18 ++++++++---------- 1 file changed, 8 insertions(+), 10 deletions(-) diff --git a/src/librustc_typeck/check/mod.rs b/src/librustc_typeck/check/mod.rs index 51381447fedca..1d68d79db8856 100644 --- a/src/librustc_typeck/check/mod.rs +++ b/src/librustc_typeck/check/mod.rs @@ -470,16 +470,6 @@ pub enum Diverges { WarnedAlways } -impl Diverges { - /// Creates a `Diverges::Always` with the provided `span` and the default note message. - fn always(span: Span) -> Diverges { - Diverges::Always { - span, - custom_note: None - } - } -} - // Convenience impls for combinig `Diverges`. impl ops::BitAnd for Diverges { @@ -509,6 +499,14 @@ impl ops::BitOrAssign for Diverges { } impl Diverges { + /// Creates a `Diverges::Always` with the provided `span` and the default note message. + fn always(span: Span) -> Diverges { + Diverges::Always { + span, + custom_note: None + } + } + fn is_always(self) -> bool { // Enum comparison ignores the // contents of fields, so we just From 37bafeafe73cff935bb444b6736a958a9a752669 Mon Sep 17 00:00:00 2001 From: Joshua Groves Date: Wed, 18 Sep 2019 23:17:36 -0600 Subject: [PATCH 14/18] Fix backticks in documentation --- src/librustc/middle/region.rs | 2 +- src/librustc/ty/query/on_disk_cache.rs | 2 +- src/libsyntax/source_map.rs | 4 ++-- src/test/ui/borrowck/two-phase-surprise-no-conflict.rs | 2 +- 4 files changed, 5 insertions(+), 5 deletions(-) diff --git a/src/librustc/middle/region.rs b/src/librustc/middle/region.rs index b9d8a4ec68fad..87470140e3148 100644 --- a/src/librustc/middle/region.rs +++ b/src/librustc/middle/region.rs @@ -1136,7 +1136,7 @@ fn resolve_local<'tcx>( // Rule A. `let (ref x, ref y) = (foo().x, 44)`. The rvalue `(22, 44)` // would have an extended lifetime, but not `foo()`. // - // Rule B. `let x = &foo().x`. The rvalue ``foo()` would have extended + // Rule B. `let x = &foo().x`. The rvalue `foo()` would have extended // lifetime. // // In some cases, multiple rules may apply (though not to the same diff --git a/src/librustc/ty/query/on_disk_cache.rs b/src/librustc/ty/query/on_disk_cache.rs index 4cef6a09925ad..c20e758688959 100644 --- a/src/librustc/ty/query/on_disk_cache.rs +++ b/src/librustc/ty/query/on_disk_cache.rs @@ -643,7 +643,7 @@ impl<'a, 'tcx> SpecializedDecoder for CacheDecoder<'a, 'tcx> { // Both the `CrateNum` and the `DefIndex` of a `DefId` can change in between two // compilation sessions. We use the `DefPathHash`, which is stable across -// sessions, to map the old DefId`` to the new one. +// sessions, to map the old `DefId` to the new one. impl<'a, 'tcx> SpecializedDecoder for CacheDecoder<'a, 'tcx> { #[inline] fn specialized_decode(&mut self) -> Result { diff --git a/src/libsyntax/source_map.rs b/src/libsyntax/source_map.rs index d7ea799e00459..7d0d2392945e5 100644 --- a/src/libsyntax/source_map.rs +++ b/src/libsyntax/source_map.rs @@ -3,7 +3,7 @@ //! of source parsed during crate parsing (typically files, in-memory strings, //! or various bits of macro expansion) cover a continuous range of bytes in the //! `SourceMap` and are represented by `SourceFile`s. Byte positions are stored in -//! `Span`` and used pervasively in the compiler. They are absolute positions +//! `Span` and used pervasively in the compiler. They are absolute positions //! within the `SourceMap`, which upon request can be converted to line and column //! information, source code snippets, etc. @@ -645,7 +645,7 @@ impl SourceMap { } /// Given a `Span`, tries to get a shorter span ending before the first occurrence of `char` - /// ``c`. + /// `c`. pub fn span_until_char(&self, sp: Span, c: char) -> Span { match self.span_to_snippet(sp) { Ok(snippet) => { diff --git a/src/test/ui/borrowck/two-phase-surprise-no-conflict.rs b/src/test/ui/borrowck/two-phase-surprise-no-conflict.rs index 3fd24bbf290b5..6d37d1ded6400 100644 --- a/src/test/ui/borrowck/two-phase-surprise-no-conflict.rs +++ b/src/test/ui/borrowck/two-phase-surprise-no-conflict.rs @@ -31,7 +31,7 @@ impl <'a> SpanlessHash<'a> { // // Not okay without two-phase borrows: the implicit // `&mut self` of the receiver is evaluated first, and - // that conflicts with the `self.cx`` access during + // that conflicts with the `self.cx` access during // argument evaluation, as demonstrated in `fn demo` // above. // From 1de533ac1a3bddd6bf61791f4ee6cf52cb186fdf Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Thu, 19 Sep 2019 09:02:45 +0200 Subject: [PATCH 15/18] first determine if the variant is a niche-variant, then compute absolute variant --- src/librustc_mir/interpret/operand.rs | 29 ++++++++++++++------------- src/librustc_mir/interpret/place.rs | 3 ++- 2 files changed, 17 insertions(+), 15 deletions(-) diff --git a/src/librustc_mir/interpret/operand.rs b/src/librustc_mir/interpret/operand.rs index 9e7ae32f1eacf..dd214c4a031f7 100644 --- a/src/librustc_mir/interpret/operand.rs +++ b/src/librustc_mir/interpret/operand.rs @@ -1,7 +1,7 @@ //! Functions concerning immediate values and operands, and reading from operands. //! All high-level functions to read from memory work on operands as sources. -use std::convert::TryInto; +use std::convert::{TryInto, TryFrom}; use rustc::{mir, ty}; use rustc::ty::layout::{ @@ -671,8 +671,8 @@ impl<'mir, 'tcx, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> { ref niche_variants, niche_start, } => { - let variants_start = niche_variants.start().as_u32() as u128; - let variants_end = niche_variants.end().as_u32() as u128; + let variants_start = niche_variants.start().as_u32(); + let variants_end = niche_variants.end().as_u32(); let raw_discr = raw_discr.not_undef().map_err(|_| { err_unsup!(InvalidDiscriminant(ScalarMaybeUndef::Undef)) })?; @@ -687,7 +687,8 @@ impl<'mir, 'tcx, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> { (dataful_variant.as_u32() as u128, dataful_variant) }, Ok(raw_discr) => { - // We need to use machine arithmetic to get the relative variant idx. + // We need to use machine arithmetic to get the relative variant idx: + // variant_index_relative = discr_val - niche_start_val let discr_layout = self.layout_of(discr_layout.value.to_int_ty(*self.tcx))?; let discr_val = ImmTy::from_uint(raw_discr, discr_layout); let niche_start_val = ImmTy::from_uint(niche_start, discr_layout); @@ -699,21 +700,21 @@ impl<'mir, 'tcx, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> { let variant_index_relative = variant_index_relative_val .to_scalar()? .assert_bits(discr_val.layout.size); - // Then computing the absolute variant idx should not overflow any more. - let variant_index = variants_start - .checked_add(variant_index_relative) - .expect("oveflow computing absolute variant idx"); // Check if this is in the range that indicates an actual discriminant. - if variants_start <= variant_index && variant_index <= variants_end { - let index = variant_index as usize; - assert_eq!(index as u128, variant_index); - assert!(index < rval.layout.ty + if variant_index_relative <= u128::from(variants_end - variants_start) { + let variant_index_relative = u32::try_from(variant_index_relative) + .expect("we checked that this fits into a u32"); + // Then computing the absolute variant idx should not overflow any more. + let variant_index = variants_start + .checked_add(variant_index_relative) + .expect("oveflow computing absolute variant idx"); + assert!((variant_index as usize) < rval.layout.ty .ty_adt_def() .expect("tagged layout for non adt") .variants.len()); - (variant_index, VariantIdx::from_usize(index)) + (u128::from(variant_index), VariantIdx::from_u32(variant_index)) } else { - (dataful_variant.as_u32() as u128, dataful_variant) + (u128::from(dataful_variant.as_u32()), dataful_variant) } }, } diff --git a/src/librustc_mir/interpret/place.rs b/src/librustc_mir/interpret/place.rs index 5f4903d61e3b2..c3660fb7a2e28 100644 --- a/src/librustc_mir/interpret/place.rs +++ b/src/librustc_mir/interpret/place.rs @@ -1064,7 +1064,8 @@ where let variant_index_relative = variant_index.as_u32() .checked_sub(variants_start) .expect("overflow computing relative variant idx"); - // We need to use machine arithmetic when taking into account `niche_start`. + // We need to use machine arithmetic when taking into account `niche_start`: + // discr_val = variant_index_relative + niche_start_val let discr_layout = self.layout_of(discr_layout.value.to_int_ty(*self.tcx))?; let niche_start_val = ImmTy::from_uint(niche_start, discr_layout); let variant_index_relative_val = From c23e78a38b9643f15b008393b2506a813e7ee2ab Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Adrian=20Heine=20n=C3=A9=20Lang?= Date: Thu, 19 Sep 2019 13:36:10 +0200 Subject: [PATCH 16/18] Remove unnecessary `mut` in doc example --- src/libstd/process.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/libstd/process.rs b/src/libstd/process.rs index b8d57cfafea0a..c6dc02fea2d84 100644 --- a/src/libstd/process.rs +++ b/src/libstd/process.rs @@ -935,7 +935,7 @@ impl Stdio { /// .expect("Failed to spawn child process"); /// /// { - /// let mut stdin = child.stdin.as_mut().expect("Failed to open stdin"); + /// let stdin = child.stdin.as_mut().expect("Failed to open stdin"); /// stdin.write_all("Hello, world!".as_bytes()).expect("Failed to write to stdin"); /// } /// From 2fff78038cf3eb38f2bd3fecbab123bccb59c1a8 Mon Sep 17 00:00:00 2001 From: Alex Crichton Date: Thu, 19 Sep 2019 07:46:24 -0700 Subject: [PATCH 17/18] rustbuild: Don't package libstd twice Looks like the packaging step for the standard library was happening twice on CI, but it only needs to happen once! The `Analysis` packaging step accidentally packaged `Std` instead of relying on compiling `Std`, which meant that we ended up packaging it twice erroneously. --- src/bootstrap/dist.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/bootstrap/dist.rs b/src/bootstrap/dist.rs index 076bcd878df71..e27a6bf7da0ac 100644 --- a/src/bootstrap/dist.rs +++ b/src/bootstrap/dist.rs @@ -762,7 +762,7 @@ impl Step for Analysis { return distdir(builder).join(format!("{}-{}.tar.gz", name, target)); } - builder.ensure(Std { compiler, target }); + builder.ensure(compile::Std { compiler, target }); let image = tmpdir(builder).join(format!("{}-{}-image", name, target)); From d7f64749c0d32f3d63c0ee5d0999fce99f222553 Mon Sep 17 00:00:00 2001 From: Alex Crichton Date: Thu, 19 Sep 2019 07:48:04 -0700 Subject: [PATCH 18/18] rustbuild: Copy crate doc files fewer times Previously when building documentation for the standard library we'd copy all the files 5 times, and these files include libcore/libstd docs which are huge! This commit instead only copies the files after rustdoc has been run for each crate, reducing the number of redundant copies we're making. --- src/bootstrap/doc.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/bootstrap/doc.rs b/src/bootstrap/doc.rs index 6faedc80ad3f5..873a3c31d1535 100644 --- a/src/bootstrap/doc.rs +++ b/src/bootstrap/doc.rs @@ -476,11 +476,11 @@ impl Step for Std { .arg("--index-page").arg(&builder.src.join("src/doc/index.md")); builder.run(&mut cargo); - builder.cp_r(&my_out, &out); }; for krate in &["alloc", "core", "std", "proc_macro", "test"] { run_cargo_rustdoc_for(krate); } + builder.cp_r(&my_out, &out); } }