From cf1406c4e37b0c0710788aa81c989618650b8e43 Mon Sep 17 00:00:00 2001 From: Georg Brandl Date: Thu, 1 Nov 2018 09:52:14 +0100 Subject: [PATCH 01/23] Clarify error message for -C opt-level The new levels s and z are not mentioned as possible values. --- src/librustc/session/config.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/librustc/session/config.rs b/src/librustc/session/config.rs index 569e7a24d2353..459a748dd6a60 100644 --- a/src/librustc/session/config.rs +++ b/src/librustc/session/config.rs @@ -2083,7 +2083,7 @@ pub fn build_session_options_and_crate_config( error_format, &format!( "optimization level needs to be \ - between 0-3 (instead was `{}`)", + between 0-3, s or z (instead was `{}`)", arg ), ); From 5aeb6c756e4b6e28366dee28802022430c66605e Mon Sep 17 00:00:00 2001 From: "Felix S. Klock II" Date: Sat, 3 Nov 2018 02:24:50 +0100 Subject: [PATCH 02/23] Sidestep an ICE by providing *some* description for `ReEmpty` when it arises. --- src/librustc/infer/error_reporting/mod.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/src/librustc/infer/error_reporting/mod.rs b/src/librustc/infer/error_reporting/mod.rs index d19c495af3b96..6c5f10332956d 100644 --- a/src/librustc/infer/error_reporting/mod.rs +++ b/src/librustc/infer/error_reporting/mod.rs @@ -178,6 +178,7 @@ impl<'a, 'gcx, 'tcx> TyCtxt<'a, 'gcx, 'tcx> { self.msg_span_from_early_bound_and_free_regions(region) } ty::ReStatic => ("the static lifetime".to_owned(), None), + ty::ReEmpty => ("an empty lifetime".to_owned(), None), _ => bug!("{:?}", region), } } From cc33aecb683b1cb08bd60ba1f24ef797827fda94 Mon Sep 17 00:00:00 2001 From: "Felix S. Klock II" Date: Sat, 3 Nov 2018 02:29:26 +0100 Subject: [PATCH 03/23] Regression test for issue 55608. --- .../issue-55608-captures-empty-region.rs | 22 +++++++++++++++++++ .../issue-55608-captures-empty-region.stderr | 11 ++++++++++ 2 files changed, 33 insertions(+) create mode 100644 src/test/ui/impl-trait/issue-55608-captures-empty-region.rs create mode 100644 src/test/ui/impl-trait/issue-55608-captures-empty-region.stderr diff --git a/src/test/ui/impl-trait/issue-55608-captures-empty-region.rs b/src/test/ui/impl-trait/issue-55608-captures-empty-region.rs new file mode 100644 index 0000000000000..7ebc348996f5e --- /dev/null +++ b/src/test/ui/impl-trait/issue-55608-captures-empty-region.rs @@ -0,0 +1,22 @@ +// This used to ICE because it creates an `impl Trait` that captures a +// hidden empty region. + +#![feature(conservative_impl_trait)] + +fn server() -> impl FilterBase2 { //~ ERROR [E0700] + segment2(|| { loop { } }).map2(|| "") +} + +trait FilterBase2 { + fn map2(self, _fn: F) -> Map2 where Self: Sized { loop { } } +} + +struct Map2 { _func: F } + +impl FilterBase2 for Map2 { } + +fn segment2(_fn: F) -> Map2 where F: Fn() -> Result<(), ()> { + loop { } +} + +fn main() { server(); } diff --git a/src/test/ui/impl-trait/issue-55608-captures-empty-region.stderr b/src/test/ui/impl-trait/issue-55608-captures-empty-region.stderr new file mode 100644 index 0000000000000..d1f147834d2ef --- /dev/null +++ b/src/test/ui/impl-trait/issue-55608-captures-empty-region.stderr @@ -0,0 +1,11 @@ +error[E0700]: hidden type for `impl Trait` captures lifetime that does not appear in bounds + --> $DIR/issue-55608-captures-empty-region.rs:6:16 + | +LL | fn server() -> impl FilterBase2 { //~ ERROR [E0700] + | ^^^^^^^^^^^^^^^^ + | + = note: hidden type `Map2<[closure@$DIR/issue-55608-captures-empty-region.rs:7:36: 7:41]>` captures an empty lifetime + +error: aborting due to previous error + +For more information about this error, try `rustc --explain E0700`. From 4c40ff6a2472124cd061721463f329184ca76fa3 Mon Sep 17 00:00:00 2001 From: Nikita Popov Date: Sat, 3 Nov 2018 15:48:29 +0100 Subject: [PATCH 04/23] Implement rotate using funnel shift on LLVM >= 7 Implement the rotate_left and rotate_right operations using llvm.fshl and llvm.fshr if they are available (LLVM >= 7). Originally I wanted to expose the funnel_shift_left and funnel_shift_right intrinsics and implement rotate_left and rotate_right on top of them. However, emulation of funnel shifts requires emitting a conditional to check for zero shift amount, which is not necessary for rotates. I was uncomfortable doing that here, as I don't want to rely on LLVM to optimize away that conditional (and for variable rotates, I'm not sure it can). We should revisit that question when we raise our minimum version requirement to LLVM 7 and don't need emulation code anymore. --- src/libcore/intrinsics.rs | 14 +++++++++++ src/libcore/num/mod.rs | 14 +++++++++-- src/librustc_codegen_llvm/context.rs | 12 +++++++++ .../debuginfo/metadata.rs | 6 ++--- src/librustc_codegen_llvm/intrinsic.rs | 25 ++++++++++++++++++- src/librustc_codegen_llvm/llvm_util.rs | 4 +++ src/librustc_codegen_llvm/mir/mod.rs | 3 ++- src/librustc_mir/interpret/intrinsics.rs | 18 +++++++++++++ src/librustc_mir/transform/qualify_consts.rs | 2 ++ src/librustc_typeck/check/intrinsic.rs | 3 ++- 10 files changed, 93 insertions(+), 8 deletions(-) diff --git a/src/libcore/intrinsics.rs b/src/libcore/intrinsics.rs index cceae9249e456..7ed6e4a8f51eb 100644 --- a/src/libcore/intrinsics.rs +++ b/src/libcore/intrinsics.rs @@ -1465,6 +1465,20 @@ extern "rust-intrinsic" { /// y < 0 or y >= N, where N is the width of T in bits. pub fn unchecked_shr(x: T, y: T) -> T; + /// Performs rotate left. + /// The stabilized versions of this intrinsic are available on the integer + /// primitives via the `rotate_left` method. For example, + /// [`std::u32::rotate_left`](../../std/primitive.u32.html#method.rotate_left) + #[cfg(not(stage0))] + pub fn rotate_left(x: T, y: T) -> T; + + /// Performs rotate right. + /// The stabilized versions of this intrinsic are available on the integer + /// primitives via the `rotate_right` method. For example, + /// [`std::u32::rotate_right`](../../std/primitive.u32.html#method.rotate_right) + #[cfg(not(stage0))] + pub fn rotate_right(x: T, y: T) -> T; + /// Returns (a + b) mod 2N, where N is the width of T in bits. /// The stabilized versions of this intrinsic are available on the integer /// primitives via the `wrapping_add` method. For example, diff --git a/src/libcore/num/mod.rs b/src/libcore/num/mod.rs index c6cbeea5a0ea6..090147c9fe4fa 100644 --- a/src/libcore/num/mod.rs +++ b/src/libcore/num/mod.rs @@ -2301,7 +2301,12 @@ assert_eq!(n.rotate_left(", $rot, "), m); #[rustc_const_unstable(feature = "const_int_rotate")] #[inline] pub const fn rotate_left(self, n: u32) -> Self { - (self << (n % $BITS)) | (self >> (($BITS - (n % $BITS)) % $BITS)) + #[cfg(not(stage0))] { + unsafe { intrinsics::rotate_left(self, n as $SelfT) } + } + #[cfg(stage0)] { + (self << (n % $BITS)) | (self >> (($BITS - (n % $BITS)) % $BITS)) + } } } @@ -2326,7 +2331,12 @@ assert_eq!(n.rotate_right(", $rot, "), m); #[rustc_const_unstable(feature = "const_int_rotate")] #[inline] pub const fn rotate_right(self, n: u32) -> Self { - (self >> (n % $BITS)) | (self << (($BITS - (n % $BITS)) % $BITS)) + #[cfg(not(stage0))] { + unsafe { intrinsics::rotate_right(self, n as $SelfT) } + } + #[cfg(stage0)] { + (self >> (n % $BITS)) | (self << (($BITS - (n % $BITS)) % $BITS)) + } } } diff --git a/src/librustc_codegen_llvm/context.rs b/src/librustc_codegen_llvm/context.rs index 241f7989e1681..db5a3937e5226 100644 --- a/src/librustc_codegen_llvm/context.rs +++ b/src/librustc_codegen_llvm/context.rs @@ -726,6 +726,18 @@ fn declare_intrinsic(cx: &CodegenCx<'ll, '_>, key: &str) -> Option<&'ll Value> { ifn!("llvm.bitreverse.i64", fn(t_i64) -> t_i64); ifn!("llvm.bitreverse.i128", fn(t_i128) -> t_i128); + ifn!("llvm.fshl.i8", fn(t_i8, t_i8, t_i8) -> t_i8); + ifn!("llvm.fshl.i16", fn(t_i16, t_i16, t_i16) -> t_i16); + ifn!("llvm.fshl.i32", fn(t_i32, t_i32, t_i32) -> t_i32); + ifn!("llvm.fshl.i64", fn(t_i64, t_i64, t_i64) -> t_i64); + ifn!("llvm.fshl.i128", fn(t_i128, t_i128, t_i128) -> t_i128); + + ifn!("llvm.fshr.i8", fn(t_i8, t_i8, t_i8) -> t_i8); + ifn!("llvm.fshr.i16", fn(t_i16, t_i16, t_i16) -> t_i16); + ifn!("llvm.fshr.i32", fn(t_i32, t_i32, t_i32) -> t_i32); + ifn!("llvm.fshr.i64", fn(t_i64, t_i64, t_i64) -> t_i64); + ifn!("llvm.fshr.i128", fn(t_i128, t_i128, t_i128) -> t_i128); + ifn!("llvm.sadd.with.overflow.i8", fn(t_i8, t_i8) -> mk_struct!{t_i8, i1}); ifn!("llvm.sadd.with.overflow.i16", fn(t_i16, t_i16) -> mk_struct!{t_i16, i1}); ifn!("llvm.sadd.with.overflow.i32", fn(t_i32, t_i32) -> mk_struct!{t_i32, i1}); diff --git a/src/librustc_codegen_llvm/debuginfo/metadata.rs b/src/librustc_codegen_llvm/debuginfo/metadata.rs index ba1e3f5960c85..00f06645930c2 100644 --- a/src/librustc_codegen_llvm/debuginfo/metadata.rs +++ b/src/librustc_codegen_llvm/debuginfo/metadata.rs @@ -23,6 +23,7 @@ use value::Value; use llvm; use llvm::debuginfo::{DIType, DIFile, DIScope, DIDescriptor, DICompositeType, DILexicalBlock, DIFlags}; +use llvm_util; use rustc_data_structures::stable_hasher::{HashStable, StableHasher}; use rustc::hir::CodegenFnAttrFlags; @@ -1169,9 +1170,8 @@ fn prepare_union_metadata( fn use_enum_fallback(cx: &CodegenCx) -> bool { // On MSVC we have to use the fallback mode, because LLVM doesn't // lower variant parts to PDB. - return cx.sess().target.target.options.is_like_msvc || unsafe { - llvm::LLVMRustVersionMajor() < 7 - }; + return cx.sess().target.target.options.is_like_msvc + || llvm_util::get_major_version() < 7; } // Describes the members of an enum value: An enum is described as a union of diff --git a/src/librustc_codegen_llvm/intrinsic.rs b/src/librustc_codegen_llvm/intrinsic.rs index 03244c18ac3e4..bdafa8b50ba31 100644 --- a/src/librustc_codegen_llvm/intrinsic.rs +++ b/src/librustc_codegen_llvm/intrinsic.rs @@ -13,6 +13,7 @@ use attributes; use intrinsics::{self, Intrinsic}; use llvm::{self, TypeKind}; +use llvm_util; use abi::{Abi, FnType, LlvmType, PassMode}; use mir::place::PlaceRef; use mir::operand::{OperandRef, OperandValue}; @@ -284,7 +285,8 @@ pub fn codegen_intrinsic_call( "ctlz" | "ctlz_nonzero" | "cttz" | "cttz_nonzero" | "ctpop" | "bswap" | "bitreverse" | "add_with_overflow" | "sub_with_overflow" | "mul_with_overflow" | "overflowing_add" | "overflowing_sub" | "overflowing_mul" | - "unchecked_div" | "unchecked_rem" | "unchecked_shl" | "unchecked_shr" | "exact_div" => { + "unchecked_div" | "unchecked_rem" | "unchecked_shl" | "unchecked_shr" | "exact_div" | + "rotate_left" | "rotate_right" => { let ty = arg_tys[0]; match int_type_width_signed(ty, cx) { Some((width, signed)) => @@ -363,6 +365,27 @@ pub fn codegen_intrinsic_call( } else { bx.lshr(args[0].immediate(), args[1].immediate()) }, + "rotate_left" | "rotate_right" => { + let is_left = name == "rotate_left"; + let val = args[0].immediate(); + let raw_shift = args[1].immediate(); + if llvm_util::get_major_version() >= 7 { + // rotate = funnel shift with first two args the same + let llvm_name = &format!("llvm.fsh{}.i{}", + if is_left { 'l' } else { 'r' }, width); + let llfn = cx.get_intrinsic(llvm_name); + bx.call(llfn, &[val, val, raw_shift], None) + } else { + // rotate_left: (X << (S % BW)) | (X >> ((BW - S) % BW)) + // rotate_right: (X << ((BW - S) % BW)) | (X >> (S % BW)) + let width = C_uint(Type::ix(cx, width), width); + let shift = bx.urem(raw_shift, width); + let inv_shift = bx.urem(bx.sub(width, raw_shift), width); + let shift1 = bx.shl(val, if is_left { shift } else { inv_shift }); + let shift2 = bx.lshr(val, if !is_left { shift } else { inv_shift }); + bx.or(shift1, shift2) + } + }, _ => bug!(), }, None => { diff --git a/src/librustc_codegen_llvm/llvm_util.rs b/src/librustc_codegen_llvm/llvm_util.rs index 0a80fdddbf9fd..126b19c0c83fa 100644 --- a/src/librustc_codegen_llvm/llvm_util.rs +++ b/src/librustc_codegen_llvm/llvm_util.rs @@ -256,6 +256,10 @@ pub fn print_version() { } } +pub fn get_major_version() -> u32 { + unsafe { llvm::LLVMRustVersionMajor() } +} + pub fn print_passes() { // Can be called without initializing LLVM unsafe { llvm::LLVMRustPrintPasses(); } diff --git a/src/librustc_codegen_llvm/mir/mod.rs b/src/librustc_codegen_llvm/mir/mod.rs index a6e2ccf92e4e3..e5b25ea068b3b 100644 --- a/src/librustc_codegen_llvm/mir/mod.rs +++ b/src/librustc_codegen_llvm/mir/mod.rs @@ -12,6 +12,7 @@ use common::{C_i32, C_null}; use libc::c_uint; use llvm::{self, BasicBlock}; use llvm::debuginfo::DIScope; +use llvm_util; use rustc::ty::{self, Ty, TypeFoldable, UpvarSubsts}; use rustc::ty::layout::{LayoutOf, TyLayout}; use rustc::mir::{self, Mir}; @@ -612,7 +613,7 @@ fn arg_local_refs( // doesn't actually strip the offset when splitting the closure // environment into its components so it ends up out of bounds. // (cuviper) It seems to be fine without the alloca on LLVM 6 and later. - let env_alloca = !env_ref && unsafe { llvm::LLVMRustVersionMajor() < 6 }; + let env_alloca = !env_ref && llvm_util::get_major_version() < 6; let env_ptr = if env_alloca { let scratch = PlaceRef::alloca(bx, bx.cx.layout_of(tcx.mk_mut_ptr(arg.layout.ty)), diff --git a/src/librustc_mir/interpret/intrinsics.rs b/src/librustc_mir/interpret/intrinsics.rs index 5fa0fef36935d..e4c4bfa7d6588 100644 --- a/src/librustc_mir/interpret/intrinsics.rs +++ b/src/librustc_mir/interpret/intrinsics.rs @@ -150,6 +150,24 @@ impl<'a, 'mir, 'tcx, M: Machine<'a, 'mir, 'tcx>> EvalContext<'a, 'mir, 'tcx, M> } self.write_scalar(val, dest)?; } + "rotate_left" | "rotate_right" => { + // rotate_left: (X << (S % BW)) | (X >> ((BW - S) % BW)) + // rotate_right: (X << ((BW - S) % BW)) | (X >> (S % BW)) + let layout = self.layout_of(substs.type_at(0))?; + let val_bits = self.read_scalar(args[0])?.to_bits(layout.size)?; + let raw_shift_bits = self.read_scalar(args[1])?.to_bits(layout.size)?; + let width_bits = layout.size.bits() as u128; + let shift_bits = raw_shift_bits % width_bits; + let inv_shift_bits = (width_bits - raw_shift_bits) % width_bits; + let result_bits = if intrinsic_name == "rotate_left" { + (val_bits << shift_bits) | (val_bits >> inv_shift_bits) + } else { + (val_bits >> shift_bits) | (val_bits << inv_shift_bits) + }; + let truncated_bits = self.truncate(result_bits, layout); + let result = Scalar::from_uint(truncated_bits, layout.size); + self.write_scalar(result, dest)?; + } "transmute" => { self.copy_op_transmute(args[0], dest)?; } diff --git a/src/librustc_mir/transform/qualify_consts.rs b/src/librustc_mir/transform/qualify_consts.rs index ca9c4eb9b8bb9..03497be03087b 100644 --- a/src/librustc_mir/transform/qualify_consts.rs +++ b/src/librustc_mir/transform/qualify_consts.rs @@ -869,6 +869,8 @@ impl<'a, 'tcx> Visitor<'tcx> for Qualifier<'a, 'tcx, 'tcx> { | "overflowing_mul" | "unchecked_shl" | "unchecked_shr" + | "rotate_left" + | "rotate_right" | "add_with_overflow" | "sub_with_overflow" | "mul_with_overflow" diff --git a/src/librustc_typeck/check/intrinsic.rs b/src/librustc_typeck/check/intrinsic.rs index 3156458b4aa4a..84967aaf72f57 100644 --- a/src/librustc_typeck/check/intrinsic.rs +++ b/src/librustc_typeck/check/intrinsic.rs @@ -292,7 +292,8 @@ pub fn check_intrinsic_type<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>, "unchecked_div" | "unchecked_rem" | "exact_div" => (1, vec![param(0), param(0)], param(0)), - "unchecked_shl" | "unchecked_shr" => + "unchecked_shl" | "unchecked_shr" | + "rotate_left" | "rotate_right" => (1, vec![param(0), param(0)], param(0)), "overflowing_add" | "overflowing_sub" | "overflowing_mul" => From e753d2105159397eff162aa3f1f7715f96b5772d Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Sun, 4 Nov 2018 11:23:34 +0100 Subject: [PATCH 05/23] miri: accept extern types in structs if they are the only field --- src/librustc_mir/interpret/eval_context.rs | 15 ++++++++++++-- src/librustc_mir/interpret/place.rs | 3 ++- src/test/ui/consts/const-eval/issue-55541.rs | 21 ++++++++++++++++++++ 3 files changed, 36 insertions(+), 3 deletions(-) create mode 100644 src/test/ui/consts/const-eval/issue-55541.rs diff --git a/src/librustc_mir/interpret/eval_context.rs b/src/librustc_mir/interpret/eval_context.rs index bc7ad16dc97bc..48a8d0bbe56d6 100644 --- a/src/librustc_mir/interpret/eval_context.rs +++ b/src/librustc_mir/interpret/eval_context.rs @@ -371,8 +371,19 @@ impl<'a, 'mir, 'tcx: 'mir, M: Machine<'a, 'mir, 'tcx>> EvalContext<'a, 'mir, 'tc // the last field). Can't have foreign types here, how would we // adjust alignment and size for them? let field = layout.field(self, layout.fields.count() - 1)?; - let (unsized_size, unsized_align) = self.size_and_align_of(metadata, field)? - .expect("Fields cannot be extern types"); + let (unsized_size, unsized_align) = match self.size_and_align_of(metadata, field)? { + Some(size_and_align) => size_and_align, + None => { + // A field with extern type. If this is the only field, + // we treat this struct just the same. Else, this is an error + // (for now). + if layout.fields.count() == 1 { + return Ok(None) + } else { + bug!("Fields cannot be extern types, unless they are the only field") + } + } + }; // FIXME (#26403, #27023): We should be adding padding // to `sized_size` (to accommodate the `unsized_align` diff --git a/src/librustc_mir/interpret/place.rs b/src/librustc_mir/interpret/place.rs index 3b104e2284fe2..8bd29aff841cf 100644 --- a/src/librustc_mir/interpret/place.rs +++ b/src/librustc_mir/interpret/place.rs @@ -354,7 +354,8 @@ where let (meta, offset) = if field_layout.is_unsized() { // re-use parent metadata to determine dynamic field layout let (_, align) = self.size_and_align_of(base.meta, field_layout)? - .expect("Fields cannot be extern types"); + // If this is an extern type, we fall back to its static size and alignment. + .unwrap_or_else(|| base.layout.size_and_align()); (base.meta, offset.abi_align(align)) } else { // base.meta could be present; we might be accessing a sized field of an unsized diff --git a/src/test/ui/consts/const-eval/issue-55541.rs b/src/test/ui/consts/const-eval/issue-55541.rs new file mode 100644 index 0000000000000..bf8965e836182 --- /dev/null +++ b/src/test/ui/consts/const-eval/issue-55541.rs @@ -0,0 +1,21 @@ +// compile-pass + +// Test that we can handle newtypes wrapping extern types + +#![feature(extern_types, const_transmute)] + +extern "C" { + pub type ExternType; +} +unsafe impl Sync for ExternType {} + +#[repr(transparent)] +pub struct Wrapper(ExternType); + +static MAGIC_FFI_STATIC: u8 = 42; + +pub static MAGIC_FFI_REF: &'static Wrapper = unsafe { + std::mem::transmute(&MAGIC_FFI_STATIC) +}; + +fn main() {} From aca76d42a05c419a539d9b34fa30b88d4cdcadcc Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Sun, 4 Nov 2018 11:55:39 +0100 Subject: [PATCH 06/23] test for offset and alignment of the sized part, instead of field count --- src/librustc_mir/interpret/eval_context.rs | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/src/librustc_mir/interpret/eval_context.rs b/src/librustc_mir/interpret/eval_context.rs index 48a8d0bbe56d6..c1c2efdae7516 100644 --- a/src/librustc_mir/interpret/eval_context.rs +++ b/src/librustc_mir/interpret/eval_context.rs @@ -374,13 +374,13 @@ impl<'a, 'mir, 'tcx: 'mir, M: Machine<'a, 'mir, 'tcx>> EvalContext<'a, 'mir, 'tc let (unsized_size, unsized_align) = match self.size_and_align_of(metadata, field)? { Some(size_and_align) => size_and_align, None => { - // A field with extern type. If this is the only field, - // we treat this struct just the same. Else, this is an error - // (for now). - if layout.fields.count() == 1 { + // A field with extern type. If this field is at offset 0 and the sized + // part makes no alignment constraints, we behave like the underlying + // extern type. + if sized_size == Size::ZERO && sized_align.abi() == 1 { return Ok(None) } else { - bug!("Fields cannot be extern types, unless they are the only field") + bug!("Fields cannot be extern types, unless they are at offset 0") } } }; From bb17f717499132a23e1b54bf3fdd0727c09715ff Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Sun, 4 Nov 2018 14:43:05 +0100 Subject: [PATCH 07/23] also test with PhantomData --- src/test/ui/consts/const-eval/issue-55541.rs | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/src/test/ui/consts/const-eval/issue-55541.rs b/src/test/ui/consts/const-eval/issue-55541.rs index bf8965e836182..611fb89341de4 100644 --- a/src/test/ui/consts/const-eval/issue-55541.rs +++ b/src/test/ui/consts/const-eval/issue-55541.rs @@ -4,18 +4,24 @@ #![feature(extern_types, const_transmute)] +use std::marker::PhantomData; + extern "C" { pub type ExternType; } unsafe impl Sync for ExternType {} +static MAGIC_FFI_STATIC: u8 = 42; #[repr(transparent)] pub struct Wrapper(ExternType); - -static MAGIC_FFI_STATIC: u8 = 42; - pub static MAGIC_FFI_REF: &'static Wrapper = unsafe { std::mem::transmute(&MAGIC_FFI_STATIC) }; +#[repr(transparent)] +pub struct Wrapper2(PhantomData>, ExternType); +pub static MAGIC_FFI_REF2: &'static Wrapper2 = unsafe { + std::mem::transmute(&MAGIC_FFI_STATIC) +}; + fn main() {} From 5ebd077f54c735aeb634d18080c9f127016f1c87 Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Mon, 5 Nov 2018 11:14:21 +0100 Subject: [PATCH 08/23] make it more obvious that the size is not relevant --- src/librustc_mir/interpret/place.rs | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/src/librustc_mir/interpret/place.rs b/src/librustc_mir/interpret/place.rs index 8bd29aff841cf..3dcd081edf9d9 100644 --- a/src/librustc_mir/interpret/place.rs +++ b/src/librustc_mir/interpret/place.rs @@ -353,9 +353,10 @@ where // Offset may need adjustment for unsized fields let (meta, offset) = if field_layout.is_unsized() { // re-use parent metadata to determine dynamic field layout - let (_, align) = self.size_and_align_of(base.meta, field_layout)? - // If this is an extern type, we fall back to its static size and alignment. - .unwrap_or_else(|| base.layout.size_and_align()); + let align = self.size_and_align_of(base.meta, field_layout)? + .map(|(_, align)| align) + // If this is an extern type, we fall back to its static alignment. + .unwrap_or_else(|| base.layout.align); (base.meta, offset.abi_align(align)) } else { // base.meta could be present; we might be accessing a sized field of an unsized From a6622c265c9a359c277af576c4849a74d476f597 Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Mon, 5 Nov 2018 11:20:01 +0100 Subject: [PATCH 09/23] note some FIXMEs --- src/librustc_mir/interpret/eval_context.rs | 3 +++ src/librustc_mir/interpret/place.rs | 3 +++ 2 files changed, 6 insertions(+) diff --git a/src/librustc_mir/interpret/eval_context.rs b/src/librustc_mir/interpret/eval_context.rs index c1c2efdae7516..797e0458e5312 100644 --- a/src/librustc_mir/interpret/eval_context.rs +++ b/src/librustc_mir/interpret/eval_context.rs @@ -377,6 +377,9 @@ impl<'a, 'mir, 'tcx: 'mir, M: Machine<'a, 'mir, 'tcx>> EvalContext<'a, 'mir, 'tc // A field with extern type. If this field is at offset 0 and the sized // part makes no alignment constraints, we behave like the underlying // extern type. + // FIXME: Once we have made decisions for how to handle size and alignment + // of `extern type`, this should be adapted. It is just a temporary hack + // to get some code to work that probably ought to work. if sized_size == Size::ZERO && sized_align.abi() == 1 { return Ok(None) } else { diff --git a/src/librustc_mir/interpret/place.rs b/src/librustc_mir/interpret/place.rs index 3dcd081edf9d9..c60ae7b4a00c4 100644 --- a/src/librustc_mir/interpret/place.rs +++ b/src/librustc_mir/interpret/place.rs @@ -356,6 +356,9 @@ where let align = self.size_and_align_of(base.meta, field_layout)? .map(|(_, align)| align) // If this is an extern type, we fall back to its static alignment. + // FIXME: Once we have made decisions for how to handle size and alignment + // of `extern type`, this should be adapted. It is just a temporary hack + // to get some code to work that probably ought to work. .unwrap_or_else(|| base.layout.align); (base.meta, offset.abi_align(align)) } else { From ba0bab39e04a13ad996e41a2ef2ca9b83fbb2cf4 Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Tue, 6 Nov 2018 08:41:56 +0100 Subject: [PATCH 10/23] make sure we only guess field alignment at offset 0 --- src/librustc_mir/interpret/eval_context.rs | 7 +++---- src/librustc_mir/interpret/place.rs | 18 +++++++++++------- 2 files changed, 14 insertions(+), 11 deletions(-) diff --git a/src/librustc_mir/interpret/eval_context.rs b/src/librustc_mir/interpret/eval_context.rs index 797e0458e5312..cca4e8a3ce31a 100644 --- a/src/librustc_mir/interpret/eval_context.rs +++ b/src/librustc_mir/interpret/eval_context.rs @@ -374,13 +374,12 @@ impl<'a, 'mir, 'tcx: 'mir, M: Machine<'a, 'mir, 'tcx>> EvalContext<'a, 'mir, 'tc let (unsized_size, unsized_align) = match self.size_and_align_of(metadata, field)? { Some(size_and_align) => size_and_align, None => { - // A field with extern type. If this field is at offset 0 and the sized - // part makes no alignment constraints, we behave like the underlying - // extern type. + // A field with extern type. If this field is at offset 0, we behave + // like the underlying extern type. // FIXME: Once we have made decisions for how to handle size and alignment // of `extern type`, this should be adapted. It is just a temporary hack // to get some code to work that probably ought to work. - if sized_size == Size::ZERO && sized_align.abi() == 1 { + if sized_size == Size::ZERO { return Ok(None) } else { bug!("Fields cannot be extern types, unless they are at offset 0") diff --git a/src/librustc_mir/interpret/place.rs b/src/librustc_mir/interpret/place.rs index c60ae7b4a00c4..a836a199f768d 100644 --- a/src/librustc_mir/interpret/place.rs +++ b/src/librustc_mir/interpret/place.rs @@ -353,13 +353,17 @@ where // Offset may need adjustment for unsized fields let (meta, offset) = if field_layout.is_unsized() { // re-use parent metadata to determine dynamic field layout - let align = self.size_and_align_of(base.meta, field_layout)? - .map(|(_, align)| align) - // If this is an extern type, we fall back to its static alignment. - // FIXME: Once we have made decisions for how to handle size and alignment - // of `extern type`, this should be adapted. It is just a temporary hack - // to get some code to work that probably ought to work. - .unwrap_or_else(|| base.layout.align); + let align = match self.size_and_align_of(base.meta, field_layout)? { + Some((_, align)) => align, + None if offset == Size::ZERO => + // An extern type at offset 0, we fall back to its static alignment. + // FIXME: Once we have made decisions for how to handle size and alignment + // of `extern type`, this should be adapted. It is just a temporary hack + // to get some code to work that probably ought to work. + field_layout.align, + None => + bug!("Cannot compute offset for extern type field at non-0 offset"), + }; (base.meta, offset.abi_align(align)) } else { // base.meta could be present; we might be accessing a sized field of an unsized From e7f8c0d1050bc8871f5bb63cfe5018ed854d91e3 Mon Sep 17 00:00:00 2001 From: "Felix S. Klock II" Date: Tue, 6 Nov 2018 14:21:52 +0100 Subject: [PATCH 11/23] Sidestep link error from rustfix'ed code by using a *defined* static. As a drive-by, added `-g` to the compile-flags so that the test more reliably fails to compile when the extern static in question is *not* provided. (I.e. this is making the test more robust in the face of potential future revisions.) Fix #54388. --- src/test/ui/extern/extern-const.fixed | 25 +++++++++++++++++++++++++ src/test/ui/extern/extern-const.rs | 24 ++++++++++++------------ src/test/ui/extern/extern-const.stderr | 2 +- 3 files changed, 38 insertions(+), 13 deletions(-) create mode 100644 src/test/ui/extern/extern-const.fixed diff --git a/src/test/ui/extern/extern-const.fixed b/src/test/ui/extern/extern-const.fixed new file mode 100644 index 0000000000000..0229362cf0eca --- /dev/null +++ b/src/test/ui/extern/extern-const.fixed @@ -0,0 +1,25 @@ +// This test is checking that extern items cannot be const. It also +// checks that `rustfix` suggests the alternative of using an extern +// static. +// +// #54388: an unused reference to an undefined static may or may not +// compile. To sidestep this by using one that *is* defined. + +// run-rustfix +// compile-flags: -g -Z continue-parse-after-error +#![feature(libc)] +extern crate libc; + +#[link(name = "rust_test_helpers", kind = "static")] +extern "C" { + static rust_dbg_static_mut: libc::c_int; //~ ERROR extern items cannot be `const` +} + +fn main() { + // We suggest turning the (illegal) extern `const` into an extern `static`, + // but this also requires `unsafe` (a deny-by-default lint at comment time, + // future error; Issue #36247) + unsafe { + let _x = rust_dbg_static_mut; + } +} diff --git a/src/test/ui/extern/extern-const.rs b/src/test/ui/extern/extern-const.rs index d8a167311d55c..e7b1715255532 100644 --- a/src/test/ui/extern/extern-const.rs +++ b/src/test/ui/extern/extern-const.rs @@ -1,18 +1,18 @@ -// Copyright 2017 The Rust Project Developers. See the COPYRIGHT -// file at the top-level directory of this distribution and at -// http://rust-lang.org/COPYRIGHT. +// This test is checking that extern items cannot be const. It also +// checks that `rustfix` suggests the alternative of using an extern +// static. // -// Licensed under the Apache License, Version 2.0 or the MIT license -// , at your -// option. This file may not be copied, modified, or distributed -// except according to those terms. +// #54388: an unused reference to an undefined static may or may not +// compile. To sidestep this by using one that *is* defined. -// FIXME(#54388): re-enable rustfix later, when this test has consistent output across targets -// compile-flags: -Z continue-parse-after-error +// run-rustfix +// compile-flags: -g -Z continue-parse-after-error +#![feature(libc)] +extern crate libc; +#[link(name = "rust_test_helpers", kind = "static")] extern "C" { - const C: u8; //~ ERROR extern items cannot be `const` + const rust_dbg_static_mut: libc::c_int; //~ ERROR extern items cannot be `const` } fn main() { @@ -20,6 +20,6 @@ fn main() { // but this also requires `unsafe` (a deny-by-default lint at comment time, // future error; Issue #36247) unsafe { - let _x = C; + let _x = rust_dbg_static_mut; } } diff --git a/src/test/ui/extern/extern-const.stderr b/src/test/ui/extern/extern-const.stderr index cbed5e56c76c4..7ebaec0368e3d 100644 --- a/src/test/ui/extern/extern-const.stderr +++ b/src/test/ui/extern/extern-const.stderr @@ -1,7 +1,7 @@ error: extern items cannot be `const` --> $DIR/extern-const.rs:15:5 | -LL | const C: u8; //~ ERROR extern items cannot be `const` +LL | const rust_dbg_static_mut: libc::c_int; //~ ERROR extern items cannot be `const` | ^^^^^ help: try using a static value: `static` error: aborting due to previous error From db3c69ec80a9a9fbb2c4a7d3e10547549152387a Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Tue, 6 Nov 2018 16:00:01 +0100 Subject: [PATCH 12/23] impl_stable_hash_for: support enums and tuple structs with generic parameters --- src/librustc/ich/impls_mir.rs | 71 ++---- src/librustc/ich/impls_ty.rs | 426 ++++++++++------------------------ src/librustc/macros.rs | 72 ++++-- 3 files changed, 192 insertions(+), 377 deletions(-) diff --git a/src/librustc/ich/impls_mir.rs b/src/librustc/ich/impls_mir.rs index a73fe2b8a1ab3..c42c19e82c7c8 100644 --- a/src/librustc/ich/impls_mir.rs +++ b/src/librustc/ich/impls_mir.rs @@ -37,68 +37,31 @@ impl_stable_hash_for!(struct mir::BasicBlockData<'tcx> { statements, terminator, impl_stable_hash_for!(struct mir::UnsafetyViolation { source_info, description, details, kind }); impl_stable_hash_for!(struct mir::UnsafetyCheckResult { violations, unsafe_blocks }); -impl<'a> HashStable> -for mir::BorrowKind { - #[inline] - fn hash_stable(&self, - hcx: &mut StableHashingContext<'a>, - hasher: &mut StableHasher) { - mem::discriminant(self).hash_stable(hcx, hasher); - - match *self { - mir::BorrowKind::Shared | - mir::BorrowKind::Shallow | - mir::BorrowKind::Unique => {} - mir::BorrowKind::Mut { allow_two_phase_borrow } => { - allow_two_phase_borrow.hash_stable(hcx, hasher); - } - } - } -} - - -impl<'a> HashStable> -for mir::UnsafetyViolationKind { - #[inline] - fn hash_stable(&self, - hcx: &mut StableHashingContext<'a>, - hasher: &mut StableHasher) { - - mem::discriminant(self).hash_stable(hcx, hasher); - - match *self { - mir::UnsafetyViolationKind::General => {} - mir::UnsafetyViolationKind::MinConstFn => {} - mir::UnsafetyViolationKind::ExternStatic(lint_node_id) | - mir::UnsafetyViolationKind::BorrowPacked(lint_node_id) => { - lint_node_id.hash_stable(hcx, hasher); - } +impl_stable_hash_for!(enum mir::BorrowKind { + Shared, + Shallow, + Unique, + Mut { allow_two_phase_borrow }, +}); - } - } -} +impl_stable_hash_for!(enum mir::UnsafetyViolationKind { + General, + MinConstFn, + ExternStatic(lint_node_id), + BorrowPacked(lint_node_id), +}); impl_stable_hash_for!(struct mir::Terminator<'tcx> { kind, source_info }); -impl<'a, 'gcx, T> HashStable> for mir::ClearCrossCrate - where T: HashStable> -{ - #[inline] - fn hash_stable(&self, - hcx: &mut StableHashingContext<'a>, - hasher: &mut StableHasher) { - mem::discriminant(self).hash_stable(hcx, hasher); - match *self { - mir::ClearCrossCrate::Clear => {} - mir::ClearCrossCrate::Set(ref value) => { - value.hash_stable(hcx, hasher); - } - } +impl_stable_hash_for!( + impl for enum mir::ClearCrossCrate [ mir::ClearCrossCrate ] { + Clear, + Set(value), } -} +); impl<'a> HashStable> for mir::Local { #[inline] diff --git a/src/librustc/ich/impls_ty.rs b/src/librustc/ich/impls_ty.rs index 7d25ecedb4e04..f3a62975dd9f4 100644 --- a/src/librustc/ich/impls_ty.rs +++ b/src/librustc/ich/impls_ty.rs @@ -224,20 +224,10 @@ impl_stable_hash_for!(enum ty::BorrowKind { MutBorrow }); -impl<'a, 'gcx> HashStable> -for ty::UpvarCapture<'gcx> { - fn hash_stable(&self, - hcx: &mut StableHashingContext<'a>, - hasher: &mut StableHasher) { - mem::discriminant(self).hash_stable(hcx, hasher); - match *self { - ty::UpvarCapture::ByValue => {} - ty::UpvarCapture::ByRef(ref up_var_borrow) => { - up_var_borrow.hash_stable(hcx, hasher); - } - } - } -} +impl_stable_hash_for!(impl<'gcx> for enum ty::UpvarCapture<'gcx> [ ty::UpvarCapture ] { + ByValue, + ByRef(up_var_borrow), +}); impl_stable_hash_for!(struct ty::GenSig<'tcx> { yield_ty, @@ -272,64 +262,23 @@ impl_stable_hash_for!(enum ty::Visibility { impl_stable_hash_for!(struct ty::TraitRef<'tcx> { def_id, substs }); impl_stable_hash_for!(struct ty::TraitPredicate<'tcx> { trait_ref }); impl_stable_hash_for!(struct ty::SubtypePredicate<'tcx> { a_is_expected, a, b }); - -impl<'a, 'gcx, A, B> HashStable> -for ty::OutlivesPredicate - where A: HashStable>, - B: HashStable>, -{ - fn hash_stable(&self, - hcx: &mut StableHashingContext<'a>, - hasher: &mut StableHasher) { - let ty::OutlivesPredicate(ref a, ref b) = *self; - a.hash_stable(hcx, hasher); - b.hash_stable(hcx, hasher); - } -} - +impl_stable_hash_for!(impl for tuple_struct ty::OutlivesPredicate { a, b }); impl_stable_hash_for!(struct ty::ProjectionPredicate<'tcx> { projection_ty, ty }); impl_stable_hash_for!(struct ty::ProjectionTy<'tcx> { substs, item_def_id }); - -impl<'a, 'gcx> HashStable> for ty::Predicate<'gcx> { - fn hash_stable(&self, - hcx: &mut StableHashingContext<'a>, - hasher: &mut StableHasher) { - mem::discriminant(self).hash_stable(hcx, hasher); - match *self { - ty::Predicate::Trait(ref pred) => { - pred.hash_stable(hcx, hasher); - } - ty::Predicate::Subtype(ref pred) => { - pred.hash_stable(hcx, hasher); - } - ty::Predicate::RegionOutlives(ref pred) => { - pred.hash_stable(hcx, hasher); - } - ty::Predicate::TypeOutlives(ref pred) => { - pred.hash_stable(hcx, hasher); - } - ty::Predicate::Projection(ref pred) => { - pred.hash_stable(hcx, hasher); - } - ty::Predicate::WellFormed(ty) => { - ty.hash_stable(hcx, hasher); - } - ty::Predicate::ObjectSafe(def_id) => { - def_id.hash_stable(hcx, hasher); - } - ty::Predicate::ClosureKind(def_id, closure_substs, closure_kind) => { - def_id.hash_stable(hcx, hasher); - closure_substs.hash_stable(hcx, hasher); - closure_kind.hash_stable(hcx, hasher); - } - ty::Predicate::ConstEvaluatable(def_id, substs) => { - def_id.hash_stable(hcx, hasher); - substs.hash_stable(hcx, hasher); - } - } +impl_stable_hash_for!( + impl<'tcx> for enum ty::Predicate<'tcx> [ ty::Predicate ] { + Trait(pred), + Subtype(pred), + RegionOutlives(pred), + TypeOutlives(pred), + Projection(pred), + WellFormed(ty), + ObjectSafe(def_id), + ClosureKind(def_id, closure_substs, closure_kind), + ConstEvaluatable(def_id, substs), } -} +); impl<'a> HashStable> for ty::AdtFlags { fn hash_stable(&self, @@ -358,70 +307,39 @@ impl_stable_hash_for!(struct ty::FieldDef { vis, }); -impl<'a, 'gcx> HashStable> -for ::mir::interpret::ConstValue<'gcx> { - fn hash_stable(&self, - hcx: &mut StableHashingContext<'a>, - hasher: &mut StableHasher) { - use mir::interpret::ConstValue::*; - - mem::discriminant(self).hash_stable(hcx, hasher); - - match *self { - Unevaluated(def_id, substs) => { - def_id.hash_stable(hcx, hasher); - substs.hash_stable(hcx, hasher); - } - Scalar(val) => { - val.hash_stable(hcx, hasher); - } - ScalarPair(a, b) => { - a.hash_stable(hcx, hasher); - b.hash_stable(hcx, hasher); - } - ByRef(id, alloc, offset) => { - id.hash_stable(hcx, hasher); - alloc.hash_stable(hcx, hasher); - offset.hash_stable(hcx, hasher); - } - } +impl_stable_hash_for!( + impl<'tcx> for enum mir::interpret::ConstValue<'tcx> [ mir::interpret::ConstValue ] { + Unevaluated(def_id, substs), + Scalar(val), + ScalarPair(a, b), + ByRef(id, alloc, offset), } -} +); -impl<'a, Tag> HashStable> -for ::mir::interpret::Pointer -where Tag: HashStable> -{ - fn hash_stable(&self, - hcx: &mut StableHashingContext<'a>, - hasher: &mut StableHasher) { - let ::mir::interpret::Pointer { alloc_id, offset, tag } = self; - alloc_id.hash_stable(hcx, hasher); - offset.hash_stable(hcx, hasher); - tag.hash_stable(hcx, hasher); +impl_stable_hash_for! { + impl for struct mir::interpret::Pointer { + alloc_id, + offset, + tag, } } -impl<'a, Tag> HashStable> -for ::mir::interpret::Scalar -where Tag: HashStable> -{ - fn hash_stable(&self, - hcx: &mut StableHashingContext<'a>, - hasher: &mut StableHasher) { - use mir::interpret::Scalar::*; +impl_stable_hash_for!( + impl for enum mir::interpret::Scalar [ mir::interpret::Scalar ] { + Bits { bits, size }, + Ptr(ptr), + } +); - mem::discriminant(self).hash_stable(hcx, hasher); - match self { - Bits { bits, size } => { - bits.hash_stable(hcx, hasher); - size.hash_stable(hcx, hasher); - }, - Ptr(ptr) => ptr.hash_stable(hcx, hasher), - } +impl_stable_hash_for!( + impl<'tcx, M> for enum mir::interpret::AllocType<'tcx, M> [ mir::interpret::AllocType ] { + Function(instance), + Static(def_id), + Memory(mem), } -} +); +// AllocIds get resolved to whatever they point to (to be stable) impl<'a> HashStable> for mir::interpret::AllocId { fn hash_stable( &self, @@ -437,23 +355,7 @@ impl<'a> HashStable> for mir::interpret::AllocId { } } -impl<'a, 'gcx, M: HashStable>> HashStable> -for mir::interpret::AllocType<'gcx, M> { - fn hash_stable(&self, - hcx: &mut StableHashingContext<'a>, - hasher: &mut StableHasher) { - use mir::interpret::AllocType::*; - - mem::discriminant(self).hash_stable(hcx, hasher); - - match *self { - Function(instance) => instance.hash_stable(hcx, hasher), - Static(def_id) => def_id.hash_stable(hcx, hasher), - Memory(ref mem) => mem.hash_stable(hcx, hasher), - } - } -} - +// Allocations treat their relocations specially impl<'a> HashStable> for mir::interpret::Allocation { fn hash_stable( &self, @@ -485,7 +387,7 @@ impl_stable_hash_for!(enum mir::interpret::ErrorHandled { TooGeneric }); -impl_stable_hash_for!(struct ::mir::interpret::FrameInfo { +impl_stable_hash_for!(struct mir::interpret::FrameInfo { span, lint_root, location @@ -499,124 +401,75 @@ impl_stable_hash_for!(struct ty::GenericPredicates<'tcx> { predicates }); -impl<'a, 'gcx, O: HashStable>> HashStable> -for ::mir::interpret::EvalErrorKind<'gcx, O> { - fn hash_stable(&self, - hcx: &mut StableHashingContext<'a>, - hasher: &mut StableHasher) { - use mir::interpret::EvalErrorKind::*; - - mem::discriminant(&self).hash_stable(hcx, hasher); - - match *self { - FunctionArgCountMismatch | - DanglingPointerDeref | - DoubleFree | - InvalidMemoryAccess | - InvalidFunctionPointer | - InvalidBool | - InvalidNullPointerUsage | - ReadPointerAsBytes | - ReadBytesAsPointer | - ReadForeignStatic | - InvalidPointerMath | - DeadLocal | - StackFrameLimitReached | - OutOfTls | - TlsOutOfBounds | - CalledClosureAsFunction | - VtableForArgumentlessMethod | - ModifiedConstantMemory | - AssumptionNotHeld | - InlineAsm | - ReallocateNonBasePtr | - DeallocateNonBasePtr | - HeapAllocZeroBytes | - Unreachable | - ReadFromReturnPointer | - UnimplementedTraitSelection | - TypeckError | - TooGeneric | - DerefFunctionPointer | - ExecuteMemory | - OverflowNeg | - RemainderByZero | - DivisionByZero | - GeneratorResumedAfterReturn | - GeneratorResumedAfterPanic | - ReferencedConstant | - InfiniteLoop => {} - ReadUndefBytes(offset) => offset.hash_stable(hcx, hasher), - InvalidDiscriminant(val) => val.hash_stable(hcx, hasher), - Panic { ref msg, ref file, line, col } => { - msg.hash_stable(hcx, hasher); - file.hash_stable(hcx, hasher); - line.hash_stable(hcx, hasher); - col.hash_stable(hcx, hasher); - }, - MachineError(ref err) => err.hash_stable(hcx, hasher), - FunctionAbiMismatch(a, b) => { - a.hash_stable(hcx, hasher); - b.hash_stable(hcx, hasher) - }, - FunctionArgMismatch(a, b) => { - a.hash_stable(hcx, hasher); - b.hash_stable(hcx, hasher) - }, - FunctionRetMismatch(a, b) => { - a.hash_stable(hcx, hasher); - b.hash_stable(hcx, hasher) - }, - NoMirFor(ref s) => s.hash_stable(hcx, hasher), - UnterminatedCString(ptr) => ptr.hash_stable(hcx, hasher), - PointerOutOfBounds { - ptr, - access, - allocation_size, - } => { - ptr.hash_stable(hcx, hasher); - access.hash_stable(hcx, hasher); - allocation_size.hash_stable(hcx, hasher) - }, - InvalidBoolOp(bop) => bop.hash_stable(hcx, hasher), - Unimplemented(ref s) => s.hash_stable(hcx, hasher), - BoundsCheck { ref len, ref index } => { - len.hash_stable(hcx, hasher); - index.hash_stable(hcx, hasher) - }, - Intrinsic(ref s) => s.hash_stable(hcx, hasher), - InvalidChar(c) => c.hash_stable(hcx, hasher), - AbiViolation(ref s) => s.hash_stable(hcx, hasher), - AlignmentCheckFailed { - required, - has, - } => { - required.hash_stable(hcx, hasher); - has.hash_stable(hcx, hasher) - }, - ValidationFailure(ref s) => s.hash_stable(hcx, hasher), - TypeNotPrimitive(ty) => ty.hash_stable(hcx, hasher), - ReallocatedWrongMemoryKind(ref a, ref b) => { - a.hash_stable(hcx, hasher); - b.hash_stable(hcx, hasher) - }, - DeallocatedWrongMemoryKind(ref a, ref b) => { - a.hash_stable(hcx, hasher); - b.hash_stable(hcx, hasher) - }, - IncorrectAllocationInformation(a, b, c, d) => { - a.hash_stable(hcx, hasher); - b.hash_stable(hcx, hasher); - c.hash_stable(hcx, hasher); - d.hash_stable(hcx, hasher) - }, - Layout(lay) => lay.hash_stable(hcx, hasher), - HeapAllocNonPowerOfTwoAlignment(n) => n.hash_stable(hcx, hasher), - PathNotFound(ref v) => v.hash_stable(hcx, hasher), - Overflow(op) => op.hash_stable(hcx, hasher), - } +impl_stable_hash_for!( + impl<'tcx, O> for enum mir::interpret::EvalErrorKind<'tcx, O> + [ mir::interpret::EvalErrorKind ] + { + FunctionArgCountMismatch, + DanglingPointerDeref, + DoubleFree, + InvalidMemoryAccess, + InvalidFunctionPointer, + InvalidBool, + InvalidNullPointerUsage, + ReadPointerAsBytes, + ReadBytesAsPointer, + ReadForeignStatic, + InvalidPointerMath, + DeadLocal, + StackFrameLimitReached, + OutOfTls, + TlsOutOfBounds, + CalledClosureAsFunction, + VtableForArgumentlessMethod, + ModifiedConstantMemory, + AssumptionNotHeld, + InlineAsm, + ReallocateNonBasePtr, + DeallocateNonBasePtr, + HeapAllocZeroBytes, + Unreachable, + ReadFromReturnPointer, + UnimplementedTraitSelection, + TypeckError, + TooGeneric, + DerefFunctionPointer, + ExecuteMemory, + OverflowNeg, + RemainderByZero, + DivisionByZero, + GeneratorResumedAfterReturn, + GeneratorResumedAfterPanic, + ReferencedConstant, + InfiniteLoop, + ReadUndefBytes(offset), + InvalidDiscriminant(val), + Panic { msg, file, line, col }, + MachineError(err), + FunctionAbiMismatch(a, b), + FunctionArgMismatch(a, b), + FunctionRetMismatch(a, b), + NoMirFor(s), + UnterminatedCString(ptr), + PointerOutOfBounds { ptr, access, allocation_size }, + InvalidBoolOp(bop), + Unimplemented(s), + BoundsCheck { len, index }, + Intrinsic(s), + InvalidChar(c), + AbiViolation(s), + AlignmentCheckFailed { required, has }, + ValidationFailure(s), + TypeNotPrimitive(ty), + ReallocatedWrongMemoryKind(a, b), + DeallocatedWrongMemoryKind(a, b), + IncorrectAllocationInformation(a, b, c, d), + Layout(lay), + HeapAllocNonPowerOfTwoAlignment(n), + PathNotFound(v), + Overflow(op), } -} +); impl_stable_hash_for!(enum mir::interpret::Lock { NoLock, @@ -663,47 +516,18 @@ impl_stable_hash_for!(struct ty::GenericParamDef { kind }); -impl<'a> HashStable> for ty::GenericParamDefKind { - fn hash_stable(&self, - hcx: &mut StableHashingContext<'a>, - hasher: &mut StableHasher) { - mem::discriminant(self).hash_stable(hcx, hasher); - match *self { - ty::GenericParamDefKind::Lifetime => {} - ty::GenericParamDefKind::Type { - has_default, - ref object_lifetime_default, - ref synthetic, - } => { - has_default.hash_stable(hcx, hasher); - object_lifetime_default.hash_stable(hcx, hasher); - synthetic.hash_stable(hcx, hasher); - } - } - } -} - -impl<'a, 'gcx, T> HashStable> -for ::middle::resolve_lifetime::Set1 - where T: HashStable> -{ - fn hash_stable(&self, - hcx: &mut StableHashingContext<'a>, - hasher: &mut StableHasher) { - use middle::resolve_lifetime::Set1; +impl_stable_hash_for!(enum ty::GenericParamDefKind { + Lifetime, + Type { has_default, object_lifetime_default, synthetic }, +}); - mem::discriminant(self).hash_stable(hcx, hasher); - match *self { - Set1::Empty | - Set1::Many => { - // Nothing to do. - } - Set1::One(ref value) => { - value.hash_stable(hcx, hasher); - } - } +impl_stable_hash_for!( + impl for enum ::middle::resolve_lifetime::Set1 [ ::middle::resolve_lifetime::Set1 ] { + Empty, + Many, + One(value), } -} +); impl_stable_hash_for!(enum ::middle::resolve_lifetime::LifetimeDefOrigin { ExplicitOrElided, @@ -1250,7 +1074,7 @@ impl_stable_hash_for!( ); impl_stable_hash_for!( - impl<'tcx> for struct infer::canonical::CanonicalVarValues<'tcx> { + struct infer::canonical::CanonicalVarValues<'tcx> { var_values } ); @@ -1369,7 +1193,7 @@ impl<'a, 'tcx> HashStable> for traits::Goal<'tcx> { } impl_stable_hash_for!( - impl<'tcx> for struct traits::ProgramClause<'tcx> { + struct traits::ProgramClause<'tcx> { goal, hypotheses, category } ); @@ -1404,7 +1228,7 @@ impl_stable_hash_for!(struct ty::subst::UserSubsts<'tcx> { substs, user_self_ty impl_stable_hash_for!(struct ty::subst::UserSelfTy<'tcx> { impl_def_id, self_ty }); impl_stable_hash_for!( - impl<'tcx> for struct traits::Environment<'tcx> { + struct traits::Environment<'tcx> { clauses, } ); diff --git a/src/librustc/macros.rs b/src/librustc/macros.rs index f21f949c9f5cd..03b50c46a8ba6 100644 --- a/src/librustc/macros.rs +++ b/src/librustc/macros.rs @@ -81,6 +81,7 @@ macro_rules! __impl_stable_hash_field { #[macro_export] macro_rules! impl_stable_hash_for { + // Enums // FIXME(mark-i-m): Some of these should be `?` rather than `*`. See the git blame and change // them back when `?` is supported again. (enum $enum_name:path { @@ -91,12 +92,37 @@ macro_rules! impl_stable_hash_for { $( { $($named_field:ident $(-> $named_delegate:tt)*),* } )* ),* $(,)* }) => { - impl<'a, 'tcx> ::rustc_data_structures::stable_hasher::HashStable<$crate::ich::StableHashingContext<'a>> for $enum_name { + impl_stable_hash_for!( + impl<> for enum $enum_name [ $enum_name ] { $( $variant + $( ( $($field $(-> $delegate)*),* ) )* + $( { $($named_field $(-> $named_delegate)*),* } )* + ),* } + ); + }; + // We want to use the enum name both in the `impl ... for $enum_name` as well as for + // importing all the variants. Unfortunately it seems we have to take the name + // twice for this purpose + (impl<$($lt:lifetime $(: $lt_bound:lifetime)* ),* $(,)* $($T:ident),* $(,)*> + for enum $enum_name:path + [ $enum_path:path ] + { + $( $variant:ident + // this incorrectly allows specifying both tuple-like and struct-like fields, as in `Variant(a,b){c,d}`, + // when it should be only one or the other + $( ( $($field:ident $(-> $delegate:tt)*),* ) )* + $( { $($named_field:ident $(-> $named_delegate:tt)*),* } )* + ),* $(,)* + }) => { + impl<'a, $($lt $(: $lt_bound)*,)* $($T,)*> + ::rustc_data_structures::stable_hasher::HashStable<$crate::ich::StableHashingContext<'a>> + for $enum_name + where $($T: ::rustc_data_structures::stable_hasher::HashStable<$crate::ich::StableHashingContext<'a>>),* + { #[inline] fn hash_stable(&self, __ctx: &mut $crate::ich::StableHashingContext<'a>, __hasher: &mut ::rustc_data_structures::stable_hasher::StableHasher) { - use $enum_name::*; + use $enum_path::*; ::std::mem::discriminant(self).hash_stable(__ctx, __hasher); match *self { @@ -110,9 +136,20 @@ macro_rules! impl_stable_hash_for { } } }; + // Structs // FIXME(mark-i-m): same here. (struct $struct_name:path { $($field:ident $(-> $delegate:tt)*),* $(,)* }) => { - impl<'a, 'tcx> ::rustc_data_structures::stable_hasher::HashStable<$crate::ich::StableHashingContext<'a>> for $struct_name { + impl_stable_hash_for!( + impl<'tcx> for struct $struct_name { $($field $(-> $delegate)*),* } + ); + }; + (impl<$($lt:lifetime $(: $lt_bound:lifetime)* ),* $(,)* $($T:ident),* $(,)*> for struct $struct_name:path { + $($field:ident $(-> $delegate:tt)*),* $(,)* + }) => { + impl<'a, $($lt $(: $lt_bound)*,)* $($T,)*> + ::rustc_data_structures::stable_hasher::HashStable<$crate::ich::StableHashingContext<'a>> for $struct_name + where $($T: ::rustc_data_structures::stable_hasher::HashStable<$crate::ich::StableHashingContext<'a>>),* + { #[inline] fn hash_stable(&self, __ctx: &mut $crate::ich::StableHashingContext<'a>, @@ -125,26 +162,17 @@ macro_rules! impl_stable_hash_for { } } }; + // Tuple structs + // We cannot use normale parentheses here, the parser won't allow it // FIXME(mark-i-m): same here. (tuple_struct $struct_name:path { $($field:ident $(-> $delegate:tt)*),* $(,)* }) => { - impl<'a, 'tcx> ::rustc_data_structures::stable_hasher::HashStable<$crate::ich::StableHashingContext<'a>> for $struct_name { - #[inline] - fn hash_stable(&self, - __ctx: &mut $crate::ich::StableHashingContext<'a>, - __hasher: &mut ::rustc_data_structures::stable_hasher::StableHasher) { - let $struct_name ( - $(ref $field),* - ) = *self; - - $( __impl_stable_hash_field!($field, __ctx, __hasher $(, $delegate)*) );* - } - } + impl_stable_hash_for!( + impl<'tcx> for tuple_struct $struct_name { $($field $(-> $delegate)*),* } + ); }; - - (impl<$tcx:lifetime $(, $lt:lifetime $(: $lt_bound:lifetime)*)* $(, $T:ident)*> for struct $struct_name:path { - $($field:ident $(-> $delegate:tt)*),* $(,)* - }) => { - impl<'a, $tcx, $($lt $(: $lt_bound)*,)* $($T,)*> + (impl<$($lt:lifetime $(: $lt_bound:lifetime)* ),* $(,)* $($T:ident),* $(,)*> + for tuple_struct $struct_name:path { $($field:ident $(-> $delegate:tt)*),* $(,)* }) => { + impl<'a, $($lt $(: $lt_bound)*,)* $($T,)*> ::rustc_data_structures::stable_hasher::HashStable<$crate::ich::StableHashingContext<'a>> for $struct_name where $($T: ::rustc_data_structures::stable_hasher::HashStable<$crate::ich::StableHashingContext<'a>>),* { @@ -152,9 +180,9 @@ macro_rules! impl_stable_hash_for { fn hash_stable(&self, __ctx: &mut $crate::ich::StableHashingContext<'a>, __hasher: &mut ::rustc_data_structures::stable_hasher::StableHasher) { - let $struct_name { + let $struct_name ( $(ref $field),* - } = *self; + ) = *self; $( __impl_stable_hash_field!($field, __ctx, __hasher $(, $delegate)*) );* } From 3ec837e763ba423ddff0cdc886eba91f2836b6f8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Esteban=20K=C3=BCber?= Date: Tue, 6 Nov 2018 14:49:09 -0800 Subject: [PATCH 13/23] Elide anon lifetimes in conflicting impl note --- src/librustc/traits/specialize/mod.rs | 5 ++++- src/test/ui/coherence/coherence-impls-copy.stderr | 4 ++-- src/test/ui/e0119/issue-28981.stderr | 2 +- 3 files changed, 7 insertions(+), 4 deletions(-) diff --git a/src/librustc/traits/specialize/mod.rs b/src/librustc/traits/specialize/mod.rs index 0ce1d8f822755..d7b5dd049e350 100644 --- a/src/librustc/traits/specialize/mod.rs +++ b/src/librustc/traits/specialize/mod.rs @@ -396,7 +396,10 @@ fn to_pretty_impl_header(tcx: TyCtxt<'_, '_, '_>, impl_def_id: DefId) -> Option< if !substs.is_noop() { types_without_default_bounds.extend(substs.types()); w.push('<'); - w.push_str(&substs.iter().map(|k| k.to_string()).collect::>().join(", ")); + w.push_str(&substs.iter() + .map(|k| k.to_string()) + .filter(|k| &k[..] != "'_") + .collect::>().join(", ")); w.push('>'); } diff --git a/src/test/ui/coherence/coherence-impls-copy.stderr b/src/test/ui/coherence/coherence-impls-copy.stderr index 613ee0a269e70..dc417957795f5 100644 --- a/src/test/ui/coherence/coherence-impls-copy.stderr +++ b/src/test/ui/coherence/coherence-impls-copy.stderr @@ -14,7 +14,7 @@ LL | impl Copy for &'static NotSync {} | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ | = note: conflicting implementation in crate `core`: - - impl<'_, T> std::marker::Copy for &T + - impl std::marker::Copy for &T where T: ?Sized; error[E0119]: conflicting implementations of trait `std::marker::Copy` for type `&[NotSync]`: @@ -24,7 +24,7 @@ LL | impl Copy for &'static [NotSync] {} | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ | = note: conflicting implementation in crate `core`: - - impl<'_, T> std::marker::Copy for &T + - impl std::marker::Copy for &T where T: ?Sized; error[E0206]: the trait `Copy` may not be implemented for this type diff --git a/src/test/ui/e0119/issue-28981.stderr b/src/test/ui/e0119/issue-28981.stderr index 4886ad7717574..76ff88d6cc623 100644 --- a/src/test/ui/e0119/issue-28981.stderr +++ b/src/test/ui/e0119/issue-28981.stderr @@ -5,7 +5,7 @@ LL | impl Deref for Foo { } //~ ERROR must be used | ^^^^^^^^^^^^^^^^^^^^^^^ | = note: conflicting implementation in crate `core`: - - impl<'_, T> std::ops::Deref for &T + - impl std::ops::Deref for &T where T: ?Sized; error[E0210]: type parameter `Foo` must be used as the type parameter for some local type (e.g. `MyStruct`) From e72afa9573ab8cb7ca2482f53b234bb59e3040ef Mon Sep 17 00:00:00 2001 From: Wesley Wiser Date: Sun, 21 Oct 2018 11:58:39 -0400 Subject: [PATCH 14/23] Consume optimization fuel from the MIR inliner This makes it easier to debug mis-optimizations that occur during inlining. Thanks to @nikomatsakis for the suggestion! --- src/librustc_mir/transform/inline.rs | 14 +++++++++++++- 1 file changed, 13 insertions(+), 1 deletion(-) diff --git a/src/librustc_mir/transform/inline.rs b/src/librustc_mir/transform/inline.rs index 199cf5650fda8..2db3bbda3233b 100644 --- a/src/librustc_mir/transform/inline.rs +++ b/src/librustc_mir/transform/inline.rs @@ -137,7 +137,7 @@ impl<'a, 'tcx> Inliner<'a, 'tcx> { let callee_mir = match self.tcx.try_optimized_mir(callsite.location.span, callsite.callee) { - Ok(callee_mir) if self.should_inline(callsite, callee_mir) => { + Ok(callee_mir) if self.consider_optimizing(callsite, callee_mir) => { self.tcx.subst_and_normalize_erasing_regions( &callsite.substs, param_env, @@ -198,6 +198,18 @@ impl<'a, 'tcx> Inliner<'a, 'tcx> { } } + fn consider_optimizing(&self, + callsite: CallSite<'tcx>, + callee_mir: &Mir<'tcx>) + -> bool + { + debug!("consider_optimizing({:?})", callsite); + self.should_inline(callsite, callee_mir) + && self.tcx.consider_optimizing(|| format!("Inline {:?} into {:?}", + callee_mir.span, + callsite)) + } + fn should_inline(&self, callsite: CallSite<'tcx>, callee_mir: &Mir<'tcx>) From 2d7426bb5a55fa27555de658fca529d8b205ef66 Mon Sep 17 00:00:00 2001 From: ljedrz Date: Wed, 7 Nov 2018 15:32:58 +0100 Subject: [PATCH 15/23] borrow_set: remove a helper function and a clone it uses --- src/librustc_mir/borrow_check/borrow_set.rs | 15 +++------------ 1 file changed, 3 insertions(+), 12 deletions(-) diff --git a/src/librustc_mir/borrow_check/borrow_set.rs b/src/librustc_mir/borrow_check/borrow_set.rs index db56ce4627410..c432826dca865 100644 --- a/src/librustc_mir/borrow_check/borrow_set.rs +++ b/src/librustc_mir/borrow_check/borrow_set.rs @@ -21,7 +21,6 @@ use rustc::util::nodemap::{FxHashMap, FxHashSet}; use rustc_data_structures::indexed_vec::IndexVec; use rustc_data_structures::bit_set::BitSet; use std::fmt; -use std::hash::Hash; use std::ops::Index; crate struct BorrowSet<'tcx> { @@ -233,21 +232,13 @@ impl<'a, 'gcx, 'tcx> Visitor<'tcx> for GatherBorrows<'a, 'gcx, 'tcx> { self.insert_as_pending_if_two_phase(location, &assigned_place, region, kind, idx); - insert(&mut self.region_map, ®ion, idx); + self.region_map.entry(region).or_default().insert(idx); if let Some(local) = borrowed_place.root_local() { - insert(&mut self.local_map, &local, idx); + self.local_map.entry(local).or_default().insert(idx); } } - return self.super_assign(block, assigned_place, rvalue, location); - - fn insert<'a, K, V>(map: &'a mut FxHashMap>, k: &K, v: V) - where - K: Clone + Eq + Hash, - V: Eq + Hash, - { - map.entry(k.clone()).or_default().insert(v); - } + self.super_assign(block, assigned_place, rvalue, location) } fn visit_place( From 299a452a75883b64ea15fc7e7f0d139cab3d750f Mon Sep 17 00:00:00 2001 From: David Wood Date: Wed, 7 Nov 2018 13:40:55 +0100 Subject: [PATCH 16/23] Ignore never-initialized locals for `unused_mut`. This commit filters out locals that have never been initialized for consideration in the `unused_mut` lint. This is intended to detect when the statement that would have initialized the local was removed as unreachable code. In these cases, we would not want to lint. This is the same behaviour as the AST borrow checker. This is achieved by taking advantage of an existing pass over the MIR for the `unused_mut` lint and creating a set of those locals that were never initialized. --- src/librustc/mir/mod.rs | 14 +++ src/librustc_mir/borrow_check/mod.rs | 20 ++-- src/librustc_mir/borrow_check/used_muts.rs | 104 +++++++++++++++++---- src/test/ui/nll/issue-55344.rs | 26 ++++++ 4 files changed, 136 insertions(+), 28 deletions(-) create mode 100644 src/test/ui/nll/issue-55344.rs diff --git a/src/librustc/mir/mod.rs b/src/librustc/mir/mod.rs index c662ed6a6bf06..722bb64a7469d 100644 --- a/src/librustc/mir/mod.rs +++ b/src/librustc/mir/mod.rs @@ -304,6 +304,20 @@ impl<'tcx> Mir<'tcx> { }) } + /// Returns an iterator over all user-declared mutable locals. + #[inline] + pub fn mut_vars_iter<'a>(&'a self) -> impl Iterator + 'a { + (self.arg_count + 1..self.local_decls.len()).filter_map(move |index| { + let local = Local::new(index); + let decl = &self.local_decls[local]; + if decl.is_user_variable.is_some() && decl.mutability == Mutability::Mut { + Some(local) + } else { + None + } + }) + } + /// Returns an iterator over all user-declared mutable arguments and locals. #[inline] pub fn mut_vars_and_args_iter<'a>(&'a self) -> impl Iterator + 'a { diff --git a/src/librustc_mir/borrow_check/mod.rs b/src/librustc_mir/borrow_check/mod.rs index d4f00ab3bb91a..4e03f6f7f5e7a 100644 --- a/src/librustc_mir/borrow_check/mod.rs +++ b/src/librustc_mir/borrow_check/mod.rs @@ -281,23 +281,21 @@ fn do_mir_borrowck<'a, 'gcx, 'tcx>( // Note that this set is expected to be small - only upvars from closures // would have a chance of erroneously adding non-user-defined mutable vars // to the set. - let temporary_used_locals: FxHashSet = mbcx - .used_mut - .iter() + let temporary_used_locals: FxHashSet = mbcx.used_mut.iter() .filter(|&local| mbcx.mir.local_decls[*local].is_user_variable.is_none()) .cloned() .collect(); - mbcx.gather_used_muts(temporary_used_locals); + // For the remaining unused locals that are marked as mutable, we avoid linting any that + // were never initialized. These locals may have been removed as unreachable code; or will be + // linted as unused variables. + let unused_mut_locals = mbcx.mir.mut_vars_iter() + .filter(|local| !mbcx.used_mut.contains(local)) + .collect(); + mbcx.gather_used_muts(temporary_used_locals, unused_mut_locals); debug!("mbcx.used_mut: {:?}", mbcx.used_mut); - let used_mut = mbcx.used_mut; - - for local in mbcx - .mir - .mut_vars_and_args_iter() - .filter(|local| !used_mut.contains(local)) - { + for local in mbcx.mir.mut_vars_and_args_iter().filter(|local| !used_mut.contains(local)) { if let ClearCrossCrate::Set(ref vsi) = mbcx.mir.source_scope_local_data { let local_decl = &mbcx.mir.local_decls[local]; diff --git a/src/librustc_mir/borrow_check/used_muts.rs b/src/librustc_mir/borrow_check/used_muts.rs index dad87ed65a7d4..7c75fb59917c0 100644 --- a/src/librustc_mir/borrow_check/used_muts.rs +++ b/src/librustc_mir/borrow_check/used_muts.rs @@ -9,43 +9,113 @@ // except according to those terms. use rustc::mir::visit::{PlaceContext, Visitor}; -use rustc::mir::{Local, Location, Place}; +use rustc::mir::{BasicBlock, Local, Location, Place, Statement, StatementKind, TerminatorKind}; use rustc_data_structures::fx::FxHashSet; use borrow_check::MirBorrowckCtxt; impl<'cx, 'gcx, 'tcx> MirBorrowckCtxt<'cx, 'gcx, 'tcx> { - /// Walks the MIR looking for assignments to a set of locals, as part of the unused mutable - /// local variables lint, to update the context's `used_mut` in a single walk. - crate fn gather_used_muts(&mut self, locals: FxHashSet) { - let mut visitor = GatherUsedMutsVisitor { - needles: locals, - mbcx: self, - }; - visitor.visit_mir(visitor.mbcx.mir); + /// Walks the MIR adding to the set of `used_mut` locals that will be ignored for the purposes + /// of the `unused_mut` lint. + /// + /// `temporary_used_locals` should contain locals that were found to be temporary, mutable and + /// used from borrow checking. This function looks for assignments into these locals from + /// user-declared locals and adds those user-defined locals to the `used_mut` set. This can + /// occur due to a rare case involving upvars in closures. + /// + /// `never_initialized_mut_locals` should contain the set of user-declared mutable locals + /// (not arguments) that have not already been marked as being used. + /// This function then looks for assignments from statements or the terminator into the locals + /// from this set and removes them from the set. This leaves only those locals that have not + /// been assigned to - this set is used as a proxy for locals that were not initialized due to + /// unreachable code. These locals are then considered "used" to silence the lint for them. + /// See #55344 for context. + crate fn gather_used_muts( + &mut self, + temporary_used_locals: FxHashSet, + mut never_initialized_mut_locals: FxHashSet, + ) { + { + let mut visitor = GatherUsedMutsVisitor { + temporary_used_locals, + never_initialized_mut_locals: &mut never_initialized_mut_locals, + mbcx: self, + }; + visitor.visit_mir(visitor.mbcx.mir); + } + + // Take the union of the existed `used_mut` set with those variables we've found were + // never initialized. + debug!("gather_used_muts: never_initialized_mut_locals={:?}", never_initialized_mut_locals); + self.used_mut = self.used_mut.union(&never_initialized_mut_locals).cloned().collect(); } } -/// MIR visitor gathering the assignments to a set of locals, in a single walk. -/// 'visit = the duration of the MIR walk +/// MIR visitor for collecting used mutable variables. +/// The 'visit lifetime represents the duration of the MIR walk. struct GatherUsedMutsVisitor<'visit, 'cx: 'visit, 'gcx: 'tcx, 'tcx: 'cx> { - needles: FxHashSet, + temporary_used_locals: FxHashSet, + never_initialized_mut_locals: &'visit mut FxHashSet, mbcx: &'visit mut MirBorrowckCtxt<'cx, 'gcx, 'tcx>, } impl<'visit, 'cx, 'gcx, 'tcx> Visitor<'tcx> for GatherUsedMutsVisitor<'visit, 'cx, 'gcx, 'tcx> { + fn visit_terminator_kind( + &mut self, + _block: BasicBlock, + kind: &TerminatorKind<'tcx>, + _location: Location, + ) { + debug!("visit_terminator_kind: kind={:?}", kind); + match &kind { + TerminatorKind::Call { destination: Some((into, _)), .. } => { + if let Some(local) = into.base_local() { + debug!( + "visit_terminator_kind: kind={:?} local={:?} \ + never_initialized_mut_locals={:?}", + kind, local, self.never_initialized_mut_locals + ); + let _ = self.never_initialized_mut_locals.remove(&local); + } + }, + _ => {}, + } + } + + fn visit_statement( + &mut self, + _block: BasicBlock, + statement: &Statement<'tcx>, + _location: Location, + ) { + match &statement.kind { + StatementKind::Assign(into, _) => { + // Remove any locals that we found were initialized from the + // `never_initialized_mut_locals` set. At the end, the only remaining locals will + // be those that were never initialized - we will consider those as being used as + // they will either have been removed by unreachable code optimizations; or linted + // as unused variables. + if let Some(local) = into.base_local() { + debug!( + "visit_statement: statement={:?} local={:?} \ + never_initialized_mut_locals={:?}", + statement, local, self.never_initialized_mut_locals + ); + let _ = self.never_initialized_mut_locals.remove(&local); + } + }, + _ => {}, + } + } + fn visit_local( &mut self, local: &Local, place_context: PlaceContext<'tcx>, location: Location, ) { - if !self.needles.contains(local) { - return; - } - - if place_context.is_place_assignment() { + if place_context.is_place_assignment() && self.temporary_used_locals.contains(local) { // Propagate the Local assigned at this Location as a used mutable local variable for moi in &self.mbcx.move_data.loc_map[location] { let mpi = &self.mbcx.move_data.moves[*moi].path; diff --git a/src/test/ui/nll/issue-55344.rs b/src/test/ui/nll/issue-55344.rs new file mode 100644 index 0000000000000..131c979a24b7d --- /dev/null +++ b/src/test/ui/nll/issue-55344.rs @@ -0,0 +1,26 @@ +// Copyright 2017 The Rust Project Developers. See the COPYRIGHT +// file at the top-level directory of this distribution and at +// http://rust-lang.org/COPYRIGHT. +// +// Licensed under the Apache License, Version 2.0 or the MIT license +// , at your +// option. This file may not be copied, modified, or distributed +// except according to those terms. + +// compile-pass + +#![feature(nll)] +#![allow(unreachable_code)] +#![deny(unused_mut)] + +pub fn foo() { + return; + + let mut v = 0; + assert_eq!(v, 0); + v = 1; + assert_eq!(v, 1); +} + +fn main() {} From 34ffbdb965dd84c6eb6309cacc1f44d6304fdd17 Mon Sep 17 00:00:00 2001 From: "Felix S. Klock II" Date: Wed, 7 Nov 2018 16:33:41 +0100 Subject: [PATCH 17/23] This test will not link on wasm32. --- src/test/ui/extern/extern-const.fixed | 6 +++--- src/test/ui/extern/extern-const.rs | 6 +++--- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/src/test/ui/extern/extern-const.fixed b/src/test/ui/extern/extern-const.fixed index 0229362cf0eca..dca5698a70c20 100644 --- a/src/test/ui/extern/extern-const.fixed +++ b/src/test/ui/extern/extern-const.fixed @@ -1,11 +1,11 @@ -// This test is checking that extern items cannot be const. It also -// checks that `rustfix` suggests the alternative of using an extern -// static. +// Check extern items cannot be const + `rustfix` suggests using +// extern static. // // #54388: an unused reference to an undefined static may or may not // compile. To sidestep this by using one that *is* defined. // run-rustfix +// ignore-wasm32 no external library to link to. // compile-flags: -g -Z continue-parse-after-error #![feature(libc)] extern crate libc; diff --git a/src/test/ui/extern/extern-const.rs b/src/test/ui/extern/extern-const.rs index e7b1715255532..07dbe545a850a 100644 --- a/src/test/ui/extern/extern-const.rs +++ b/src/test/ui/extern/extern-const.rs @@ -1,11 +1,11 @@ -// This test is checking that extern items cannot be const. It also -// checks that `rustfix` suggests the alternative of using an extern -// static. +// Check extern items cannot be const + `rustfix` suggests using +// extern static. // // #54388: an unused reference to an undefined static may or may not // compile. To sidestep this by using one that *is* defined. // run-rustfix +// ignore-wasm32 no external library to link to. // compile-flags: -g -Z continue-parse-after-error #![feature(libc)] extern crate libc; From 5159b32997c92323ac7dd746b6898c1e96e7078c Mon Sep 17 00:00:00 2001 From: ljedrz Date: Wed, 7 Nov 2018 17:00:51 +0100 Subject: [PATCH 18/23] mir: remove a hacky recursive helper function --- src/librustc_mir/transform/promote_consts.rs | 15 +++++---------- 1 file changed, 5 insertions(+), 10 deletions(-) diff --git a/src/librustc_mir/transform/promote_consts.rs b/src/librustc_mir/transform/promote_consts.rs index e79da88a2464b..c5add6260789a 100644 --- a/src/librustc_mir/transform/promote_consts.rs +++ b/src/librustc_mir/transform/promote_consts.rs @@ -310,16 +310,11 @@ impl<'a, 'tcx> Promoter<'a, 'tcx> { match statement.kind { StatementKind::Assign(_, box Rvalue::Ref(_, _, ref mut place)) => { // Find the underlying local for this (necessarily interior) borrow. - // HACK(eddyb) using a recursive function because of mutable borrows. - fn interior_base<'a, 'tcx>(place: &'a mut Place<'tcx>) - -> &'a mut Place<'tcx> { - if let Place::Projection(ref mut proj) = *place { - assert_ne!(proj.elem, ProjectionElem::Deref); - return interior_base(&mut proj.base); - } - place - } - let place = interior_base(place); + let mut place = place; + while let Place::Projection(ref mut proj) = *place { + assert_ne!(proj.elem, ProjectionElem::Deref); + place = &mut proj.base; + }; let ty = place.ty(local_decls, self.tcx).to_ty(self.tcx); let span = statement.source_info.span; From 36f815bf8da2b05ef8919711223a23d849beeb3f Mon Sep 17 00:00:00 2001 From: Jon Gjengset Date: Wed, 7 Nov 2018 11:02:08 -0500 Subject: [PATCH 19/23] Remove intermediate font specs This is a (much) more constrained version of #54772 that also aims at improving the situation in #34681. It removes any font specifications that are not the "official" rustdoc font, and instead relies on the browser to provide the fallback font if the official on is not available. On Linux systems, this is particularly important, as fonts like Helvetica, Arial, and Times often look pretty bad since they're pulled from extracted MS fonts. A specification like `serif` or `sans-serif` lets the browser instead choose a good font. --- src/librustdoc/html/static/rustdoc.css | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/src/librustdoc/html/static/rustdoc.css b/src/librustdoc/html/static/rustdoc.css index 8f679b4d22b25..1ae3b0b88c6dd 100644 --- a/src/librustdoc/html/static/rustdoc.css +++ b/src/librustdoc/html/static/rustdoc.css @@ -70,7 +70,7 @@ /* General structure and fonts */ body { - font: 16px/1.4 "Source Serif Pro", Georgia, Times, "Times New Roman", serif; + font: 16px/1.4 "Source Serif Pro", serif; margin: 0; position: relative; padding: 10px 15px 20px 15px; @@ -114,7 +114,7 @@ h3.impl, h3.method, h3.type { h1, h2, h3, h4, .sidebar, a.source, .search-input, .content table :not(code)>a, .collapse-toggle, div.item-list .out-of-band { - font-family: "Fira Sans", "Helvetica Neue", Helvetica, Arial, sans-serif; + font-family: "Fira Sans", sans-serif; } ol, ul { @@ -133,7 +133,7 @@ summary { } code, pre { - font-family: "Source Code Pro", Menlo, Monaco, Consolas, "DejaVu Sans Mono", Inconsolata, monospace; + font-family: "Source Code Pro", monospace; white-space: pre-wrap; } .docblock code, .docblock-short code { @@ -415,7 +415,7 @@ h4 > code, h3 > code, .invisible > code { } #main > .since { top: inherit; - font-family: "Fira Sans", "Helvetica Neue", Helvetica, Arial, sans-serif; + font-family: "Fira Sans", sans-serif; } .content table:not(.table-display) { @@ -1338,7 +1338,7 @@ h3.important { kbd { display: inline-block; padding: 3px 5px; - font: 15px "SFMono-Regular", Consolas, "Liberation Mono", Menlo, Courier, monospace; + font: 15px monospace; line-height: 10px; vertical-align: middle; border: solid 1px; From a9b598884719cccdcf699567cece2a6ac46fc84c Mon Sep 17 00:00:00 2001 From: Christopher Serr Date: Thu, 8 Nov 2018 02:56:25 +0100 Subject: [PATCH 20/23] wasm32-unknown-emscripten expects the rust_eh_personality symbol The `wasm32-unknown-emscripten` expects the `rust_eh_personality` symbol to be there, but a cfg checking for `target_arch = "wasm32"` which was meant to remove the symbol from the `wasm32-unknown-unknown` target, didn't check for whether `emscripten` is targeted or not, so the symbol accidentally got filtered out there as well. Fixes #55276 --- src/libpanic_abort/lib.rs | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/libpanic_abort/lib.rs b/src/libpanic_abort/lib.rs index b86928534cb24..9235f8e7660a1 100644 --- a/src/libpanic_abort/lib.rs +++ b/src/libpanic_abort/lib.rs @@ -97,7 +97,10 @@ pub unsafe extern fn __rust_start_panic(_payload: usize) -> u32 { pub mod personalities { #[no_mangle] #[cfg(not(any( - target_arch = "wasm32", + all( + target_arch = "wasm32", + not(target_os = "emscripten"), + ), all( target_os = "windows", target_env = "gnu", From e3390d89ea7a58a35d3aca62247e2a5d3c327181 Mon Sep 17 00:00:00 2001 From: ljedrz Date: Wed, 7 Nov 2018 15:38:06 +0100 Subject: [PATCH 21/23] Improve creation of 3 IndexVecs --- src/librustc/ty/query/on_disk_cache.rs | 3 +-- src/librustc_mir/shim.rs | 5 +++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/src/librustc/ty/query/on_disk_cache.rs b/src/librustc/ty/query/on_disk_cache.rs index 3dc31c517169f..54550b8a2055f 100644 --- a/src/librustc/ty/query/on_disk_cache.rs +++ b/src/librustc/ty/query/on_disk_cache.rs @@ -450,8 +450,7 @@ impl<'sess> OnDiskCache<'sess> { .map(|&(cnum, ..)| cnum) .max() .unwrap_or(0) + 1; - let mut map = IndexVec::new(); - map.resize(map_size as usize, None); + let mut map = IndexVec::from_elem_n(None, map_size as usize); for &(prev_cnum, ref crate_name, crate_disambiguator) in prev_cnums { let key = (crate_name.clone(), crate_disambiguator); diff --git a/src/librustc_mir/shim.rs b/src/librustc_mir/shim.rs index 76a8501fb177a..54983b8f4e026 100644 --- a/src/librustc_mir/shim.rs +++ b/src/librustc_mir/shim.rs @@ -196,7 +196,7 @@ fn build_drop_shim<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>, let source_info = SourceInfo { span, scope: OUTERMOST_SOURCE_SCOPE }; let return_block = BasicBlock::new(1); - let mut blocks = IndexVec::new(); + let mut blocks = IndexVec::with_capacity(2); let block = |blocks: &mut IndexVec<_, _>, kind| { blocks.push(BasicBlockData { statements: vec![], @@ -768,7 +768,8 @@ fn build_call_shim<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>, })); } - let mut blocks = IndexVec::new(); + let n_blocks = if let Adjustment::RefMut = rcvr_adjustment { 5 } else { 2 }; + let mut blocks = IndexVec::with_capacity(n_blocks); let block = |blocks: &mut IndexVec<_, _>, statements, kind, is_cleanup| { blocks.push(BasicBlockData { statements, From 255cc1aed33442567c29c95fa445a534575e925c Mon Sep 17 00:00:00 2001 From: Alex Crichton Date: Thu, 8 Nov 2018 07:49:28 -0800 Subject: [PATCH 22/23] rustc: Request ansi colors if stderr isn't a tty Currently Cargo will always capture the output of rustc meaning that rustc is never hooked up to a tty. To retain colors Cargo uses the `fwdansi` crate to ensure that ansi color codes are translated to windows terminal methods (and ansi codes otherwise just go their natural route on Unix). Cargo passes `--color always` to rustc to ensure that using a pipe doesn't trick it into not emitting colors at all. It turns out, however, that `--color always` ends up still accidentally using the native shell api on native windows shells. The fix here is to instead pass `AlwaysAnsi` to `termcolor` instead of `Always`, ensuring that when `--color always` is passed to rustc and its output isn't a terminal, we're always generating ansi colors regardless of the platform. Closes #55769 --- src/librustc_errors/emitter.rs | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/src/librustc_errors/emitter.rs b/src/librustc_errors/emitter.rs index 720e8def5ab1c..7e69e98071d4b 100644 --- a/src/librustc_errors/emitter.rs +++ b/src/librustc_errors/emitter.rs @@ -108,7 +108,13 @@ pub enum ColorConfig { impl ColorConfig { fn to_color_choice(&self) -> ColorChoice { match *self { - ColorConfig::Always => ColorChoice::Always, + ColorConfig::Always => { + if atty::is(atty::Stream::Stderr) { + ColorChoice::Always + } else { + ColorChoice::AlwaysAnsi + } + } ColorConfig::Never => ColorChoice::Never, ColorConfig::Auto if atty::is(atty::Stream::Stderr) => { ColorChoice::Auto From 50a2d47b4f57f8f9433b18692fe2d1975f9e84e1 Mon Sep 17 00:00:00 2001 From: Tim Neumann Date: Thu, 11 Oct 2018 17:50:00 +0200 Subject: [PATCH 23/23] Support for the program data address space option of LLVM's Target Datalayout. https://llvm.org/docs/LangRef.html#data-layout --- src/librustc_codegen_llvm/abi.rs | 10 +++++++++- src/librustc_codegen_llvm/meth.rs | 2 +- src/librustc_codegen_llvm/type_.rs | 2 ++ src/librustc_codegen_llvm/type_of.rs | 2 +- src/librustc_target/abi/mod.rs | 17 +++++++++++++++-- 5 files changed, 28 insertions(+), 5 deletions(-) diff --git a/src/librustc_codegen_llvm/abi.rs b/src/librustc_codegen_llvm/abi.rs index e50534a4e1dc9..6b08a74067566 100644 --- a/src/librustc_codegen_llvm/abi.rs +++ b/src/librustc_codegen_llvm/abi.rs @@ -19,7 +19,7 @@ use type_::Type; use type_of::{LayoutLlvmExt, PointerKind}; use value::Value; -use rustc_target::abi::{LayoutOf, Size, TyLayout, Abi as LayoutAbi}; +use rustc_target::abi::{HasDataLayout, LayoutOf, Size, TyLayout, Abi as LayoutAbi}; use rustc::ty::{self, Ty}; use rustc::ty::layout; @@ -276,6 +276,7 @@ pub trait FnTypeExt<'tcx> { cx: &CodegenCx<'ll, 'tcx>, abi: Abi); fn llvm_type(&self, cx: &CodegenCx<'ll, 'tcx>) -> &'ll Type; + fn ptr_to_llvm_type(&self, cx: &CodegenCx<'ll, 'tcx>) -> &'ll Type; fn llvm_cconv(&self) -> llvm::CallConv; fn apply_attrs_llfn(&self, llfn: &'ll Value); fn apply_attrs_callsite(&self, bx: &Builder<'a, 'll, 'tcx>, callsite: &'ll Value); @@ -657,6 +658,13 @@ impl<'tcx> FnTypeExt<'tcx> for FnType<'tcx, Ty<'tcx>> { } } + fn ptr_to_llvm_type(&self, cx: &CodegenCx<'ll, 'tcx>) -> &'ll Type { + unsafe { + llvm::LLVMPointerType(self.llvm_type(cx), + cx.data_layout().instruction_address_space as c_uint) + } + } + fn llvm_cconv(&self) -> llvm::CallConv { match self.conv { Conv::C => llvm::CCallConv, diff --git a/src/librustc_codegen_llvm/meth.rs b/src/librustc_codegen_llvm/meth.rs index d38f343d01f34..0dc5a4ddde82c 100644 --- a/src/librustc_codegen_llvm/meth.rs +++ b/src/librustc_codegen_llvm/meth.rs @@ -39,7 +39,7 @@ impl<'a, 'tcx> VirtualIndex { // Load the data pointer from the object. debug!("get_fn({:?}, {:?})", llvtable, self); - let llvtable = bx.pointercast(llvtable, fn_ty.llvm_type(bx.cx).ptr_to().ptr_to()); + let llvtable = bx.pointercast(llvtable, fn_ty.ptr_to_llvm_type(bx.cx).ptr_to()); let ptr_align = bx.tcx().data_layout.pointer_align; let ptr = bx.load(bx.inbounds_gep(llvtable, &[C_usize(bx.cx, self.0)]), ptr_align); bx.nonnull_metadata(ptr); diff --git a/src/librustc_codegen_llvm/type_.rs b/src/librustc_codegen_llvm/type_.rs index 51a233d791625..6fb78fe4aa5a4 100644 --- a/src/librustc_codegen_llvm/type_.rs +++ b/src/librustc_codegen_llvm/type_.rs @@ -234,6 +234,8 @@ impl Type { } pub fn ptr_to(&self) -> &Type { + assert_ne!(self.kind(), TypeKind::Function, + "don't call ptr_to on function types, use ptr_to_llvm_type on FnType instead"); unsafe { llvm::LLVMPointerType(self, 0) } diff --git a/src/librustc_codegen_llvm/type_of.rs b/src/librustc_codegen_llvm/type_of.rs index b01d7e3a776f7..fea02edf7be01 100644 --- a/src/librustc_codegen_llvm/type_of.rs +++ b/src/librustc_codegen_llvm/type_of.rs @@ -265,7 +265,7 @@ impl<'tcx> LayoutLlvmExt<'tcx> for TyLayout<'tcx> { ty::ParamEnv::reveal_all(), &sig, ); - FnType::new(cx, sig, &[]).llvm_type(cx).ptr_to() + FnType::new(cx, sig, &[]).ptr_to_llvm_type(cx) } _ => self.scalar_llvm_type_at(cx, scalar, Size::ZERO) }; diff --git a/src/librustc_target/abi/mod.rs b/src/librustc_target/abi/mod.rs index c7d0469e556a7..15f61a033447f 100644 --- a/src/librustc_target/abi/mod.rs +++ b/src/librustc_target/abi/mod.rs @@ -35,7 +35,8 @@ pub struct TargetDataLayout { pub aggregate_align: Align, /// Alignments for vector types. - pub vector_align: Vec<(Size, Align)> + pub vector_align: Vec<(Size, Align)>, + pub instruction_address_space: u32, } impl Default for TargetDataLayout { @@ -57,13 +58,22 @@ impl Default for TargetDataLayout { vector_align: vec![ (Size::from_bits(64), Align::from_bits(64, 64).unwrap()), (Size::from_bits(128), Align::from_bits(128, 128).unwrap()) - ] + ], + instruction_address_space: 0, } } } impl TargetDataLayout { pub fn parse(target: &Target) -> Result { + // Parse an address space index from a string. + let parse_address_space = |s: &str, cause: &str| { + s.parse::().map_err(|err| { + format!("invalid address space `{}` for `{}` in \"data-layout\": {}", + s, cause, err) + }) + }; + // Parse a bit count from a string. let parse_bits = |s: &str, kind: &str, cause: &str| { s.parse::().map_err(|err| { @@ -96,6 +106,9 @@ impl TargetDataLayout { match spec.split(':').collect::>()[..] { ["e"] => dl.endian = Endian::Little, ["E"] => dl.endian = Endian::Big, + [p] if p.starts_with("P") => { + dl.instruction_address_space = parse_address_space(&p[1..], "P")? + } ["a", ref a..] => dl.aggregate_align = align(a, "a")?, ["f32", ref a..] => dl.f32_align = align(a, "f32")?, ["f64", ref a..] => dl.f64_align = align(a, "f64")?,