From f847ff1511825224a19055f6d8646136becc15eb Mon Sep 17 00:00:00 2001 From: Jad Ghalayini Date: Mon, 8 Feb 2021 12:07:59 +0000 Subject: [PATCH 01/19] Added #[repr(transparent)] to core::cmp::Reverse --- library/core/src/cmp.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/library/core/src/cmp.rs b/library/core/src/cmp.rs index 4a15b185a83e7..5bab1fb93dbb5 100644 --- a/library/core/src/cmp.rs +++ b/library/core/src/cmp.rs @@ -579,6 +579,7 @@ impl Ordering { /// ``` #[derive(PartialEq, Eq, Debug, Copy, Clone, Default, Hash)] #[stable(feature = "reverse_cmp_key", since = "1.19.0")] +#[repr(transparent)] pub struct Reverse(#[stable(feature = "reverse_cmp_key", since = "1.19.0")] pub T); #[stable(feature = "reverse_cmp_key", since = "1.19.0")] From 095bf01649a202b088c817bcdd3612d2604187e0 Mon Sep 17 00:00:00 2001 From: Han Mertens Date: Sun, 17 Jan 2021 17:49:02 +0100 Subject: [PATCH 02/19] Improve sift_down performance in BinaryHeap Because child > 0, the two statements are equivalent, but using saturating_sub and <= yields in faster code. This is most notable in the binary_heap::bench_into_sorted_vec benchmark, which shows a speedup of 1.26x, which uses sift_down_range internally. The speedup of pop (that uses sift_down_to_bottom internally) is much less significant as the sifting method is not called in a loop. --- library/alloc/src/collections/binary_heap.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/library/alloc/src/collections/binary_heap.rs b/library/alloc/src/collections/binary_heap.rs index 33bd98d467cec..e025da74da471 100644 --- a/library/alloc/src/collections/binary_heap.rs +++ b/library/alloc/src/collections/binary_heap.rs @@ -565,7 +565,7 @@ impl BinaryHeap { let mut child = 2 * hole.pos() + 1; // Loop invariant: child == 2 * hole.pos() + 1. - while child < end - 1 { + while child <= end.saturating_sub(2) { // compare with the greater of the two children // SAFETY: child < end - 1 < self.len() and // child + 1 < end <= self.len(), so they're valid indexes. @@ -624,7 +624,7 @@ impl BinaryHeap { let mut child = 2 * hole.pos() + 1; // Loop invariant: child == 2 * hole.pos() + 1. - while child < end - 1 { + while child <= end.saturating_sub(2) { // SAFETY: child < end - 1 < self.len() and // child + 1 < end <= self.len(), so they're valid indexes. // child == 2 * hole.pos() + 1 != hole.pos() and From 516988b268abcfa8cda96db0074e5795fb11aee1 Mon Sep 17 00:00:00 2001 From: kadmin Date: Sat, 3 Oct 2020 20:57:47 +0000 Subject: [PATCH 03/19] Impl StatementKind::CopyNonOverlapping --- compiler/rustc_codegen_ssa/src/mir/analyze.rs | 6 ++++-- .../rustc_codegen_ssa/src/mir/statement.rs | 15 +++++++++++++ compiler/rustc_middle/src/mir/mod.rs | 17 +++++++++++++++ compiler/rustc_middle/src/mir/visit.rs | 21 +++++++++++++++++++ 4 files changed, 57 insertions(+), 2 deletions(-) diff --git a/compiler/rustc_codegen_ssa/src/mir/analyze.rs b/compiler/rustc_codegen_ssa/src/mir/analyze.rs index 289629d921545..5ad9f46147282 100644 --- a/compiler/rustc_codegen_ssa/src/mir/analyze.rs +++ b/compiler/rustc_codegen_ssa/src/mir/analyze.rs @@ -293,7 +293,8 @@ impl<'mir, 'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> Visitor<'tcx> | MutatingUseContext::AsmOutput | MutatingUseContext::Borrow | MutatingUseContext::AddressOf - | MutatingUseContext::Projection, + | MutatingUseContext::Projection + | MutatingUseContext::CopyNonOverlapping, ) | PlaceContext::NonMutatingUse( NonMutatingUseContext::Inspect @@ -301,7 +302,8 @@ impl<'mir, 'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> Visitor<'tcx> | NonMutatingUseContext::UniqueBorrow | NonMutatingUseContext::ShallowBorrow | NonMutatingUseContext::AddressOf - | NonMutatingUseContext::Projection, + | NonMutatingUseContext::Projection + | NonMutatingUseContext::CopyNonOverlapping, ) => { self.not_ssa(local); } diff --git a/compiler/rustc_codegen_ssa/src/mir/statement.rs b/compiler/rustc_codegen_ssa/src/mir/statement.rs index 6f74ba77d4c16..2c90054b6c7f5 100644 --- a/compiler/rustc_codegen_ssa/src/mir/statement.rs +++ b/compiler/rustc_codegen_ssa/src/mir/statement.rs @@ -115,6 +115,21 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> { self.codegen_coverage(&mut bx, coverage.clone()); bx } + mir::StatementKind::CopyNonOverlapping(box mir::CopyNonOverlapping { + ref src, + ref dst, + ref size, + }) => { + bx.memcpy( + dst, + todo!(), + src, + todo!(), + size, + todo!(), + ); + bx + } mir::StatementKind::FakeRead(..) | mir::StatementKind::Retag { .. } | mir::StatementKind::AscribeUserType(..) diff --git a/compiler/rustc_middle/src/mir/mod.rs b/compiler/rustc_middle/src/mir/mod.rs index f0b52c698819a..857ee8bcd7fbb 100644 --- a/compiler/rustc_middle/src/mir/mod.rs +++ b/compiler/rustc_middle/src/mir/mod.rs @@ -1515,6 +1515,11 @@ pub enum StatementKind<'tcx> { /// counter varible at runtime, each time the code region is executed. Coverage(Box), + /// Denotes a call to the intrinsic function copy_overlapping, where `src_dst` denotes the + /// memory being read from and written to(one field to save memory), and size + /// indicates how many bytes are being copied over. + CopyNonOverlapping(Box>), + /// No-op. Useful for deleting instructions without affecting statement indices. Nop, } @@ -1633,6 +1638,11 @@ impl Debug for Statement<'_> { write!(fmt, "Coverage::{:?}", coverage.kind) } } + CopyNonOverlapping(box crate::mir::CopyNonOverlapping { + ref src, + ref dst, + ref size, + }) => write!(fmt, "src {:?} -> dst {:?}, {:?} bytes", src, dst, size), Nop => write!(fmt, "nop"), } } @@ -1644,6 +1654,13 @@ pub struct Coverage { pub code_region: Option, } +#[derive(Clone, Debug, PartialEq, TyEncodable, TyDecodable, HashStable, TypeFoldable)] +pub struct CopyNonOverlapping<'tcx> { + pub src: Place<'tcx>, + pub dst: Place<'tcx>, + pub size: Operand<'tcx>, +} + /////////////////////////////////////////////////////////////////////////// // Places diff --git a/compiler/rustc_middle/src/mir/visit.rs b/compiler/rustc_middle/src/mir/visit.rs index bb8c5f175b8c0..edd5ac43a7d79 100644 --- a/compiler/rustc_middle/src/mir/visit.rs +++ b/compiler/rustc_middle/src/mir/visit.rs @@ -434,6 +434,23 @@ macro_rules! make_mir_visitor { location ) } + StatementKind::CopyNonOverlapping(box crate::mir::CopyNonOverlapping{ + ref $($mutability)? src, + ref $($mutability)? dst, + ref $($mutability)? size, + }) => { + self.visit_place( + src, + PlaceContext::NonMutatingUse(NonMutatingUseContext::CopyNonOverlapping), + location + ); + self.visit_place( + dst, + PlaceContext::MutatingUse(MutatingUseContext::CopyNonOverlapping), + location + ); + self.visit_operand(size, location) + } StatementKind::Nop => {} } } @@ -1149,6 +1166,8 @@ pub enum NonMutatingUseContext { /// f(&x.y); /// Projection, + /// Source from copy_nonoverlapping. + CopyNonOverlapping, } #[derive(Copy, Clone, Debug, PartialEq, Eq)] @@ -1178,6 +1197,8 @@ pub enum MutatingUseContext { Projection, /// Retagging, a "Stacked Borrows" shadow state operation Retag, + /// Memory written to in copy_nonoverlapping. + CopyNonOverlapping, } #[derive(Copy, Clone, Debug, PartialEq, Eq)] From a058d447593154004de1b6ac1e7bd228cfc6fcf8 Mon Sep 17 00:00:00 2001 From: kadmin Date: Mon, 5 Oct 2020 20:13:36 +0000 Subject: [PATCH 04/19] Update fmt and use of memcpy I'm still not totally sure if this is the right way to implement the memcpy, but that portion compiles correctly now. Now to fix the compile errors everywhere else :). --- .../rustc_codegen_ssa/src/mir/statement.rs | 23 +++++++++++-------- compiler/rustc_middle/src/mir/mod.rs | 2 +- 2 files changed, 15 insertions(+), 10 deletions(-) diff --git a/compiler/rustc_codegen_ssa/src/mir/statement.rs b/compiler/rustc_codegen_ssa/src/mir/statement.rs index 2c90054b6c7f5..b507cb0a82312 100644 --- a/compiler/rustc_codegen_ssa/src/mir/statement.rs +++ b/compiler/rustc_codegen_ssa/src/mir/statement.rs @@ -120,15 +120,20 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> { ref dst, ref size, }) => { - bx.memcpy( - dst, - todo!(), - src, - todo!(), - size, - todo!(), - ); - bx + let dst_val = self.codegen_place(&mut bx, dst.as_ref()); + let src_val = self.codegen_place(&mut bx, src.as_ref()); + let size_val = self.codegen_operand(&mut bx, size); + let size = size_val.immediate_or_packed_pair(&mut bx); + bx.memcpy( + dst_val.llval, + dst_val.align, + src_val.llval, + src_val.align, + size, + // TODO probably want to have this change based on alignment above? + crate::MemFlags::empty(), + ); + bx } mir::StatementKind::FakeRead(..) | mir::StatementKind::Retag { .. } diff --git a/compiler/rustc_middle/src/mir/mod.rs b/compiler/rustc_middle/src/mir/mod.rs index 857ee8bcd7fbb..9a000446fc820 100644 --- a/compiler/rustc_middle/src/mir/mod.rs +++ b/compiler/rustc_middle/src/mir/mod.rs @@ -1642,7 +1642,7 @@ impl Debug for Statement<'_> { ref src, ref dst, ref size, - }) => write!(fmt, "src {:?} -> dst {:?}, {:?} bytes", src, dst, size), + }) => write!(fmt, "copy_nonoverlapping(src={:?}, dst={:?},bytes={:?})", src, dst, size), Nop => write!(fmt, "nop"), } } From 5aba3daf2f2630c50ec05cf308178dfe266477cd Mon Sep 17 00:00:00 2001 From: kadmin Date: Mon, 5 Oct 2020 22:53:00 +0000 Subject: [PATCH 05/19] Update match branches This updates all places where match branches check on StatementKind or UseContext. This doesn't properly implement them, but adds TODOs where they are, and also adds some best guesses to what they should be in some cases. --- compiler/rustc_codegen_ssa/src/mir/analyze.rs | 6 +- .../rustc_codegen_ssa/src/mir/statement.rs | 20 +- compiler/rustc_middle/src/mir/mod.rs | 6 +- compiler/rustc_middle/src/mir/visit.rs | 10 +- .../src/borrow_check/invalidation.rs | 15 + compiler/rustc_mir/src/borrow_check/mod.rs | 2 + .../src/borrow_check/type_check/mod.rs | 1 + .../rustc_mir/src/dataflow/impls/borrows.rs | 2 + .../src/dataflow/impls/storage_liveness.rs | 2 + .../src/dataflow/move_paths/builder.rs | 2 + compiler/rustc_mir/src/interpret/step.rs | 17 + .../src/transform/check_consts/validation.rs | 1 + .../rustc_mir/src/transform/check_unsafety.rs | 1 + compiler/rustc_mir/src/transform/dest_prop.rs | 1 + compiler/rustc_mir/src/transform/generator.rs | 1 + .../src/transform/instrument_coverage.rs | 1272 +++++++++++++++++ .../src/transform/remove_noop_landing_pads.rs | 1 + compiler/rustc_mir/src/util/spanview.rs | 1 + .../clippy_utils/src/qualify_min_const_fn.rs | 9 +- 19 files changed, 1346 insertions(+), 24 deletions(-) create mode 100644 compiler/rustc_mir/src/transform/instrument_coverage.rs diff --git a/compiler/rustc_codegen_ssa/src/mir/analyze.rs b/compiler/rustc_codegen_ssa/src/mir/analyze.rs index 5ad9f46147282..289629d921545 100644 --- a/compiler/rustc_codegen_ssa/src/mir/analyze.rs +++ b/compiler/rustc_codegen_ssa/src/mir/analyze.rs @@ -293,8 +293,7 @@ impl<'mir, 'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> Visitor<'tcx> | MutatingUseContext::AsmOutput | MutatingUseContext::Borrow | MutatingUseContext::AddressOf - | MutatingUseContext::Projection - | MutatingUseContext::CopyNonOverlapping, + | MutatingUseContext::Projection, ) | PlaceContext::NonMutatingUse( NonMutatingUseContext::Inspect @@ -302,8 +301,7 @@ impl<'mir, 'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> Visitor<'tcx> | NonMutatingUseContext::UniqueBorrow | NonMutatingUseContext::ShallowBorrow | NonMutatingUseContext::AddressOf - | NonMutatingUseContext::Projection - | NonMutatingUseContext::CopyNonOverlapping, + | NonMutatingUseContext::Projection, ) => { self.not_ssa(local); } diff --git a/compiler/rustc_codegen_ssa/src/mir/statement.rs b/compiler/rustc_codegen_ssa/src/mir/statement.rs index b507cb0a82312..1dafc8cd2c16d 100644 --- a/compiler/rustc_codegen_ssa/src/mir/statement.rs +++ b/compiler/rustc_codegen_ssa/src/mir/statement.rs @@ -120,18 +120,22 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> { ref dst, ref size, }) => { - let dst_val = self.codegen_place(&mut bx, dst.as_ref()); - let src_val = self.codegen_place(&mut bx, src.as_ref()); + let dst_val = self.codegen_operand(&mut bx, dst); + let src_val = self.codegen_operand(&mut bx, src); let size_val = self.codegen_operand(&mut bx, size); let size = size_val.immediate_or_packed_pair(&mut bx); + let dst = dst_val.immediate_or_packed_pair(&mut bx); + let src = src_val.immediate_or_packed_pair(&mut bx); + use crate::MemFlags; + let flags = + (!MemFlags::UNALIGNED) & (!MemFlags::VOLATILE) & (!MemFlags::NONTEMPORAL); bx.memcpy( - dst_val.llval, - dst_val.align, - src_val.llval, - src_val.align, + dst, + dst_val.layout.layout.align.pref, + src, + src_val.layout.layout.align.pref, size, - // TODO probably want to have this change based on alignment above? - crate::MemFlags::empty(), + flags, ); bx } diff --git a/compiler/rustc_middle/src/mir/mod.rs b/compiler/rustc_middle/src/mir/mod.rs index 9a000446fc820..5fb04eccca811 100644 --- a/compiler/rustc_middle/src/mir/mod.rs +++ b/compiler/rustc_middle/src/mir/mod.rs @@ -1642,7 +1642,7 @@ impl Debug for Statement<'_> { ref src, ref dst, ref size, - }) => write!(fmt, "copy_nonoverlapping(src={:?}, dst={:?},bytes={:?})", src, dst, size), + }) => write!(fmt, "copy_nonoverlapping(src={:?}, dst={:?}, size={:?})", src, dst, size), Nop => write!(fmt, "nop"), } } @@ -1656,8 +1656,8 @@ pub struct Coverage { #[derive(Clone, Debug, PartialEq, TyEncodable, TyDecodable, HashStable, TypeFoldable)] pub struct CopyNonOverlapping<'tcx> { - pub src: Place<'tcx>, - pub dst: Place<'tcx>, + pub src: Operand<'tcx>, + pub dst: Operand<'tcx>, pub size: Operand<'tcx>, } diff --git a/compiler/rustc_middle/src/mir/visit.rs b/compiler/rustc_middle/src/mir/visit.rs index edd5ac43a7d79..5762048d30afe 100644 --- a/compiler/rustc_middle/src/mir/visit.rs +++ b/compiler/rustc_middle/src/mir/visit.rs @@ -439,14 +439,12 @@ macro_rules! make_mir_visitor { ref $($mutability)? dst, ref $($mutability)? size, }) => { - self.visit_place( + self.visit_operand( src, - PlaceContext::NonMutatingUse(NonMutatingUseContext::CopyNonOverlapping), location ); - self.visit_place( + self.visit_operand( dst, - PlaceContext::MutatingUse(MutatingUseContext::CopyNonOverlapping), location ); self.visit_operand(size, location) @@ -1166,8 +1164,6 @@ pub enum NonMutatingUseContext { /// f(&x.y); /// Projection, - /// Source from copy_nonoverlapping. - CopyNonOverlapping, } #[derive(Copy, Clone, Debug, PartialEq, Eq)] @@ -1197,8 +1193,6 @@ pub enum MutatingUseContext { Projection, /// Retagging, a "Stacked Borrows" shadow state operation Retag, - /// Memory written to in copy_nonoverlapping. - CopyNonOverlapping, } #[derive(Copy, Clone, Debug, PartialEq, Eq)] diff --git a/compiler/rustc_mir/src/borrow_check/invalidation.rs b/compiler/rustc_mir/src/borrow_check/invalidation.rs index 8c05e6fd5d0e4..8eb6e8a91f1f1 100644 --- a/compiler/rustc_mir/src/borrow_check/invalidation.rs +++ b/compiler/rustc_mir/src/borrow_check/invalidation.rs @@ -92,6 +92,21 @@ impl<'cx, 'tcx> Visitor<'tcx> for InvalidationGenerator<'cx, 'tcx> { self.consume_operand(location, input); } } + StatementKind::CopyNonOverlapping(box rustc_middle::mir::CopyNonOverlapping { + ref src, + ref dst, + ref size, + }) => { + self.consume_operand(location, src); + self.consume_operand(location, dst); + self.consume_operand(location, size); + match dst { + Operand::Move(ref place) | Operand::Copy(ref place) => { + self.mutate_place(location, *place, Deep, JustWrite); + } + _ => {} + } + } StatementKind::Nop | StatementKind::Coverage(..) | StatementKind::AscribeUserType(..) diff --git a/compiler/rustc_mir/src/borrow_check/mod.rs b/compiler/rustc_mir/src/borrow_check/mod.rs index 375d464917118..cdff4715fc1ab 100644 --- a/compiler/rustc_mir/src/borrow_check/mod.rs +++ b/compiler/rustc_mir/src/borrow_check/mod.rs @@ -626,6 +626,8 @@ impl<'cx, 'tcx> dataflow::ResultsVisitor<'cx, 'tcx> for MirBorrowckCtxt<'cx, 'tc self.consume_operand(location, (input, span), flow_state); } } + + StatementKind::CopyNonOverlapping(..) => todo!(), StatementKind::Nop | StatementKind::Coverage(..) | StatementKind::AscribeUserType(..) diff --git a/compiler/rustc_mir/src/borrow_check/type_check/mod.rs b/compiler/rustc_mir/src/borrow_check/type_check/mod.rs index aa946fdafa912..f34a12099d72f 100644 --- a/compiler/rustc_mir/src/borrow_check/type_check/mod.rs +++ b/compiler/rustc_mir/src/borrow_check/type_check/mod.rs @@ -1520,6 +1520,7 @@ impl<'a, 'tcx> TypeChecker<'a, 'tcx> { ); } } + StatementKind::CopyNonOverlapping(..) => todo!(), StatementKind::FakeRead(..) | StatementKind::StorageLive(..) | StatementKind::StorageDead(..) diff --git a/compiler/rustc_mir/src/dataflow/impls/borrows.rs b/compiler/rustc_mir/src/dataflow/impls/borrows.rs index b149ffa9667a3..ffa02f855c979 100644 --- a/compiler/rustc_mir/src/dataflow/impls/borrows.rs +++ b/compiler/rustc_mir/src/dataflow/impls/borrows.rs @@ -306,6 +306,8 @@ impl<'tcx> dataflow::GenKillAnalysis<'tcx> for Borrows<'_, 'tcx> { | mir::StatementKind::AscribeUserType(..) | mir::StatementKind::Coverage(..) | mir::StatementKind::Nop => {} + + mir::StatementKind::CopyNonOverlapping(..) => todo!(), } } diff --git a/compiler/rustc_mir/src/dataflow/impls/storage_liveness.rs b/compiler/rustc_mir/src/dataflow/impls/storage_liveness.rs index 9250cd408479a..e407f394c51ec 100644 --- a/compiler/rustc_mir/src/dataflow/impls/storage_liveness.rs +++ b/compiler/rustc_mir/src/dataflow/impls/storage_liveness.rs @@ -150,6 +150,8 @@ impl<'mir, 'tcx> dataflow::GenKillAnalysis<'tcx> for MaybeRequiresStorage<'mir, | StatementKind::Nop | StatementKind::Retag(..) | StatementKind::StorageLive(..) => {} + + StatementKind::CopyNonOverlapping(..) => todo!(), } } diff --git a/compiler/rustc_mir/src/dataflow/move_paths/builder.rs b/compiler/rustc_mir/src/dataflow/move_paths/builder.rs index ee78ff00c9b2b..aaf78046a46a0 100644 --- a/compiler/rustc_mir/src/dataflow/move_paths/builder.rs +++ b/compiler/rustc_mir/src/dataflow/move_paths/builder.rs @@ -319,6 +319,8 @@ impl<'b, 'a, 'tcx> Gatherer<'b, 'a, 'tcx> { | StatementKind::AscribeUserType(..) | StatementKind::Coverage(..) | StatementKind::Nop => {} + + StatementKind::CopyNonOverlapping(..) => todo!(), } } diff --git a/compiler/rustc_mir/src/interpret/step.rs b/compiler/rustc_mir/src/interpret/step.rs index 64d7c8ef2c719..f4030366c53d4 100644 --- a/compiler/rustc_mir/src/interpret/step.rs +++ b/compiler/rustc_mir/src/interpret/step.rs @@ -113,6 +113,23 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> { M::retag(self, *kind, &dest)?; } + // Call CopyNonOverlapping + CopyNonOverlapping(box rustc_middle::mir::CopyNonOverlapping { dst, src, size }) => { + let size = self.eval_operand(size, None)?; + + let dst = { + let dst = self.eval_operand(dst, None)?; + dst.assert_mem_place(self) + }; + let src = { + let src = self.eval_operand(src, None)?; + src.assert_mem_place(self) + }; + // Not sure how to convert an MPlaceTy<'_, >::PointerTag> + // to a pointer, or OpTy to a size + self.memory.copy(src, dst, size, /*nonoverlapping*/ true)?; + } + // Statements we do not track. AscribeUserType(..) => {} diff --git a/compiler/rustc_mir/src/transform/check_consts/validation.rs b/compiler/rustc_mir/src/transform/check_consts/validation.rs index a82636837122f..39ca6f3a9a830 100644 --- a/compiler/rustc_mir/src/transform/check_consts/validation.rs +++ b/compiler/rustc_mir/src/transform/check_consts/validation.rs @@ -808,6 +808,7 @@ impl Visitor<'tcx> for Validator<'mir, 'tcx> { | StatementKind::Retag { .. } | StatementKind::AscribeUserType(..) | StatementKind::Coverage(..) + | StatementKind::CopyNonOverlapping(..) | StatementKind::Nop => {} } } diff --git a/compiler/rustc_mir/src/transform/check_unsafety.rs b/compiler/rustc_mir/src/transform/check_unsafety.rs index f0472758dfb8e..e7fdd5496cb40 100644 --- a/compiler/rustc_mir/src/transform/check_unsafety.rs +++ b/compiler/rustc_mir/src/transform/check_unsafety.rs @@ -115,6 +115,7 @@ impl<'a, 'tcx> Visitor<'tcx> for UnsafetyChecker<'a, 'tcx> { | StatementKind::Retag { .. } | StatementKind::AscribeUserType(..) | StatementKind::Coverage(..) + | StatementKind::CopyNonOverlapping(..) | StatementKind::Nop => { // safe (at least as emitted during MIR construction) } diff --git a/compiler/rustc_mir/src/transform/dest_prop.rs b/compiler/rustc_mir/src/transform/dest_prop.rs index 46de5dba6e0ed..c84baa94009a6 100644 --- a/compiler/rustc_mir/src/transform/dest_prop.rs +++ b/compiler/rustc_mir/src/transform/dest_prop.rs @@ -582,6 +582,7 @@ impl Conflicts<'a> { | StatementKind::FakeRead(..) | StatementKind::AscribeUserType(..) | StatementKind::Coverage(..) + | StatementKind::CopyNonOverlapping(..) | StatementKind::Nop => {} } } diff --git a/compiler/rustc_mir/src/transform/generator.rs b/compiler/rustc_mir/src/transform/generator.rs index 9f996e8452835..6683166d4fe51 100644 --- a/compiler/rustc_mir/src/transform/generator.rs +++ b/compiler/rustc_mir/src/transform/generator.rs @@ -1453,6 +1453,7 @@ impl Visitor<'tcx> for EnsureGeneratorFieldAssignmentsNeverAlias<'_> { | StatementKind::Retag(..) | StatementKind::AscribeUserType(..) | StatementKind::Coverage(..) + | StatementKind::CopyNonOverlapping(..) | StatementKind::Nop => {} } } diff --git a/compiler/rustc_mir/src/transform/instrument_coverage.rs b/compiler/rustc_mir/src/transform/instrument_coverage.rs new file mode 100644 index 0000000000000..7125e1516e9fd --- /dev/null +++ b/compiler/rustc_mir/src/transform/instrument_coverage.rs @@ -0,0 +1,1272 @@ +use crate::transform::MirPass; +use crate::util::pretty; +use crate::util::spanview::{self, SpanViewable}; + +use rustc_data_structures::fingerprint::Fingerprint; +use rustc_data_structures::graph::dominators::Dominators; +use rustc_data_structures::stable_hasher::{HashStable, StableHasher}; +use rustc_data_structures::sync::Lrc; +use rustc_index::bit_set::BitSet; +use rustc_index::vec::IndexVec; +use rustc_middle::hir; +use rustc_middle::hir::map::blocks::FnLikeNode; +use rustc_middle::ich::StableHashingContext; +use rustc_middle::mir; +use rustc_middle::mir::coverage::*; +use rustc_middle::mir::visit::Visitor; +use rustc_middle::mir::{ + AggregateKind, BasicBlock, BasicBlockData, Coverage, CoverageInfo, FakeReadCause, Location, + Rvalue, SourceInfo, Statement, StatementKind, Terminator, TerminatorKind, +}; +use rustc_middle::ty::query::Providers; +use rustc_middle::ty::TyCtxt; +use rustc_span::def_id::DefId; +use rustc_span::source_map::original_sp; +use rustc_span::{BytePos, CharPos, Pos, SourceFile, Span, Symbol, SyntaxContext}; + +use std::cmp::Ordering; + +const ID_SEPARATOR: &str = ","; + +/// Inserts `StatementKind::Coverage` statements that either instrument the binary with injected +/// counters, via intrinsic `llvm.instrprof.increment`, and/or inject metadata used during codegen +/// to construct the coverage map. +pub struct InstrumentCoverage; + +/// The `query` provider for `CoverageInfo`, requested by `codegen_coverage()` (to inject each +/// counter) and `FunctionCoverage::new()` (to extract the coverage map metadata from the MIR). +pub(crate) fn provide(providers: &mut Providers) { + providers.coverageinfo = |tcx, def_id| coverageinfo_from_mir(tcx, def_id); +} + +/// The `num_counters` argument to `llvm.instrprof.increment` is the max counter_id + 1, or in +/// other words, the number of counter value references injected into the MIR (plus 1 for the +/// reserved `ZERO` counter, which uses counter ID `0` when included in an expression). Injected +/// counters have a counter ID from `1..num_counters-1`. +/// +/// `num_expressions` is the number of counter expressions added to the MIR body. +/// +/// Both `num_counters` and `num_expressions` are used to initialize new vectors, during backend +/// code generate, to lookup counters and expressions by simple u32 indexes. +/// +/// MIR optimization may split and duplicate some BasicBlock sequences, or optimize out some code +/// including injected counters. (It is OK if some counters are optimized out, but those counters +/// are still included in the total `num_counters` or `num_expressions`.) Simply counting the +/// calls may not work; but computing the number of counters or expressions by adding `1` to the +/// highest ID (for a given instrumented function) is valid. +struct CoverageVisitor { + info: CoverageInfo, +} + +impl Visitor<'_> for CoverageVisitor { + fn visit_coverage(&mut self, coverage: &Coverage, _location: Location) { + match coverage.kind { + CoverageKind::Counter { id, .. } => { + let counter_id = u32::from(id); + self.info.num_counters = std::cmp::max(self.info.num_counters, counter_id + 1); + } + CoverageKind::Expression { id, .. } => { + let expression_index = u32::MAX - u32::from(id); + self.info.num_expressions = + std::cmp::max(self.info.num_expressions, expression_index + 1); + } + _ => {} + } + } +} + +fn coverageinfo_from_mir<'tcx>(tcx: TyCtxt<'tcx>, def_id: DefId) -> CoverageInfo { + let mir_body = tcx.optimized_mir(def_id); + + let mut coverage_visitor = + CoverageVisitor { info: CoverageInfo { num_counters: 0, num_expressions: 0 } }; + + coverage_visitor.visit_body(mir_body); + coverage_visitor.info +} + +impl<'tcx> MirPass<'tcx> for InstrumentCoverage { + fn run_pass(&self, tcx: TyCtxt<'tcx>, mir_body: &mut mir::Body<'tcx>) { + // If the InstrumentCoverage pass is called on promoted MIRs, skip them. + // See: https://github.com/rust-lang/rust/pull/73011#discussion_r438317601 + if mir_body.source.promoted.is_some() { + trace!( + "InstrumentCoverage skipped for {:?} (already promoted for Miri evaluation)", + mir_body.source.def_id() + ); + return; + } + + let hir_id = tcx.hir().local_def_id_to_hir_id(mir_body.source.def_id().expect_local()); + let is_fn_like = FnLikeNode::from_node(tcx.hir().get(hir_id)).is_some(); + + // Only instrument functions, methods, and closures (not constants since they are evaluated + // at compile time by Miri). + // FIXME(#73156): Handle source code coverage in const eval + if !is_fn_like { + trace!( + "InstrumentCoverage skipped for {:?} (not an FnLikeNode)", + mir_body.source.def_id(), + ); + return; + } + // FIXME(richkadel): By comparison, the MIR pass `ConstProp` includes associated constants, + // with functions, methods, and closures. I assume Miri is used for associated constants as + // well. If not, we may need to include them here too. + + trace!("InstrumentCoverage starting for {:?}", mir_body.source.def_id()); + Instrumentor::new(&self.name(), tcx, mir_body).inject_counters(); + trace!("InstrumentCoverage starting for {:?}", mir_body.source.def_id()); + } +} + +/// A BasicCoverageBlock (BCB) represents the maximal-length sequence of CFG (MIR) BasicBlocks +/// without conditional branches. +/// +/// The BCB allows coverage analysis to be performed on a simplified projection of the underlying +/// MIR CFG, without altering the original CFG. Note that running the MIR `SimplifyCfg` transform, +/// is not sufficient, and therefore not necessary, since the BCB-based CFG projection is a more +/// aggressive simplification. For example: +/// +/// * The BCB CFG projection ignores (trims) branches not relevant to coverage, such as unwind- +/// related code that is injected by the Rust compiler but has no physical source code to +/// count. This also means a BasicBlock with a `Call` terminator can be merged into its +/// primary successor target block, in the same BCB. +/// * Some BasicBlock terminators support Rust-specific concerns--like borrow-checking--that are +/// not relevant to coverage analysis. `FalseUnwind`, for example, can be treated the same as +/// a `Goto`, and merged with its successor into the same BCB. +/// +/// Each BCB with at least one computed `CoverageSpan` will have no more than one `Counter`. +/// In some cases, a BCB's execution count can be computed by `CounterExpression`. Additional +/// disjoint `CoverageSpan`s in a BCB can also be counted by `CounterExpression` (by adding `ZERO` +/// to the BCB's primary counter or expression). +/// +/// Dominator/dominated relationships (which are fundamental to the coverage analysis algorithm) +/// between two BCBs can be computed using the `mir::Body` `dominators()` with any `BasicBlock` +/// member of each BCB. (For consistency, BCB's use the first `BasicBlock`, also referred to as the +/// `bcb_leader_bb`.) +/// +/// The BCB CFG projection is critical to simplifying the coverage analysis by ensuring graph +/// path-based queries (`is_dominated_by()`, `predecessors`, `successors`, etc.) have branch +/// (control flow) significance. +#[derive(Debug, Clone)] +struct BasicCoverageBlock { + pub blocks: Vec, +} + +impl BasicCoverageBlock { + pub fn leader_bb(&self) -> BasicBlock { + self.blocks[0] + } + + pub fn id(&self) -> String { + format!( + "@{}", + self.blocks + .iter() + .map(|bb| bb.index().to_string()) + .collect::>() + .join(ID_SEPARATOR) + ) + } +} + +struct BasicCoverageBlocks { + vec: IndexVec>, +} + +impl BasicCoverageBlocks { + pub fn from_mir(mir_body: &mir::Body<'tcx>) -> Self { + let mut basic_coverage_blocks = + BasicCoverageBlocks { vec: IndexVec::from_elem_n(None, mir_body.basic_blocks().len()) }; + basic_coverage_blocks.extract_from_mir(mir_body); + basic_coverage_blocks + } + + pub fn iter(&self) -> impl Iterator { + self.vec.iter().filter_map(|option| option.as_ref()) + } + + fn extract_from_mir(&mut self, mir_body: &mir::Body<'tcx>) { + // Traverse the CFG but ignore anything following an `unwind` + let cfg_without_unwind = ShortCircuitPreorder::new(mir_body, |term_kind| { + let mut successors = term_kind.successors(); + match &term_kind { + // SwitchInt successors are never unwind, and all of them should be traversed. + + // NOTE: TerminatorKind::FalseEdge targets from SwitchInt don't appear to be + // helpful in identifying unreachable code. I did test the theory, but the following + // changes were not beneficial. (I assumed that replacing some constants with + // non-deterministic variables might effect which blocks were targeted by a + // `FalseEdge` `imaginary_target`. It did not.) + // + // Also note that, if there is a way to identify BasicBlocks that are part of the + // MIR CFG, but not actually reachable, here are some other things to consider: + // + // Injecting unreachable code regions will probably require computing the set + // difference between the basic blocks found without filtering out unreachable + // blocks, and the basic blocks found with the filter; then computing the + // `CoverageSpans` without the filter; and then injecting `Counter`s or + // `CounterExpression`s for blocks that are not unreachable, or injecting + // `Unreachable` code regions otherwise. This seems straightforward, but not + // trivial. + // + // Alternatively, we might instead want to leave the unreachable blocks in + // (bypass the filter here), and inject the counters. This will result in counter + // values of zero (0) for unreachable code (and, notably, the code will be displayed + // with a red background by `llvm-cov show`). + // + // TerminatorKind::SwitchInt { .. } => { + // let some_imaginary_target = successors.clone().find_map(|&successor| { + // let term = mir_body.basic_blocks()[successor].terminator(); + // if let TerminatorKind::FalseEdge { imaginary_target, .. } = term.kind { + // if mir_body.predecessors()[imaginary_target].len() == 1 { + // return Some(imaginary_target); + // } + // } + // None + // }); + // if let Some(imaginary_target) = some_imaginary_target { + // box successors.filter(move |&&successor| successor != imaginary_target) + // } else { + // box successors + // } + // } + // + // Note this also required changing the closure signature for the + // `ShortCurcuitPreorder` to: + // + // F: Fn(&'tcx TerminatorKind<'tcx>) -> Box + 'a>, + TerminatorKind::SwitchInt { .. } => successors, + + // For all other kinds, return only the first successor, if any, and ignore unwinds + _ => successors.next().into_iter().chain(&[]), + } + }); + + // Walk the CFG using a Preorder traversal, which starts from `START_BLOCK` and follows + // each block terminator's `successors()`. Coverage spans must map to actual source code, + // so compiler generated blocks and paths can be ignored. To that end the CFG traversal + // intentionally omits unwind paths. + let mut blocks = Vec::new(); + for (bb, data) in cfg_without_unwind { + if let Some(last) = blocks.last() { + let predecessors = &mir_body.predecessors()[bb]; + if predecessors.len() > 1 || !predecessors.contains(last) { + // The `bb` has more than one _incoming_ edge, and should start its own + // `BasicCoverageBlock`. (Note, the `blocks` vector does not yet include `bb`; + // it contains a sequence of one or more sequential blocks with no intermediate + // branches in or out. Save these as a new `BasicCoverageBlock` before starting + // the new one.) + self.add_basic_coverage_block(blocks.split_off(0)); + debug!( + " because {}", + if predecessors.len() > 1 { + "predecessors.len() > 1".to_owned() + } else { + format!("bb {} is not in precessors: {:?}", bb.index(), predecessors) + } + ); + } + } + blocks.push(bb); + + let term = data.terminator(); + + match term.kind { + TerminatorKind::Return { .. } + | TerminatorKind::Abort + | TerminatorKind::Assert { .. } + | TerminatorKind::Yield { .. } + | TerminatorKind::SwitchInt { .. } => { + // The `bb` has more than one _outgoing_ edge, or exits the function. Save the + // current sequence of `blocks` gathered to this point, as a new + // `BasicCoverageBlock`. + self.add_basic_coverage_block(blocks.split_off(0)); + debug!(" because term.kind = {:?}", term.kind); + // Note that this condition is based on `TerminatorKind`, even though it + // theoretically boils down to `successors().len() != 1`; that is, either zero + // (e.g., `Return`, `Abort`) or multiple successors (e.g., `SwitchInt`), but + // since the Coverage graph (the BCB CFG projection) ignores things like unwind + // branches (which exist in the `Terminator`s `successors()` list) checking the + // number of successors won't work. + } + TerminatorKind::Goto { .. } + | TerminatorKind::Resume + | TerminatorKind::Unreachable + | TerminatorKind::Drop { .. } + | TerminatorKind::DropAndReplace { .. } + | TerminatorKind::Call { .. } + | TerminatorKind::GeneratorDrop + | TerminatorKind::FalseEdge { .. } + | TerminatorKind::FalseUnwind { .. } + | TerminatorKind::InlineAsm { .. } => {} + } + } + + if !blocks.is_empty() { + // process any remaining blocks into a final `BasicCoverageBlock` + self.add_basic_coverage_block(blocks.split_off(0)); + debug!(" because the end of the CFG was reached while traversing"); + } + } + + fn add_basic_coverage_block(&mut self, blocks: Vec) { + let leader_bb = blocks[0]; + let bcb = BasicCoverageBlock { blocks }; + debug!("adding BCB: {:?}", bcb); + self.vec[leader_bb] = Some(bcb); + } +} + +impl std::ops::Index for BasicCoverageBlocks { + type Output = BasicCoverageBlock; + + fn index(&self, index: BasicBlock) -> &Self::Output { + self.vec[index].as_ref().expect("is_some if BasicBlock is a BasicCoverageBlock leader") + } +} + +#[derive(Debug, Copy, Clone)] +enum CoverageStatement { + Statement(BasicBlock, Span, usize), + Terminator(BasicBlock, Span), +} + +impl CoverageStatement { + pub fn format(&self, tcx: TyCtxt<'tcx>, mir_body: &'a mir::Body<'tcx>) -> String { + match *self { + Self::Statement(bb, span, stmt_index) => { + let stmt = &mir_body.basic_blocks()[bb].statements[stmt_index]; + format!( + "{}: @{}[{}]: {:?}", + spanview::source_range_no_file(tcx, &span), + bb.index(), + stmt_index, + stmt + ) + } + Self::Terminator(bb, span) => { + let term = mir_body.basic_blocks()[bb].terminator(); + format!( + "{}: @{}.{}: {:?}", + spanview::source_range_no_file(tcx, &span), + bb.index(), + term_type(&term.kind), + term.kind + ) + } + } + } + + pub fn span(&self) -> &Span { + match self { + Self::Statement(_, span, _) | Self::Terminator(_, span) => span, + } + } +} + +fn term_type(kind: &TerminatorKind<'tcx>) -> &'static str { + match kind { + TerminatorKind::Goto { .. } => "Goto", + TerminatorKind::SwitchInt { .. } => "SwitchInt", + TerminatorKind::Resume => "Resume", + TerminatorKind::Abort => "Abort", + TerminatorKind::Return => "Return", + TerminatorKind::Unreachable => "Unreachable", + TerminatorKind::Drop { .. } => "Drop", + TerminatorKind::DropAndReplace { .. } => "DropAndReplace", + TerminatorKind::Call { .. } => "Call", + TerminatorKind::Assert { .. } => "Assert", + TerminatorKind::Yield { .. } => "Yield", + TerminatorKind::GeneratorDrop => "GeneratorDrop", + TerminatorKind::FalseEdge { .. } => "FalseEdge", + TerminatorKind::FalseUnwind { .. } => "FalseUnwind", + TerminatorKind::InlineAsm { .. } => "InlineAsm", + } +} + +/// A BCB is deconstructed into one or more `Span`s. Each `Span` maps to a `CoverageSpan` that +/// references the originating BCB and one or more MIR `Statement`s and/or `Terminator`s. +/// Initially, the `Span`s come from the `Statement`s and `Terminator`s, but subsequent +/// transforms can combine adjacent `Span`s and `CoverageSpan` from the same BCB, merging the +/// `CoverageStatement` vectors, and the `Span`s to cover the extent of the combined `Span`s. +/// +/// Note: A `CoverageStatement` merged into another CoverageSpan may come from a `BasicBlock` that +/// is not part of the `CoverageSpan` bcb if the statement was included because it's `Span` matches +/// or is subsumed by the `Span` associated with this `CoverageSpan`, and it's `BasicBlock` +/// `is_dominated_by()` the `BasicBlock`s in this `CoverageSpan`. +#[derive(Debug, Clone)] +struct CoverageSpan { + span: Span, + bcb_leader_bb: BasicBlock, + coverage_statements: Vec, + is_closure: bool, +} + +impl CoverageSpan { + pub fn for_statement( + statement: &Statement<'tcx>, + span: Span, + bcb: &BasicCoverageBlock, + bb: BasicBlock, + stmt_index: usize, + ) -> Self { + let is_closure = match statement.kind { + StatementKind::Assign(box ( + _, + Rvalue::Aggregate(box AggregateKind::Closure(_, _), _), + )) => true, + _ => false, + }; + + Self { + span, + bcb_leader_bb: bcb.leader_bb(), + coverage_statements: vec![CoverageStatement::Statement(bb, span, stmt_index)], + is_closure, + } + } + + pub fn for_terminator(span: Span, bcb: &'a BasicCoverageBlock, bb: BasicBlock) -> Self { + Self { + span, + bcb_leader_bb: bcb.leader_bb(), + coverage_statements: vec![CoverageStatement::Terminator(bb, span)], + is_closure: false, + } + } + + pub fn merge_from(&mut self, mut other: CoverageSpan) { + debug_assert!(self.is_mergeable(&other)); + self.span = self.span.to(other.span); + if other.is_closure { + self.is_closure = true; + } + self.coverage_statements.append(&mut other.coverage_statements); + } + + pub fn cutoff_statements_at(&mut self, cutoff_pos: BytePos) { + self.coverage_statements.retain(|covstmt| covstmt.span().hi() <= cutoff_pos); + if let Some(highest_covstmt) = + self.coverage_statements.iter().max_by_key(|covstmt| covstmt.span().hi()) + { + self.span = self.span.with_hi(highest_covstmt.span().hi()); + } + } + + pub fn is_dominated_by( + &self, + other: &CoverageSpan, + dominators: &Dominators, + ) -> bool { + debug_assert!(!self.is_in_same_bcb(other)); + dominators.is_dominated_by(self.bcb_leader_bb, other.bcb_leader_bb) + } + + pub fn is_mergeable(&self, other: &Self) -> bool { + self.is_in_same_bcb(other) && !(self.is_closure || other.is_closure) + } + + pub fn is_in_same_bcb(&self, other: &Self) -> bool { + self.bcb_leader_bb == other.bcb_leader_bb + } +} + +struct Instrumentor<'a, 'tcx> { + pass_name: &'a str, + tcx: TyCtxt<'tcx>, + mir_body: &'a mut mir::Body<'tcx>, + hir_body: &'tcx rustc_hir::Body<'tcx>, + dominators: Option>, + basic_coverage_blocks: Option, + function_source_hash: Option, + next_counter_id: u32, + num_expressions: u32, +} + +impl<'a, 'tcx> Instrumentor<'a, 'tcx> { + fn new(pass_name: &'a str, tcx: TyCtxt<'tcx>, mir_body: &'a mut mir::Body<'tcx>) -> Self { + let hir_body = hir_body(tcx, mir_body.source.def_id()); + Self { + pass_name, + tcx, + mir_body, + hir_body, + dominators: None, + basic_coverage_blocks: None, + function_source_hash: None, + next_counter_id: CounterValueReference::START.as_u32(), + num_expressions: 0, + } + } + + /// Counter IDs start from one and go up. + fn next_counter(&mut self) -> CounterValueReference { + assert!(self.next_counter_id < u32::MAX - self.num_expressions); + let next = self.next_counter_id; + self.next_counter_id += 1; + CounterValueReference::from(next) + } + + /// Expression IDs start from u32::MAX and go down because a CounterExpression can reference + /// (add or subtract counts) of both Counter regions and CounterExpression regions. The counter + /// expression operand IDs must be unique across both types. + fn next_expression(&mut self) -> InjectedExpressionIndex { + assert!(self.next_counter_id < u32::MAX - self.num_expressions); + let next = u32::MAX - self.num_expressions; + self.num_expressions += 1; + InjectedExpressionIndex::from(next) + } + + fn dominators(&self) -> &Dominators { + self.dominators.as_ref().expect("dominators must be initialized before calling") + } + + fn basic_coverage_blocks(&self) -> &BasicCoverageBlocks { + self.basic_coverage_blocks + .as_ref() + .expect("basic_coverage_blocks must be initialized before calling") + } + + fn function_source_hash(&mut self) -> u64 { + match self.function_source_hash { + Some(hash) => hash, + None => { + let hash = hash_mir_source(self.tcx, self.hir_body); + self.function_source_hash.replace(hash); + hash + } + } + } + + fn inject_counters(&mut self) { + let tcx = self.tcx; + let source_map = tcx.sess.source_map(); + let def_id = self.mir_body.source.def_id(); + let mir_body = &self.mir_body; + let body_span = self.body_span(); + let source_file = source_map.lookup_source_file(body_span.lo()); + let file_name = Symbol::intern(&source_file.name.to_string()); + + debug!("instrumenting {:?}, span: {}", def_id, source_map.span_to_string(body_span)); + + self.dominators.replace(mir_body.dominators()); + self.basic_coverage_blocks.replace(BasicCoverageBlocks::from_mir(mir_body)); + + let coverage_spans = self.coverage_spans(); + + let span_viewables = if pretty::dump_enabled(tcx, self.pass_name, def_id) { + Some(self.span_viewables(&coverage_spans)) + } else { + None + }; + + // Inject a counter for each `CoverageSpan`. There can be multiple `CoverageSpan`s for a + // given BCB, but only one actual counter needs to be incremented per BCB. `bb_counters` + // maps each `bcb_leader_bb` to its `Counter`, when injected. Subsequent `CoverageSpan`s + // for a BCB that already has a `Counter` will inject a `CounterExpression` instead, and + // compute its value by adding `ZERO` to the BCB `Counter` value. + let mut bb_counters = IndexVec::from_elem_n(None, mir_body.basic_blocks().len()); + for CoverageSpan { span, bcb_leader_bb: bb, .. } in coverage_spans { + if let Some(&counter_operand) = bb_counters[bb].as_ref() { + let expression = + self.make_expression(counter_operand, Op::Add, ExpressionOperandId::ZERO); + debug!( + "Injecting counter expression {:?} at: {:?}:\n{}\n==========", + expression, + span, + source_map.span_to_snippet(span).expect("Error getting source for span"), + ); + self.inject_statement(file_name, &source_file, expression, span, bb); + } else { + let counter = self.make_counter(); + debug!( + "Injecting counter {:?} at: {:?}:\n{}\n==========", + counter, + span, + source_map.span_to_snippet(span).expect("Error getting source for span"), + ); + let counter_operand = counter.as_operand_id(); + bb_counters[bb] = Some(counter_operand); + self.inject_statement(file_name, &source_file, counter, span, bb); + } + } + + if let Some(span_viewables) = span_viewables { + let mut file = pretty::create_dump_file( + tcx, + "html", + None, + self.pass_name, + &0, + self.mir_body.source, + ) + .expect("Unexpected error creating MIR spanview HTML file"); + let crate_name = tcx.crate_name(def_id.krate); + let item_name = tcx.def_path(def_id).to_filename_friendly_no_crate(); + let title = format!("{}.{} - Coverage Spans", crate_name, item_name); + spanview::write_document(tcx, def_id, span_viewables, &title, &mut file) + .expect("Unexpected IO error dumping coverage spans as HTML"); + } + } + + fn make_counter(&mut self) -> CoverageKind { + CoverageKind::Counter { + function_source_hash: self.function_source_hash(), + id: self.next_counter(), + } + } + + fn make_expression( + &mut self, + lhs: ExpressionOperandId, + op: Op, + rhs: ExpressionOperandId, + ) -> CoverageKind { + CoverageKind::Expression { id: self.next_expression(), lhs, op, rhs } + } + + fn inject_statement( + &mut self, + file_name: Symbol, + source_file: &Lrc, + coverage_kind: CoverageKind, + span: Span, + block: BasicBlock, + ) { + let code_region = make_code_region(file_name, source_file, span); + debug!(" injecting statement {:?} covering {:?}", coverage_kind, code_region); + + let data = &mut self.mir_body[block]; + let source_info = data.terminator().source_info; + let statement = Statement { + source_info, + kind: StatementKind::Coverage(box Coverage { kind: coverage_kind, code_region }), + }; + data.statements.push(statement); + } + + /// Converts the computed `BasicCoverageBlock`s into `SpanViewable`s. + fn span_viewables(&self, coverage_spans: &Vec) -> Vec { + let tcx = self.tcx; + let mir_body = &self.mir_body; + let mut span_viewables = Vec::new(); + for coverage_span in coverage_spans { + let bcb = self.bcb_from_coverage_span(coverage_span); + let CoverageSpan { span, bcb_leader_bb: bb, coverage_statements, .. } = coverage_span; + let id = bcb.id(); + let mut sorted_coverage_statements = coverage_statements.clone(); + sorted_coverage_statements.sort_unstable_by_key(|covstmt| match *covstmt { + CoverageStatement::Statement(bb, _, index) => (bb, index), + CoverageStatement::Terminator(bb, _) => (bb, usize::MAX), + }); + let tooltip = sorted_coverage_statements + .iter() + .map(|covstmt| covstmt.format(tcx, mir_body)) + .collect::>() + .join("\n"); + span_viewables.push(SpanViewable { bb: *bb, span: *span, id, tooltip }); + } + span_viewables + } + + #[inline(always)] + fn bcb_from_coverage_span(&self, coverage_span: &CoverageSpan) -> &BasicCoverageBlock { + &self.basic_coverage_blocks()[coverage_span.bcb_leader_bb] + } + + #[inline(always)] + fn body_span(&self) -> Span { + self.hir_body.value.span + } + + // Generate a set of `CoverageSpan`s from the filtered set of `Statement`s and `Terminator`s of + // the `BasicBlock`(s) in the given `BasicCoverageBlock`. One `CoverageSpan` is generated for + // each `Statement` and `Terminator`. (Note that subsequent stages of coverage analysis will + // merge some `CoverageSpan`s, at which point a `CoverageSpan` may represent multiple + // `Statement`s and/or `Terminator`s.) + fn extract_spans(&self, bcb: &'a BasicCoverageBlock) -> Vec { + let body_span = self.body_span(); + let mir_basic_blocks = self.mir_body.basic_blocks(); + bcb.blocks + .iter() + .map(|bbref| { + let bb = *bbref; + let data = &mir_basic_blocks[bb]; + data.statements + .iter() + .enumerate() + .filter_map(move |(index, statement)| { + filtered_statement_span(statement, body_span).map(|span| { + CoverageSpan::for_statement(statement, span, bcb, bb, index) + }) + }) + .chain( + filtered_terminator_span(data.terminator(), body_span) + .map(|span| CoverageSpan::for_terminator(span, bcb, bb)), + ) + }) + .flatten() + .collect() + } + + /// Generate a minimal set of `CoverageSpan`s, each representing a contiguous code region to be + /// counted. + /// + /// The basic steps are: + /// + /// 1. Extract an initial set of spans from the `Statement`s and `Terminator`s of each + /// `BasicCoverageBlock`. + /// 2. Sort the spans by span.lo() (starting position). Spans that start at the same position + /// are sorted with longer spans before shorter spans; and equal spans are sorted + /// (deterministically) based on "dominator" relationship (if any). + /// 3. Traverse the spans in sorted order to identify spans that can be dropped (for instance, + /// if another span or spans are already counting the same code region), or should be merged + /// into a broader combined span (because it represents a contiguous, non-branching, and + /// uninterrupted region of source code). + /// + /// Closures are exposed in their enclosing functions as `Assign` `Rvalue`s, and since + /// closures have their own MIR, their `Span` in their enclosing function should be left + /// "uncovered". + /// + /// Note the resulting vector of `CoverageSpan`s does may not be fully sorted (and does not need + /// to be). + fn coverage_spans(&self) -> Vec { + let mut initial_spans = + Vec::::with_capacity(self.mir_body.basic_blocks().len() * 2); + for bcb in self.basic_coverage_blocks().iter() { + for coverage_span in self.extract_spans(bcb) { + initial_spans.push(coverage_span); + } + } + + if initial_spans.is_empty() { + // This can happen if, for example, the function is unreachable (contains only a + // `BasicBlock`(s) with an `Unreachable` terminator). + return initial_spans; + } + + initial_spans.sort_unstable_by(|a, b| { + if a.span.lo() == b.span.lo() { + if a.span.hi() == b.span.hi() { + if a.is_in_same_bcb(b) { + Some(Ordering::Equal) + } else { + // Sort equal spans by dominator relationship, in reverse order (so + // dominators always come after the dominated equal spans). When later + // comparing two spans in order, the first will either dominate the second, + // or they will have no dominator relationship. + self.dominators().rank_partial_cmp(b.bcb_leader_bb, a.bcb_leader_bb) + } + } else { + // Sort hi() in reverse order so shorter spans are attempted after longer spans. + // This guarantees that, if a `prev` span overlaps, and is not equal to, a `curr` + // span, the prev span either extends further left of the curr span, or they + // start at the same position and the prev span extends further right of the end + // of the curr span. + b.span.hi().partial_cmp(&a.span.hi()) + } + } else { + a.span.lo().partial_cmp(&b.span.lo()) + } + .unwrap() + }); + + let refinery = CoverageSpanRefinery::from_sorted_spans(initial_spans, self.dominators()); + refinery.to_refined_spans() + } +} + +struct CoverageSpanRefinery<'a> { + sorted_spans_iter: std::vec::IntoIter, + dominators: &'a Dominators, + some_curr: Option, + curr_original_span: Span, + some_prev: Option, + prev_original_span: Span, + pending_dups: Vec, + refined_spans: Vec, +} + +impl<'a> CoverageSpanRefinery<'a> { + fn from_sorted_spans( + sorted_spans: Vec, + dominators: &'a Dominators, + ) -> Self { + let refined_spans = Vec::with_capacity(sorted_spans.len()); + let mut sorted_spans_iter = sorted_spans.into_iter(); + let prev = sorted_spans_iter.next().expect("at least one span"); + let prev_original_span = prev.span; + Self { + sorted_spans_iter, + dominators, + refined_spans, + some_curr: None, + curr_original_span: Span::with_root_ctxt(BytePos(0), BytePos(0)), + some_prev: Some(prev), + prev_original_span, + pending_dups: Vec::new(), + } + } + + /// Iterate through the sorted `CoverageSpan`s, and return the refined list of merged and + /// de-duplicated `CoverageSpan`s. + fn to_refined_spans(mut self) -> Vec { + while self.next_coverage_span() { + if self.curr().is_mergeable(self.prev()) { + debug!(" same bcb (and neither is a closure), merge with prev={:?}", self.prev()); + let prev = self.take_prev(); + self.curr_mut().merge_from(prev); + // Note that curr.span may now differ from curr_original_span + } else if self.prev_ends_before_curr() { + debug!( + " different bcbs and disjoint spans, so keep curr for next iter, and add \ + prev={:?}", + self.prev() + ); + let prev = self.take_prev(); + self.add_refined_span(prev); + } else if self.prev().is_closure { + // drop any equal or overlapping span (`curr`) and keep `prev` to test again in the + // next iter + debug!( + " curr overlaps a closure (prev). Drop curr and keep prev for next iter. \ + prev={:?}", + self.prev() + ); + self.discard_curr(); + } else if self.curr().is_closure { + self.carve_out_span_for_closure(); + } else if self.prev_original_span == self.curr().span { + self.hold_pending_dups_unless_dominated(); + } else { + self.cutoff_prev_at_overlapping_curr(); + } + } + debug!(" AT END, adding last prev={:?}", self.prev()); + let pending_dups = self.pending_dups.split_off(0); + for dup in pending_dups.into_iter() { + debug!(" ...adding at least one pending dup={:?}", dup); + self.add_refined_span(dup); + } + let prev = self.take_prev(); + self.add_refined_span(prev); + + // FIXME(richkadel): Replace some counters with expressions if they can be calculated based + // on branching. (For example, one branch of a SwitchInt can be computed from the counter + // for the CoverageSpan just prior to the SwitchInt minus the sum of the counters of all + // other branches). + + self.to_refined_spans_without_closures() + } + + fn add_refined_span(&mut self, coverage_span: CoverageSpan) { + self.refined_spans.push(coverage_span); + } + + /// Remove `CoverageSpan`s derived from closures, originally added to ensure the coverage + /// regions for the current function leave room for the closure's own coverage regions + /// (injected separately, from the closure's own MIR). + fn to_refined_spans_without_closures(mut self) -> Vec { + self.refined_spans.retain(|covspan| !covspan.is_closure); + self.refined_spans + } + + fn curr(&self) -> &CoverageSpan { + self.some_curr + .as_ref() + .unwrap_or_else(|| bug!("invalid attempt to unwrap a None some_curr")) + } + + fn curr_mut(&mut self) -> &mut CoverageSpan { + self.some_curr + .as_mut() + .unwrap_or_else(|| bug!("invalid attempt to unwrap a None some_curr")) + } + + fn prev(&self) -> &CoverageSpan { + self.some_prev + .as_ref() + .unwrap_or_else(|| bug!("invalid attempt to unwrap a None some_prev")) + } + + fn prev_mut(&mut self) -> &mut CoverageSpan { + self.some_prev + .as_mut() + .unwrap_or_else(|| bug!("invalid attempt to unwrap a None some_prev")) + } + + fn take_prev(&mut self) -> CoverageSpan { + self.some_prev.take().unwrap_or_else(|| bug!("invalid attempt to unwrap a None some_prev")) + } + + /// If there are `pending_dups` but `prev` is not a matching dup (`prev.span` doesn't match the + /// `pending_dups` spans), then one of the following two things happened during the previous + /// iteration: + /// * the `span` of prev was modified (by `curr_mut().merge_from(prev)`); or + /// * the `span` of prev advanced past the end of the span of pending_dups + /// (`prev().span.hi() <= curr().span.lo()`) + /// In either case, no more spans will match the span of `pending_dups`, so + /// add the `pending_dups` if they don't overlap `curr`, and clear the list. + fn check_pending_dups(&mut self) { + if let Some(dup) = self.pending_dups.last() { + if dup.span != self.prev().span { + debug!( + " SAME spans, but pending_dups are NOT THE SAME, so BCBs matched on \ + previous iteration, or prev started a new disjoint span" + ); + if dup.span.hi() <= self.curr().span.lo() { + let pending_dups = self.pending_dups.split_off(0); + for dup in pending_dups.into_iter() { + debug!(" ...adding at least one pending={:?}", dup); + self.add_refined_span(dup); + } + } else { + self.pending_dups.clear(); + } + } + } + } + + /// Advance `prev` to `curr` (if any), and `curr` to the next `CoverageSpan` in sorted order. + fn next_coverage_span(&mut self) -> bool { + if let Some(curr) = self.some_curr.take() { + self.some_prev = Some(curr); + self.prev_original_span = self.curr_original_span; + } + while let Some(curr) = self.sorted_spans_iter.next() { + debug!("FOR curr={:?}", curr); + if self.prev_starts_after_next(&curr) { + debug!( + " prev.span starts after curr.span, so curr will be dropped (skipping past \ + closure?); prev={:?}", + self.prev() + ); + } else { + // Save a copy of the original span for `curr` in case the `CoverageSpan` is changed + // by `self.curr_mut().merge_from(prev)`. + self.curr_original_span = curr.span; + self.some_curr.replace(curr); + self.check_pending_dups(); + return true; + } + } + false + } + + /// If called, then the next call to `next_coverage_span()` will *not* update `prev` with the + /// `curr` coverage span. + fn discard_curr(&mut self) { + self.some_curr = None; + } + + /// Returns true if the curr span should be skipped because prev has already advanced beyond the + /// end of curr. This can only happen if a prior iteration updated `prev` to skip past a region + /// of code, such as skipping past a closure. + fn prev_starts_after_next(&self, next_curr: &CoverageSpan) -> bool { + self.prev().span.lo() > next_curr.span.lo() + } + + /// Returns true if the curr span starts past the end of the prev span, which means they don't + /// overlap, so we now know the prev can be added to the refined coverage spans. + fn prev_ends_before_curr(&self) -> bool { + self.prev().span.hi() <= self.curr().span.lo() + } + + /// If `prev`s span extends left of the closure (`curr`), carve out the closure's + /// span from `prev`'s span. (The closure's coverage counters will be injected when + /// processing the closure's own MIR.) Add the portion of the span to the left of the + /// closure; and if the span extends to the right of the closure, update `prev` to + /// that portion of the span. For any `pending_dups`, repeat the same process. + fn carve_out_span_for_closure(&mut self) { + let curr_span = self.curr().span; + let left_cutoff = curr_span.lo(); + let right_cutoff = curr_span.hi(); + let has_pre_closure_span = self.prev().span.lo() < right_cutoff; + let has_post_closure_span = self.prev().span.hi() > right_cutoff; + let mut pending_dups = self.pending_dups.split_off(0); + if has_pre_closure_span { + let mut pre_closure = self.prev().clone(); + pre_closure.span = pre_closure.span.with_hi(left_cutoff); + debug!(" prev overlaps a closure. Adding span for pre_closure={:?}", pre_closure); + if !pending_dups.is_empty() { + for mut dup in pending_dups.iter().cloned() { + dup.span = dup.span.with_hi(left_cutoff); + debug!(" ...and at least one pre_closure dup={:?}", dup); + self.add_refined_span(dup); + } + } + self.add_refined_span(pre_closure); + } + if has_post_closure_span { + // Update prev.span to start after the closure (and discard curr) + self.prev_mut().span = self.prev().span.with_lo(right_cutoff); + self.prev_original_span = self.prev().span; + for dup in pending_dups.iter_mut() { + dup.span = dup.span.with_lo(right_cutoff); + } + self.pending_dups.append(&mut pending_dups); + self.discard_curr(); // since self.prev() was already updated + } else { + pending_dups.clear(); + } + } + + /// When two `CoverageSpan`s have the same `Span`, dominated spans can be discarded; but if + /// neither `CoverageSpan` dominates the other, both (or possibly more than two) are held, + /// until their disposition is determined. In this latter case, the `prev` dup is moved into + /// `pending_dups` so the new `curr` dup can be moved to `prev` for the next iteration. + fn hold_pending_dups_unless_dominated(&mut self) { + // equal coverage spans are ordered by dominators before dominated (if any) + debug_assert!(!self.prev().is_dominated_by(self.curr(), self.dominators)); + + if self.curr().is_dominated_by(&self.prev(), self.dominators) { + // If one span dominates the other, assocate the span with the dominator only. + // + // For example: + // match somenum { + // x if x < 1 => { ... } + // }... + // The span for the first `x` is referenced by both the pattern block (every + // time it is evaluated) and the arm code (only when matched). The counter + // will be applied only to the dominator block. + // + // The dominator's (`prev`) execution count may be higher than the dominated + // block's execution count, so drop `curr`. + debug!( + " different bcbs but SAME spans, and prev dominates curr. Drop curr and \ + keep prev for next iter. prev={:?}", + self.prev() + ); + self.discard_curr(); + } else { + // Save `prev` in `pending_dups`. (`curr` will become `prev` in the next iteration.) + // If the `curr` CoverageSpan is later discarded, `pending_dups` can be discarded as + // well; but if `curr` is added to refined_spans, the `pending_dups` will also be added. + debug!( + " different bcbs but SAME spans, and neither dominates, so keep curr for \ + next iter, and, pending upcoming spans (unless overlapping) add prev={:?}", + self.prev() + ); + let prev = self.take_prev(); + self.pending_dups.push(prev); + } + } + + /// `curr` overlaps `prev`. If `prev`s span extends left of `curr`s span, keep _only_ + /// statements that end before `curr.lo()` (if any), and add the portion of the + /// combined span for those statements. Any other statements have overlapping spans + /// that can be ignored because `curr` and/or other upcoming statements/spans inside + /// the overlap area will produce their own counters. This disambiguation process + /// avoids injecting multiple counters for overlapping spans, and the potential for + /// double-counting. + fn cutoff_prev_at_overlapping_curr(&mut self) { + debug!( + " different bcbs, overlapping spans, so ignore/drop pending and only add prev \ + if it has statements that end before curr={:?}", + self.prev() + ); + if self.pending_dups.is_empty() { + let curr_span = self.curr().span; + self.prev_mut().cutoff_statements_at(curr_span.lo()); + if self.prev().coverage_statements.is_empty() { + debug!(" ... no non-overlapping statements to add"); + } else { + debug!(" ... adding modified prev={:?}", self.prev()); + let prev = self.take_prev(); + self.add_refined_span(prev); + } + } else { + // with `pending_dups`, `prev` cannot have any statements that don't overlap + self.pending_dups.clear(); + } + } +} + +fn filtered_statement_span(statement: &'a Statement<'tcx>, body_span: Span) -> Option { + match statement.kind { + // These statements have spans that are often outside the scope of the executed source code + // for their parent `BasicBlock`. + StatementKind::StorageLive(_) + | StatementKind::StorageDead(_) + // Coverage should not be encountered, but don't inject coverage coverage + | StatementKind::Coverage(_) + // Ignore `Nop`s + | StatementKind::Nop => None, + + // FIXME(richkadel): Look into a possible issue assigning the span to a + // FakeReadCause::ForGuardBinding, in this example: + // match somenum { + // x if x < 1 => { ... } + // }... + // The BasicBlock within the match arm code included one of these statements, but the span + // for it covered the `1` in this source. The actual statements have nothing to do with that + // source span: + // FakeRead(ForGuardBinding, _4); + // where `_4` is: + // _4 = &_1; (at the span for the first `x`) + // and `_1` is the `Place` for `somenum`. + // + // The arm code BasicBlock already has its own assignment for `x` itself, `_3 = 1`, and I've + // decided it's reasonable for that span (even though outside the arm code) to be part of + // the counted coverage of the arm code execution, but I can't justify including the literal + // `1` in the arm code. I'm pretty sure that, if the `FakeRead(ForGuardBinding)` has a + // purpose in codegen, it's probably in the right BasicBlock, but if so, the `Statement`s + // `source_info.span` can't be right. + // + // Consider correcting the span assignment, assuming there is a better solution, and see if + // the following pattern can be removed here: + StatementKind::FakeRead(cause, _) if cause == FakeReadCause::ForGuardBinding => None, + + // Retain spans from all other statements + StatementKind::FakeRead(_, _) // Not including `ForGuardBinding` + | StatementKind::CopyNonOverlapping(..) + | StatementKind::Assign(_) + | StatementKind::SetDiscriminant { .. } + | StatementKind::LlvmInlineAsm(_) + | StatementKind::Retag(_, _) + | StatementKind::AscribeUserType(_, _) => { + Some(source_info_span(&statement.source_info, body_span)) + } + } +} + +fn filtered_terminator_span(terminator: &'a Terminator<'tcx>, body_span: Span) -> Option { + match terminator.kind { + // These terminators have spans that don't positively contribute to computing a reasonable + // span of actually executed source code. (For example, SwitchInt terminators extracted from + // an `if condition { block }` has a span that includes the executed block, if true, + // but for coverage, the code region executed, up to *and* through the SwitchInt, + // actually stops before the if's block.) + TerminatorKind::Unreachable // Unreachable blocks are not connected to the CFG + | TerminatorKind::Assert { .. } + | TerminatorKind::Drop { .. } + | TerminatorKind::DropAndReplace { .. } + | TerminatorKind::SwitchInt { .. } + | TerminatorKind::Goto { .. } + // For `FalseEdge`, only the `real` branch is taken, so it is similar to a `Goto`. + | TerminatorKind::FalseEdge { .. } => None, + + // Retain spans from all other terminators + TerminatorKind::Resume + | TerminatorKind::Abort + | TerminatorKind::Return + | TerminatorKind::Call { .. } + | TerminatorKind::Yield { .. } + | TerminatorKind::GeneratorDrop + | TerminatorKind::FalseUnwind { .. } + | TerminatorKind::InlineAsm { .. } => { + Some(source_info_span(&terminator.source_info, body_span)) + } + } +} + +#[inline(always)] +fn source_info_span(source_info: &SourceInfo, body_span: Span) -> Span { + let span = original_sp(source_info.span, body_span).with_ctxt(SyntaxContext::root()); + if body_span.contains(span) { span } else { body_span } +} + +/// Convert the Span into its file name, start line and column, and end line and column +fn make_code_region(file_name: Symbol, source_file: &Lrc, span: Span) -> CodeRegion { + let (start_line, mut start_col) = source_file.lookup_file_pos(span.lo()); + let (end_line, end_col) = if span.hi() == span.lo() { + let (end_line, mut end_col) = (start_line, start_col); + // Extend an empty span by one character so the region will be counted. + let CharPos(char_pos) = start_col; + if char_pos > 0 { + start_col = CharPos(char_pos - 1); + } else { + end_col = CharPos(char_pos + 1); + } + (end_line, end_col) + } else { + source_file.lookup_file_pos(span.hi()) + }; + CodeRegion { + file_name, + start_line: start_line as u32, + start_col: start_col.to_u32() + 1, + end_line: end_line as u32, + end_col: end_col.to_u32() + 1, + } +} + +fn hir_body<'tcx>(tcx: TyCtxt<'tcx>, def_id: DefId) -> &'tcx rustc_hir::Body<'tcx> { + let hir_node = tcx.hir().get_if_local(def_id).expect("expected DefId is local"); + let fn_body_id = hir::map::associated_body(hir_node).expect("HIR node is a function with body"); + tcx.hir().body(fn_body_id) +} + +fn hash_mir_source<'tcx>(tcx: TyCtxt<'tcx>, hir_body: &'tcx rustc_hir::Body<'tcx>) -> u64 { + let mut hcx = tcx.create_no_span_stable_hashing_context(); + hash(&mut hcx, &hir_body.value).to_smaller_hash() +} + +fn hash( + hcx: &mut StableHashingContext<'tcx>, + node: &impl HashStable>, +) -> Fingerprint { + let mut stable_hasher = StableHasher::new(); + node.hash_stable(hcx, &mut stable_hasher); + stable_hasher.finish() +} + +pub struct ShortCircuitPreorder< + 'a, + 'tcx, + F: Fn(&'tcx TerminatorKind<'tcx>) -> mir::Successors<'tcx>, +> { + body: &'a mir::Body<'tcx>, + visited: BitSet, + worklist: Vec, + filtered_successors: F, +} + +impl<'a, 'tcx, F: Fn(&'tcx TerminatorKind<'tcx>) -> mir::Successors<'tcx>> + ShortCircuitPreorder<'a, 'tcx, F> +{ + pub fn new( + body: &'a mir::Body<'tcx>, + filtered_successors: F, + ) -> ShortCircuitPreorder<'a, 'tcx, F> { + let worklist = vec![mir::START_BLOCK]; + + ShortCircuitPreorder { + body, + visited: BitSet::new_empty(body.basic_blocks().len()), + worklist, + filtered_successors, + } + } +} + +impl<'a: 'tcx, 'tcx, F: Fn(&'tcx TerminatorKind<'tcx>) -> mir::Successors<'tcx>> Iterator + for ShortCircuitPreorder<'a, 'tcx, F> +{ + type Item = (BasicBlock, &'a BasicBlockData<'tcx>); + + fn next(&mut self) -> Option<(BasicBlock, &'a BasicBlockData<'tcx>)> { + while let Some(idx) = self.worklist.pop() { + if !self.visited.insert(idx) { + continue; + } + + let data = &self.body[idx]; + + if let Some(ref term) = data.terminator { + self.worklist.extend((self.filtered_successors)(&term.kind)); + } + + return Some((idx, data)); + } + + None + } + + fn size_hint(&self) -> (usize, Option) { + let size = self.body.basic_blocks().len() - self.visited.count(); + (size, Some(size)) + } +} diff --git a/compiler/rustc_mir/src/transform/remove_noop_landing_pads.rs b/compiler/rustc_mir/src/transform/remove_noop_landing_pads.rs index 31e201c3a5bbe..a37f5d4f329f3 100644 --- a/compiler/rustc_mir/src/transform/remove_noop_landing_pads.rs +++ b/compiler/rustc_mir/src/transform/remove_noop_landing_pads.rs @@ -39,6 +39,7 @@ impl RemoveNoopLandingPads { | StatementKind::StorageDead(_) | StatementKind::AscribeUserType(..) | StatementKind::Coverage(..) + | StatementKind::CopyNonOverlapping(..) | StatementKind::Nop => { // These are all nops in a landing pad } diff --git a/compiler/rustc_mir/src/util/spanview.rs b/compiler/rustc_mir/src/util/spanview.rs index d3ef8c64565c6..a9a30e407b4b0 100644 --- a/compiler/rustc_mir/src/util/spanview.rs +++ b/compiler/rustc_mir/src/util/spanview.rs @@ -245,6 +245,7 @@ pub fn statement_kind_name(statement: &Statement<'_>) -> &'static str { Retag(..) => "Retag", AscribeUserType(..) => "AscribeUserType", Coverage(..) => "Coverage", + CopyNonOverlapping(..) => "CopyNonOverlapping", Nop => "Nop", } } diff --git a/src/tools/clippy/clippy_utils/src/qualify_min_const_fn.rs b/src/tools/clippy/clippy_utils/src/qualify_min_const_fn.rs index a482017afeb13..aa596bcb4975e 100644 --- a/src/tools/clippy/clippy_utils/src/qualify_min_const_fn.rs +++ b/src/tools/clippy/clippy_utils/src/qualify_min_const_fn.rs @@ -210,7 +210,7 @@ fn check_statement(tcx: TyCtxt<'tcx>, body: &Body<'tcx>, def_id: DefId, statemen StatementKind::Assign(box (place, rval)) => { check_place(tcx, *place, span, body)?; check_rvalue(tcx, body, def_id, rval, span) - }, + } StatementKind::FakeRead(_, place) | // just an assignment @@ -218,6 +218,13 @@ fn check_statement(tcx: TyCtxt<'tcx>, body: &Body<'tcx>, def_id: DefId, statemen StatementKind::LlvmInlineAsm { .. } => Err((span, "cannot use inline assembly in const fn".into())), + StatementKind::CopyNonOverlapping(box rustc_middle::mir::CopyNonOverlapping{ + dst, src, size, + }) => { + check_operand(tcx, dst, span, body)?; + check_operand(tcx, src, span, body)?; + check_operand(tcx, size, span, body) + }, // These are all NOPs StatementKind::StorageLive(_) | StatementKind::StorageDead(_) From 9fde03854dea401a77fb828dbc8c6c1514ef7966 Mon Sep 17 00:00:00 2001 From: kadmin Date: Sat, 7 Nov 2020 00:49:55 +0000 Subject: [PATCH 06/19] Update interpret step --- compiler/rustc_mir/src/interpret/step.rs | 14 +++++++++++--- compiler/rustc_mir/src/transform/coverage/spans.rs | 1 + compiler/rustc_mir/src/transform/simplify.rs | 1 + 3 files changed, 13 insertions(+), 3 deletions(-) diff --git a/compiler/rustc_mir/src/interpret/step.rs b/compiler/rustc_mir/src/interpret/step.rs index f4030366c53d4..4e8f43e2a455a 100644 --- a/compiler/rustc_mir/src/interpret/step.rs +++ b/compiler/rustc_mir/src/interpret/step.rs @@ -119,15 +119,23 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> { let dst = { let dst = self.eval_operand(dst, None)?; - dst.assert_mem_place(self) + let mplace = *dst.assert_mem_place(self); + match mplace.ptr { + Scalar::Ptr(ptr) => ptr, + _ => panic!(), + } }; let src = { let src = self.eval_operand(src, None)?; - src.assert_mem_place(self) + let mplace = *src.assert_mem_place(self); + match mplace.ptr { + Scalar::Ptr(ptr) => ptr, + _ => panic!(), + } }; // Not sure how to convert an MPlaceTy<'_, >::PointerTag> // to a pointer, or OpTy to a size - self.memory.copy(src, dst, size, /*nonoverlapping*/ true)?; + self.memory.copy(src, dst, size.layout.layout.size, /*nonoverlapping*/ true)?; } // Statements we do not track. diff --git a/compiler/rustc_mir/src/transform/coverage/spans.rs b/compiler/rustc_mir/src/transform/coverage/spans.rs index fd3e782f6df43..e7097ce861902 100644 --- a/compiler/rustc_mir/src/transform/coverage/spans.rs +++ b/compiler/rustc_mir/src/transform/coverage/spans.rs @@ -687,6 +687,7 @@ pub(super) fn filtered_statement_span( // Retain spans from all other statements StatementKind::FakeRead(_, _) // Not including `ForGuardBinding` + | StatementKind::CopyNonOverlapping(..) | StatementKind::Assign(_) | StatementKind::SetDiscriminant { .. } | StatementKind::LlvmInlineAsm(_) diff --git a/compiler/rustc_mir/src/transform/simplify.rs b/compiler/rustc_mir/src/transform/simplify.rs index 85f27428bbbf4..a5764d9bf4e3d 100644 --- a/compiler/rustc_mir/src/transform/simplify.rs +++ b/compiler/rustc_mir/src/transform/simplify.rs @@ -428,6 +428,7 @@ impl Visitor<'_> for UsedLocals { fn visit_statement(&mut self, statement: &Statement<'tcx>, location: Location) { match statement.kind { StatementKind::LlvmInlineAsm(..) + | StatementKind::CopyNonOverlapping(..) | StatementKind::Retag(..) | StatementKind::Coverage(..) | StatementKind::FakeRead(..) From 0686d5d4c3213df906f2d67f90f9eefe563eb2c4 Mon Sep 17 00:00:00 2001 From: kadmin Date: Tue, 29 Dec 2020 02:00:04 +0000 Subject: [PATCH 07/19] Update cranelift --- compiler/rustc_codegen_cranelift/src/base.rs | 10 + compiler/rustc_codegen_cranelift/src/lib.rs | 1 + .../rustc_codegen_ssa/src/mir/statement.rs | 11 +- compiler/rustc_middle/src/mir/mod.rs | 9 +- compiler/rustc_middle/src/mir/visit.rs | 14 +- .../src/borrow_check/invalidation.rs | 4 +- compiler/rustc_mir/src/interpret/step.rs | 36 +- .../src/transform/instrument_coverage.rs | 1272 ----------------- .../clippy_utils/src/qualify_min_const_fn.rs | 4 +- 9 files changed, 52 insertions(+), 1309 deletions(-) delete mode 100644 compiler/rustc_mir/src/transform/instrument_coverage.rs diff --git a/compiler/rustc_codegen_cranelift/src/base.rs b/compiler/rustc_codegen_cranelift/src/base.rs index 4842628a99da7..94f57f9a4e19e 100644 --- a/compiler/rustc_codegen_cranelift/src/base.rs +++ b/compiler/rustc_codegen_cranelift/src/base.rs @@ -919,6 +919,16 @@ fn codegen_stmt<'tcx>( } } StatementKind::Coverage { .. } => fx.tcx.sess.fatal("-Zcoverage is unimplemented"), + StatementKind::CopyNonOverlapping(box rustc_middle::mir::CopyNonOverlapping { + src, + dst, + count, + }) => { + let dst = codegen_operand(fx, dst).load_scalar(fx); + let src = codegen_operand(fx, src).load_scalar(fx); + let count = codegen_operand(fx, count).load_scalar(fx); + fx.bcx.call_memcpy(fx.cx.module.target_config(), dst, src, count); + } } } diff --git a/compiler/rustc_codegen_cranelift/src/lib.rs b/compiler/rustc_codegen_cranelift/src/lib.rs index 1480ab2513378..7d1d99ef372a0 100644 --- a/compiler/rustc_codegen_cranelift/src/lib.rs +++ b/compiler/rustc_codegen_cranelift/src/lib.rs @@ -10,6 +10,7 @@ #![warn(rust_2018_idioms)] #![warn(unused_lifetimes)] #![warn(unreachable_pub)] +#![feature(box_patterns)] #[cfg(feature = "jit")] extern crate libc; diff --git a/compiler/rustc_codegen_ssa/src/mir/statement.rs b/compiler/rustc_codegen_ssa/src/mir/statement.rs index 1dafc8cd2c16d..774c920ee9666 100644 --- a/compiler/rustc_codegen_ssa/src/mir/statement.rs +++ b/compiler/rustc_codegen_ssa/src/mir/statement.rs @@ -118,23 +118,22 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> { mir::StatementKind::CopyNonOverlapping(box mir::CopyNonOverlapping { ref src, ref dst, - ref size, + ref count, }) => { let dst_val = self.codegen_operand(&mut bx, dst); let src_val = self.codegen_operand(&mut bx, src); - let size_val = self.codegen_operand(&mut bx, size); - let size = size_val.immediate_or_packed_pair(&mut bx); + let count_val = self.codegen_operand(&mut bx, count); + let count = count_val.immediate_or_packed_pair(&mut bx); let dst = dst_val.immediate_or_packed_pair(&mut bx); let src = src_val.immediate_or_packed_pair(&mut bx); use crate::MemFlags; - let flags = - (!MemFlags::UNALIGNED) & (!MemFlags::VOLATILE) & (!MemFlags::NONTEMPORAL); + let flags = MemFlags::empty(); bx.memcpy( dst, dst_val.layout.layout.align.pref, src, src_val.layout.layout.align.pref, - size, + count, flags, ); bx diff --git a/compiler/rustc_middle/src/mir/mod.rs b/compiler/rustc_middle/src/mir/mod.rs index 5fb04eccca811..20393ce46602c 100644 --- a/compiler/rustc_middle/src/mir/mod.rs +++ b/compiler/rustc_middle/src/mir/mod.rs @@ -1641,8 +1641,10 @@ impl Debug for Statement<'_> { CopyNonOverlapping(box crate::mir::CopyNonOverlapping { ref src, ref dst, - ref size, - }) => write!(fmt, "copy_nonoverlapping(src={:?}, dst={:?}, size={:?})", src, dst, size), + ref count, + }) => { + write!(fmt, "copy_nonoverlapping(src={:?}, dst={:?}, count={:?})", src, dst, count) + } Nop => write!(fmt, "nop"), } } @@ -1658,7 +1660,8 @@ pub struct Coverage { pub struct CopyNonOverlapping<'tcx> { pub src: Operand<'tcx>, pub dst: Operand<'tcx>, - pub size: Operand<'tcx>, + /// Number of elements to copy from src to dest, not bytes. + pub count: Operand<'tcx>, } /////////////////////////////////////////////////////////////////////////// diff --git a/compiler/rustc_middle/src/mir/visit.rs b/compiler/rustc_middle/src/mir/visit.rs index 5762048d30afe..9d7cfa7be71dd 100644 --- a/compiler/rustc_middle/src/mir/visit.rs +++ b/compiler/rustc_middle/src/mir/visit.rs @@ -437,17 +437,11 @@ macro_rules! make_mir_visitor { StatementKind::CopyNonOverlapping(box crate::mir::CopyNonOverlapping{ ref $($mutability)? src, ref $($mutability)? dst, - ref $($mutability)? size, + ref $($mutability)? count, }) => { - self.visit_operand( - src, - location - ); - self.visit_operand( - dst, - location - ); - self.visit_operand(size, location) + self.visit_operand(src, location); + self.visit_operand(dst, location); + self.visit_operand(count, location) } StatementKind::Nop => {} } diff --git a/compiler/rustc_mir/src/borrow_check/invalidation.rs b/compiler/rustc_mir/src/borrow_check/invalidation.rs index 8eb6e8a91f1f1..168c373800044 100644 --- a/compiler/rustc_mir/src/borrow_check/invalidation.rs +++ b/compiler/rustc_mir/src/borrow_check/invalidation.rs @@ -95,11 +95,11 @@ impl<'cx, 'tcx> Visitor<'tcx> for InvalidationGenerator<'cx, 'tcx> { StatementKind::CopyNonOverlapping(box rustc_middle::mir::CopyNonOverlapping { ref src, ref dst, - ref size, + ref count, }) => { self.consume_operand(location, src); self.consume_operand(location, dst); - self.consume_operand(location, size); + self.consume_operand(location, count); match dst { Operand::Move(ref place) | Operand::Copy(ref place) => { self.mutate_place(location, *place, Deep, JustWrite); diff --git a/compiler/rustc_mir/src/interpret/step.rs b/compiler/rustc_mir/src/interpret/step.rs index 4e8f43e2a455a..eda9a4ac89026 100644 --- a/compiler/rustc_mir/src/interpret/step.rs +++ b/compiler/rustc_mir/src/interpret/step.rs @@ -114,28 +114,36 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> { } // Call CopyNonOverlapping - CopyNonOverlapping(box rustc_middle::mir::CopyNonOverlapping { dst, src, size }) => { - let size = self.eval_operand(size, None)?; + CopyNonOverlapping(box rustc_middle::mir::CopyNonOverlapping { dst, src, count }) => { + let (src, size) = { + let src = self.eval_operand(src, None)?; + let size = src.layout.layout.size; + let mplace = *src.assert_mem_place(self); + let ptr = match mplace.ptr { + Scalar::Ptr(ptr) => ptr, + _ => panic!(), + }; + (ptr, size) + }; let dst = { let dst = self.eval_operand(dst, None)?; let mplace = *dst.assert_mem_place(self); match mplace.ptr { - Scalar::Ptr(ptr) => ptr, - _ => panic!(), + Scalar::Ptr(ptr) => ptr, + _ => panic!(), } }; - let src = { - let src = self.eval_operand(src, None)?; - let mplace = *src.assert_mem_place(self); - match mplace.ptr { - Scalar::Ptr(ptr) => ptr, - _ => panic!(), - } + + let count = self.eval_operand(count, None)?; + let count = self.read_immediate(count)?.to_scalar()?; + let count = if let Scalar::Int(i) = count { + core::convert::TryFrom::try_from(i).unwrap() + } else { + panic!(); }; - // Not sure how to convert an MPlaceTy<'_, >::PointerTag> - // to a pointer, or OpTy to a size - self.memory.copy(src, dst, size.layout.layout.size, /*nonoverlapping*/ true)?; + + self.memory.copy_repeatedly(src, dst, size, count, /*nonoverlapping*/ true)?; } // Statements we do not track. diff --git a/compiler/rustc_mir/src/transform/instrument_coverage.rs b/compiler/rustc_mir/src/transform/instrument_coverage.rs deleted file mode 100644 index 7125e1516e9fd..0000000000000 --- a/compiler/rustc_mir/src/transform/instrument_coverage.rs +++ /dev/null @@ -1,1272 +0,0 @@ -use crate::transform::MirPass; -use crate::util::pretty; -use crate::util::spanview::{self, SpanViewable}; - -use rustc_data_structures::fingerprint::Fingerprint; -use rustc_data_structures::graph::dominators::Dominators; -use rustc_data_structures::stable_hasher::{HashStable, StableHasher}; -use rustc_data_structures::sync::Lrc; -use rustc_index::bit_set::BitSet; -use rustc_index::vec::IndexVec; -use rustc_middle::hir; -use rustc_middle::hir::map::blocks::FnLikeNode; -use rustc_middle::ich::StableHashingContext; -use rustc_middle::mir; -use rustc_middle::mir::coverage::*; -use rustc_middle::mir::visit::Visitor; -use rustc_middle::mir::{ - AggregateKind, BasicBlock, BasicBlockData, Coverage, CoverageInfo, FakeReadCause, Location, - Rvalue, SourceInfo, Statement, StatementKind, Terminator, TerminatorKind, -}; -use rustc_middle::ty::query::Providers; -use rustc_middle::ty::TyCtxt; -use rustc_span::def_id::DefId; -use rustc_span::source_map::original_sp; -use rustc_span::{BytePos, CharPos, Pos, SourceFile, Span, Symbol, SyntaxContext}; - -use std::cmp::Ordering; - -const ID_SEPARATOR: &str = ","; - -/// Inserts `StatementKind::Coverage` statements that either instrument the binary with injected -/// counters, via intrinsic `llvm.instrprof.increment`, and/or inject metadata used during codegen -/// to construct the coverage map. -pub struct InstrumentCoverage; - -/// The `query` provider for `CoverageInfo`, requested by `codegen_coverage()` (to inject each -/// counter) and `FunctionCoverage::new()` (to extract the coverage map metadata from the MIR). -pub(crate) fn provide(providers: &mut Providers) { - providers.coverageinfo = |tcx, def_id| coverageinfo_from_mir(tcx, def_id); -} - -/// The `num_counters` argument to `llvm.instrprof.increment` is the max counter_id + 1, or in -/// other words, the number of counter value references injected into the MIR (plus 1 for the -/// reserved `ZERO` counter, which uses counter ID `0` when included in an expression). Injected -/// counters have a counter ID from `1..num_counters-1`. -/// -/// `num_expressions` is the number of counter expressions added to the MIR body. -/// -/// Both `num_counters` and `num_expressions` are used to initialize new vectors, during backend -/// code generate, to lookup counters and expressions by simple u32 indexes. -/// -/// MIR optimization may split and duplicate some BasicBlock sequences, or optimize out some code -/// including injected counters. (It is OK if some counters are optimized out, but those counters -/// are still included in the total `num_counters` or `num_expressions`.) Simply counting the -/// calls may not work; but computing the number of counters or expressions by adding `1` to the -/// highest ID (for a given instrumented function) is valid. -struct CoverageVisitor { - info: CoverageInfo, -} - -impl Visitor<'_> for CoverageVisitor { - fn visit_coverage(&mut self, coverage: &Coverage, _location: Location) { - match coverage.kind { - CoverageKind::Counter { id, .. } => { - let counter_id = u32::from(id); - self.info.num_counters = std::cmp::max(self.info.num_counters, counter_id + 1); - } - CoverageKind::Expression { id, .. } => { - let expression_index = u32::MAX - u32::from(id); - self.info.num_expressions = - std::cmp::max(self.info.num_expressions, expression_index + 1); - } - _ => {} - } - } -} - -fn coverageinfo_from_mir<'tcx>(tcx: TyCtxt<'tcx>, def_id: DefId) -> CoverageInfo { - let mir_body = tcx.optimized_mir(def_id); - - let mut coverage_visitor = - CoverageVisitor { info: CoverageInfo { num_counters: 0, num_expressions: 0 } }; - - coverage_visitor.visit_body(mir_body); - coverage_visitor.info -} - -impl<'tcx> MirPass<'tcx> for InstrumentCoverage { - fn run_pass(&self, tcx: TyCtxt<'tcx>, mir_body: &mut mir::Body<'tcx>) { - // If the InstrumentCoverage pass is called on promoted MIRs, skip them. - // See: https://github.com/rust-lang/rust/pull/73011#discussion_r438317601 - if mir_body.source.promoted.is_some() { - trace!( - "InstrumentCoverage skipped for {:?} (already promoted for Miri evaluation)", - mir_body.source.def_id() - ); - return; - } - - let hir_id = tcx.hir().local_def_id_to_hir_id(mir_body.source.def_id().expect_local()); - let is_fn_like = FnLikeNode::from_node(tcx.hir().get(hir_id)).is_some(); - - // Only instrument functions, methods, and closures (not constants since they are evaluated - // at compile time by Miri). - // FIXME(#73156): Handle source code coverage in const eval - if !is_fn_like { - trace!( - "InstrumentCoverage skipped for {:?} (not an FnLikeNode)", - mir_body.source.def_id(), - ); - return; - } - // FIXME(richkadel): By comparison, the MIR pass `ConstProp` includes associated constants, - // with functions, methods, and closures. I assume Miri is used for associated constants as - // well. If not, we may need to include them here too. - - trace!("InstrumentCoverage starting for {:?}", mir_body.source.def_id()); - Instrumentor::new(&self.name(), tcx, mir_body).inject_counters(); - trace!("InstrumentCoverage starting for {:?}", mir_body.source.def_id()); - } -} - -/// A BasicCoverageBlock (BCB) represents the maximal-length sequence of CFG (MIR) BasicBlocks -/// without conditional branches. -/// -/// The BCB allows coverage analysis to be performed on a simplified projection of the underlying -/// MIR CFG, without altering the original CFG. Note that running the MIR `SimplifyCfg` transform, -/// is not sufficient, and therefore not necessary, since the BCB-based CFG projection is a more -/// aggressive simplification. For example: -/// -/// * The BCB CFG projection ignores (trims) branches not relevant to coverage, such as unwind- -/// related code that is injected by the Rust compiler but has no physical source code to -/// count. This also means a BasicBlock with a `Call` terminator can be merged into its -/// primary successor target block, in the same BCB. -/// * Some BasicBlock terminators support Rust-specific concerns--like borrow-checking--that are -/// not relevant to coverage analysis. `FalseUnwind`, for example, can be treated the same as -/// a `Goto`, and merged with its successor into the same BCB. -/// -/// Each BCB with at least one computed `CoverageSpan` will have no more than one `Counter`. -/// In some cases, a BCB's execution count can be computed by `CounterExpression`. Additional -/// disjoint `CoverageSpan`s in a BCB can also be counted by `CounterExpression` (by adding `ZERO` -/// to the BCB's primary counter or expression). -/// -/// Dominator/dominated relationships (which are fundamental to the coverage analysis algorithm) -/// between two BCBs can be computed using the `mir::Body` `dominators()` with any `BasicBlock` -/// member of each BCB. (For consistency, BCB's use the first `BasicBlock`, also referred to as the -/// `bcb_leader_bb`.) -/// -/// The BCB CFG projection is critical to simplifying the coverage analysis by ensuring graph -/// path-based queries (`is_dominated_by()`, `predecessors`, `successors`, etc.) have branch -/// (control flow) significance. -#[derive(Debug, Clone)] -struct BasicCoverageBlock { - pub blocks: Vec, -} - -impl BasicCoverageBlock { - pub fn leader_bb(&self) -> BasicBlock { - self.blocks[0] - } - - pub fn id(&self) -> String { - format!( - "@{}", - self.blocks - .iter() - .map(|bb| bb.index().to_string()) - .collect::>() - .join(ID_SEPARATOR) - ) - } -} - -struct BasicCoverageBlocks { - vec: IndexVec>, -} - -impl BasicCoverageBlocks { - pub fn from_mir(mir_body: &mir::Body<'tcx>) -> Self { - let mut basic_coverage_blocks = - BasicCoverageBlocks { vec: IndexVec::from_elem_n(None, mir_body.basic_blocks().len()) }; - basic_coverage_blocks.extract_from_mir(mir_body); - basic_coverage_blocks - } - - pub fn iter(&self) -> impl Iterator { - self.vec.iter().filter_map(|option| option.as_ref()) - } - - fn extract_from_mir(&mut self, mir_body: &mir::Body<'tcx>) { - // Traverse the CFG but ignore anything following an `unwind` - let cfg_without_unwind = ShortCircuitPreorder::new(mir_body, |term_kind| { - let mut successors = term_kind.successors(); - match &term_kind { - // SwitchInt successors are never unwind, and all of them should be traversed. - - // NOTE: TerminatorKind::FalseEdge targets from SwitchInt don't appear to be - // helpful in identifying unreachable code. I did test the theory, but the following - // changes were not beneficial. (I assumed that replacing some constants with - // non-deterministic variables might effect which blocks were targeted by a - // `FalseEdge` `imaginary_target`. It did not.) - // - // Also note that, if there is a way to identify BasicBlocks that are part of the - // MIR CFG, but not actually reachable, here are some other things to consider: - // - // Injecting unreachable code regions will probably require computing the set - // difference between the basic blocks found without filtering out unreachable - // blocks, and the basic blocks found with the filter; then computing the - // `CoverageSpans` without the filter; and then injecting `Counter`s or - // `CounterExpression`s for blocks that are not unreachable, or injecting - // `Unreachable` code regions otherwise. This seems straightforward, but not - // trivial. - // - // Alternatively, we might instead want to leave the unreachable blocks in - // (bypass the filter here), and inject the counters. This will result in counter - // values of zero (0) for unreachable code (and, notably, the code will be displayed - // with a red background by `llvm-cov show`). - // - // TerminatorKind::SwitchInt { .. } => { - // let some_imaginary_target = successors.clone().find_map(|&successor| { - // let term = mir_body.basic_blocks()[successor].terminator(); - // if let TerminatorKind::FalseEdge { imaginary_target, .. } = term.kind { - // if mir_body.predecessors()[imaginary_target].len() == 1 { - // return Some(imaginary_target); - // } - // } - // None - // }); - // if let Some(imaginary_target) = some_imaginary_target { - // box successors.filter(move |&&successor| successor != imaginary_target) - // } else { - // box successors - // } - // } - // - // Note this also required changing the closure signature for the - // `ShortCurcuitPreorder` to: - // - // F: Fn(&'tcx TerminatorKind<'tcx>) -> Box + 'a>, - TerminatorKind::SwitchInt { .. } => successors, - - // For all other kinds, return only the first successor, if any, and ignore unwinds - _ => successors.next().into_iter().chain(&[]), - } - }); - - // Walk the CFG using a Preorder traversal, which starts from `START_BLOCK` and follows - // each block terminator's `successors()`. Coverage spans must map to actual source code, - // so compiler generated blocks and paths can be ignored. To that end the CFG traversal - // intentionally omits unwind paths. - let mut blocks = Vec::new(); - for (bb, data) in cfg_without_unwind { - if let Some(last) = blocks.last() { - let predecessors = &mir_body.predecessors()[bb]; - if predecessors.len() > 1 || !predecessors.contains(last) { - // The `bb` has more than one _incoming_ edge, and should start its own - // `BasicCoverageBlock`. (Note, the `blocks` vector does not yet include `bb`; - // it contains a sequence of one or more sequential blocks with no intermediate - // branches in or out. Save these as a new `BasicCoverageBlock` before starting - // the new one.) - self.add_basic_coverage_block(blocks.split_off(0)); - debug!( - " because {}", - if predecessors.len() > 1 { - "predecessors.len() > 1".to_owned() - } else { - format!("bb {} is not in precessors: {:?}", bb.index(), predecessors) - } - ); - } - } - blocks.push(bb); - - let term = data.terminator(); - - match term.kind { - TerminatorKind::Return { .. } - | TerminatorKind::Abort - | TerminatorKind::Assert { .. } - | TerminatorKind::Yield { .. } - | TerminatorKind::SwitchInt { .. } => { - // The `bb` has more than one _outgoing_ edge, or exits the function. Save the - // current sequence of `blocks` gathered to this point, as a new - // `BasicCoverageBlock`. - self.add_basic_coverage_block(blocks.split_off(0)); - debug!(" because term.kind = {:?}", term.kind); - // Note that this condition is based on `TerminatorKind`, even though it - // theoretically boils down to `successors().len() != 1`; that is, either zero - // (e.g., `Return`, `Abort`) or multiple successors (e.g., `SwitchInt`), but - // since the Coverage graph (the BCB CFG projection) ignores things like unwind - // branches (which exist in the `Terminator`s `successors()` list) checking the - // number of successors won't work. - } - TerminatorKind::Goto { .. } - | TerminatorKind::Resume - | TerminatorKind::Unreachable - | TerminatorKind::Drop { .. } - | TerminatorKind::DropAndReplace { .. } - | TerminatorKind::Call { .. } - | TerminatorKind::GeneratorDrop - | TerminatorKind::FalseEdge { .. } - | TerminatorKind::FalseUnwind { .. } - | TerminatorKind::InlineAsm { .. } => {} - } - } - - if !blocks.is_empty() { - // process any remaining blocks into a final `BasicCoverageBlock` - self.add_basic_coverage_block(blocks.split_off(0)); - debug!(" because the end of the CFG was reached while traversing"); - } - } - - fn add_basic_coverage_block(&mut self, blocks: Vec) { - let leader_bb = blocks[0]; - let bcb = BasicCoverageBlock { blocks }; - debug!("adding BCB: {:?}", bcb); - self.vec[leader_bb] = Some(bcb); - } -} - -impl std::ops::Index for BasicCoverageBlocks { - type Output = BasicCoverageBlock; - - fn index(&self, index: BasicBlock) -> &Self::Output { - self.vec[index].as_ref().expect("is_some if BasicBlock is a BasicCoverageBlock leader") - } -} - -#[derive(Debug, Copy, Clone)] -enum CoverageStatement { - Statement(BasicBlock, Span, usize), - Terminator(BasicBlock, Span), -} - -impl CoverageStatement { - pub fn format(&self, tcx: TyCtxt<'tcx>, mir_body: &'a mir::Body<'tcx>) -> String { - match *self { - Self::Statement(bb, span, stmt_index) => { - let stmt = &mir_body.basic_blocks()[bb].statements[stmt_index]; - format!( - "{}: @{}[{}]: {:?}", - spanview::source_range_no_file(tcx, &span), - bb.index(), - stmt_index, - stmt - ) - } - Self::Terminator(bb, span) => { - let term = mir_body.basic_blocks()[bb].terminator(); - format!( - "{}: @{}.{}: {:?}", - spanview::source_range_no_file(tcx, &span), - bb.index(), - term_type(&term.kind), - term.kind - ) - } - } - } - - pub fn span(&self) -> &Span { - match self { - Self::Statement(_, span, _) | Self::Terminator(_, span) => span, - } - } -} - -fn term_type(kind: &TerminatorKind<'tcx>) -> &'static str { - match kind { - TerminatorKind::Goto { .. } => "Goto", - TerminatorKind::SwitchInt { .. } => "SwitchInt", - TerminatorKind::Resume => "Resume", - TerminatorKind::Abort => "Abort", - TerminatorKind::Return => "Return", - TerminatorKind::Unreachable => "Unreachable", - TerminatorKind::Drop { .. } => "Drop", - TerminatorKind::DropAndReplace { .. } => "DropAndReplace", - TerminatorKind::Call { .. } => "Call", - TerminatorKind::Assert { .. } => "Assert", - TerminatorKind::Yield { .. } => "Yield", - TerminatorKind::GeneratorDrop => "GeneratorDrop", - TerminatorKind::FalseEdge { .. } => "FalseEdge", - TerminatorKind::FalseUnwind { .. } => "FalseUnwind", - TerminatorKind::InlineAsm { .. } => "InlineAsm", - } -} - -/// A BCB is deconstructed into one or more `Span`s. Each `Span` maps to a `CoverageSpan` that -/// references the originating BCB and one or more MIR `Statement`s and/or `Terminator`s. -/// Initially, the `Span`s come from the `Statement`s and `Terminator`s, but subsequent -/// transforms can combine adjacent `Span`s and `CoverageSpan` from the same BCB, merging the -/// `CoverageStatement` vectors, and the `Span`s to cover the extent of the combined `Span`s. -/// -/// Note: A `CoverageStatement` merged into another CoverageSpan may come from a `BasicBlock` that -/// is not part of the `CoverageSpan` bcb if the statement was included because it's `Span` matches -/// or is subsumed by the `Span` associated with this `CoverageSpan`, and it's `BasicBlock` -/// `is_dominated_by()` the `BasicBlock`s in this `CoverageSpan`. -#[derive(Debug, Clone)] -struct CoverageSpan { - span: Span, - bcb_leader_bb: BasicBlock, - coverage_statements: Vec, - is_closure: bool, -} - -impl CoverageSpan { - pub fn for_statement( - statement: &Statement<'tcx>, - span: Span, - bcb: &BasicCoverageBlock, - bb: BasicBlock, - stmt_index: usize, - ) -> Self { - let is_closure = match statement.kind { - StatementKind::Assign(box ( - _, - Rvalue::Aggregate(box AggregateKind::Closure(_, _), _), - )) => true, - _ => false, - }; - - Self { - span, - bcb_leader_bb: bcb.leader_bb(), - coverage_statements: vec![CoverageStatement::Statement(bb, span, stmt_index)], - is_closure, - } - } - - pub fn for_terminator(span: Span, bcb: &'a BasicCoverageBlock, bb: BasicBlock) -> Self { - Self { - span, - bcb_leader_bb: bcb.leader_bb(), - coverage_statements: vec![CoverageStatement::Terminator(bb, span)], - is_closure: false, - } - } - - pub fn merge_from(&mut self, mut other: CoverageSpan) { - debug_assert!(self.is_mergeable(&other)); - self.span = self.span.to(other.span); - if other.is_closure { - self.is_closure = true; - } - self.coverage_statements.append(&mut other.coverage_statements); - } - - pub fn cutoff_statements_at(&mut self, cutoff_pos: BytePos) { - self.coverage_statements.retain(|covstmt| covstmt.span().hi() <= cutoff_pos); - if let Some(highest_covstmt) = - self.coverage_statements.iter().max_by_key(|covstmt| covstmt.span().hi()) - { - self.span = self.span.with_hi(highest_covstmt.span().hi()); - } - } - - pub fn is_dominated_by( - &self, - other: &CoverageSpan, - dominators: &Dominators, - ) -> bool { - debug_assert!(!self.is_in_same_bcb(other)); - dominators.is_dominated_by(self.bcb_leader_bb, other.bcb_leader_bb) - } - - pub fn is_mergeable(&self, other: &Self) -> bool { - self.is_in_same_bcb(other) && !(self.is_closure || other.is_closure) - } - - pub fn is_in_same_bcb(&self, other: &Self) -> bool { - self.bcb_leader_bb == other.bcb_leader_bb - } -} - -struct Instrumentor<'a, 'tcx> { - pass_name: &'a str, - tcx: TyCtxt<'tcx>, - mir_body: &'a mut mir::Body<'tcx>, - hir_body: &'tcx rustc_hir::Body<'tcx>, - dominators: Option>, - basic_coverage_blocks: Option, - function_source_hash: Option, - next_counter_id: u32, - num_expressions: u32, -} - -impl<'a, 'tcx> Instrumentor<'a, 'tcx> { - fn new(pass_name: &'a str, tcx: TyCtxt<'tcx>, mir_body: &'a mut mir::Body<'tcx>) -> Self { - let hir_body = hir_body(tcx, mir_body.source.def_id()); - Self { - pass_name, - tcx, - mir_body, - hir_body, - dominators: None, - basic_coverage_blocks: None, - function_source_hash: None, - next_counter_id: CounterValueReference::START.as_u32(), - num_expressions: 0, - } - } - - /// Counter IDs start from one and go up. - fn next_counter(&mut self) -> CounterValueReference { - assert!(self.next_counter_id < u32::MAX - self.num_expressions); - let next = self.next_counter_id; - self.next_counter_id += 1; - CounterValueReference::from(next) - } - - /// Expression IDs start from u32::MAX and go down because a CounterExpression can reference - /// (add or subtract counts) of both Counter regions and CounterExpression regions. The counter - /// expression operand IDs must be unique across both types. - fn next_expression(&mut self) -> InjectedExpressionIndex { - assert!(self.next_counter_id < u32::MAX - self.num_expressions); - let next = u32::MAX - self.num_expressions; - self.num_expressions += 1; - InjectedExpressionIndex::from(next) - } - - fn dominators(&self) -> &Dominators { - self.dominators.as_ref().expect("dominators must be initialized before calling") - } - - fn basic_coverage_blocks(&self) -> &BasicCoverageBlocks { - self.basic_coverage_blocks - .as_ref() - .expect("basic_coverage_blocks must be initialized before calling") - } - - fn function_source_hash(&mut self) -> u64 { - match self.function_source_hash { - Some(hash) => hash, - None => { - let hash = hash_mir_source(self.tcx, self.hir_body); - self.function_source_hash.replace(hash); - hash - } - } - } - - fn inject_counters(&mut self) { - let tcx = self.tcx; - let source_map = tcx.sess.source_map(); - let def_id = self.mir_body.source.def_id(); - let mir_body = &self.mir_body; - let body_span = self.body_span(); - let source_file = source_map.lookup_source_file(body_span.lo()); - let file_name = Symbol::intern(&source_file.name.to_string()); - - debug!("instrumenting {:?}, span: {}", def_id, source_map.span_to_string(body_span)); - - self.dominators.replace(mir_body.dominators()); - self.basic_coverage_blocks.replace(BasicCoverageBlocks::from_mir(mir_body)); - - let coverage_spans = self.coverage_spans(); - - let span_viewables = if pretty::dump_enabled(tcx, self.pass_name, def_id) { - Some(self.span_viewables(&coverage_spans)) - } else { - None - }; - - // Inject a counter for each `CoverageSpan`. There can be multiple `CoverageSpan`s for a - // given BCB, but only one actual counter needs to be incremented per BCB. `bb_counters` - // maps each `bcb_leader_bb` to its `Counter`, when injected. Subsequent `CoverageSpan`s - // for a BCB that already has a `Counter` will inject a `CounterExpression` instead, and - // compute its value by adding `ZERO` to the BCB `Counter` value. - let mut bb_counters = IndexVec::from_elem_n(None, mir_body.basic_blocks().len()); - for CoverageSpan { span, bcb_leader_bb: bb, .. } in coverage_spans { - if let Some(&counter_operand) = bb_counters[bb].as_ref() { - let expression = - self.make_expression(counter_operand, Op::Add, ExpressionOperandId::ZERO); - debug!( - "Injecting counter expression {:?} at: {:?}:\n{}\n==========", - expression, - span, - source_map.span_to_snippet(span).expect("Error getting source for span"), - ); - self.inject_statement(file_name, &source_file, expression, span, bb); - } else { - let counter = self.make_counter(); - debug!( - "Injecting counter {:?} at: {:?}:\n{}\n==========", - counter, - span, - source_map.span_to_snippet(span).expect("Error getting source for span"), - ); - let counter_operand = counter.as_operand_id(); - bb_counters[bb] = Some(counter_operand); - self.inject_statement(file_name, &source_file, counter, span, bb); - } - } - - if let Some(span_viewables) = span_viewables { - let mut file = pretty::create_dump_file( - tcx, - "html", - None, - self.pass_name, - &0, - self.mir_body.source, - ) - .expect("Unexpected error creating MIR spanview HTML file"); - let crate_name = tcx.crate_name(def_id.krate); - let item_name = tcx.def_path(def_id).to_filename_friendly_no_crate(); - let title = format!("{}.{} - Coverage Spans", crate_name, item_name); - spanview::write_document(tcx, def_id, span_viewables, &title, &mut file) - .expect("Unexpected IO error dumping coverage spans as HTML"); - } - } - - fn make_counter(&mut self) -> CoverageKind { - CoverageKind::Counter { - function_source_hash: self.function_source_hash(), - id: self.next_counter(), - } - } - - fn make_expression( - &mut self, - lhs: ExpressionOperandId, - op: Op, - rhs: ExpressionOperandId, - ) -> CoverageKind { - CoverageKind::Expression { id: self.next_expression(), lhs, op, rhs } - } - - fn inject_statement( - &mut self, - file_name: Symbol, - source_file: &Lrc, - coverage_kind: CoverageKind, - span: Span, - block: BasicBlock, - ) { - let code_region = make_code_region(file_name, source_file, span); - debug!(" injecting statement {:?} covering {:?}", coverage_kind, code_region); - - let data = &mut self.mir_body[block]; - let source_info = data.terminator().source_info; - let statement = Statement { - source_info, - kind: StatementKind::Coverage(box Coverage { kind: coverage_kind, code_region }), - }; - data.statements.push(statement); - } - - /// Converts the computed `BasicCoverageBlock`s into `SpanViewable`s. - fn span_viewables(&self, coverage_spans: &Vec) -> Vec { - let tcx = self.tcx; - let mir_body = &self.mir_body; - let mut span_viewables = Vec::new(); - for coverage_span in coverage_spans { - let bcb = self.bcb_from_coverage_span(coverage_span); - let CoverageSpan { span, bcb_leader_bb: bb, coverage_statements, .. } = coverage_span; - let id = bcb.id(); - let mut sorted_coverage_statements = coverage_statements.clone(); - sorted_coverage_statements.sort_unstable_by_key(|covstmt| match *covstmt { - CoverageStatement::Statement(bb, _, index) => (bb, index), - CoverageStatement::Terminator(bb, _) => (bb, usize::MAX), - }); - let tooltip = sorted_coverage_statements - .iter() - .map(|covstmt| covstmt.format(tcx, mir_body)) - .collect::>() - .join("\n"); - span_viewables.push(SpanViewable { bb: *bb, span: *span, id, tooltip }); - } - span_viewables - } - - #[inline(always)] - fn bcb_from_coverage_span(&self, coverage_span: &CoverageSpan) -> &BasicCoverageBlock { - &self.basic_coverage_blocks()[coverage_span.bcb_leader_bb] - } - - #[inline(always)] - fn body_span(&self) -> Span { - self.hir_body.value.span - } - - // Generate a set of `CoverageSpan`s from the filtered set of `Statement`s and `Terminator`s of - // the `BasicBlock`(s) in the given `BasicCoverageBlock`. One `CoverageSpan` is generated for - // each `Statement` and `Terminator`. (Note that subsequent stages of coverage analysis will - // merge some `CoverageSpan`s, at which point a `CoverageSpan` may represent multiple - // `Statement`s and/or `Terminator`s.) - fn extract_spans(&self, bcb: &'a BasicCoverageBlock) -> Vec { - let body_span = self.body_span(); - let mir_basic_blocks = self.mir_body.basic_blocks(); - bcb.blocks - .iter() - .map(|bbref| { - let bb = *bbref; - let data = &mir_basic_blocks[bb]; - data.statements - .iter() - .enumerate() - .filter_map(move |(index, statement)| { - filtered_statement_span(statement, body_span).map(|span| { - CoverageSpan::for_statement(statement, span, bcb, bb, index) - }) - }) - .chain( - filtered_terminator_span(data.terminator(), body_span) - .map(|span| CoverageSpan::for_terminator(span, bcb, bb)), - ) - }) - .flatten() - .collect() - } - - /// Generate a minimal set of `CoverageSpan`s, each representing a contiguous code region to be - /// counted. - /// - /// The basic steps are: - /// - /// 1. Extract an initial set of spans from the `Statement`s and `Terminator`s of each - /// `BasicCoverageBlock`. - /// 2. Sort the spans by span.lo() (starting position). Spans that start at the same position - /// are sorted with longer spans before shorter spans; and equal spans are sorted - /// (deterministically) based on "dominator" relationship (if any). - /// 3. Traverse the spans in sorted order to identify spans that can be dropped (for instance, - /// if another span or spans are already counting the same code region), or should be merged - /// into a broader combined span (because it represents a contiguous, non-branching, and - /// uninterrupted region of source code). - /// - /// Closures are exposed in their enclosing functions as `Assign` `Rvalue`s, and since - /// closures have their own MIR, their `Span` in their enclosing function should be left - /// "uncovered". - /// - /// Note the resulting vector of `CoverageSpan`s does may not be fully sorted (and does not need - /// to be). - fn coverage_spans(&self) -> Vec { - let mut initial_spans = - Vec::::with_capacity(self.mir_body.basic_blocks().len() * 2); - for bcb in self.basic_coverage_blocks().iter() { - for coverage_span in self.extract_spans(bcb) { - initial_spans.push(coverage_span); - } - } - - if initial_spans.is_empty() { - // This can happen if, for example, the function is unreachable (contains only a - // `BasicBlock`(s) with an `Unreachable` terminator). - return initial_spans; - } - - initial_spans.sort_unstable_by(|a, b| { - if a.span.lo() == b.span.lo() { - if a.span.hi() == b.span.hi() { - if a.is_in_same_bcb(b) { - Some(Ordering::Equal) - } else { - // Sort equal spans by dominator relationship, in reverse order (so - // dominators always come after the dominated equal spans). When later - // comparing two spans in order, the first will either dominate the second, - // or they will have no dominator relationship. - self.dominators().rank_partial_cmp(b.bcb_leader_bb, a.bcb_leader_bb) - } - } else { - // Sort hi() in reverse order so shorter spans are attempted after longer spans. - // This guarantees that, if a `prev` span overlaps, and is not equal to, a `curr` - // span, the prev span either extends further left of the curr span, or they - // start at the same position and the prev span extends further right of the end - // of the curr span. - b.span.hi().partial_cmp(&a.span.hi()) - } - } else { - a.span.lo().partial_cmp(&b.span.lo()) - } - .unwrap() - }); - - let refinery = CoverageSpanRefinery::from_sorted_spans(initial_spans, self.dominators()); - refinery.to_refined_spans() - } -} - -struct CoverageSpanRefinery<'a> { - sorted_spans_iter: std::vec::IntoIter, - dominators: &'a Dominators, - some_curr: Option, - curr_original_span: Span, - some_prev: Option, - prev_original_span: Span, - pending_dups: Vec, - refined_spans: Vec, -} - -impl<'a> CoverageSpanRefinery<'a> { - fn from_sorted_spans( - sorted_spans: Vec, - dominators: &'a Dominators, - ) -> Self { - let refined_spans = Vec::with_capacity(sorted_spans.len()); - let mut sorted_spans_iter = sorted_spans.into_iter(); - let prev = sorted_spans_iter.next().expect("at least one span"); - let prev_original_span = prev.span; - Self { - sorted_spans_iter, - dominators, - refined_spans, - some_curr: None, - curr_original_span: Span::with_root_ctxt(BytePos(0), BytePos(0)), - some_prev: Some(prev), - prev_original_span, - pending_dups: Vec::new(), - } - } - - /// Iterate through the sorted `CoverageSpan`s, and return the refined list of merged and - /// de-duplicated `CoverageSpan`s. - fn to_refined_spans(mut self) -> Vec { - while self.next_coverage_span() { - if self.curr().is_mergeable(self.prev()) { - debug!(" same bcb (and neither is a closure), merge with prev={:?}", self.prev()); - let prev = self.take_prev(); - self.curr_mut().merge_from(prev); - // Note that curr.span may now differ from curr_original_span - } else if self.prev_ends_before_curr() { - debug!( - " different bcbs and disjoint spans, so keep curr for next iter, and add \ - prev={:?}", - self.prev() - ); - let prev = self.take_prev(); - self.add_refined_span(prev); - } else if self.prev().is_closure { - // drop any equal or overlapping span (`curr`) and keep `prev` to test again in the - // next iter - debug!( - " curr overlaps a closure (prev). Drop curr and keep prev for next iter. \ - prev={:?}", - self.prev() - ); - self.discard_curr(); - } else if self.curr().is_closure { - self.carve_out_span_for_closure(); - } else if self.prev_original_span == self.curr().span { - self.hold_pending_dups_unless_dominated(); - } else { - self.cutoff_prev_at_overlapping_curr(); - } - } - debug!(" AT END, adding last prev={:?}", self.prev()); - let pending_dups = self.pending_dups.split_off(0); - for dup in pending_dups.into_iter() { - debug!(" ...adding at least one pending dup={:?}", dup); - self.add_refined_span(dup); - } - let prev = self.take_prev(); - self.add_refined_span(prev); - - // FIXME(richkadel): Replace some counters with expressions if they can be calculated based - // on branching. (For example, one branch of a SwitchInt can be computed from the counter - // for the CoverageSpan just prior to the SwitchInt minus the sum of the counters of all - // other branches). - - self.to_refined_spans_without_closures() - } - - fn add_refined_span(&mut self, coverage_span: CoverageSpan) { - self.refined_spans.push(coverage_span); - } - - /// Remove `CoverageSpan`s derived from closures, originally added to ensure the coverage - /// regions for the current function leave room for the closure's own coverage regions - /// (injected separately, from the closure's own MIR). - fn to_refined_spans_without_closures(mut self) -> Vec { - self.refined_spans.retain(|covspan| !covspan.is_closure); - self.refined_spans - } - - fn curr(&self) -> &CoverageSpan { - self.some_curr - .as_ref() - .unwrap_or_else(|| bug!("invalid attempt to unwrap a None some_curr")) - } - - fn curr_mut(&mut self) -> &mut CoverageSpan { - self.some_curr - .as_mut() - .unwrap_or_else(|| bug!("invalid attempt to unwrap a None some_curr")) - } - - fn prev(&self) -> &CoverageSpan { - self.some_prev - .as_ref() - .unwrap_or_else(|| bug!("invalid attempt to unwrap a None some_prev")) - } - - fn prev_mut(&mut self) -> &mut CoverageSpan { - self.some_prev - .as_mut() - .unwrap_or_else(|| bug!("invalid attempt to unwrap a None some_prev")) - } - - fn take_prev(&mut self) -> CoverageSpan { - self.some_prev.take().unwrap_or_else(|| bug!("invalid attempt to unwrap a None some_prev")) - } - - /// If there are `pending_dups` but `prev` is not a matching dup (`prev.span` doesn't match the - /// `pending_dups` spans), then one of the following two things happened during the previous - /// iteration: - /// * the `span` of prev was modified (by `curr_mut().merge_from(prev)`); or - /// * the `span` of prev advanced past the end of the span of pending_dups - /// (`prev().span.hi() <= curr().span.lo()`) - /// In either case, no more spans will match the span of `pending_dups`, so - /// add the `pending_dups` if they don't overlap `curr`, and clear the list. - fn check_pending_dups(&mut self) { - if let Some(dup) = self.pending_dups.last() { - if dup.span != self.prev().span { - debug!( - " SAME spans, but pending_dups are NOT THE SAME, so BCBs matched on \ - previous iteration, or prev started a new disjoint span" - ); - if dup.span.hi() <= self.curr().span.lo() { - let pending_dups = self.pending_dups.split_off(0); - for dup in pending_dups.into_iter() { - debug!(" ...adding at least one pending={:?}", dup); - self.add_refined_span(dup); - } - } else { - self.pending_dups.clear(); - } - } - } - } - - /// Advance `prev` to `curr` (if any), and `curr` to the next `CoverageSpan` in sorted order. - fn next_coverage_span(&mut self) -> bool { - if let Some(curr) = self.some_curr.take() { - self.some_prev = Some(curr); - self.prev_original_span = self.curr_original_span; - } - while let Some(curr) = self.sorted_spans_iter.next() { - debug!("FOR curr={:?}", curr); - if self.prev_starts_after_next(&curr) { - debug!( - " prev.span starts after curr.span, so curr will be dropped (skipping past \ - closure?); prev={:?}", - self.prev() - ); - } else { - // Save a copy of the original span for `curr` in case the `CoverageSpan` is changed - // by `self.curr_mut().merge_from(prev)`. - self.curr_original_span = curr.span; - self.some_curr.replace(curr); - self.check_pending_dups(); - return true; - } - } - false - } - - /// If called, then the next call to `next_coverage_span()` will *not* update `prev` with the - /// `curr` coverage span. - fn discard_curr(&mut self) { - self.some_curr = None; - } - - /// Returns true if the curr span should be skipped because prev has already advanced beyond the - /// end of curr. This can only happen if a prior iteration updated `prev` to skip past a region - /// of code, such as skipping past a closure. - fn prev_starts_after_next(&self, next_curr: &CoverageSpan) -> bool { - self.prev().span.lo() > next_curr.span.lo() - } - - /// Returns true if the curr span starts past the end of the prev span, which means they don't - /// overlap, so we now know the prev can be added to the refined coverage spans. - fn prev_ends_before_curr(&self) -> bool { - self.prev().span.hi() <= self.curr().span.lo() - } - - /// If `prev`s span extends left of the closure (`curr`), carve out the closure's - /// span from `prev`'s span. (The closure's coverage counters will be injected when - /// processing the closure's own MIR.) Add the portion of the span to the left of the - /// closure; and if the span extends to the right of the closure, update `prev` to - /// that portion of the span. For any `pending_dups`, repeat the same process. - fn carve_out_span_for_closure(&mut self) { - let curr_span = self.curr().span; - let left_cutoff = curr_span.lo(); - let right_cutoff = curr_span.hi(); - let has_pre_closure_span = self.prev().span.lo() < right_cutoff; - let has_post_closure_span = self.prev().span.hi() > right_cutoff; - let mut pending_dups = self.pending_dups.split_off(0); - if has_pre_closure_span { - let mut pre_closure = self.prev().clone(); - pre_closure.span = pre_closure.span.with_hi(left_cutoff); - debug!(" prev overlaps a closure. Adding span for pre_closure={:?}", pre_closure); - if !pending_dups.is_empty() { - for mut dup in pending_dups.iter().cloned() { - dup.span = dup.span.with_hi(left_cutoff); - debug!(" ...and at least one pre_closure dup={:?}", dup); - self.add_refined_span(dup); - } - } - self.add_refined_span(pre_closure); - } - if has_post_closure_span { - // Update prev.span to start after the closure (and discard curr) - self.prev_mut().span = self.prev().span.with_lo(right_cutoff); - self.prev_original_span = self.prev().span; - for dup in pending_dups.iter_mut() { - dup.span = dup.span.with_lo(right_cutoff); - } - self.pending_dups.append(&mut pending_dups); - self.discard_curr(); // since self.prev() was already updated - } else { - pending_dups.clear(); - } - } - - /// When two `CoverageSpan`s have the same `Span`, dominated spans can be discarded; but if - /// neither `CoverageSpan` dominates the other, both (or possibly more than two) are held, - /// until their disposition is determined. In this latter case, the `prev` dup is moved into - /// `pending_dups` so the new `curr` dup can be moved to `prev` for the next iteration. - fn hold_pending_dups_unless_dominated(&mut self) { - // equal coverage spans are ordered by dominators before dominated (if any) - debug_assert!(!self.prev().is_dominated_by(self.curr(), self.dominators)); - - if self.curr().is_dominated_by(&self.prev(), self.dominators) { - // If one span dominates the other, assocate the span with the dominator only. - // - // For example: - // match somenum { - // x if x < 1 => { ... } - // }... - // The span for the first `x` is referenced by both the pattern block (every - // time it is evaluated) and the arm code (only when matched). The counter - // will be applied only to the dominator block. - // - // The dominator's (`prev`) execution count may be higher than the dominated - // block's execution count, so drop `curr`. - debug!( - " different bcbs but SAME spans, and prev dominates curr. Drop curr and \ - keep prev for next iter. prev={:?}", - self.prev() - ); - self.discard_curr(); - } else { - // Save `prev` in `pending_dups`. (`curr` will become `prev` in the next iteration.) - // If the `curr` CoverageSpan is later discarded, `pending_dups` can be discarded as - // well; but if `curr` is added to refined_spans, the `pending_dups` will also be added. - debug!( - " different bcbs but SAME spans, and neither dominates, so keep curr for \ - next iter, and, pending upcoming spans (unless overlapping) add prev={:?}", - self.prev() - ); - let prev = self.take_prev(); - self.pending_dups.push(prev); - } - } - - /// `curr` overlaps `prev`. If `prev`s span extends left of `curr`s span, keep _only_ - /// statements that end before `curr.lo()` (if any), and add the portion of the - /// combined span for those statements. Any other statements have overlapping spans - /// that can be ignored because `curr` and/or other upcoming statements/spans inside - /// the overlap area will produce their own counters. This disambiguation process - /// avoids injecting multiple counters for overlapping spans, and the potential for - /// double-counting. - fn cutoff_prev_at_overlapping_curr(&mut self) { - debug!( - " different bcbs, overlapping spans, so ignore/drop pending and only add prev \ - if it has statements that end before curr={:?}", - self.prev() - ); - if self.pending_dups.is_empty() { - let curr_span = self.curr().span; - self.prev_mut().cutoff_statements_at(curr_span.lo()); - if self.prev().coverage_statements.is_empty() { - debug!(" ... no non-overlapping statements to add"); - } else { - debug!(" ... adding modified prev={:?}", self.prev()); - let prev = self.take_prev(); - self.add_refined_span(prev); - } - } else { - // with `pending_dups`, `prev` cannot have any statements that don't overlap - self.pending_dups.clear(); - } - } -} - -fn filtered_statement_span(statement: &'a Statement<'tcx>, body_span: Span) -> Option { - match statement.kind { - // These statements have spans that are often outside the scope of the executed source code - // for their parent `BasicBlock`. - StatementKind::StorageLive(_) - | StatementKind::StorageDead(_) - // Coverage should not be encountered, but don't inject coverage coverage - | StatementKind::Coverage(_) - // Ignore `Nop`s - | StatementKind::Nop => None, - - // FIXME(richkadel): Look into a possible issue assigning the span to a - // FakeReadCause::ForGuardBinding, in this example: - // match somenum { - // x if x < 1 => { ... } - // }... - // The BasicBlock within the match arm code included one of these statements, but the span - // for it covered the `1` in this source. The actual statements have nothing to do with that - // source span: - // FakeRead(ForGuardBinding, _4); - // where `_4` is: - // _4 = &_1; (at the span for the first `x`) - // and `_1` is the `Place` for `somenum`. - // - // The arm code BasicBlock already has its own assignment for `x` itself, `_3 = 1`, and I've - // decided it's reasonable for that span (even though outside the arm code) to be part of - // the counted coverage of the arm code execution, but I can't justify including the literal - // `1` in the arm code. I'm pretty sure that, if the `FakeRead(ForGuardBinding)` has a - // purpose in codegen, it's probably in the right BasicBlock, but if so, the `Statement`s - // `source_info.span` can't be right. - // - // Consider correcting the span assignment, assuming there is a better solution, and see if - // the following pattern can be removed here: - StatementKind::FakeRead(cause, _) if cause == FakeReadCause::ForGuardBinding => None, - - // Retain spans from all other statements - StatementKind::FakeRead(_, _) // Not including `ForGuardBinding` - | StatementKind::CopyNonOverlapping(..) - | StatementKind::Assign(_) - | StatementKind::SetDiscriminant { .. } - | StatementKind::LlvmInlineAsm(_) - | StatementKind::Retag(_, _) - | StatementKind::AscribeUserType(_, _) => { - Some(source_info_span(&statement.source_info, body_span)) - } - } -} - -fn filtered_terminator_span(terminator: &'a Terminator<'tcx>, body_span: Span) -> Option { - match terminator.kind { - // These terminators have spans that don't positively contribute to computing a reasonable - // span of actually executed source code. (For example, SwitchInt terminators extracted from - // an `if condition { block }` has a span that includes the executed block, if true, - // but for coverage, the code region executed, up to *and* through the SwitchInt, - // actually stops before the if's block.) - TerminatorKind::Unreachable // Unreachable blocks are not connected to the CFG - | TerminatorKind::Assert { .. } - | TerminatorKind::Drop { .. } - | TerminatorKind::DropAndReplace { .. } - | TerminatorKind::SwitchInt { .. } - | TerminatorKind::Goto { .. } - // For `FalseEdge`, only the `real` branch is taken, so it is similar to a `Goto`. - | TerminatorKind::FalseEdge { .. } => None, - - // Retain spans from all other terminators - TerminatorKind::Resume - | TerminatorKind::Abort - | TerminatorKind::Return - | TerminatorKind::Call { .. } - | TerminatorKind::Yield { .. } - | TerminatorKind::GeneratorDrop - | TerminatorKind::FalseUnwind { .. } - | TerminatorKind::InlineAsm { .. } => { - Some(source_info_span(&terminator.source_info, body_span)) - } - } -} - -#[inline(always)] -fn source_info_span(source_info: &SourceInfo, body_span: Span) -> Span { - let span = original_sp(source_info.span, body_span).with_ctxt(SyntaxContext::root()); - if body_span.contains(span) { span } else { body_span } -} - -/// Convert the Span into its file name, start line and column, and end line and column -fn make_code_region(file_name: Symbol, source_file: &Lrc, span: Span) -> CodeRegion { - let (start_line, mut start_col) = source_file.lookup_file_pos(span.lo()); - let (end_line, end_col) = if span.hi() == span.lo() { - let (end_line, mut end_col) = (start_line, start_col); - // Extend an empty span by one character so the region will be counted. - let CharPos(char_pos) = start_col; - if char_pos > 0 { - start_col = CharPos(char_pos - 1); - } else { - end_col = CharPos(char_pos + 1); - } - (end_line, end_col) - } else { - source_file.lookup_file_pos(span.hi()) - }; - CodeRegion { - file_name, - start_line: start_line as u32, - start_col: start_col.to_u32() + 1, - end_line: end_line as u32, - end_col: end_col.to_u32() + 1, - } -} - -fn hir_body<'tcx>(tcx: TyCtxt<'tcx>, def_id: DefId) -> &'tcx rustc_hir::Body<'tcx> { - let hir_node = tcx.hir().get_if_local(def_id).expect("expected DefId is local"); - let fn_body_id = hir::map::associated_body(hir_node).expect("HIR node is a function with body"); - tcx.hir().body(fn_body_id) -} - -fn hash_mir_source<'tcx>(tcx: TyCtxt<'tcx>, hir_body: &'tcx rustc_hir::Body<'tcx>) -> u64 { - let mut hcx = tcx.create_no_span_stable_hashing_context(); - hash(&mut hcx, &hir_body.value).to_smaller_hash() -} - -fn hash( - hcx: &mut StableHashingContext<'tcx>, - node: &impl HashStable>, -) -> Fingerprint { - let mut stable_hasher = StableHasher::new(); - node.hash_stable(hcx, &mut stable_hasher); - stable_hasher.finish() -} - -pub struct ShortCircuitPreorder< - 'a, - 'tcx, - F: Fn(&'tcx TerminatorKind<'tcx>) -> mir::Successors<'tcx>, -> { - body: &'a mir::Body<'tcx>, - visited: BitSet, - worklist: Vec, - filtered_successors: F, -} - -impl<'a, 'tcx, F: Fn(&'tcx TerminatorKind<'tcx>) -> mir::Successors<'tcx>> - ShortCircuitPreorder<'a, 'tcx, F> -{ - pub fn new( - body: &'a mir::Body<'tcx>, - filtered_successors: F, - ) -> ShortCircuitPreorder<'a, 'tcx, F> { - let worklist = vec![mir::START_BLOCK]; - - ShortCircuitPreorder { - body, - visited: BitSet::new_empty(body.basic_blocks().len()), - worklist, - filtered_successors, - } - } -} - -impl<'a: 'tcx, 'tcx, F: Fn(&'tcx TerminatorKind<'tcx>) -> mir::Successors<'tcx>> Iterator - for ShortCircuitPreorder<'a, 'tcx, F> -{ - type Item = (BasicBlock, &'a BasicBlockData<'tcx>); - - fn next(&mut self) -> Option<(BasicBlock, &'a BasicBlockData<'tcx>)> { - while let Some(idx) = self.worklist.pop() { - if !self.visited.insert(idx) { - continue; - } - - let data = &self.body[idx]; - - if let Some(ref term) = data.terminator { - self.worklist.extend((self.filtered_successors)(&term.kind)); - } - - return Some((idx, data)); - } - - None - } - - fn size_hint(&self) -> (usize, Option) { - let size = self.body.basic_blocks().len() - self.visited.count(); - (size, Some(size)) - } -} diff --git a/src/tools/clippy/clippy_utils/src/qualify_min_const_fn.rs b/src/tools/clippy/clippy_utils/src/qualify_min_const_fn.rs index aa596bcb4975e..03f1644386532 100644 --- a/src/tools/clippy/clippy_utils/src/qualify_min_const_fn.rs +++ b/src/tools/clippy/clippy_utils/src/qualify_min_const_fn.rs @@ -219,11 +219,11 @@ fn check_statement(tcx: TyCtxt<'tcx>, body: &Body<'tcx>, def_id: DefId, statemen StatementKind::LlvmInlineAsm { .. } => Err((span, "cannot use inline assembly in const fn".into())), StatementKind::CopyNonOverlapping(box rustc_middle::mir::CopyNonOverlapping{ - dst, src, size, + dst, src, count, }) => { check_operand(tcx, dst, span, body)?; check_operand(tcx, src, span, body)?; - check_operand(tcx, size, span, body) + check_operand(tcx, count, span, body) }, // These are all NOPs StatementKind::StorageLive(_) From 8eb6a16e9792d92f70f279da19673984e90eeab5 Mon Sep 17 00:00:00 2001 From: kadmin Date: Sat, 23 Jan 2021 00:23:13 +0000 Subject: [PATCH 08/19] Replace todos with impls Changed to various implementations, copying the style of prior function calls in places I was unsure of. Also one minor style nit. --- .../rustc_codegen_ssa/src/mir/statement.rs | 3 +- .../src/borrow_check/invalidation.rs | 6 ---- compiler/rustc_mir/src/borrow_check/mod.rs | 10 +++++- .../src/borrow_check/type_check/mod.rs | 36 ++++++++++++++++++- 4 files changed, 45 insertions(+), 10 deletions(-) diff --git a/compiler/rustc_codegen_ssa/src/mir/statement.rs b/compiler/rustc_codegen_ssa/src/mir/statement.rs index 774c920ee9666..054273262f79c 100644 --- a/compiler/rustc_codegen_ssa/src/mir/statement.rs +++ b/compiler/rustc_codegen_ssa/src/mir/statement.rs @@ -126,8 +126,7 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> { let count = count_val.immediate_or_packed_pair(&mut bx); let dst = dst_val.immediate_or_packed_pair(&mut bx); let src = src_val.immediate_or_packed_pair(&mut bx); - use crate::MemFlags; - let flags = MemFlags::empty(); + let flags = crate::MemFlags::empty(); bx.memcpy( dst, dst_val.layout.layout.align.pref, diff --git a/compiler/rustc_mir/src/borrow_check/invalidation.rs b/compiler/rustc_mir/src/borrow_check/invalidation.rs index 168c373800044..51c0c811c44d0 100644 --- a/compiler/rustc_mir/src/borrow_check/invalidation.rs +++ b/compiler/rustc_mir/src/borrow_check/invalidation.rs @@ -100,12 +100,6 @@ impl<'cx, 'tcx> Visitor<'tcx> for InvalidationGenerator<'cx, 'tcx> { self.consume_operand(location, src); self.consume_operand(location, dst); self.consume_operand(location, count); - match dst { - Operand::Move(ref place) | Operand::Copy(ref place) => { - self.mutate_place(location, *place, Deep, JustWrite); - } - _ => {} - } } StatementKind::Nop | StatementKind::Coverage(..) diff --git a/compiler/rustc_mir/src/borrow_check/mod.rs b/compiler/rustc_mir/src/borrow_check/mod.rs index cdff4715fc1ab..e7e936cb97e53 100644 --- a/compiler/rustc_mir/src/borrow_check/mod.rs +++ b/compiler/rustc_mir/src/borrow_check/mod.rs @@ -627,7 +627,15 @@ impl<'cx, 'tcx> dataflow::ResultsVisitor<'cx, 'tcx> for MirBorrowckCtxt<'cx, 'tc } } - StatementKind::CopyNonOverlapping(..) => todo!(), + StatementKind::CopyNonOverlapping(box rustc_middle::mir::CopyNonOverlapping { + src, + dst, + count, + }) => { + self.consume_operand(location, (src, span), flow_state); + self.consume_operand(location, (dst, span), flow_state); + self.consume_operand(location, (count, span), flow_state); + } StatementKind::Nop | StatementKind::Coverage(..) | StatementKind::AscribeUserType(..) diff --git a/compiler/rustc_mir/src/borrow_check/type_check/mod.rs b/compiler/rustc_mir/src/borrow_check/type_check/mod.rs index f34a12099d72f..9bdd0b0247900 100644 --- a/compiler/rustc_mir/src/borrow_check/type_check/mod.rs +++ b/compiler/rustc_mir/src/borrow_check/type_check/mod.rs @@ -1520,7 +1520,41 @@ impl<'a, 'tcx> TypeChecker<'a, 'tcx> { ); } } - StatementKind::CopyNonOverlapping(..) => todo!(), + StatementKind::CopyNonOverlapping(box rustc_middle::mir::CopyNonOverlapping { + ref src, + ref dst, + ref count, + }) => { + let op_src_ty = self.normalize(src.ty(body, self.tcx()), location); + let op_dst_ty = self.normalize(dst.ty(body, self.tcx()), location); + // since CopyNonOverlapping is parametrized by 1 type, + // we only need to check that they are equal and not keep an extra parameter. + if let Err(terr) = self.eq_types( + op_src_ty, + op_dst_ty, + location.to_locations(), + ConstraintCategory::Internal, + ) { + span_mirbug!( + self, + stmt, + "bad arg ({:?} != {:?}): {:?}", + op_src_ty, + op_dst_ty, + terr + ); + } + + let op_cnt_ty = self.normalize(count.ty(body, self.tcx()), location); + if let Err(terr) = self.eq_types( + op_cnt_ty, + tcx.types.usize, + location.to_locations(), + ConstraintCategory::Internal, + ) { + span_mirbug!(self, stmt, "bad arg ({:?} != usize): {:?}", op_cnt_ty, terr); + } + } StatementKind::FakeRead(..) | StatementKind::StorageLive(..) | StatementKind::StorageDead(..) From ed665d29a2894ccaa39ec042f341e5bc5545c8c2 Mon Sep 17 00:00:00 2001 From: kadmin Date: Sat, 23 Jan 2021 02:28:08 +0000 Subject: [PATCH 09/19] Change CopyNonOverlapping::codegen_ssa Fixes copy_non_overlapping codegen_ssa to properly handle pointees, and use bytes instead of elem count --- .../rustc_codegen_ssa/src/mir/statement.rs | 28 ++++++++++--------- 1 file changed, 15 insertions(+), 13 deletions(-) diff --git a/compiler/rustc_codegen_ssa/src/mir/statement.rs b/compiler/rustc_codegen_ssa/src/mir/statement.rs index 054273262f79c..e11539c14760c 100644 --- a/compiler/rustc_codegen_ssa/src/mir/statement.rs +++ b/compiler/rustc_codegen_ssa/src/mir/statement.rs @@ -122,19 +122,21 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> { }) => { let dst_val = self.codegen_operand(&mut bx, dst); let src_val = self.codegen_operand(&mut bx, src); - let count_val = self.codegen_operand(&mut bx, count); - let count = count_val.immediate_or_packed_pair(&mut bx); - let dst = dst_val.immediate_or_packed_pair(&mut bx); - let src = src_val.immediate_or_packed_pair(&mut bx); - let flags = crate::MemFlags::empty(); - bx.memcpy( - dst, - dst_val.layout.layout.align.pref, - src, - src_val.layout.layout.align.pref, - count, - flags, - ); + let count = self.codegen_operand(&mut bx, count).immediate(); + let get_val_align = |oper_ref: crate::mir::OperandRef<'_, _>| match oper_ref.val { + OperandValue::Ref(val, _, align) => (val, align), + _ => unreachable!(), + }; + let pointee_layout = dst_val + .layout + .pointee_info_at(&mut bx, rustc_target::abi::Size::ZERO) + .expect("Expected pointer"); + let elem_size = bx.const_u64(pointee_layout.size.bytes()); + let byte_count = bx.mul(count, elem_size); + + let (dst, dst_align) = get_val_align(dst_val); + let (src, src_align) = get_val_align(src_val); + bx.memcpy(dst, dst_align, src, src_align, byte_count, crate::MemFlags::empty()); bx } mir::StatementKind::FakeRead(..) From 250b7b77491232160419e6f7e25e7c9ff45bfeee Mon Sep 17 00:00:00 2001 From: kadmin Date: Sat, 23 Jan 2021 03:55:41 +0000 Subject: [PATCH 10/19] Build StKind::CopyOverlapping This replaces where it was previously being constructed in intrinsics, with direct construction of the Statement. --- compiler/rustc_codegen_cranelift/src/base.rs | 15 +- compiler/rustc_codegen_ssa/src/lib.rs | 1 + compiler/rustc_codegen_ssa/src/mir/block.rs | 130 ++++++++++-------- .../rustc_codegen_ssa/src/mir/intrinsic.rs | 13 +- .../rustc_codegen_ssa/src/mir/statement.rs | 14 +- 5 files changed, 98 insertions(+), 75 deletions(-) diff --git a/compiler/rustc_codegen_cranelift/src/base.rs b/compiler/rustc_codegen_cranelift/src/base.rs index 94f57f9a4e19e..a9f0528006c9c 100644 --- a/compiler/rustc_codegen_cranelift/src/base.rs +++ b/compiler/rustc_codegen_cranelift/src/base.rs @@ -924,10 +924,21 @@ fn codegen_stmt<'tcx>( dst, count, }) => { - let dst = codegen_operand(fx, dst).load_scalar(fx); + let dst = codegen_operand(fx, dst); + let pointee = dst + .layout() + .pointee_info_at(fx, rustc_target::abi::Size::ZERO) + .expect("Expected pointer"); + let dst = dst.load_scalar(fx); let src = codegen_operand(fx, src).load_scalar(fx); let count = codegen_operand(fx, count).load_scalar(fx); - fx.bcx.call_memcpy(fx.cx.module.target_config(), dst, src, count); + let elem_size: u64 = pointee.size.bytes(); + let bytes = if elem_size != 1 { + fx.bcx.ins().imul_imm(count, elem_size as i64) + } else { + count + }; + fx.bcx.call_memcpy(fx.cx.module.target_config(), dst, src, bytes); } } } diff --git a/compiler/rustc_codegen_ssa/src/lib.rs b/compiler/rustc_codegen_ssa/src/lib.rs index 0307117e1c8b2..2c2330409fd70 100644 --- a/compiler/rustc_codegen_ssa/src/lib.rs +++ b/compiler/rustc_codegen_ssa/src/lib.rs @@ -9,6 +9,7 @@ #![feature(or_patterns)] #![feature(associated_type_bounds)] #![recursion_limit = "256"] +#![feature(box_syntax)] //! This crate contains codegen code that is used by all codegen backends (LLVM and others). //! The backend-agnostic functions of this crate use functions defined in various traits that diff --git a/compiler/rustc_codegen_ssa/src/mir/block.rs b/compiler/rustc_codegen_ssa/src/mir/block.rs index 9ce9066980066..1150d4d734870 100644 --- a/compiler/rustc_codegen_ssa/src/mir/block.rs +++ b/compiler/rustc_codegen_ssa/src/mir/block.rs @@ -641,67 +641,89 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> { return; } - if intrinsic.is_some() && intrinsic != Some(sym::drop_in_place) { - let intrinsic = intrinsic.unwrap(); - let dest = match ret_dest { - _ if fn_abi.ret.is_indirect() => llargs[0], - ReturnDest::Nothing => { - bx.const_undef(bx.type_ptr_to(bx.arg_memory_ty(&fn_abi.ret))) - } - ReturnDest::IndirectOperand(dst, _) | ReturnDest::Store(dst) => dst.llval, - ReturnDest::DirectOperand(_) => { - bug!("Cannot use direct operand with an intrinsic call") - } - }; + match intrinsic { + None | Some(sym::drop_in_place) => {} + Some(sym::copy_nonoverlapping) => { + bx = self.codegen_statement( + bx, + &rustc_middle::mir::Statement { + source_info: rustc_middle::mir::SourceInfo::outermost(span), + kind: rustc_middle::mir::StatementKind::CopyNonOverlapping( + box rustc_middle::mir::CopyNonOverlapping { + src: args[0].clone(), + dst: args[1].clone(), + count: args[2].clone(), + }, + ), + }, + ); + helper.funclet_br(self, &mut bx, destination.unwrap().1); + return; + } + Some(intrinsic) => { + let dest = match ret_dest { + _ if fn_abi.ret.is_indirect() => llargs[0], + ReturnDest::Nothing => { + bx.const_undef(bx.type_ptr_to(bx.arg_memory_ty(&fn_abi.ret))) + } + ReturnDest::IndirectOperand(dst, _) | ReturnDest::Store(dst) => dst.llval, + ReturnDest::DirectOperand(_) => { + bug!("Cannot use direct operand with an intrinsic call") + } + }; - let args: Vec<_> = args - .iter() - .enumerate() - .map(|(i, arg)| { - // The indices passed to simd_shuffle* in the - // third argument must be constant. This is - // checked by const-qualification, which also - // promotes any complex rvalues to constants. - if i == 2 && intrinsic.as_str().starts_with("simd_shuffle") { - if let mir::Operand::Constant(constant) = arg { - let c = self.eval_mir_constant(constant); - let (llval, ty) = self.simd_shuffle_indices( - &bx, - constant.span, - constant.literal.ty, - c, - ); - return OperandRef { val: Immediate(llval), layout: bx.layout_of(ty) }; - } else { - span_bug!(span, "shuffle indices must be constant"); + let args: Vec<_> = args + .iter() + .enumerate() + .map(|(i, arg)| { + // The indices passed to simd_shuffle* in the + // third argument must be constant. This is + // checked by const-qualification, which also + // promotes any complex rvalues to constants. + if i == 2 && intrinsic.as_str().starts_with("simd_shuffle") { + if let mir::Operand::Constant(constant) = arg { + let c = self.eval_mir_constant(constant); + let (llval, ty) = self.simd_shuffle_indices( + &bx, + constant.span, + constant.literal.ty, + c, + ); + return OperandRef { + val: Immediate(llval), + layout: bx.layout_of(ty), + }; + } else { + span_bug!(span, "shuffle indices must be constant"); + } } - } - self.codegen_operand(&mut bx, arg) - }) - .collect(); + self.codegen_operand(&mut bx, arg) + }) + .collect(); + + self.codegen_intrinsic_call( + &mut bx, + *instance.as_ref().unwrap(), + &fn_abi, + &args, + dest, + span, + ); - Self::codegen_intrinsic_call( - &mut bx, - *instance.as_ref().unwrap(), - &fn_abi, - &args, - dest, - span, - ); + if let ReturnDest::IndirectOperand(dst, _) = ret_dest { + self.store_return(&mut bx, ret_dest, &fn_abi.ret, dst.llval); + } - if let ReturnDest::IndirectOperand(dst, _) = ret_dest { - self.store_return(&mut bx, ret_dest, &fn_abi.ret, dst.llval); - } + if let Some((_, target)) = *destination { + helper.maybe_sideeffect(self.mir, &mut bx, &[target]); + helper.funclet_br(self, &mut bx, target); + } else { + bx.unreachable(); + } - if let Some((_, target)) = *destination { - helper.maybe_sideeffect(self.mir, &mut bx, &[target]); - helper.funclet_br(self, &mut bx, target); - } else { - bx.unreachable(); + return; } - - return; } // Split the rust-call tupled arguments off. diff --git a/compiler/rustc_codegen_ssa/src/mir/intrinsic.rs b/compiler/rustc_codegen_ssa/src/mir/intrinsic.rs index 80e3ed75b8585..00fc5b6606151 100644 --- a/compiler/rustc_codegen_ssa/src/mir/intrinsic.rs +++ b/compiler/rustc_codegen_ssa/src/mir/intrinsic.rs @@ -49,6 +49,7 @@ fn memset_intrinsic<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>>( impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> { pub fn codegen_intrinsic_call( + &self, bx: &mut Bx, instance: ty::Instance<'tcx>, fn_abi: &FnAbi<'tcx, Ty<'tcx>>, @@ -127,16 +128,8 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> { } sym::copy_nonoverlapping => { - copy_intrinsic( - bx, - false, - false, - substs.type_at(0), - args[1].immediate(), - args[0].immediate(), - args[2].immediate(), - ); - return; + // handled explicitly in compiler/rustc_codegen_ssa/src/mir/block.rs + unreachable!(); } sym::copy => { copy_intrinsic( diff --git a/compiler/rustc_codegen_ssa/src/mir/statement.rs b/compiler/rustc_codegen_ssa/src/mir/statement.rs index e11539c14760c..5523e5f2e8604 100644 --- a/compiler/rustc_codegen_ssa/src/mir/statement.rs +++ b/compiler/rustc_codegen_ssa/src/mir/statement.rs @@ -123,20 +123,16 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> { let dst_val = self.codegen_operand(&mut bx, dst); let src_val = self.codegen_operand(&mut bx, src); let count = self.codegen_operand(&mut bx, count).immediate(); - let get_val_align = |oper_ref: crate::mir::OperandRef<'_, _>| match oper_ref.val { - OperandValue::Ref(val, _, align) => (val, align), - _ => unreachable!(), - }; let pointee_layout = dst_val .layout .pointee_info_at(&mut bx, rustc_target::abi::Size::ZERO) .expect("Expected pointer"); - let elem_size = bx.const_u64(pointee_layout.size.bytes()); - let byte_count = bx.mul(count, elem_size); + let bytes = bx.mul(count, bx.const_usize(pointee_layout.size.bytes())); - let (dst, dst_align) = get_val_align(dst_val); - let (src, src_align) = get_val_align(src_val); - bx.memcpy(dst, dst_align, src, src_align, byte_count, crate::MemFlags::empty()); + let align = pointee_layout.align; + let dst = dst_val.immediate(); + let src = src_val.immediate(); + bx.memcpy(dst, align, src, align, bytes, crate::MemFlags::empty()); bx } mir::StatementKind::FakeRead(..) From aff18fa9882c55559d36435175fea5879135eabb Mon Sep 17 00:00:00 2001 From: kadmin Date: Sat, 23 Jan 2021 08:57:04 +0000 Subject: [PATCH 11/19] Switch to changing cp_non_overlap in tform It was suggested to lower this in MIR instead of ssa, so do that instead. --- compiler/rustc_codegen_ssa/src/mir/block.rs | 20 +------ .../rustc_codegen_ssa/src/mir/intrinsic.rs | 6 -- compiler/rustc_middle/src/mir/mod.rs | 2 +- compiler/rustc_mir/src/borrow_check/mod.rs | 6 ++ .../rustc_mir/src/interpret/intrinsics.rs | 12 ++-- compiler/rustc_mir/src/interpret/step.rs | 56 ++++++++++--------- .../rustc_mir/src/transform/check_unsafety.rs | 2 +- .../src/transform/lower_intrinsics.rs | 21 +++++++ .../src/transform/remove_noop_landing_pads.rs | 2 +- .../clippy_utils/src/qualify_min_const_fn.rs | 2 +- 10 files changed, 67 insertions(+), 62 deletions(-) diff --git a/compiler/rustc_codegen_ssa/src/mir/block.rs b/compiler/rustc_codegen_ssa/src/mir/block.rs index 1150d4d734870..e148ed7ad3bce 100644 --- a/compiler/rustc_codegen_ssa/src/mir/block.rs +++ b/compiler/rustc_codegen_ssa/src/mir/block.rs @@ -643,23 +643,7 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> { match intrinsic { None | Some(sym::drop_in_place) => {} - Some(sym::copy_nonoverlapping) => { - bx = self.codegen_statement( - bx, - &rustc_middle::mir::Statement { - source_info: rustc_middle::mir::SourceInfo::outermost(span), - kind: rustc_middle::mir::StatementKind::CopyNonOverlapping( - box rustc_middle::mir::CopyNonOverlapping { - src: args[0].clone(), - dst: args[1].clone(), - count: args[2].clone(), - }, - ), - }, - ); - helper.funclet_br(self, &mut bx, destination.unwrap().1); - return; - } + Some(sym::copy_nonoverlapping) => unreachable!(), Some(intrinsic) => { let dest = match ret_dest { _ if fn_abi.ret.is_indirect() => llargs[0], @@ -702,7 +686,7 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> { }) .collect(); - self.codegen_intrinsic_call( + Self::codegen_intrinsic_call( &mut bx, *instance.as_ref().unwrap(), &fn_abi, diff --git a/compiler/rustc_codegen_ssa/src/mir/intrinsic.rs b/compiler/rustc_codegen_ssa/src/mir/intrinsic.rs index 00fc5b6606151..8502309b90e5a 100644 --- a/compiler/rustc_codegen_ssa/src/mir/intrinsic.rs +++ b/compiler/rustc_codegen_ssa/src/mir/intrinsic.rs @@ -49,7 +49,6 @@ fn memset_intrinsic<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>>( impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> { pub fn codegen_intrinsic_call( - &self, bx: &mut Bx, instance: ty::Instance<'tcx>, fn_abi: &FnAbi<'tcx, Ty<'tcx>>, @@ -126,11 +125,6 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> { let offset = args[1].immediate(); bx.gep(ptr, &[offset]) } - - sym::copy_nonoverlapping => { - // handled explicitly in compiler/rustc_codegen_ssa/src/mir/block.rs - unreachable!(); - } sym::copy => { copy_intrinsic( bx, diff --git a/compiler/rustc_middle/src/mir/mod.rs b/compiler/rustc_middle/src/mir/mod.rs index 20393ce46602c..ec8d7f8a55bbe 100644 --- a/compiler/rustc_middle/src/mir/mod.rs +++ b/compiler/rustc_middle/src/mir/mod.rs @@ -1656,7 +1656,7 @@ pub struct Coverage { pub code_region: Option, } -#[derive(Clone, Debug, PartialEq, TyEncodable, TyDecodable, HashStable, TypeFoldable)] +#[derive(Clone, Debug, PartialEq, TyEncodable, TyDecodable, Hash, HashStable, TypeFoldable)] pub struct CopyNonOverlapping<'tcx> { pub src: Operand<'tcx>, pub dst: Operand<'tcx>, diff --git a/compiler/rustc_mir/src/borrow_check/mod.rs b/compiler/rustc_mir/src/borrow_check/mod.rs index e7e936cb97e53..254ced49e2f3e 100644 --- a/compiler/rustc_mir/src/borrow_check/mod.rs +++ b/compiler/rustc_mir/src/borrow_check/mod.rs @@ -628,13 +628,19 @@ impl<'cx, 'tcx> dataflow::ResultsVisitor<'cx, 'tcx> for MirBorrowckCtxt<'cx, 'tc } StatementKind::CopyNonOverlapping(box rustc_middle::mir::CopyNonOverlapping { + .. + /* src, dst, count, + */ }) => { + unreachable!() + /* self.consume_operand(location, (src, span), flow_state); self.consume_operand(location, (dst, span), flow_state); self.consume_operand(location, (count, span), flow_state); + */ } StatementKind::Nop | StatementKind::Coverage(..) diff --git a/compiler/rustc_mir/src/interpret/intrinsics.rs b/compiler/rustc_mir/src/interpret/intrinsics.rs index d36b3a7d9b56e..b0a2d906519ac 100644 --- a/compiler/rustc_mir/src/interpret/intrinsics.rs +++ b/compiler/rustc_mir/src/interpret/intrinsics.rs @@ -323,7 +323,10 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> { let result = Scalar::from_uint(truncated_bits, layout.size); self.write_scalar(result, dest)?; } - sym::copy | sym::copy_nonoverlapping => { + sym::copy_nonoverlapping => { + self.copy_nonoverlapping(args[0], args[1], args[2])?; + } + sym::copy => { let elem_ty = instance.substs.type_at(0); let elem_layout = self.layout_of(elem_ty)?; let count = self.read_scalar(&args[2])?.to_machine_usize(self)?; @@ -338,12 +341,7 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> { let dest = self.memory.check_ptr_access(dest, size, elem_align)?; if let (Some(src), Some(dest)) = (src, dest) { - self.memory.copy( - src, - dest, - size, - intrinsic_name == sym::copy_nonoverlapping, - )?; + self.memory.copy(src, dest, size, false)?; } } sym::offset => { diff --git a/compiler/rustc_mir/src/interpret/step.rs b/compiler/rustc_mir/src/interpret/step.rs index eda9a4ac89026..64f6ec4832ec7 100644 --- a/compiler/rustc_mir/src/interpret/step.rs +++ b/compiler/rustc_mir/src/interpret/step.rs @@ -2,6 +2,7 @@ //! //! The main entry point is the `step` method. +use crate::interpret::OpTy; use rustc_middle::mir; use rustc_middle::mir::interpret::{InterpResult, Scalar}; use rustc_target::abi::LayoutOf; @@ -115,35 +116,11 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> { // Call CopyNonOverlapping CopyNonOverlapping(box rustc_middle::mir::CopyNonOverlapping { dst, src, count }) => { - let (src, size) = { - let src = self.eval_operand(src, None)?; - let size = src.layout.layout.size; - let mplace = *src.assert_mem_place(self); - let ptr = match mplace.ptr { - Scalar::Ptr(ptr) => ptr, - _ => panic!(), - }; - (ptr, size) - }; - - let dst = { - let dst = self.eval_operand(dst, None)?; - let mplace = *dst.assert_mem_place(self); - match mplace.ptr { - Scalar::Ptr(ptr) => ptr, - _ => panic!(), - } - }; - let count = self.eval_operand(count, None)?; - let count = self.read_immediate(count)?.to_scalar()?; - let count = if let Scalar::Int(i) = count { - core::convert::TryFrom::try_from(i).unwrap() - } else { - panic!(); - }; - self.memory.copy_repeatedly(src, dst, size, count, /*nonoverlapping*/ true)?; + let src = self.eval_operand(src, None)?; + let dst = self.eval_operand(dst, None)?; + self.copy_nonoverlapping(src, dst, count)?; } // Statements we do not track. @@ -173,6 +150,31 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> { Ok(()) } + pub(crate) fn copy_nonoverlapping( + &mut self, + src: OpTy<'tcx, >::PointerTag>, + dst: OpTy<'tcx, >::PointerTag>, + count: OpTy<'tcx, >::PointerTag>, + ) -> InterpResult<'tcx> { + let count = self.read_scalar(&count)?.to_machine_usize(self)?; + let layout = self.layout_of(src.layout.ty.builtin_deref(true).unwrap().ty)?; + let (size, align) = (layout.size, layout.align.abi); + let src = + self.memory.check_ptr_access(self.read_scalar(&src)?.check_init()?, size, align)?; + + let dst = + self.memory.check_ptr_access(self.read_scalar(&dst)?.check_init()?, size, align)?; + + let size = size.checked_mul(count, self).ok_or_else(|| { + err_ub_format!("overflow computing total size of `copy_nonoverlapping`") + })?; + + if let (Some(src), Some(dst)) = (src, dst) { + self.memory.copy(src, dst, size, /*nonoverlapping*/ true)?; + } + Ok(()) + } + /// Evaluate an assignment statement. /// /// There is no separate `eval_rvalue` function. Instead, the code for handling each rvalue diff --git a/compiler/rustc_mir/src/transform/check_unsafety.rs b/compiler/rustc_mir/src/transform/check_unsafety.rs index e7fdd5496cb40..33848bc130581 100644 --- a/compiler/rustc_mir/src/transform/check_unsafety.rs +++ b/compiler/rustc_mir/src/transform/check_unsafety.rs @@ -115,7 +115,6 @@ impl<'a, 'tcx> Visitor<'tcx> for UnsafetyChecker<'a, 'tcx> { | StatementKind::Retag { .. } | StatementKind::AscribeUserType(..) | StatementKind::Coverage(..) - | StatementKind::CopyNonOverlapping(..) | StatementKind::Nop => { // safe (at least as emitted during MIR construction) } @@ -124,6 +123,7 @@ impl<'a, 'tcx> Visitor<'tcx> for UnsafetyChecker<'a, 'tcx> { UnsafetyViolationKind::General, UnsafetyViolationDetails::UseOfInlineAssembly, ), + StatementKind::CopyNonOverlapping(..) => unreachable!(), } self.super_statement(statement, location); } diff --git a/compiler/rustc_mir/src/transform/lower_intrinsics.rs b/compiler/rustc_mir/src/transform/lower_intrinsics.rs index f5968532eb396..9749b31552d9a 100644 --- a/compiler/rustc_mir/src/transform/lower_intrinsics.rs +++ b/compiler/rustc_mir/src/transform/lower_intrinsics.rs @@ -40,6 +40,27 @@ impl<'tcx> MirPass<'tcx> for LowerIntrinsics { terminator.kind = TerminatorKind::Goto { target }; } } + sym::copy_nonoverlapping => { + let target = destination.unwrap().1; + let mut args = args.drain(..); + block.statements.push(Statement { + source_info: terminator.source_info, + kind: StatementKind::CopyNonOverlapping( + box rustc_middle::mir::CopyNonOverlapping { + src: args.next().unwrap(), + dst: args.next().unwrap(), + count: args.next().unwrap(), + }, + ), + }); + assert_eq!( + args.next(), + None, + "Extra argument for copy_non_overlapping intrinsic" + ); + drop(args); + terminator.kind = TerminatorKind::Goto { target }; + } sym::wrapping_add | sym::wrapping_sub | sym::wrapping_mul => { if let Some((destination, target)) = *destination { let lhs; diff --git a/compiler/rustc_mir/src/transform/remove_noop_landing_pads.rs b/compiler/rustc_mir/src/transform/remove_noop_landing_pads.rs index a37f5d4f329f3..5347846a4b334 100644 --- a/compiler/rustc_mir/src/transform/remove_noop_landing_pads.rs +++ b/compiler/rustc_mir/src/transform/remove_noop_landing_pads.rs @@ -39,7 +39,6 @@ impl RemoveNoopLandingPads { | StatementKind::StorageDead(_) | StatementKind::AscribeUserType(..) | StatementKind::Coverage(..) - | StatementKind::CopyNonOverlapping(..) | StatementKind::Nop => { // These are all nops in a landing pad } @@ -56,6 +55,7 @@ impl RemoveNoopLandingPads { StatementKind::Assign { .. } | StatementKind::SetDiscriminant { .. } | StatementKind::LlvmInlineAsm { .. } + | StatementKind::CopyNonOverlapping(..) | StatementKind::Retag { .. } => { return false; } diff --git a/src/tools/clippy/clippy_utils/src/qualify_min_const_fn.rs b/src/tools/clippy/clippy_utils/src/qualify_min_const_fn.rs index 03f1644386532..d401e25e316db 100644 --- a/src/tools/clippy/clippy_utils/src/qualify_min_const_fn.rs +++ b/src/tools/clippy/clippy_utils/src/qualify_min_const_fn.rs @@ -224,7 +224,7 @@ fn check_statement(tcx: TyCtxt<'tcx>, body: &Body<'tcx>, def_id: DefId, statemen check_operand(tcx, dst, span, body)?; check_operand(tcx, src, span, body)?; check_operand(tcx, count, span, body) - }, + } // These are all NOPs StatementKind::StorageLive(_) | StatementKind::StorageDead(_) From 1dda668a775515e5cf46a2edb8066e782dee3ac4 Mon Sep 17 00:00:00 2001 From: kadmin Date: Fri, 26 Feb 2021 16:42:51 +0000 Subject: [PATCH 12/19] Clean up todos Also add some span_bugs where it is unreachable --- compiler/rustc_mir/src/borrow_check/mod.rs | 15 ++----- .../src/borrow_check/type_check/mod.rs | 39 +++------------- .../rustc_mir/src/dataflow/impls/borrows.rs | 3 +- .../src/dataflow/impls/storage_liveness.rs | 3 +- .../src/dataflow/move_paths/builder.rs | 3 +- .../rustc_mir/src/interpret/intrinsics.rs | 3 -- compiler/rustc_mir/src/transform/validate.rs | 44 ++++++++++++++++++- 7 files changed, 55 insertions(+), 55 deletions(-) diff --git a/compiler/rustc_mir/src/borrow_check/mod.rs b/compiler/rustc_mir/src/borrow_check/mod.rs index 254ced49e2f3e..73d5a357086a9 100644 --- a/compiler/rustc_mir/src/borrow_check/mod.rs +++ b/compiler/rustc_mir/src/borrow_check/mod.rs @@ -629,18 +629,11 @@ impl<'cx, 'tcx> dataflow::ResultsVisitor<'cx, 'tcx> for MirBorrowckCtxt<'cx, 'tc StatementKind::CopyNonOverlapping(box rustc_middle::mir::CopyNonOverlapping { .. - /* - src, - dst, - count, - */ }) => { - unreachable!() - /* - self.consume_operand(location, (src, span), flow_state); - self.consume_operand(location, (dst, span), flow_state); - self.consume_operand(location, (count, span), flow_state); - */ + span_bug!( + span, + "Unexpected CopyNonOverlapping, should only appear after lower_intrinsics", + ) } StatementKind::Nop | StatementKind::Coverage(..) diff --git a/compiler/rustc_mir/src/borrow_check/type_check/mod.rs b/compiler/rustc_mir/src/borrow_check/type_check/mod.rs index 9bdd0b0247900..bc41ac1e5fe0d 100644 --- a/compiler/rustc_mir/src/borrow_check/type_check/mod.rs +++ b/compiler/rustc_mir/src/borrow_check/type_check/mod.rs @@ -1521,40 +1521,11 @@ impl<'a, 'tcx> TypeChecker<'a, 'tcx> { } } StatementKind::CopyNonOverlapping(box rustc_middle::mir::CopyNonOverlapping { - ref src, - ref dst, - ref count, - }) => { - let op_src_ty = self.normalize(src.ty(body, self.tcx()), location); - let op_dst_ty = self.normalize(dst.ty(body, self.tcx()), location); - // since CopyNonOverlapping is parametrized by 1 type, - // we only need to check that they are equal and not keep an extra parameter. - if let Err(terr) = self.eq_types( - op_src_ty, - op_dst_ty, - location.to_locations(), - ConstraintCategory::Internal, - ) { - span_mirbug!( - self, - stmt, - "bad arg ({:?} != {:?}): {:?}", - op_src_ty, - op_dst_ty, - terr - ); - } - - let op_cnt_ty = self.normalize(count.ty(body, self.tcx()), location); - if let Err(terr) = self.eq_types( - op_cnt_ty, - tcx.types.usize, - location.to_locations(), - ConstraintCategory::Internal, - ) { - span_mirbug!(self, stmt, "bad arg ({:?} != usize): {:?}", op_cnt_ty, terr); - } - } + .. + }) => span_bug!( + stmt.source_info.span, + "Unexpected StatementKind::CopyNonOverlapping, should only appear after lowering_intrinsics", + ), StatementKind::FakeRead(..) | StatementKind::StorageLive(..) | StatementKind::StorageDead(..) diff --git a/compiler/rustc_mir/src/dataflow/impls/borrows.rs b/compiler/rustc_mir/src/dataflow/impls/borrows.rs index ffa02f855c979..f24d0f0266d9f 100644 --- a/compiler/rustc_mir/src/dataflow/impls/borrows.rs +++ b/compiler/rustc_mir/src/dataflow/impls/borrows.rs @@ -305,9 +305,8 @@ impl<'tcx> dataflow::GenKillAnalysis<'tcx> for Borrows<'_, 'tcx> { | mir::StatementKind::Retag { .. } | mir::StatementKind::AscribeUserType(..) | mir::StatementKind::Coverage(..) + | mir::StatementKind::CopyNonOverlapping(..) | mir::StatementKind::Nop => {} - - mir::StatementKind::CopyNonOverlapping(..) => todo!(), } } diff --git a/compiler/rustc_mir/src/dataflow/impls/storage_liveness.rs b/compiler/rustc_mir/src/dataflow/impls/storage_liveness.rs index e407f394c51ec..792664597fd9a 100644 --- a/compiler/rustc_mir/src/dataflow/impls/storage_liveness.rs +++ b/compiler/rustc_mir/src/dataflow/impls/storage_liveness.rs @@ -149,9 +149,8 @@ impl<'mir, 'tcx> dataflow::GenKillAnalysis<'tcx> for MaybeRequiresStorage<'mir, | StatementKind::FakeRead(..) | StatementKind::Nop | StatementKind::Retag(..) + | StatementKind::CopyNonOverlapping(..) | StatementKind::StorageLive(..) => {} - - StatementKind::CopyNonOverlapping(..) => todo!(), } } diff --git a/compiler/rustc_mir/src/dataflow/move_paths/builder.rs b/compiler/rustc_mir/src/dataflow/move_paths/builder.rs index aaf78046a46a0..03094f7bf9a8e 100644 --- a/compiler/rustc_mir/src/dataflow/move_paths/builder.rs +++ b/compiler/rustc_mir/src/dataflow/move_paths/builder.rs @@ -318,9 +318,8 @@ impl<'b, 'a, 'tcx> Gatherer<'b, 'a, 'tcx> { StatementKind::Retag { .. } | StatementKind::AscribeUserType(..) | StatementKind::Coverage(..) + | StatementKind::CopyNonOverlapping(..) | StatementKind::Nop => {} - - StatementKind::CopyNonOverlapping(..) => todo!(), } } diff --git a/compiler/rustc_mir/src/interpret/intrinsics.rs b/compiler/rustc_mir/src/interpret/intrinsics.rs index b0a2d906519ac..93979b8558f7a 100644 --- a/compiler/rustc_mir/src/interpret/intrinsics.rs +++ b/compiler/rustc_mir/src/interpret/intrinsics.rs @@ -323,9 +323,6 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> { let result = Scalar::from_uint(truncated_bits, layout.size); self.write_scalar(result, dest)?; } - sym::copy_nonoverlapping => { - self.copy_nonoverlapping(args[0], args[1], args[2])?; - } sym::copy => { let elem_ty = instance.substs.type_at(0); let elem_layout = self.layout_of(elem_ty)?; diff --git a/compiler/rustc_mir/src/transform/validate.rs b/compiler/rustc_mir/src/transform/validate.rs index 29b90bff210bd..d009b0b1b2384 100644 --- a/compiler/rustc_mir/src/transform/validate.rs +++ b/compiler/rustc_mir/src/transform/validate.rs @@ -294,7 +294,49 @@ impl<'a, 'tcx> Visitor<'tcx> for TypeChecker<'a, 'tcx> { ); } } - _ => {} + StatementKind::CopyNonOverlapping(box rustc_middle::mir::CopyNonOverlapping { + ref src, + ref dst, + ref count, + }) => { + let src_ty = src.ty(&self.body.local_decls, self.tcx); + let op_src_ty = if let Some(src_deref) = src_ty.builtin_deref(true) { + src_deref.ty + } else { + self.fail( + location, + format!("Expected src to be ptr in copy_nonoverlapping, got: {}", src_ty), + ); + return; + }; + let dst_ty = dst.ty(&self.body.local_decls, self.tcx); + let op_dst_ty = if let Some(dst_deref) = dst_ty.builtin_deref(true) { + dst_deref.ty + } else { + self.fail( + location, + format!("Expected dst to be ptr in copy_nonoverlapping, got: {}", dst_ty), + ); + return; + }; + // since CopyNonOverlapping is parametrized by 1 type, + // we only need to check that they are equal and not keep an extra parameter. + if op_src_ty != op_dst_ty { + self.fail(location, format!("bad arg ({:?} != {:?})", op_src_ty, op_dst_ty)); + } + + let op_cnt_ty = count.ty(&self.body.local_decls, self.tcx); + if op_cnt_ty != self.tcx.types.usize { + self.fail(location, format!("bad arg ({:?} != usize)", op_cnt_ty)) + } + } + StatementKind::SetDiscriminant { .. } + | StatementKind::StorageLive(..) + | StatementKind::StorageDead(..) + | StatementKind::LlvmInlineAsm(..) + | StatementKind::Retag(_, _) + | StatementKind::Coverage(_) + | StatementKind::Nop => {} } self.super_statement(statement, location); From 10a4c67a613d84e80cba5fd52442d5cc30970cc6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?D=C3=A1niel=20Buga?= Date: Fri, 5 Mar 2021 16:01:46 +0100 Subject: [PATCH 13/19] Add regression test for #75525 --- src/test/codegen/issue-75525-bounds-checks.rs | 27 +++++++++++++++++++ 1 file changed, 27 insertions(+) create mode 100644 src/test/codegen/issue-75525-bounds-checks.rs diff --git a/src/test/codegen/issue-75525-bounds-checks.rs b/src/test/codegen/issue-75525-bounds-checks.rs new file mode 100644 index 0000000000000..a03c63c1d0948 --- /dev/null +++ b/src/test/codegen/issue-75525-bounds-checks.rs @@ -0,0 +1,27 @@ +// Regression test for #75525, verifies that no bounds checks are generated. + +// min-llvm-version: 12.0.0 +// compile-flags: -O + +#![crate_type = "lib"] + +// CHECK-LABEL: @f0 +// CHECK-NOT: panic +#[no_mangle] +pub fn f0(idx: usize, buf: &[u8; 10]) -> u8 { + if idx < 8 { buf[idx + 1] } else { 0 } +} + +// CHECK-LABEL: @f1 +// CHECK-NOT: panic +#[no_mangle] +pub fn f1(idx: usize, buf: &[u8; 10]) -> u8 { + if idx > 5 && idx < 8 { buf[idx - 1] } else { 0 } +} + +// CHECK-LABEL: @f2 +// CHECK-NOT: panic +#[no_mangle] +pub fn f2(idx: usize, buf: &[u8; 10]) -> u8 { + if idx > 5 && idx < 8 { buf[idx] } else { 0 } +} From e64138c534d356e555842ddc9da20a952a20ab46 Mon Sep 17 00:00:00 2001 From: mark Date: Fri, 12 Feb 2021 18:04:37 -0600 Subject: [PATCH 14/19] use pat for patterns in let bindings --- compiler/rustc_lint/src/unused.rs | 2 +- compiler/rustc_parse/src/parser/item.rs | 5 +- compiler/rustc_parse/src/parser/pat.rs | 184 ++++++++++++------ compiler/rustc_parse/src/parser/stmt.rs | 8 +- src/test/ui/or-patterns/already-bound-name.rs | 10 +- .../ui/or-patterns/already-bound-name.stderr | 66 +++---- .../ui/or-patterns/consistent-bindings.rs | 10 +- src/test/ui/or-patterns/const-fn.rs | 10 +- .../feature-gate-or_patterns-leading-let.rs | 1 + ...eature-gate-or_patterns-leading-let.stderr | 8 +- .../or-patterns/feature-gate-or_patterns.rs | 2 + .../feature-gate-or_patterns.stderr | 38 ++-- .../ui/or-patterns/fn-param-wrap-parens.fixed | 2 +- .../ui/or-patterns/fn-param-wrap-parens.rs | 2 +- .../or-patterns/fn-param-wrap-parens.stderr | 2 +- src/test/ui/or-patterns/inconsistent-modes.rs | 12 +- .../ui/or-patterns/inconsistent-modes.stderr | 82 ++++---- ...ve-been-expanded-earlier-non-exhaustive.rs | 2 +- ...een-expanded-earlier-non-exhaustive.stderr | 10 +- ...69875-should-have-been-expanded-earlier.rs | 2 +- src/test/ui/or-patterns/let-pattern.rs | 2 +- .../mismatched-bindings-async-fn.rs | 2 +- .../mismatched-bindings-async-fn.stderr | 20 +- src/test/ui/or-patterns/missing-bindings.rs | 20 +- .../ui/or-patterns/missing-bindings.stderr | 172 ++++++++-------- .../nested-undelimited-precedence.rs | 46 +++++ .../nested-undelimited-precedence.stderr | 86 ++++++++ .../or-patterns-binding-type-mismatch.rs | 4 +- .../or-patterns-binding-type-mismatch.stderr | 36 ++-- .../or-patterns-default-binding-modes.rs | 8 +- .../or-patterns/or-patterns-syntactic-fail.rs | 15 +- .../or-patterns-syntactic-fail.stderr | 20 +- .../or-patterns/or-patterns-syntactic-pass.rs | 22 +-- .../ui/or-patterns/remove-leading-vert.fixed | 5 +- .../ui/or-patterns/remove-leading-vert.rs | 5 +- .../ui/or-patterns/remove-leading-vert.stderr | 10 +- 36 files changed, 588 insertions(+), 343 deletions(-) create mode 100644 src/test/ui/or-patterns/nested-undelimited-precedence.rs create mode 100644 src/test/ui/or-patterns/nested-undelimited-precedence.stderr diff --git a/compiler/rustc_lint/src/unused.rs b/compiler/rustc_lint/src/unused.rs index b611aebad01b0..335ceb775e85a 100644 --- a/compiler/rustc_lint/src/unused.rs +++ b/compiler/rustc_lint/src/unused.rs @@ -847,7 +847,7 @@ impl EarlyLintPass for UnusedParens { fn check_stmt(&mut self, cx: &EarlyContext<'_>, s: &ast::Stmt) { if let StmtKind::Local(ref local) = s.kind { - self.check_unused_parens_pat(cx, &local.pat, false, false); + self.check_unused_parens_pat(cx, &local.pat, true, false); } ::check_stmt(self, cx, s) diff --git a/compiler/rustc_parse/src/parser/item.rs b/compiler/rustc_parse/src/parser/item.rs index 9668a24bf8ab6..a28595e6fae67 100644 --- a/compiler/rustc_parse/src/parser/item.rs +++ b/compiler/rustc_parse/src/parser/item.rs @@ -1757,8 +1757,9 @@ impl<'a> Parser<'a> { let (pat, ty) = if is_name_required || this.is_named_param() { debug!("parse_param_general parse_pat (is_name_required:{})", is_name_required); - let pat = this.parse_fn_param_pat()?; - if let Err(mut err) = this.expect(&token::Colon) { + let (pat, colon) = this.parse_fn_param_pat_colon()?; + if !colon { + let mut err = this.unexpected::<()>().unwrap_err(); return if let Some(ident) = this.parameter_without_type(&mut err, pat, is_name_required, first_param) { diff --git a/compiler/rustc_parse/src/parser/pat.rs b/compiler/rustc_parse/src/parser/pat.rs index 9e2e7359ca96e..5a68afdfa59f7 100644 --- a/compiler/rustc_parse/src/parser/pat.rs +++ b/compiler/rustc_parse/src/parser/pat.rs @@ -31,6 +31,18 @@ pub enum RecoverComma { No, } +/// The result of `eat_or_separator`. We want to distinguish which case we are in to avoid +/// emitting duplicate diagnostics. +#[derive(Debug, Clone, Copy)] +enum EatOrResult { + /// We recovered from a trailing vert. + TrailingVert, + /// We ate an `|` (or `||` and recovered). + AteOr, + /// We did not eat anything (i.e. the current token is not `|` or `||`). + None, +} + impl<'a> Parser<'a> { /// Parses a pattern. /// @@ -55,9 +67,26 @@ impl<'a> Parser<'a> { gate_or: GateOr, rc: RecoverComma, ) -> PResult<'a, P> { + self.parse_pat_allow_top_alt_inner(expected, gate_or, rc).map(|(pat, _)| pat) + } + + /// Returns the pattern and a bool indicating whether we recovered from a trailing vert (true = + /// recovered). + fn parse_pat_allow_top_alt_inner( + &mut self, + expected: Expected, + gate_or: GateOr, + rc: RecoverComma, + ) -> PResult<'a, (P, bool)> { + // Keep track of whether we recovered from a trailing vert so that we can avoid duplicated + // suggestions (which bothers rustfix). + // // Allow a '|' before the pats (RFCs 1925, 2530, and 2535). - let leading_vert_span = - if self.eat_or_separator(None) { Some(self.prev_token.span) } else { None }; + let (leading_vert_span, mut trailing_vert) = match self.eat_or_separator(None) { + EatOrResult::AteOr => (Some(self.prev_token.span), false), + EatOrResult::TrailingVert => (None, true), + EatOrResult::None => (None, false), + }; // Parse the first pattern (`p_0`). let first_pat = self.parse_pat_no_top_alt(expected)?; @@ -77,16 +106,24 @@ impl<'a> Parser<'a> { // If there was a leading vert, treat this as an or-pattern. This improves // diagnostics. let span = leading_vert_span.to(self.prev_token.span); - return Ok(self.mk_pat(span, PatKind::Or(vec![first_pat]))); + return Ok((self.mk_pat(span, PatKind::Or(vec![first_pat])), trailing_vert)); } - return Ok(first_pat); + return Ok((first_pat, trailing_vert)); } // Parse the patterns `p_1 | ... | p_n` where `n > 0`. let lo = leading_vert_span.unwrap_or(first_pat.span); let mut pats = vec![first_pat]; - while self.eat_or_separator(Some(lo)) { + loop { + match self.eat_or_separator(Some(lo)) { + EatOrResult::AteOr => {} + EatOrResult::None => break, + EatOrResult::TrailingVert => { + trailing_vert = true; + break; + } + } let pat = self.parse_pat_no_top_alt(expected).map_err(|mut err| { err.span_label(lo, WHILE_PARSING_OR_MSG); err @@ -101,15 +138,63 @@ impl<'a> Parser<'a> { self.sess.gated_spans.gate(sym::or_patterns, or_pattern_span); } - Ok(self.mk_pat(or_pattern_span, PatKind::Or(pats))) + Ok((self.mk_pat(or_pattern_span, PatKind::Or(pats)), trailing_vert)) } - /// Parse the pattern for a function or function pointer parameter. - pub(super) fn parse_fn_param_pat(&mut self) -> PResult<'a, P> { - // We actually do _not_ allow top-level or-patterns in function params, but we use - // `parse_pat_allow_top_alt` anyway so that we can detect when a user tries to use it. This - // allows us to print a better error message. - // + /// Parse a pattern and (maybe) a `Colon` in positions where a pattern may be followed by a + /// type annotation (e.g. for `let` bindings or `fn` params). + /// + /// Generally, this corresponds to `pat_no_top_alt` followed by an optional `Colon`. It will + /// eat the `Colon` token if one is present. + /// + /// The return value represents the parsed pattern and `true` if a `Colon` was parsed (`false` + /// otherwise). + pub(super) fn parse_pat_before_ty( + &mut self, + expected: Expected, + gate_or: GateOr, + rc: RecoverComma, + syntax_loc: &str, + ) -> PResult<'a, (P, bool)> { + // We use `parse_pat_allow_top_alt` regardless of whether we actually want top-level + // or-patterns so that we can detect when a user tries to use it. This allows us to print a + // better error message. + let (pat, trailing_vert) = self.parse_pat_allow_top_alt_inner(expected, gate_or, rc)?; + let colon = self.eat(&token::Colon); + + if let PatKind::Or(pats) = &pat.kind { + let msg = format!("top-level or-patterns are not allowed in {}", syntax_loc); + let (help, fix) = if pats.len() == 1 { + // If all we have is a leading vert, then print a special message. This is the case + // if `parse_pat_allow_top_alt` returns an or-pattern with one variant. + let msg = "remove the `|`"; + let fix = pprust::pat_to_string(&pat); + (msg, fix) + } else { + let msg = "wrap the pattern in parentheses"; + let fix = format!("({})", pprust::pat_to_string(&pat)); + (msg, fix) + }; + + if trailing_vert { + // We already emitted an error and suggestion to remove the trailing vert. Don't + // emit again. + self.sess.span_diagnostic.delay_span_bug(pat.span, &msg); + } else { + self.struct_span_err(pat.span, &msg) + .span_suggestion(pat.span, help, fix, Applicability::MachineApplicable) + .emit(); + } + } + + Ok((pat, colon)) + } + + /// Parse the pattern for a function or function pointer parameter, followed by a colon. + /// + /// The return value represents the parsed pattern and `true` if a `Colon` was parsed (`false` + /// otherwise). + pub(super) fn parse_fn_param_pat_colon(&mut self) -> PResult<'a, (P, bool)> { // In order to get good UX, we first recover in the case of a leading vert for an illegal // top-level or-pat. Normally, this means recovering both `|` and `||`, but in this case, // a leading `||` probably doesn't indicate an or-pattern attempt, so we handle that @@ -128,53 +213,28 @@ impl<'a> Parser<'a> { self.bump(); } - let pat = self.parse_pat_allow_top_alt(PARAM_EXPECTED, GateOr::No, RecoverComma::No)?; - - if let PatKind::Or(..) = &pat.kind { - self.ban_illegal_fn_param_or_pat(&pat); - } - - Ok(pat) - } - - /// Ban `A | B` immediately in a parameter pattern and suggest wrapping in parens. - fn ban_illegal_fn_param_or_pat(&self, pat: &Pat) { - // If all we have a leading vert, then print a special message. This is the case if - // `parse_pat_allow_top_alt` returns an or-pattern with one variant. - let (msg, fix) = match &pat.kind { - PatKind::Or(pats) if pats.len() == 1 => { - let msg = "remove the leading `|`"; - let fix = pprust::pat_to_string(pat); - (msg, fix) - } - - _ => { - let msg = "wrap the pattern in parentheses"; - let fix = format!("({})", pprust::pat_to_string(pat)); - (msg, fix) - } - }; - - self.struct_span_err(pat.span, "an or-pattern parameter must be wrapped in parentheses") - .span_suggestion(pat.span, msg, fix, Applicability::MachineApplicable) - .emit(); + self.parse_pat_before_ty( + PARAM_EXPECTED, + GateOr::No, + RecoverComma::No, + "function parameters", + ) } /// Eat the or-pattern `|` separator. /// If instead a `||` token is encountered, recover and pretend we parsed `|`. - fn eat_or_separator(&mut self, lo: Option) -> bool { + fn eat_or_separator(&mut self, lo: Option) -> EatOrResult { if self.recover_trailing_vert(lo) { - return false; - } - - match self.token.kind { - token::OrOr => { - // Found `||`; Recover and pretend we parsed `|`. - self.ban_unexpected_or_or(lo); - self.bump(); - true - } - _ => self.eat(&token::BinOp(token::Or)), + EatOrResult::TrailingVert + } else if matches!(self.token.kind, token::OrOr) { + // Found `||`; Recover and pretend we parsed `|`. + self.ban_unexpected_or_or(lo); + self.bump(); + EatOrResult::AteOr + } else if self.eat(&token::BinOp(token::Or)) { + EatOrResult::AteOr + } else { + EatOrResult::None } } @@ -190,14 +250,14 @@ impl<'a> Parser<'a> { matches!( &token.uninterpolate().kind, token::FatArrow // e.g. `a | => 0,`. - | token::Ident(kw::If, false) // e.g. `a | if expr`. - | token::Eq // e.g. `let a | = 0`. - | token::Semi // e.g. `let a |;`. - | token::Colon // e.g. `let a | :`. - | token::Comma // e.g. `let (a |,)`. - | token::CloseDelim(token::Bracket) // e.g. `let [a | ]`. - | token::CloseDelim(token::Paren) // e.g. `let (a | )`. - | token::CloseDelim(token::Brace) // e.g. `let A { f: a | }`. + | token::Ident(kw::If, false) // e.g. `a | if expr`. + | token::Eq // e.g. `let a | = 0`. + | token::Semi // e.g. `let a |;`. + | token::Colon // e.g. `let a | :`. + | token::Comma // e.g. `let (a |,)`. + | token::CloseDelim(token::Bracket) // e.g. `let [a | ]`. + | token::CloseDelim(token::Paren) // e.g. `let (a | )`. + | token::CloseDelim(token::Brace) // e.g. `let A { f: a | }`. ) }); match (is_end_ahead, &self.token.kind) { diff --git a/compiler/rustc_parse/src/parser/stmt.rs b/compiler/rustc_parse/src/parser/stmt.rs index 07746f2390dd9..cabe2b95a49f6 100644 --- a/compiler/rustc_parse/src/parser/stmt.rs +++ b/compiler/rustc_parse/src/parser/stmt.rs @@ -13,7 +13,8 @@ use rustc_ast::token::{self, TokenKind}; use rustc_ast::util::classify; use rustc_ast::AstLike; use rustc_ast::{AttrStyle, AttrVec, Attribute, MacCall, MacCallStmt, MacStmtStyle}; -use rustc_ast::{Block, BlockCheckMode, Expr, ExprKind, Local, Stmt, StmtKind, DUMMY_NODE_ID}; +use rustc_ast::{Block, BlockCheckMode, Expr, ExprKind, Local, Stmt}; +use rustc_ast::{StmtKind, DUMMY_NODE_ID}; use rustc_errors::{Applicability, PResult}; use rustc_span::source_map::{BytePos, Span}; use rustc_span::symbol::{kw, sym}; @@ -220,9 +221,10 @@ impl<'a> Parser<'a> { /// Parses a local variable declaration. fn parse_local(&mut self, attrs: AttrVec) -> PResult<'a, P> { let lo = self.prev_token.span; - let pat = self.parse_pat_allow_top_alt(None, GateOr::Yes, RecoverComma::Yes)?; + let (pat, colon) = + self.parse_pat_before_ty(None, GateOr::Yes, RecoverComma::Yes, "`let` bindings")?; - let (err, ty) = if self.eat(&token::Colon) { + let (err, ty) = if colon { // Save the state of the parser before parsing type normally, in case there is a `:` // instead of an `=` typo. let parser_snapshot_before_type = self.clone(); diff --git a/src/test/ui/or-patterns/already-bound-name.rs b/src/test/ui/or-patterns/already-bound-name.rs index 726e17b7ec226..543d7c21c8317 100644 --- a/src/test/ui/or-patterns/already-bound-name.rs +++ b/src/test/ui/or-patterns/already-bound-name.rs @@ -18,10 +18,10 @@ fn main() { let (A(a, _) | B(a), a) = (A(0, 1), 2); //~^ ERROR identifier `a` is bound more than once in the same pattern - let A(a, a) | B(a) = A(0, 1); + let (A(a, a) | B(a)) = A(0, 1); //~^ ERROR identifier `a` is bound more than once in the same pattern - let B(a) | A(a, a) = A(0, 1); + let (B(a) | A(a, a)) = A(0, 1); //~^ ERROR identifier `a` is bound more than once in the same pattern match A(0, 1) { @@ -29,17 +29,17 @@ fn main() { //~^ ERROR identifier `a` is bound more than once in the same pattern } - let B(A(a, _) | B(a)) | A(a, A(a, _) | B(a)) = B(B(1)); + let (B(A(a, _) | B(a)) | A(a, A(a, _) | B(a))) = B(B(1)); //~^ ERROR identifier `a` is bound more than once in the same pattern //~| ERROR identifier `a` is bound more than once in the same pattern //~| ERROR mismatched types - let B(_) | A(A(a, _) | B(a), A(a, _) | B(a)) = B(B(1)); + let (B(_) | A(A(a, _) | B(a), A(a, _) | B(a))) = B(B(1)); //~^ ERROR identifier `a` is bound more than once in the same pattern //~| ERROR identifier `a` is bound more than once in the same pattern //~| ERROR variable `a` is not bound in all patterns - let B(A(a, _) | B(a)) | A(A(a, _) | B(a), A(a, _) | B(a)) = B(B(1)); + let (B(A(a, _) | B(a)) | A(A(a, _) | B(a), A(a, _) | B(a))) = B(B(1)); //~^ ERROR identifier `a` is bound more than once in the same pattern //~| ERROR identifier `a` is bound more than once in the same pattern } diff --git a/src/test/ui/or-patterns/already-bound-name.stderr b/src/test/ui/or-patterns/already-bound-name.stderr index 97933ca122944..483154a5e3274 100644 --- a/src/test/ui/or-patterns/already-bound-name.stderr +++ b/src/test/ui/or-patterns/already-bound-name.stderr @@ -23,16 +23,16 @@ LL | let (A(a, _) | B(a), a) = (A(0, 1), 2); | ^ used in a pattern more than once error[E0416]: identifier `a` is bound more than once in the same pattern - --> $DIR/already-bound-name.rs:21:14 + --> $DIR/already-bound-name.rs:21:15 | -LL | let A(a, a) | B(a) = A(0, 1); - | ^ used in a pattern more than once +LL | let (A(a, a) | B(a)) = A(0, 1); + | ^ used in a pattern more than once error[E0416]: identifier `a` is bound more than once in the same pattern - --> $DIR/already-bound-name.rs:24:21 + --> $DIR/already-bound-name.rs:24:22 | -LL | let B(a) | A(a, a) = A(0, 1); - | ^ used in a pattern more than once +LL | let (B(a) | A(a, a)) = A(0, 1); + | ^ used in a pattern more than once error[E0416]: identifier `a` is bound more than once in the same pattern --> $DIR/already-bound-name.rs:28:21 @@ -41,55 +41,55 @@ LL | B(a) | A(a, a) => {} // Let's ensure `match` has no funny business. | ^ used in a pattern more than once error[E0416]: identifier `a` is bound more than once in the same pattern - --> $DIR/already-bound-name.rs:32:36 + --> $DIR/already-bound-name.rs:32:37 | -LL | let B(A(a, _) | B(a)) | A(a, A(a, _) | B(a)) = B(B(1)); - | ^ used in a pattern more than once +LL | let (B(A(a, _) | B(a)) | A(a, A(a, _) | B(a))) = B(B(1)); + | ^ used in a pattern more than once error[E0416]: identifier `a` is bound more than once in the same pattern - --> $DIR/already-bound-name.rs:32:46 + --> $DIR/already-bound-name.rs:32:47 | -LL | let B(A(a, _) | B(a)) | A(a, A(a, _) | B(a)) = B(B(1)); - | ^ used in a pattern more than once +LL | let (B(A(a, _) | B(a)) | A(a, A(a, _) | B(a))) = B(B(1)); + | ^ used in a pattern more than once error[E0416]: identifier `a` is bound more than once in the same pattern - --> $DIR/already-bound-name.rs:37:36 + --> $DIR/already-bound-name.rs:37:37 | -LL | let B(_) | A(A(a, _) | B(a), A(a, _) | B(a)) = B(B(1)); - | ^ used in a pattern more than once +LL | let (B(_) | A(A(a, _) | B(a), A(a, _) | B(a))) = B(B(1)); + | ^ used in a pattern more than once error[E0416]: identifier `a` is bound more than once in the same pattern - --> $DIR/already-bound-name.rs:37:46 + --> $DIR/already-bound-name.rs:37:47 | -LL | let B(_) | A(A(a, _) | B(a), A(a, _) | B(a)) = B(B(1)); - | ^ used in a pattern more than once +LL | let (B(_) | A(A(a, _) | B(a), A(a, _) | B(a))) = B(B(1)); + | ^ used in a pattern more than once error[E0408]: variable `a` is not bound in all patterns - --> $DIR/already-bound-name.rs:37:9 + --> $DIR/already-bound-name.rs:37:10 | -LL | let B(_) | A(A(a, _) | B(a), A(a, _) | B(a)) = B(B(1)); - | ^^^^ pattern doesn't bind `a` - variable not in all patterns +LL | let (B(_) | A(A(a, _) | B(a), A(a, _) | B(a))) = B(B(1)); + | ^^^^ pattern doesn't bind `a` - variable not in all patterns error[E0416]: identifier `a` is bound more than once in the same pattern - --> $DIR/already-bound-name.rs:42:49 + --> $DIR/already-bound-name.rs:42:50 | -LL | let B(A(a, _) | B(a)) | A(A(a, _) | B(a), A(a, _) | B(a)) = B(B(1)); - | ^ used in a pattern more than once +LL | let (B(A(a, _) | B(a)) | A(A(a, _) | B(a), A(a, _) | B(a))) = B(B(1)); + | ^ used in a pattern more than once error[E0416]: identifier `a` is bound more than once in the same pattern - --> $DIR/already-bound-name.rs:42:59 + --> $DIR/already-bound-name.rs:42:60 | -LL | let B(A(a, _) | B(a)) | A(A(a, _) | B(a), A(a, _) | B(a)) = B(B(1)); - | ^ used in a pattern more than once +LL | let (B(A(a, _) | B(a)) | A(A(a, _) | B(a), A(a, _) | B(a))) = B(B(1)); + | ^ used in a pattern more than once error[E0308]: mismatched types - --> $DIR/already-bound-name.rs:32:31 + --> $DIR/already-bound-name.rs:32:32 | -LL | let B(A(a, _) | B(a)) | A(a, A(a, _) | B(a)) = B(B(1)); - | - ^ ------- this expression has type `E>` - | | | - | | expected integer, found enum `E` - | first introduced with type `{integer}` here +LL | let (B(A(a, _) | B(a)) | A(a, A(a, _) | B(a))) = B(B(1)); + | - ^ ------- this expression has type `E>` + | | | + | | expected integer, found enum `E` + | first introduced with type `{integer}` here | = note: expected type `{integer}` found type `E<{integer}>` diff --git a/src/test/ui/or-patterns/consistent-bindings.rs b/src/test/ui/or-patterns/consistent-bindings.rs index 3ee57978bb009..853ddcf241232 100644 --- a/src/test/ui/or-patterns/consistent-bindings.rs +++ b/src/test/ui/or-patterns/consistent-bindings.rs @@ -8,9 +8,9 @@ fn main() { // One level: - let Ok(a) | Err(a) = Ok(0); - let Ok(ref a) | Err(ref a) = Ok(0); - let Ok(ref mut a) | Err(ref mut a) = Ok(0); + let (Ok(a) | Err(a)) = Ok(0); + let (Ok(ref a) | Err(ref a)) = Ok(0); + let (Ok(ref mut a) | Err(ref mut a)) = Ok(0); // Two levels: enum Tri { @@ -20,10 +20,10 @@ fn main() { } use Tri::*; - let Ok((V1(a) | V2(a) | V3(a), b)) | Err(Ok((a, b)) | Err((a, b))): Result<_, Result<_, _>> = + let (Ok((V1(a) | V2(a) | V3(a), b)) | Err(Ok((a, b)) | Err((a, b)))): Result<_, Result<_, _>> = Ok((V1(1), 1)); - let Ok((V1(a) | V2(a) | V3(a), ref b)) | Err(Ok((a, ref b)) | Err((a, ref b))): Result< + let (Ok((V1(a) | V2(a) | V3(a), ref b)) | Err(Ok((a, ref b)) | Err((a, ref b)))): Result< _, Result<_, _>, > = Ok((V1(1), 1)); diff --git a/src/test/ui/or-patterns/const-fn.rs b/src/test/ui/or-patterns/const-fn.rs index f4af2f0d2dd40..55c6f60915fa8 100644 --- a/src/test/ui/or-patterns/const-fn.rs +++ b/src/test/ui/or-patterns/const-fn.rs @@ -3,28 +3,28 @@ const fn foo((Ok(a) | Err(a)): Result) { let x = Ok(3); - let Ok(y) | Err(y) = x; + let (Ok(y) | Err(y)) = x; } const X: () = { let x = Ok(3); - let Ok(y) | Err(y) = x; + let (Ok(y) | Err(y)) = x; }; static Y: () = { let x = Ok(3); - let Ok(y) | Err(y) = x; + let (Ok(y) | Err(y)) = x; }; static mut Z: () = { let x = Ok(3); - let Ok(y) | Err(y) = x; + let (Ok(y) | Err(y)) = x; }; fn main() { let _: [(); { let x = Ok(3); - let Ok(y) | Err(y) = x; + let (Ok(y) | Err(y)) = x; 2 }]; } diff --git a/src/test/ui/or-patterns/feature-gate-or_patterns-leading-let.rs b/src/test/ui/or-patterns/feature-gate-or_patterns-leading-let.rs index a4ea4e25d861e..6c592550ec257 100644 --- a/src/test/ui/or-patterns/feature-gate-or_patterns-leading-let.rs +++ b/src/test/ui/or-patterns/feature-gate-or_patterns-leading-let.rs @@ -5,4 +5,5 @@ fn main() {} #[cfg(FALSE)] fn gated_leading_vert_in_let() { let | A; //~ ERROR or-patterns syntax is experimental + //~^ ERROR top-level or-patterns are not allowed } diff --git a/src/test/ui/or-patterns/feature-gate-or_patterns-leading-let.stderr b/src/test/ui/or-patterns/feature-gate-or_patterns-leading-let.stderr index 499f60dd545ff..d556532cd6ad8 100644 --- a/src/test/ui/or-patterns/feature-gate-or_patterns-leading-let.stderr +++ b/src/test/ui/or-patterns/feature-gate-or_patterns-leading-let.stderr @@ -1,3 +1,9 @@ +error: top-level or-patterns are not allowed in `let` bindings + --> $DIR/feature-gate-or_patterns-leading-let.rs:7:9 + | +LL | let | A; + | ^^^ help: remove the `|`: `A` + error[E0658]: or-patterns syntax is experimental --> $DIR/feature-gate-or_patterns-leading-let.rs:7:9 | @@ -7,6 +13,6 @@ LL | let | A; = note: see issue #54883 for more information = help: add `#![feature(or_patterns)]` to the crate attributes to enable -error: aborting due to previous error +error: aborting due to 2 previous errors For more information about this error, try `rustc --explain E0658`. diff --git a/src/test/ui/or-patterns/feature-gate-or_patterns.rs b/src/test/ui/or-patterns/feature-gate-or_patterns.rs index e638838147a4d..8bb45e606be96 100644 --- a/src/test/ui/or-patterns/feature-gate-or_patterns.rs +++ b/src/test/ui/or-patterns/feature-gate-or_patterns.rs @@ -26,7 +26,9 @@ fn or_patterns() { // Gated: let | A | B; //~ ERROR or-patterns syntax is experimental + //~^ ERROR top-level or-patterns are not allowed let A | B; //~ ERROR or-patterns syntax is experimental + //~^ ERROR top-level or-patterns are not allowed for | A | B in 0 {} //~ ERROR or-patterns syntax is experimental for A | B in 0 {} //~ ERROR or-patterns syntax is experimental fn fun((A | B): _) {} //~ ERROR or-patterns syntax is experimental diff --git a/src/test/ui/or-patterns/feature-gate-or_patterns.stderr b/src/test/ui/or-patterns/feature-gate-or_patterns.stderr index c01d17c0b3d7e..7988af5b94213 100644 --- a/src/test/ui/or-patterns/feature-gate-or_patterns.stderr +++ b/src/test/ui/or-patterns/feature-gate-or_patterns.stderr @@ -1,3 +1,15 @@ +error: top-level or-patterns are not allowed in `let` bindings + --> $DIR/feature-gate-or_patterns.rs:28:9 + | +LL | let | A | B; + | ^^^^^^^ help: wrap the pattern in parentheses: `(A | B)` + +error: top-level or-patterns are not allowed in `let` bindings + --> $DIR/feature-gate-or_patterns.rs:30:9 + | +LL | let A | B; + | ^^^^^ help: wrap the pattern in parentheses: `(A | B)` + error[E0658]: or-patterns syntax is experimental --> $DIR/feature-gate-or_patterns.rs:5:14 | @@ -17,7 +29,7 @@ LL | let | A | B; = help: add `#![feature(or_patterns)]` to the crate attributes to enable error[E0658]: or-patterns syntax is experimental - --> $DIR/feature-gate-or_patterns.rs:29:9 + --> $DIR/feature-gate-or_patterns.rs:30:9 | LL | let A | B; | ^^^^^ @@ -26,7 +38,7 @@ LL | let A | B; = help: add `#![feature(or_patterns)]` to the crate attributes to enable error[E0658]: or-patterns syntax is experimental - --> $DIR/feature-gate-or_patterns.rs:30:9 + --> $DIR/feature-gate-or_patterns.rs:32:9 | LL | for | A | B in 0 {} | ^^^^^^^ @@ -35,7 +47,7 @@ LL | for | A | B in 0 {} = help: add `#![feature(or_patterns)]` to the crate attributes to enable error[E0658]: or-patterns syntax is experimental - --> $DIR/feature-gate-or_patterns.rs:31:9 + --> $DIR/feature-gate-or_patterns.rs:33:9 | LL | for A | B in 0 {} | ^^^^^ @@ -44,7 +56,7 @@ LL | for A | B in 0 {} = help: add `#![feature(or_patterns)]` to the crate attributes to enable error[E0658]: or-patterns syntax is experimental - --> $DIR/feature-gate-or_patterns.rs:32:13 + --> $DIR/feature-gate-or_patterns.rs:34:13 | LL | fn fun((A | B): _) {} | ^^^^^ @@ -53,7 +65,7 @@ LL | fn fun((A | B): _) {} = help: add `#![feature(or_patterns)]` to the crate attributes to enable error[E0658]: or-patterns syntax is experimental - --> $DIR/feature-gate-or_patterns.rs:33:15 + --> $DIR/feature-gate-or_patterns.rs:35:15 | LL | let _ = |(A | B): u8| (); | ^^^^^ @@ -62,7 +74,7 @@ LL | let _ = |(A | B): u8| (); = help: add `#![feature(or_patterns)]` to the crate attributes to enable error[E0658]: or-patterns syntax is experimental - --> $DIR/feature-gate-or_patterns.rs:34:10 + --> $DIR/feature-gate-or_patterns.rs:36:10 | LL | let (A | B); | ^^^^^ @@ -71,7 +83,7 @@ LL | let (A | B); = help: add `#![feature(or_patterns)]` to the crate attributes to enable error[E0658]: or-patterns syntax is experimental - --> $DIR/feature-gate-or_patterns.rs:35:10 + --> $DIR/feature-gate-or_patterns.rs:37:10 | LL | let (A | B,); | ^^^^^ @@ -80,7 +92,7 @@ LL | let (A | B,); = help: add `#![feature(or_patterns)]` to the crate attributes to enable error[E0658]: or-patterns syntax is experimental - --> $DIR/feature-gate-or_patterns.rs:36:11 + --> $DIR/feature-gate-or_patterns.rs:38:11 | LL | let A(B | C); | ^^^^^ @@ -89,7 +101,7 @@ LL | let A(B | C); = help: add `#![feature(or_patterns)]` to the crate attributes to enable error[E0658]: or-patterns syntax is experimental - --> $DIR/feature-gate-or_patterns.rs:37:14 + --> $DIR/feature-gate-or_patterns.rs:39:14 | LL | let E::V(B | C); | ^^^^^ @@ -98,7 +110,7 @@ LL | let E::V(B | C); = help: add `#![feature(or_patterns)]` to the crate attributes to enable error[E0658]: or-patterns syntax is experimental - --> $DIR/feature-gate-or_patterns.rs:38:17 + --> $DIR/feature-gate-or_patterns.rs:40:17 | LL | let S { f1: B | C, f2 }; | ^^^^^ @@ -107,7 +119,7 @@ LL | let S { f1: B | C, f2 }; = help: add `#![feature(or_patterns)]` to the crate attributes to enable error[E0658]: or-patterns syntax is experimental - --> $DIR/feature-gate-or_patterns.rs:39:20 + --> $DIR/feature-gate-or_patterns.rs:41:20 | LL | let E::V { f1: B | C, f2 }; | ^^^^^ @@ -116,7 +128,7 @@ LL | let E::V { f1: B | C, f2 }; = help: add `#![feature(or_patterns)]` to the crate attributes to enable error[E0658]: or-patterns syntax is experimental - --> $DIR/feature-gate-or_patterns.rs:40:10 + --> $DIR/feature-gate-or_patterns.rs:42:10 | LL | let [A | B]; | ^^^^^ @@ -169,6 +181,6 @@ LL | accept_pat!([p | q]); = note: see issue #54883 for more information = help: add `#![feature(or_patterns)]` to the crate attributes to enable -error: aborting due to 19 previous errors +error: aborting due to 21 previous errors For more information about this error, try `rustc --explain E0658`. diff --git a/src/test/ui/or-patterns/fn-param-wrap-parens.fixed b/src/test/ui/or-patterns/fn-param-wrap-parens.fixed index bbc75d2b411eb..65e04325e16bd 100644 --- a/src/test/ui/or-patterns/fn-param-wrap-parens.fixed +++ b/src/test/ui/or-patterns/fn-param-wrap-parens.fixed @@ -11,4 +11,4 @@ enum E { A, B } use E::*; #[cfg(FALSE)] -fn fun1((A | B): E) {} //~ ERROR an or-pattern parameter must be wrapped in parentheses +fn fun1((A | B): E) {} //~ ERROR top-level or-patterns are not allowed diff --git a/src/test/ui/or-patterns/fn-param-wrap-parens.rs b/src/test/ui/or-patterns/fn-param-wrap-parens.rs index 65b93dcbf7467..aeb4a05ea808e 100644 --- a/src/test/ui/or-patterns/fn-param-wrap-parens.rs +++ b/src/test/ui/or-patterns/fn-param-wrap-parens.rs @@ -11,4 +11,4 @@ enum E { A, B } use E::*; #[cfg(FALSE)] -fn fun1(A | B: E) {} //~ ERROR an or-pattern parameter must be wrapped in parentheses +fn fun1(A | B: E) {} //~ ERROR top-level or-patterns are not allowed diff --git a/src/test/ui/or-patterns/fn-param-wrap-parens.stderr b/src/test/ui/or-patterns/fn-param-wrap-parens.stderr index 0e6424a430043..96193c17ae23f 100644 --- a/src/test/ui/or-patterns/fn-param-wrap-parens.stderr +++ b/src/test/ui/or-patterns/fn-param-wrap-parens.stderr @@ -1,4 +1,4 @@ -error: an or-pattern parameter must be wrapped in parentheses +error: top-level or-patterns are not allowed in function parameters --> $DIR/fn-param-wrap-parens.rs:14:9 | LL | fn fun1(A | B: E) {} diff --git a/src/test/ui/or-patterns/inconsistent-modes.rs b/src/test/ui/or-patterns/inconsistent-modes.rs index fd5cb01ab42c5..2300e9f9f3b7b 100644 --- a/src/test/ui/or-patterns/inconsistent-modes.rs +++ b/src/test/ui/or-patterns/inconsistent-modes.rs @@ -4,23 +4,23 @@ #![allow(non_camel_case_types)] fn main() { // One level: - let Ok(a) | Err(ref a): Result<&u8, u8> = Ok(&0); + let (Ok(a) | Err(ref a)): Result<&u8, u8> = Ok(&0); //~^ ERROR variable `a` is bound inconsistently - let Ok(ref mut a) | Err(a): Result = Ok(0); + let (Ok(ref mut a) | Err(a)): Result = Ok(0); //~^ ERROR variable `a` is bound inconsistently - let Ok(ref a) | Err(ref mut a): Result<&u8, &mut u8> = Ok(&0); + let (Ok(ref a) | Err(ref mut a)): Result<&u8, &mut u8> = Ok(&0); //~^ ERROR variable `a` is bound inconsistently //~| ERROR mismatched types - let Ok((ref a, b)) | Err((ref mut a, ref b)) = Ok((0, &0)); + let (Ok((ref a, b)) | Err((ref mut a, ref b))) = Ok((0, &0)); //~^ ERROR variable `a` is bound inconsistently //~| ERROR variable `b` is bound inconsistently //~| ERROR mismatched types // Two levels: - let Ok(Ok(a) | Err(a)) | Err(ref a) = Err(0); + let (Ok(Ok(a) | Err(a)) | Err(ref a)) = Err(0); //~^ ERROR variable `a` is bound inconsistently // Three levels: - let Ok([Ok((Ok(ref a) | Err(a),)) | Err(a)]) | Err(a) = Err(&1); + let (Ok([Ok((Ok(ref a) | Err(a),)) | Err(a)]) | Err(a)) = Err(&1); //~^ ERROR variable `a` is bound inconsistently } diff --git a/src/test/ui/or-patterns/inconsistent-modes.stderr b/src/test/ui/or-patterns/inconsistent-modes.stderr index 15790771043df..99791431eaf25 100644 --- a/src/test/ui/or-patterns/inconsistent-modes.stderr +++ b/src/test/ui/or-patterns/inconsistent-modes.stderr @@ -1,74 +1,74 @@ error[E0409]: variable `a` is bound inconsistently across alternatives separated by `|` - --> $DIR/inconsistent-modes.rs:7:25 + --> $DIR/inconsistent-modes.rs:7:26 | -LL | let Ok(a) | Err(ref a): Result<&u8, u8> = Ok(&0); - | - ^ bound in different ways - | | - | first binding +LL | let (Ok(a) | Err(ref a)): Result<&u8, u8> = Ok(&0); + | - ^ bound in different ways + | | + | first binding error[E0409]: variable `a` is bound inconsistently across alternatives separated by `|` - --> $DIR/inconsistent-modes.rs:9:29 + --> $DIR/inconsistent-modes.rs:9:30 | -LL | let Ok(ref mut a) | Err(a): Result = Ok(0); - | - ^ bound in different ways - | | - | first binding +LL | let (Ok(ref mut a) | Err(a)): Result = Ok(0); + | - ^ bound in different ways + | | + | first binding error[E0409]: variable `a` is bound inconsistently across alternatives separated by `|` - --> $DIR/inconsistent-modes.rs:11:33 + --> $DIR/inconsistent-modes.rs:11:34 | -LL | let Ok(ref a) | Err(ref mut a): Result<&u8, &mut u8> = Ok(&0); - | - first binding ^ bound in different ways +LL | let (Ok(ref a) | Err(ref mut a)): Result<&u8, &mut u8> = Ok(&0); + | - first binding ^ bound in different ways error[E0409]: variable `a` is bound inconsistently across alternatives separated by `|` - --> $DIR/inconsistent-modes.rs:14:39 + --> $DIR/inconsistent-modes.rs:14:40 | -LL | let Ok((ref a, b)) | Err((ref mut a, ref b)) = Ok((0, &0)); - | - first binding ^ bound in different ways +LL | let (Ok((ref a, b)) | Err((ref mut a, ref b))) = Ok((0, &0)); + | - first binding ^ bound in different ways error[E0409]: variable `b` is bound inconsistently across alternatives separated by `|` - --> $DIR/inconsistent-modes.rs:14:46 + --> $DIR/inconsistent-modes.rs:14:47 | -LL | let Ok((ref a, b)) | Err((ref mut a, ref b)) = Ok((0, &0)); - | - first binding ^ bound in different ways +LL | let (Ok((ref a, b)) | Err((ref mut a, ref b))) = Ok((0, &0)); + | - first binding ^ bound in different ways error[E0409]: variable `a` is bound inconsistently across alternatives separated by `|` - --> $DIR/inconsistent-modes.rs:20:38 + --> $DIR/inconsistent-modes.rs:20:39 | -LL | let Ok(Ok(a) | Err(a)) | Err(ref a) = Err(0); - | - ^ bound in different ways - | | - | first binding +LL | let (Ok(Ok(a) | Err(a)) | Err(ref a)) = Err(0); + | - ^ bound in different ways + | | + | first binding error[E0409]: variable `a` is bound inconsistently across alternatives separated by `|` - --> $DIR/inconsistent-modes.rs:24:33 + --> $DIR/inconsistent-modes.rs:24:34 | -LL | let Ok([Ok((Ok(ref a) | Err(a),)) | Err(a)]) | Err(a) = Err(&1); - | - ^ bound in different ways - | | - | first binding +LL | let (Ok([Ok((Ok(ref a) | Err(a),)) | Err(a)]) | Err(a)) = Err(&1); + | - ^ bound in different ways + | | + | first binding error[E0308]: mismatched types - --> $DIR/inconsistent-modes.rs:11:25 + --> $DIR/inconsistent-modes.rs:11:26 | -LL | let Ok(ref a) | Err(ref mut a): Result<&u8, &mut u8> = Ok(&0); - | ----- ^^^^^^^^^ -------------------- expected due to this - | | | - | | types differ in mutability - | first introduced with type `&&u8` here +LL | let (Ok(ref a) | Err(ref mut a)): Result<&u8, &mut u8> = Ok(&0); + | ----- ^^^^^^^^^ -------------------- expected due to this + | | | + | | types differ in mutability + | first introduced with type `&&u8` here | = note: expected type `&&u8` found type `&mut &mut u8` = note: a binding must have the same type in all alternatives error[E0308]: mismatched types - --> $DIR/inconsistent-modes.rs:14:31 + --> $DIR/inconsistent-modes.rs:14:32 | -LL | let Ok((ref a, b)) | Err((ref mut a, ref b)) = Ok((0, &0)); - | ----- ^^^^^^^^^ ----------- this expression has type `Result<({integer}, &{integer}), (_, _)>` - | | | - | | types differ in mutability - | first introduced with type `&{integer}` here +LL | let (Ok((ref a, b)) | Err((ref mut a, ref b))) = Ok((0, &0)); + | ----- ^^^^^^^^^ ----------- this expression has type `Result<({integer}, &{integer}), (_, _)>` + | | | + | | types differ in mutability + | first introduced with type `&{integer}` here | = note: expected type `&{integer}` found type `&mut _` diff --git a/src/test/ui/or-patterns/issue-69875-should-have-been-expanded-earlier-non-exhaustive.rs b/src/test/ui/or-patterns/issue-69875-should-have-been-expanded-earlier-non-exhaustive.rs index 59533cefea64c..8e83acc6dcbc0 100644 --- a/src/test/ui/or-patterns/issue-69875-should-have-been-expanded-earlier-non-exhaustive.rs +++ b/src/test/ui/or-patterns/issue-69875-should-have-been-expanded-earlier-non-exhaustive.rs @@ -1,7 +1,7 @@ #![feature(or_patterns)] fn main() { - let 0 | (1 | 2) = 0; //~ ERROR refutable pattern in local binding + let (0 | (1 | 2)) = 0; //~ ERROR refutable pattern in local binding match 0 { //~^ ERROR non-exhaustive patterns 0 | (1 | 2) => {} diff --git a/src/test/ui/or-patterns/issue-69875-should-have-been-expanded-earlier-non-exhaustive.stderr b/src/test/ui/or-patterns/issue-69875-should-have-been-expanded-earlier-non-exhaustive.stderr index 2acf1f41c6fa6..9ed942d9e0fd5 100644 --- a/src/test/ui/or-patterns/issue-69875-should-have-been-expanded-earlier-non-exhaustive.stderr +++ b/src/test/ui/or-patterns/issue-69875-should-have-been-expanded-earlier-non-exhaustive.stderr @@ -1,16 +1,16 @@ error[E0005]: refutable pattern in local binding: `i32::MIN..=-1_i32` and `3_i32..=i32::MAX` not covered - --> $DIR/issue-69875-should-have-been-expanded-earlier-non-exhaustive.rs:4:9 + --> $DIR/issue-69875-should-have-been-expanded-earlier-non-exhaustive.rs:4:10 | -LL | let 0 | (1 | 2) = 0; - | ^^^^^^^^^^^ patterns `i32::MIN..=-1_i32` and `3_i32..=i32::MAX` not covered +LL | let (0 | (1 | 2)) = 0; + | ^^^^^^^^^^^ patterns `i32::MIN..=-1_i32` and `3_i32..=i32::MAX` not covered | = note: `let` bindings require an "irrefutable pattern", like a `struct` or an `enum` with only one variant = note: for more information, visit https://doc.rust-lang.org/book/ch18-02-refutability.html = note: the matched value is of type `i32` help: you might want to use `if let` to ignore the variant that isn't matched | -LL | if let 0 | (1 | 2) = 0 { /* */ } - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ +LL | if let (0 | (1 | 2)) = 0 { /* */ } + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ error[E0004]: non-exhaustive patterns: `i32::MIN..=-1_i32` and `3_i32..=i32::MAX` not covered --> $DIR/issue-69875-should-have-been-expanded-earlier-non-exhaustive.rs:5:11 diff --git a/src/test/ui/or-patterns/issue-69875-should-have-been-expanded-earlier.rs b/src/test/ui/or-patterns/issue-69875-should-have-been-expanded-earlier.rs index 1de563dedbf18..2f080ebf7830b 100644 --- a/src/test/ui/or-patterns/issue-69875-should-have-been-expanded-earlier.rs +++ b/src/test/ui/or-patterns/issue-69875-should-have-been-expanded-earlier.rs @@ -3,7 +3,7 @@ #![feature(or_patterns)] fn main() { - let 0 | (1 | _) = 0; + let (0 | (1 | _)) = 0; if let 0 | (1 | 2) = 0 {} if let x @ 0 | x @ (1 | 2) = 0 {} } diff --git a/src/test/ui/or-patterns/let-pattern.rs b/src/test/ui/or-patterns/let-pattern.rs index 07e37412ce842..7f22aa9d9f980 100644 --- a/src/test/ui/or-patterns/let-pattern.rs +++ b/src/test/ui/or-patterns/let-pattern.rs @@ -3,7 +3,7 @@ // run-pass fn or_pat_let(x: Result) -> u32 { - let Ok(y) | Err(y) = x; + let (Ok(y) | Err(y)) = x; y } diff --git a/src/test/ui/or-patterns/mismatched-bindings-async-fn.rs b/src/test/ui/or-patterns/mismatched-bindings-async-fn.rs index 5c5c68f81d1f2..cf98a472106e8 100644 --- a/src/test/ui/or-patterns/mismatched-bindings-async-fn.rs +++ b/src/test/ui/or-patterns/mismatched-bindings-async-fn.rs @@ -8,7 +8,7 @@ async fn a((x | s): String) {} //~| ERROR variable `s` is not bound in all patterns async fn b() { - let x | s = String::new(); + let (x | s) = String::new(); //~^ ERROR variable `x` is not bound in all patterns //~| ERROR variable `s` is not bound in all patterns } diff --git a/src/test/ui/or-patterns/mismatched-bindings-async-fn.stderr b/src/test/ui/or-patterns/mismatched-bindings-async-fn.stderr index 998577cf4b5e0..d5c77ee39c99c 100644 --- a/src/test/ui/or-patterns/mismatched-bindings-async-fn.stderr +++ b/src/test/ui/or-patterns/mismatched-bindings-async-fn.stderr @@ -15,20 +15,20 @@ LL | async fn a((x | s): String) {} | variable not in all patterns error[E0408]: variable `s` is not bound in all patterns - --> $DIR/mismatched-bindings-async-fn.rs:11:9 + --> $DIR/mismatched-bindings-async-fn.rs:11:10 | -LL | let x | s = String::new(); - | ^ - variable not in all patterns - | | - | pattern doesn't bind `s` +LL | let (x | s) = String::new(); + | ^ - variable not in all patterns + | | + | pattern doesn't bind `s` error[E0408]: variable `x` is not bound in all patterns - --> $DIR/mismatched-bindings-async-fn.rs:11:13 + --> $DIR/mismatched-bindings-async-fn.rs:11:14 | -LL | let x | s = String::new(); - | - ^ pattern doesn't bind `x` - | | - | variable not in all patterns +LL | let (x | s) = String::new(); + | - ^ pattern doesn't bind `x` + | | + | variable not in all patterns error: aborting due to 4 previous errors diff --git a/src/test/ui/or-patterns/missing-bindings.rs b/src/test/ui/or-patterns/missing-bindings.rs index 67cf52fa8c418..5dd1f16b65580 100644 --- a/src/test/ui/or-patterns/missing-bindings.rs +++ b/src/test/ui/or-patterns/missing-bindings.rs @@ -17,7 +17,7 @@ fn check_handling_of_paths() { } use bar::foo::{alpha, charlie}; - let alpha | beta | charlie = alpha; //~ ERROR variable `beta` is not bound in all patterns + let (alpha | beta | charlie) = alpha; //~ ERROR variable `beta` is not bound in all patterns match Some(alpha) { Some(alpha | beta) => {} //~ ERROR variable `beta` is not bound in all patterns } @@ -31,19 +31,19 @@ fn check_misc_nesting() { // One level: const X: E = B(0); - let A(a, _) | _ = X; //~ ERROR variable `a` is not bound in all patterns - let _ | B(a) = X; //~ ERROR variable `a` is not bound in all patterns - let A(..) | B(a) = X; //~ ERROR variable `a` is not bound in all patterns - let A(a, _) | B(_) = X; //~ ERROR variable `a` is not bound in all patterns - let A(_, a) | B(_) = X; //~ ERROR variable `a` is not bound in all patterns - let A(a, b) | B(a) = X; //~ ERROR variable `b` is not bound in all patterns + let (A(a, _) | _) = X; //~ ERROR variable `a` is not bound in all patterns + let (_ | B(a)) = X; //~ ERROR variable `a` is not bound in all patterns + let (A(..) | B(a)) = X; //~ ERROR variable `a` is not bound in all patterns + let (A(a, _) | B(_)) = X; //~ ERROR variable `a` is not bound in all patterns + let (A(_, a) | B(_)) = X; //~ ERROR variable `a` is not bound in all patterns + let (A(a, b) | B(a)) = X; //~ ERROR variable `b` is not bound in all patterns // Two levels: const Y: E> = B(B(0)); - let A(A(..) | B(_), _) | B(a) = Y; //~ ERROR variable `a` is not bound in all patterns - let A(A(..) | B(a), _) | B(A(a, _) | B(a)) = Y; + let (A(A(..) | B(_), _) | B(a)) = Y; //~ ERROR variable `a` is not bound in all patterns + let (A(A(..) | B(a), _) | B(A(a, _) | B(a))) = Y; //~^ ERROR variable `a` is not bound in all patterns - let A(A(a, b) | B(c), d) | B(e) = Y; + let (A(A(a, b) | B(c), d) | B(e)) = Y; //~^ ERROR variable `a` is not bound in all patterns //~| ERROR variable `a` is not bound in all patterns //~| ERROR variable `b` is not bound in all patterns diff --git a/src/test/ui/or-patterns/missing-bindings.stderr b/src/test/ui/or-patterns/missing-bindings.stderr index 57270e4412351..4702bc6bbf322 100644 --- a/src/test/ui/or-patterns/missing-bindings.stderr +++ b/src/test/ui/or-patterns/missing-bindings.stderr @@ -1,11 +1,11 @@ error[E0408]: variable `beta` is not bound in all patterns - --> $DIR/missing-bindings.rs:20:9 + --> $DIR/missing-bindings.rs:20:10 | -LL | let alpha | beta | charlie = alpha; - | ^^^^^ ---- ^^^^^^^ pattern doesn't bind `beta` - | | | - | | variable not in all patterns - | pattern doesn't bind `beta` +LL | let (alpha | beta | charlie) = alpha; + | ^^^^^ ---- ^^^^^^^ pattern doesn't bind `beta` + | | | + | | variable not in all patterns + | pattern doesn't bind `beta` error[E0408]: variable `beta` is not bound in all patterns --> $DIR/missing-bindings.rs:22:14 @@ -16,132 +16,132 @@ LL | Some(alpha | beta) => {} | pattern doesn't bind `beta` error[E0408]: variable `a` is not bound in all patterns - --> $DIR/missing-bindings.rs:34:19 + --> $DIR/missing-bindings.rs:34:20 | -LL | let A(a, _) | _ = X; - | - ^ pattern doesn't bind `a` - | | - | variable not in all patterns +LL | let (A(a, _) | _) = X; + | - ^ pattern doesn't bind `a` + | | + | variable not in all patterns error[E0408]: variable `a` is not bound in all patterns - --> $DIR/missing-bindings.rs:35:9 + --> $DIR/missing-bindings.rs:35:10 | -LL | let _ | B(a) = X; - | ^ - variable not in all patterns - | | - | pattern doesn't bind `a` +LL | let (_ | B(a)) = X; + | ^ - variable not in all patterns + | | + | pattern doesn't bind `a` error[E0408]: variable `a` is not bound in all patterns - --> $DIR/missing-bindings.rs:36:9 + --> $DIR/missing-bindings.rs:36:10 | -LL | let A(..) | B(a) = X; - | ^^^^^ - variable not in all patterns - | | - | pattern doesn't bind `a` +LL | let (A(..) | B(a)) = X; + | ^^^^^ - variable not in all patterns + | | + | pattern doesn't bind `a` error[E0408]: variable `a` is not bound in all patterns - --> $DIR/missing-bindings.rs:37:19 + --> $DIR/missing-bindings.rs:37:20 | -LL | let A(a, _) | B(_) = X; - | - ^^^^ pattern doesn't bind `a` - | | - | variable not in all patterns +LL | let (A(a, _) | B(_)) = X; + | - ^^^^ pattern doesn't bind `a` + | | + | variable not in all patterns error[E0408]: variable `a` is not bound in all patterns - --> $DIR/missing-bindings.rs:38:19 + --> $DIR/missing-bindings.rs:38:20 | -LL | let A(_, a) | B(_) = X; - | - ^^^^ pattern doesn't bind `a` - | | - | variable not in all patterns +LL | let (A(_, a) | B(_)) = X; + | - ^^^^ pattern doesn't bind `a` + | | + | variable not in all patterns error[E0408]: variable `b` is not bound in all patterns - --> $DIR/missing-bindings.rs:39:19 + --> $DIR/missing-bindings.rs:39:20 | -LL | let A(a, b) | B(a) = X; - | - ^^^^ pattern doesn't bind `b` - | | - | variable not in all patterns +LL | let (A(a, b) | B(a)) = X; + | - ^^^^ pattern doesn't bind `b` + | | + | variable not in all patterns error[E0408]: variable `a` is not bound in all patterns - --> $DIR/missing-bindings.rs:43:9 + --> $DIR/missing-bindings.rs:43:10 | -LL | let A(A(..) | B(_), _) | B(a) = Y; - | ^^^^^^^^^^^^^^^^^^ - variable not in all patterns - | | - | pattern doesn't bind `a` +LL | let (A(A(..) | B(_), _) | B(a)) = Y; + | ^^^^^^^^^^^^^^^^^^ - variable not in all patterns + | | + | pattern doesn't bind `a` error[E0408]: variable `a` is not bound in all patterns - --> $DIR/missing-bindings.rs:44:11 + --> $DIR/missing-bindings.rs:44:12 | -LL | let A(A(..) | B(a), _) | B(A(a, _) | B(a)) = Y; - | ^^^^^ - variable not in all patterns - | | - | pattern doesn't bind `a` +LL | let (A(A(..) | B(a), _) | B(A(a, _) | B(a))) = Y; + | ^^^^^ - variable not in all patterns + | | + | pattern doesn't bind `a` error[E0408]: variable `a` is not bound in all patterns - --> $DIR/missing-bindings.rs:46:21 + --> $DIR/missing-bindings.rs:46:22 | -LL | let A(A(a, b) | B(c), d) | B(e) = Y; - | - ^^^^ pattern doesn't bind `a` - | | - | variable not in all patterns +LL | let (A(A(a, b) | B(c), d) | B(e)) = Y; + | - ^^^^ pattern doesn't bind `a` + | | + | variable not in all patterns error[E0408]: variable `b` is not bound in all patterns - --> $DIR/missing-bindings.rs:46:21 + --> $DIR/missing-bindings.rs:46:22 | -LL | let A(A(a, b) | B(c), d) | B(e) = Y; - | - ^^^^ pattern doesn't bind `b` - | | - | variable not in all patterns +LL | let (A(A(a, b) | B(c), d) | B(e)) = Y; + | - ^^^^ pattern doesn't bind `b` + | | + | variable not in all patterns error[E0408]: variable `c` is not bound in all patterns - --> $DIR/missing-bindings.rs:46:11 + --> $DIR/missing-bindings.rs:46:12 | -LL | let A(A(a, b) | B(c), d) | B(e) = Y; - | ^^^^^^^ - variable not in all patterns - | | - | pattern doesn't bind `c` +LL | let (A(A(a, b) | B(c), d) | B(e)) = Y; + | ^^^^^^^ - variable not in all patterns + | | + | pattern doesn't bind `c` error[E0408]: variable `a` is not bound in all patterns - --> $DIR/missing-bindings.rs:46:32 + --> $DIR/missing-bindings.rs:46:33 | -LL | let A(A(a, b) | B(c), d) | B(e) = Y; - | - ^^^^ pattern doesn't bind `a` - | | - | variable not in all patterns +LL | let (A(A(a, b) | B(c), d) | B(e)) = Y; + | - ^^^^ pattern doesn't bind `a` + | | + | variable not in all patterns error[E0408]: variable `b` is not bound in all patterns - --> $DIR/missing-bindings.rs:46:32 + --> $DIR/missing-bindings.rs:46:33 | -LL | let A(A(a, b) | B(c), d) | B(e) = Y; - | - ^^^^ pattern doesn't bind `b` - | | - | variable not in all patterns +LL | let (A(A(a, b) | B(c), d) | B(e)) = Y; + | - ^^^^ pattern doesn't bind `b` + | | + | variable not in all patterns error[E0408]: variable `c` is not bound in all patterns - --> $DIR/missing-bindings.rs:46:32 + --> $DIR/missing-bindings.rs:46:33 | -LL | let A(A(a, b) | B(c), d) | B(e) = Y; - | - ^^^^ pattern doesn't bind `c` - | | - | variable not in all patterns +LL | let (A(A(a, b) | B(c), d) | B(e)) = Y; + | - ^^^^ pattern doesn't bind `c` + | | + | variable not in all patterns error[E0408]: variable `d` is not bound in all patterns - --> $DIR/missing-bindings.rs:46:32 + --> $DIR/missing-bindings.rs:46:33 | -LL | let A(A(a, b) | B(c), d) | B(e) = Y; - | - ^^^^ pattern doesn't bind `d` - | | - | variable not in all patterns +LL | let (A(A(a, b) | B(c), d) | B(e)) = Y; + | - ^^^^ pattern doesn't bind `d` + | | + | variable not in all patterns error[E0408]: variable `e` is not bound in all patterns - --> $DIR/missing-bindings.rs:46:9 + --> $DIR/missing-bindings.rs:46:10 | -LL | let A(A(a, b) | B(c), d) | B(e) = Y; - | ^^^^^^^^^^^^^^^^^^^^ - variable not in all patterns - | | - | pattern doesn't bind `e` +LL | let (A(A(a, b) | B(c), d) | B(e)) = Y; + | ^^^^^^^^^^^^^^^^^^^^ - variable not in all patterns + | | + | pattern doesn't bind `e` error[E0408]: variable `a` is not bound in all patterns --> $DIR/missing-bindings.rs:62:29 diff --git a/src/test/ui/or-patterns/nested-undelimited-precedence.rs b/src/test/ui/or-patterns/nested-undelimited-precedence.rs new file mode 100644 index 0000000000000..208662b1c425d --- /dev/null +++ b/src/test/ui/or-patterns/nested-undelimited-precedence.rs @@ -0,0 +1,46 @@ +// This test tests the precedence of `|` (or-patterns) undelimited nested patterns. In particular, +// we want to reserve the syntactic space of a pattern followed by a type annotation for possible +// future type ascription, so we need to make sure that any time a pattern is followed by type +// annotation (for now), the pattern is not a top-level or-pattern. However, there are also a few +// types of patterns that allow undelimited subpatterns that could cause the same ambiguity. +// Currently, those should be impossible due to precedence rule. This test enforces that. + +#![feature(or_patterns)] + +enum E { + A, + B, +} + +fn foo() { + use E::*; + + // ok + let b @ (A | B): E = A; + + let b @ A | B: E = A; //~ERROR `b` is not bound in all patterns + //~^ ERROR top-level or-patterns are not allowed +} + +enum F { + A(usize), + B(usize), +} + +fn bar() { + use F::*; + + // ok + let (A(x) | B(x)): F = A(3); + + let &A(_) | B(_): F = A(3); //~ERROR mismatched types + //~^ ERROR top-level or-patterns are not allowed + let &&A(_) | B(_): F = A(3); //~ERROR mismatched types + //~^ ERROR top-level or-patterns are not allowed + let &mut A(_) | B(_): F = A(3); //~ERROR mismatched types + //~^ ERROR top-level or-patterns are not allowed + let &&mut A(_) | B(_): F = A(3); //~ERROR mismatched types + //~^ ERROR top-level or-patterns are not allowed +} + +fn main() {} diff --git a/src/test/ui/or-patterns/nested-undelimited-precedence.stderr b/src/test/ui/or-patterns/nested-undelimited-precedence.stderr new file mode 100644 index 0000000000000..1d78d5193cb88 --- /dev/null +++ b/src/test/ui/or-patterns/nested-undelimited-precedence.stderr @@ -0,0 +1,86 @@ +error: top-level or-patterns are not allowed in `let` bindings + --> $DIR/nested-undelimited-precedence.rs:21:9 + | +LL | let b @ A | B: E = A; + | ^^^^^^^^^ help: wrap the pattern in parentheses: `(b @ A | B)` + +error: top-level or-patterns are not allowed in `let` bindings + --> $DIR/nested-undelimited-precedence.rs:36:9 + | +LL | let &A(_) | B(_): F = A(3); + | ^^^^^^^^^^^^ help: wrap the pattern in parentheses: `(&A(_) | B(_))` + +error: top-level or-patterns are not allowed in `let` bindings + --> $DIR/nested-undelimited-precedence.rs:38:9 + | +LL | let &&A(_) | B(_): F = A(3); + | ^^^^^^^^^^^^^ help: wrap the pattern in parentheses: `(&&A(_) | B(_))` + +error: top-level or-patterns are not allowed in `let` bindings + --> $DIR/nested-undelimited-precedence.rs:40:9 + | +LL | let &mut A(_) | B(_): F = A(3); + | ^^^^^^^^^^^^^^^^ help: wrap the pattern in parentheses: `(&mut A(_) | B(_))` + +error: top-level or-patterns are not allowed in `let` bindings + --> $DIR/nested-undelimited-precedence.rs:42:9 + | +LL | let &&mut A(_) | B(_): F = A(3); + | ^^^^^^^^^^^^^^^^^ help: wrap the pattern in parentheses: `(&&mut A(_) | B(_))` + +error[E0408]: variable `b` is not bound in all patterns + --> $DIR/nested-undelimited-precedence.rs:21:17 + | +LL | let b @ A | B: E = A; + | - ^ pattern doesn't bind `b` + | | + | variable not in all patterns + +error[E0308]: mismatched types + --> $DIR/nested-undelimited-precedence.rs:36:9 + | +LL | let &A(_) | B(_): F = A(3); + | ^^^^^ - expected due to this + | | + | expected enum `F`, found reference + | + = note: expected enum `F` + found reference `&_` + +error[E0308]: mismatched types + --> $DIR/nested-undelimited-precedence.rs:38:9 + | +LL | let &&A(_) | B(_): F = A(3); + | ^^^^^^ - expected due to this + | | + | expected enum `F`, found reference + | + = note: expected enum `F` + found reference `&_` + +error[E0308]: mismatched types + --> $DIR/nested-undelimited-precedence.rs:40:9 + | +LL | let &mut A(_) | B(_): F = A(3); + | ^^^^^^^^^ - expected due to this + | | + | expected enum `F`, found `&mut _` + | + = note: expected enum `F` + found mutable reference `&mut _` + +error[E0308]: mismatched types + --> $DIR/nested-undelimited-precedence.rs:42:9 + | +LL | let &&mut A(_) | B(_): F = A(3); + | ^^^^^^^^^^ - expected due to this + | | + | expected enum `F`, found reference + | + = note: expected enum `F` + found reference `&_` + +error: aborting due to 10 previous errors + +Some errors have detailed explanations: E0308, E0408. +For more information about an error, try `rustc --explain E0308`. diff --git a/src/test/ui/or-patterns/or-patterns-binding-type-mismatch.rs b/src/test/ui/or-patterns/or-patterns-binding-type-mismatch.rs index 5ec7dc6962c18..11c8a7b69f948 100644 --- a/src/test/ui/or-patterns/or-patterns-binding-type-mismatch.rs +++ b/src/test/ui/or-patterns/or-patterns-binding-type-mismatch.rs @@ -52,10 +52,10 @@ fn main() { = Some((0u8, Some((1u16, 2u32)))) {} - let Blah::A(_, x, y) | Blah::B(x, y) = Blah::A(1, 1, 2); + let (Blah::A(_, x, y) | Blah::B(x, y)) = Blah::A(1, 1, 2); //~^ ERROR mismatched types - let (x, y) | (y, x) = (0u8, 1u16); + let ((x, y) | (y, x)) = (0u8, 1u16); //~^ ERROR mismatched types //~| ERROR mismatched types diff --git a/src/test/ui/or-patterns/or-patterns-binding-type-mismatch.stderr b/src/test/ui/or-patterns/or-patterns-binding-type-mismatch.stderr index 00dba053a59d3..26e14b539dbf2 100644 --- a/src/test/ui/or-patterns/or-patterns-binding-type-mismatch.stderr +++ b/src/test/ui/or-patterns/or-patterns-binding-type-mismatch.stderr @@ -187,35 +187,35 @@ LL | = Some((0u8, Some((1u16, 2u32)))) = note: a binding must have the same type in all alternatives error[E0308]: mismatched types - --> $DIR/or-patterns-binding-type-mismatch.rs:55:39 + --> $DIR/or-patterns-binding-type-mismatch.rs:55:40 | -LL | let Blah::A(_, x, y) | Blah::B(x, y) = Blah::A(1, 1, 2); - | - ^ ---------------- this expression has type `Blah` - | | | - | | expected `usize`, found `isize` - | first introduced with type `usize` here +LL | let (Blah::A(_, x, y) | Blah::B(x, y)) = Blah::A(1, 1, 2); + | - ^ ---------------- this expression has type `Blah` + | | | + | | expected `usize`, found `isize` + | first introduced with type `usize` here | = note: a binding must have the same type in all alternatives error[E0308]: mismatched types - --> $DIR/or-patterns-binding-type-mismatch.rs:58:19 + --> $DIR/or-patterns-binding-type-mismatch.rs:58:20 | -LL | let (x, y) | (y, x) = (0u8, 1u16); - | - ^ ----------- this expression has type `(u8, u16)` - | | | - | | expected `u16`, found `u8` - | first introduced with type `u16` here +LL | let ((x, y) | (y, x)) = (0u8, 1u16); + | - ^ ----------- this expression has type `(u8, u16)` + | | | + | | expected `u16`, found `u8` + | first introduced with type `u16` here | = note: a binding must have the same type in all alternatives error[E0308]: mismatched types - --> $DIR/or-patterns-binding-type-mismatch.rs:58:22 + --> $DIR/or-patterns-binding-type-mismatch.rs:58:23 | -LL | let (x, y) | (y, x) = (0u8, 1u16); - | - ^ ----------- this expression has type `(u8, u16)` - | | | - | | expected `u8`, found `u16` - | first introduced with type `u8` here +LL | let ((x, y) | (y, x)) = (0u8, 1u16); + | - ^ ----------- this expression has type `(u8, u16)` + | | | + | | expected `u8`, found `u16` + | first introduced with type `u8` here | = note: a binding must have the same type in all alternatives diff --git a/src/test/ui/or-patterns/or-patterns-default-binding-modes.rs b/src/test/ui/or-patterns/or-patterns-default-binding-modes.rs index 3b6047c7be47d..f98b038ae81af 100644 --- a/src/test/ui/or-patterns/or-patterns-default-binding-modes.rs +++ b/src/test/ui/or-patterns/or-patterns-default-binding-modes.rs @@ -37,11 +37,11 @@ fn main() { if let &(Ok(x) | Err(x)) = res { drop::(x); } - let Ok(mut x) | &Err(mut x) = res; + let (Ok(mut x) | &Err(mut x)) = res; drop::(x); let &(Ok(x) | Err(x)) = res; drop::(x); - let Ok(x) | Err(x) = res; + let (Ok(x) | Err(x)) = res; drop::<&u8>(x); for Ok(mut x) | &Err(mut x) in std::iter::once(res) { drop::(x); @@ -119,9 +119,9 @@ fn main() { } let tri = &Tri::A(&Ok(0)); - let Tri::A(Ok(mut x) | Err(mut x)) + let (Tri::A(Ok(mut x) | Err(mut x)) | Tri::B(&Ok(mut x) | Err(mut x)) - | &Tri::C(Ok(mut x) | Err(mut x)) = tri; + | &Tri::C(Ok(mut x) | Err(mut x))) = tri; drop::(x); match tri { diff --git a/src/test/ui/or-patterns/or-patterns-syntactic-fail.rs b/src/test/ui/or-patterns/or-patterns-syntactic-fail.rs index cbc24eb26fa47..27a5374ff1848 100644 --- a/src/test/ui/or-patterns/or-patterns-syntactic-fail.rs +++ b/src/test/ui/or-patterns/or-patterns-syntactic-fail.rs @@ -14,8 +14,19 @@ fn no_top_level_or_patterns() { // -------- This looks like an or-pattern but is in fact `|A| (B: E | ())`. // ...and for now neither do we allow or-patterns at the top level of functions. - fn fun1(A | B: E) {} //~ ERROR an or-pattern parameter must be wrapped in parentheses + fn fun1(A | B: E) {} + //~^ ERROR top-level or-patterns are not allowed fn fun2(| A | B: E) {} - //~^ ERROR an or-pattern parameter must be wrapped in parentheses + //~^ ERROR top-level or-patterns are not allowed + + // We don't allow top-level or-patterns before type annotation in let-statements because we + // want to reserve this syntactic space for possible future type ascription. + let A | B: E = A; + //~^ ERROR top-level or-patterns are not allowed + + let | A | B: E = A; + //~^ ERROR top-level or-patterns are not allowed + + let (A | B): E = A; // ok -- wrapped in parens } diff --git a/src/test/ui/or-patterns/or-patterns-syntactic-fail.stderr b/src/test/ui/or-patterns/or-patterns-syntactic-fail.stderr index db4d827757b03..929b2088f76ce 100644 --- a/src/test/ui/or-patterns/or-patterns-syntactic-fail.stderr +++ b/src/test/ui/or-patterns/or-patterns-syntactic-fail.stderr @@ -1,15 +1,27 @@ -error: an or-pattern parameter must be wrapped in parentheses +error: top-level or-patterns are not allowed in function parameters --> $DIR/or-patterns-syntactic-fail.rs:17:13 | LL | fn fun1(A | B: E) {} | ^^^^^ help: wrap the pattern in parentheses: `(A | B)` -error: an or-pattern parameter must be wrapped in parentheses - --> $DIR/or-patterns-syntactic-fail.rs:19:13 +error: top-level or-patterns are not allowed in function parameters + --> $DIR/or-patterns-syntactic-fail.rs:20:13 | LL | fn fun2(| A | B: E) {} | ^^^^^^^ help: wrap the pattern in parentheses: `(A | B)` +error: top-level or-patterns are not allowed in `let` bindings + --> $DIR/or-patterns-syntactic-fail.rs:25:9 + | +LL | let A | B: E = A; + | ^^^^^ help: wrap the pattern in parentheses: `(A | B)` + +error: top-level or-patterns are not allowed in `let` bindings + --> $DIR/or-patterns-syntactic-fail.rs:28:9 + | +LL | let | A | B: E = A; + | ^^^^^^^ help: wrap the pattern in parentheses: `(A | B)` + error[E0369]: no implementation for `E | ()` --> $DIR/or-patterns-syntactic-fail.rs:13:22 | @@ -20,6 +32,6 @@ LL | let _ = |A | B: E| (); | = note: an implementation of `std::ops::BitOr` might be missing for `E` -error: aborting due to 3 previous errors +error: aborting due to 5 previous errors For more information about this error, try `rustc --explain E0369`. diff --git a/src/test/ui/or-patterns/or-patterns-syntactic-pass.rs b/src/test/ui/or-patterns/or-patterns-syntactic-pass.rs index 5fe72caf9c1ff..3da238f7b9a45 100644 --- a/src/test/ui/or-patterns/or-patterns-syntactic-pass.rs +++ b/src/test/ui/or-patterns/or-patterns-syntactic-pass.rs @@ -23,11 +23,11 @@ accept_pat!([p | q]); #[cfg(FALSE)] fn or_patterns() { // Top level of `let`: - let | A | B; - let A | B; - let A | B: u8; - let A | B = 0; - let A | B: u8 = 0; + let (| A | B); + let (A | B); + let (A | B): u8; + let (A | B) = 0; + let (A | B): u8 = 0; // Top level of `for`: for | A | B in 0 {} @@ -69,10 +69,10 @@ fn or_patterns() { let [A | B, .. | ..]; // These bind as `(prefix p) | q` as opposed to `prefix (p | q)`: - let box 0 | 1; // Unstable; we *can* the precedence if we want. - let &0 | 1; - let &mut 0 | 1; - let x @ 0 | 1; - let ref x @ 0 | 1; - let ref mut x @ 0 | 1; + let (box 0 | 1); // Unstable; we *can* change the precedence if we want. + let (&0 | 1); + let (&mut 0 | 1); + let (x @ 0 | 1); + let (ref x @ 0 | 1); + let (ref mut x @ 0 | 1); } diff --git a/src/test/ui/or-patterns/remove-leading-vert.fixed b/src/test/ui/or-patterns/remove-leading-vert.fixed index c8fac4faa2a66..d23858d42d146 100644 --- a/src/test/ui/or-patterns/remove-leading-vert.fixed +++ b/src/test/ui/or-patterns/remove-leading-vert.fixed @@ -9,7 +9,7 @@ fn main() {} #[cfg(FALSE)] fn leading() { - fn fun1( A: E) {} //~ ERROR an or-pattern parameter must be wrapped in parentheses + fn fun1( A: E) {} //~ ERROR top-level or-patterns are not allowed fn fun2( A: E) {} //~ ERROR unexpected `||` before function parameter let ( | A): E; let ( | A): (E); //~ ERROR unexpected token `||` in pattern @@ -40,6 +40,9 @@ fn trailing() { //~^ ERROR a trailing `|` is not allowed in an or-pattern } + // These test trailing-vert in `let` bindings, but they also test that we don't emit a + // duplicate suggestion that would confuse rustfix. + let a : u8 = 0; //~ ERROR a trailing `|` is not allowed in an or-pattern let a = 0; //~ ERROR a trailing `|` is not allowed in an or-pattern let a ; //~ ERROR a trailing `|` is not allowed in an or-pattern diff --git a/src/test/ui/or-patterns/remove-leading-vert.rs b/src/test/ui/or-patterns/remove-leading-vert.rs index 2cf6b27ab1aac..e753765b3e795 100644 --- a/src/test/ui/or-patterns/remove-leading-vert.rs +++ b/src/test/ui/or-patterns/remove-leading-vert.rs @@ -9,7 +9,7 @@ fn main() {} #[cfg(FALSE)] fn leading() { - fn fun1( | A: E) {} //~ ERROR an or-pattern parameter must be wrapped in parentheses + fn fun1( | A: E) {} //~ ERROR top-level or-patterns are not allowed fn fun2( || A: E) {} //~ ERROR unexpected `||` before function parameter let ( | A): E; let ( || A): (E); //~ ERROR unexpected token `||` in pattern @@ -40,6 +40,9 @@ fn trailing() { //~^ ERROR a trailing `|` is not allowed in an or-pattern } + // These test trailing-vert in `let` bindings, but they also test that we don't emit a + // duplicate suggestion that would confuse rustfix. + let a | : u8 = 0; //~ ERROR a trailing `|` is not allowed in an or-pattern let a | = 0; //~ ERROR a trailing `|` is not allowed in an or-pattern let a | ; //~ ERROR a trailing `|` is not allowed in an or-pattern diff --git a/src/test/ui/or-patterns/remove-leading-vert.stderr b/src/test/ui/or-patterns/remove-leading-vert.stderr index 5c9efd44a187f..0a2b143288dab 100644 --- a/src/test/ui/or-patterns/remove-leading-vert.stderr +++ b/src/test/ui/or-patterns/remove-leading-vert.stderr @@ -1,8 +1,8 @@ -error: an or-pattern parameter must be wrapped in parentheses +error: top-level or-patterns are not allowed in function parameters --> $DIR/remove-leading-vert.rs:12:14 | LL | fn fun1( | A: E) {} - | ^^^ help: remove the leading `|`: `A` + | ^^^ help: remove the `|`: `A` error: unexpected `||` before function parameter --> $DIR/remove-leading-vert.rs:13:14 @@ -135,7 +135,7 @@ LL | | A | B | => {} | while parsing this or-pattern starting here error: a trailing `|` is not allowed in an or-pattern - --> $DIR/remove-leading-vert.rs:43:11 + --> $DIR/remove-leading-vert.rs:46:11 | LL | let a | : u8 = 0; | - ^ help: remove the `|` @@ -143,7 +143,7 @@ LL | let a | : u8 = 0; | while parsing this or-pattern starting here error: a trailing `|` is not allowed in an or-pattern - --> $DIR/remove-leading-vert.rs:44:11 + --> $DIR/remove-leading-vert.rs:47:11 | LL | let a | = 0; | - ^ help: remove the `|` @@ -151,7 +151,7 @@ LL | let a | = 0; | while parsing this or-pattern starting here error: a trailing `|` is not allowed in an or-pattern - --> $DIR/remove-leading-vert.rs:45:11 + --> $DIR/remove-leading-vert.rs:48:11 | LL | let a | ; | - ^ help: remove the `|` From c946d1d620f5babbd9ac9ad3d1a9782957386329 Mon Sep 17 00:00:00 2001 From: Maarten de Vries Date: Wed, 3 Mar 2021 20:34:13 +0100 Subject: [PATCH 15/19] Bump libc dependency of std to 0.2.88. --- Cargo.lock | 4 ++-- library/std/Cargo.toml | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 0666abadcba33..506321fdf9a73 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1910,9 +1910,9 @@ checksum = "830d08ce1d1d941e6b30645f1a0eb5643013d835ce3779a5fc208261dbe10f55" [[package]] name = "libc" -version = "0.2.85" +version = "0.2.88" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "7ccac4b00700875e6a07c6cde370d44d32fa01c5a65cdd2fca6858c479d28bb3" +checksum = "03b07a082330a35e43f63177cc01689da34fbffa0105e1246cf0311472cac73a" dependencies = [ "rustc-std-workspace-core", ] diff --git a/library/std/Cargo.toml b/library/std/Cargo.toml index 275fcc4c29299..f0f5558fd1608 100644 --- a/library/std/Cargo.toml +++ b/library/std/Cargo.toml @@ -16,7 +16,7 @@ cfg-if = { version = "0.1.8", features = ['rustc-dep-of-std'] } panic_unwind = { path = "../panic_unwind", optional = true } panic_abort = { path = "../panic_abort" } core = { path = "../core" } -libc = { version = "0.2.85", default-features = false, features = ['rustc-dep-of-std'] } +libc = { version = "0.2.88", default-features = false, features = ['rustc-dep-of-std'] } compiler_builtins = { version = "0.1.39" } profiler_builtins = { path = "../profiler_builtins", optional = true } unwind = { path = "../unwind" } From 95e096d6237298f523e5566be87684d208f7d128 Mon Sep 17 00:00:00 2001 From: Harald van Dijk Date: Sat, 6 Mar 2021 16:02:48 +0000 Subject: [PATCH 16/19] Change x64 size checks to not apply to x32. Rust contains various size checks conditional on target_arch = "x86_64", but these checks were never intended to apply to x86_64-unknown-linux-gnux32. Add target_pointer_width = "64" to the conditions. --- compiler/rustc_ast/src/ast.rs | 8 ++++---- compiler/rustc_ast/src/token.rs | 4 ++-- compiler/rustc_ast/src/tokenstream.rs | 2 +- compiler/rustc_errors/src/lib.rs | 2 +- compiler/rustc_hir/src/hir.rs | 2 +- compiler/rustc_infer/src/infer/mod.rs | 2 +- compiler/rustc_infer/src/lib.rs | 2 +- compiler/rustc_infer/src/traits/mod.rs | 2 +- compiler/rustc_middle/src/mir/interpret/error.rs | 4 ++-- compiler/rustc_middle/src/mir/interpret/value.rs | 6 +++--- compiler/rustc_middle/src/mir/mod.rs | 6 +++--- compiler/rustc_middle/src/mir/tcx.rs | 2 +- compiler/rustc_middle/src/traits/mod.rs | 2 +- compiler/rustc_middle/src/ty/consts.rs | 2 +- compiler/rustc_middle/src/ty/consts/kind.rs | 2 +- compiler/rustc_middle/src/ty/mod.rs | 4 ++-- compiler/rustc_middle/src/ty/sty.rs | 2 +- compiler/rustc_mir/src/interpret/operand.rs | 6 +++--- compiler/rustc_mir/src/interpret/place.rs | 10 +++++----- compiler/rustc_mir_build/src/thir/mod.rs | 2 +- compiler/rustc_trait_selection/src/lib.rs | 2 +- compiler/rustc_trait_selection/src/traits/fulfill.rs | 2 +- src/librustdoc/clean/types.rs | 2 +- 23 files changed, 39 insertions(+), 39 deletions(-) diff --git a/compiler/rustc_ast/src/ast.rs b/compiler/rustc_ast/src/ast.rs index de44a2031ab82..0f5fabbc5742c 100644 --- a/compiler/rustc_ast/src/ast.rs +++ b/compiler/rustc_ast/src/ast.rs @@ -1083,7 +1083,7 @@ pub struct Expr { } // `Expr` is used a lot. Make sure it doesn't unintentionally get bigger. -#[cfg(target_arch = "x86_64")] +#[cfg(all(target_arch = "x86_64", target_pointer_width = "64"))] rustc_data_structures::static_assert_size!(Expr, 120); impl Expr { @@ -2757,7 +2757,7 @@ pub enum ItemKind { MacroDef(MacroDef), } -#[cfg(target_arch = "x86_64")] +#[cfg(all(target_arch = "x86_64", target_pointer_width = "64"))] rustc_data_structures::static_assert_size!(ItemKind, 112); impl ItemKind { @@ -2831,7 +2831,7 @@ pub enum AssocItemKind { MacCall(MacCall), } -#[cfg(target_arch = "x86_64")] +#[cfg(all(target_arch = "x86_64", target_pointer_width = "64"))] rustc_data_structures::static_assert_size!(AssocItemKind, 72); impl AssocItemKind { @@ -2883,7 +2883,7 @@ pub enum ForeignItemKind { MacCall(MacCall), } -#[cfg(target_arch = "x86_64")] +#[cfg(all(target_arch = "x86_64", target_pointer_width = "64"))] rustc_data_structures::static_assert_size!(ForeignItemKind, 72); impl From for ItemKind { diff --git a/compiler/rustc_ast/src/token.rs b/compiler/rustc_ast/src/token.rs index 90bfb01d6c791..71792acb37d13 100644 --- a/compiler/rustc_ast/src/token.rs +++ b/compiler/rustc_ast/src/token.rs @@ -244,7 +244,7 @@ pub enum TokenKind { } // `TokenKind` is used a lot. Make sure it doesn't unintentionally get bigger. -#[cfg(target_arch = "x86_64")] +#[cfg(all(target_arch = "x86_64", target_pointer_width = "64"))] rustc_data_structures::static_assert_size!(TokenKind, 16); #[derive(Clone, PartialEq, Encodable, Decodable, Debug, HashStable_Generic)] @@ -682,7 +682,7 @@ pub enum Nonterminal { } // `Nonterminal` is used a lot. Make sure it doesn't unintentionally get bigger. -#[cfg(target_arch = "x86_64")] +#[cfg(all(target_arch = "x86_64", target_pointer_width = "64"))] rustc_data_structures::static_assert_size!(Nonterminal, 48); #[derive(Debug, Copy, Clone, PartialEq, Encodable, Decodable)] diff --git a/compiler/rustc_ast/src/tokenstream.rs b/compiler/rustc_ast/src/tokenstream.rs index 9ac05f316f034..c5c3142602b9b 100644 --- a/compiler/rustc_ast/src/tokenstream.rs +++ b/compiler/rustc_ast/src/tokenstream.rs @@ -189,7 +189,7 @@ pub struct TokenStream(pub(crate) Lrc>); pub type TreeAndSpacing = (TokenTree, Spacing); // `TokenStream` is used a lot. Make sure it doesn't unintentionally get bigger. -#[cfg(target_arch = "x86_64")] +#[cfg(all(target_arch = "x86_64", target_pointer_width = "64"))] rustc_data_structures::static_assert_size!(TokenStream, 8); #[derive(Clone, Copy, Debug, PartialEq, Encodable, Decodable)] diff --git a/compiler/rustc_errors/src/lib.rs b/compiler/rustc_errors/src/lib.rs index a0be7442d597a..1db39fbfba502 100644 --- a/compiler/rustc_errors/src/lib.rs +++ b/compiler/rustc_errors/src/lib.rs @@ -52,7 +52,7 @@ pub type PResult<'a, T> = Result>; // `PResult` is used a lot. Make sure it doesn't unintentionally get bigger. // (See also the comment on `DiagnosticBuilderInner`.) -#[cfg(target_arch = "x86_64")] +#[cfg(all(target_arch = "x86_64", target_pointer_width = "64"))] rustc_data_structures::static_assert_size!(PResult<'_, bool>, 16); #[derive(Debug, PartialEq, Eq, Clone, Copy, Hash, Encodable, Decodable)] diff --git a/compiler/rustc_hir/src/hir.rs b/compiler/rustc_hir/src/hir.rs index 3cc501e423c0b..1938cdd1e46f4 100644 --- a/compiler/rustc_hir/src/hir.rs +++ b/compiler/rustc_hir/src/hir.rs @@ -3088,7 +3088,7 @@ impl<'hir> Node<'hir> { } // Some nodes are used a lot. Make sure they don't unintentionally get bigger. -#[cfg(target_arch = "x86_64")] +#[cfg(all(target_arch = "x86_64", target_pointer_width = "64"))] mod size_asserts { rustc_data_structures::static_assert_size!(super::Block<'static>, 48); rustc_data_structures::static_assert_size!(super::Expr<'static>, 72); diff --git a/compiler/rustc_infer/src/infer/mod.rs b/compiler/rustc_infer/src/infer/mod.rs index 09eecd715f03b..3df58cb7857f1 100644 --- a/compiler/rustc_infer/src/infer/mod.rs +++ b/compiler/rustc_infer/src/infer/mod.rs @@ -408,7 +408,7 @@ pub enum SubregionOrigin<'tcx> { } // `SubregionOrigin` is used a lot. Make sure it doesn't unintentionally get bigger. -#[cfg(target_arch = "x86_64")] +#[cfg(all(target_arch = "x86_64", target_pointer_width = "64"))] static_assert_size!(SubregionOrigin<'_>, 32); /// Times when we replace late-bound regions with variables: diff --git a/compiler/rustc_infer/src/lib.rs b/compiler/rustc_infer/src/lib.rs index 3690a88c0d973..f9170ef5dc346 100644 --- a/compiler/rustc_infer/src/lib.rs +++ b/compiler/rustc_infer/src/lib.rs @@ -27,7 +27,7 @@ #[macro_use] extern crate rustc_macros; -#[cfg(target_arch = "x86_64")] +#[cfg(all(target_arch = "x86_64", target_pointer_width = "64"))] #[macro_use] extern crate rustc_data_structures; #[macro_use] diff --git a/compiler/rustc_infer/src/traits/mod.rs b/compiler/rustc_infer/src/traits/mod.rs index aaf5e958c269d..0882d682e1537 100644 --- a/compiler/rustc_infer/src/traits/mod.rs +++ b/compiler/rustc_infer/src/traits/mod.rs @@ -56,7 +56,7 @@ pub type PredicateObligation<'tcx> = Obligation<'tcx, ty::Predicate<'tcx>>; pub type TraitObligation<'tcx> = Obligation<'tcx, ty::PolyTraitPredicate<'tcx>>; // `PredicateObligation` is used a lot. Make sure it doesn't unintentionally get bigger. -#[cfg(target_arch = "x86_64")] +#[cfg(all(target_arch = "x86_64", target_pointer_width = "64"))] static_assert_size!(PredicateObligation<'_>, 32); pub type PredicateObligations<'tcx> = Vec>; diff --git a/compiler/rustc_middle/src/mir/interpret/error.rs b/compiler/rustc_middle/src/mir/interpret/error.rs index 1589ab28e4043..b2b969e9b34e8 100644 --- a/compiler/rustc_middle/src/mir/interpret/error.rs +++ b/compiler/rustc_middle/src/mir/interpret/error.rs @@ -40,7 +40,7 @@ pub fn struct_error<'tcx>(tcx: TyCtxtAt<'tcx>, msg: &str) -> DiagnosticBuilder<' struct_span_err!(tcx.sess, tcx.span, E0080, "{}", msg) } -#[cfg(target_arch = "x86_64")] +#[cfg(all(target_arch = "x86_64", target_pointer_width = "64"))] static_assert_size!(InterpErrorInfo<'_>, 8); /// Packages the kind of error we got from the const code interpreter @@ -444,7 +444,7 @@ impl dyn MachineStopType { } } -#[cfg(target_arch = "x86_64")] +#[cfg(all(target_arch = "x86_64", target_pointer_width = "64"))] static_assert_size!(InterpError<'_>, 72); pub enum InterpError<'tcx> { diff --git a/compiler/rustc_middle/src/mir/interpret/value.rs b/compiler/rustc_middle/src/mir/interpret/value.rs index 4bb39fe4a527e..a07ccd4d2b528 100644 --- a/compiler/rustc_middle/src/mir/interpret/value.rs +++ b/compiler/rustc_middle/src/mir/interpret/value.rs @@ -44,7 +44,7 @@ pub enum ConstValue<'tcx> { }, } -#[cfg(target_arch = "x86_64")] +#[cfg(all(target_arch = "x86_64", target_pointer_width = "64"))] static_assert_size!(ConstValue<'_>, 32); impl<'tcx> ConstValue<'tcx> { @@ -111,7 +111,7 @@ pub enum Scalar { Ptr(Pointer), } -#[cfg(target_arch = "x86_64")] +#[cfg(all(target_arch = "x86_64", target_pointer_width = "64"))] static_assert_size!(Scalar, 24); // We want the `Debug` output to be readable as it is used by `derive(Debug)` for @@ -509,7 +509,7 @@ pub enum ScalarMaybeUninit { Uninit, } -#[cfg(target_arch = "x86_64")] +#[cfg(all(target_arch = "x86_64", target_pointer_width = "64"))] static_assert_size!(ScalarMaybeUninit, 24); impl From> for ScalarMaybeUninit { diff --git a/compiler/rustc_middle/src/mir/mod.rs b/compiler/rustc_middle/src/mir/mod.rs index 4d8615a215fc0..17117c5d85c2f 100644 --- a/compiler/rustc_middle/src/mir/mod.rs +++ b/compiler/rustc_middle/src/mir/mod.rs @@ -951,7 +951,7 @@ pub struct LocalDecl<'tcx> { } // `LocalDecl` is used a lot. Make sure it doesn't unintentionally get bigger. -#[cfg(target_arch = "x86_64")] +#[cfg(all(target_arch = "x86_64", target_pointer_width = "64"))] static_assert_size!(LocalDecl<'_>, 56); /// Extra information about a some locals that's used for diagnostics and for @@ -1468,7 +1468,7 @@ pub struct Statement<'tcx> { } // `Statement` is used a lot. Make sure it doesn't unintentionally get bigger. -#[cfg(target_arch = "x86_64")] +#[cfg(all(target_arch = "x86_64", target_pointer_width = "64"))] static_assert_size!(Statement<'_>, 32); impl Statement<'_> { @@ -1752,7 +1752,7 @@ impl ProjectionElem { pub type PlaceElem<'tcx> = ProjectionElem>; // At least on 64 bit systems, `PlaceElem` should not be larger than two pointers. -#[cfg(target_arch = "x86_64")] +#[cfg(all(target_arch = "x86_64", target_pointer_width = "64"))] static_assert_size!(PlaceElem<'_>, 24); /// Alias for projections as they appear in `UserTypeProjection`, where we diff --git a/compiler/rustc_middle/src/mir/tcx.rs b/compiler/rustc_middle/src/mir/tcx.rs index 92f46ef6a638c..0682ab62d0881 100644 --- a/compiler/rustc_middle/src/mir/tcx.rs +++ b/compiler/rustc_middle/src/mir/tcx.rs @@ -17,7 +17,7 @@ pub struct PlaceTy<'tcx> { } // At least on 64 bit systems, `PlaceTy` should not be larger than two or three pointers. -#[cfg(target_arch = "x86_64")] +#[cfg(all(target_arch = "x86_64", target_pointer_width = "64"))] static_assert_size!(PlaceTy<'_>, 16); impl<'tcx> PlaceTy<'tcx> { diff --git a/compiler/rustc_middle/src/traits/mod.rs b/compiler/rustc_middle/src/traits/mod.rs index 9deeaf462d65d..0bd0a701fb2e7 100644 --- a/compiler/rustc_middle/src/traits/mod.rs +++ b/compiler/rustc_middle/src/traits/mod.rs @@ -340,7 +340,7 @@ impl ObligationCauseCode<'_> { } // `ObligationCauseCode` is used a lot. Make sure it doesn't unintentionally get bigger. -#[cfg(target_arch = "x86_64")] +#[cfg(all(target_arch = "x86_64", target_pointer_width = "64"))] static_assert_size!(ObligationCauseCode<'_>, 32); #[derive(Copy, Clone, Debug, PartialEq, Eq, Hash)] diff --git a/compiler/rustc_middle/src/ty/consts.rs b/compiler/rustc_middle/src/ty/consts.rs index ed953b981130a..e7b2c9efd63cf 100644 --- a/compiler/rustc_middle/src/ty/consts.rs +++ b/compiler/rustc_middle/src/ty/consts.rs @@ -23,7 +23,7 @@ pub struct Const<'tcx> { pub val: ConstKind<'tcx>, } -#[cfg(target_arch = "x86_64")] +#[cfg(all(target_arch = "x86_64", target_pointer_width = "64"))] static_assert_size!(Const<'_>, 48); impl<'tcx> Const<'tcx> { diff --git a/compiler/rustc_middle/src/ty/consts/kind.rs b/compiler/rustc_middle/src/ty/consts/kind.rs index a2638d8bddad0..98c215407a8e0 100644 --- a/compiler/rustc_middle/src/ty/consts/kind.rs +++ b/compiler/rustc_middle/src/ty/consts/kind.rs @@ -37,7 +37,7 @@ pub enum ConstKind<'tcx> { Error(ty::DelaySpanBugEmitted), } -#[cfg(target_arch = "x86_64")] +#[cfg(all(target_arch = "x86_64", target_pointer_width = "64"))] static_assert_size!(ConstKind<'_>, 40); impl<'tcx> ConstKind<'tcx> { diff --git a/compiler/rustc_middle/src/ty/mod.rs b/compiler/rustc_middle/src/ty/mod.rs index c27a337554e67..6b9186d476b69 100644 --- a/compiler/rustc_middle/src/ty/mod.rs +++ b/compiler/rustc_middle/src/ty/mod.rs @@ -483,7 +483,7 @@ impl<'tcx> TyS<'tcx> { } // `TyS` is used a lot. Make sure it doesn't unintentionally get bigger. -#[cfg(target_arch = "x86_64")] +#[cfg(all(target_arch = "x86_64", target_pointer_width = "64"))] static_assert_size!(TyS<'_>, 32); impl<'tcx> Ord for TyS<'tcx> { @@ -1030,7 +1030,7 @@ crate struct PredicateInner<'tcx> { outer_exclusive_binder: ty::DebruijnIndex, } -#[cfg(target_arch = "x86_64")] +#[cfg(all(target_arch = "x86_64", target_pointer_width = "64"))] static_assert_size!(PredicateInner<'_>, 40); #[derive(Clone, Copy, Lift)] diff --git a/compiler/rustc_middle/src/ty/sty.rs b/compiler/rustc_middle/src/ty/sty.rs index 6c074d3af5c4b..2cd969d7a16ab 100644 --- a/compiler/rustc_middle/src/ty/sty.rs +++ b/compiler/rustc_middle/src/ty/sty.rs @@ -231,7 +231,7 @@ impl TyKind<'tcx> { } // `TyKind` is used a lot. Make sure it doesn't unintentionally get bigger. -#[cfg(target_arch = "x86_64")] +#[cfg(all(target_arch = "x86_64", target_pointer_width = "64"))] static_assert_size!(TyKind<'_>, 24); /// A closure can be modeled as a struct that looks like: diff --git a/compiler/rustc_mir/src/interpret/operand.rs b/compiler/rustc_mir/src/interpret/operand.rs index 901ed6809f24f..2c4aba19e4a2e 100644 --- a/compiler/rustc_mir/src/interpret/operand.rs +++ b/compiler/rustc_mir/src/interpret/operand.rs @@ -32,7 +32,7 @@ pub enum Immediate { ScalarPair(ScalarMaybeUninit, ScalarMaybeUninit), } -#[cfg(target_arch = "x86_64")] +#[cfg(all(target_arch = "x86_64", target_pointer_width = "64"))] rustc_data_structures::static_assert_size!(Immediate, 56); impl From> for Immediate { @@ -95,7 +95,7 @@ pub struct ImmTy<'tcx, Tag = ()> { pub layout: TyAndLayout<'tcx>, } -#[cfg(target_arch = "x86_64")] +#[cfg(all(target_arch = "x86_64", target_pointer_width = "64"))] rustc_data_structures::static_assert_size!(ImmTy<'_>, 72); impl std::fmt::Display for ImmTy<'tcx, Tag> { @@ -162,7 +162,7 @@ pub struct OpTy<'tcx, Tag = ()> { pub layout: TyAndLayout<'tcx>, } -#[cfg(target_arch = "x86_64")] +#[cfg(all(target_arch = "x86_64", target_pointer_width = "64"))] rustc_data_structures::static_assert_size!(OpTy<'_, ()>, 80); impl<'tcx, Tag> std::ops::Deref for OpTy<'tcx, Tag> { diff --git a/compiler/rustc_mir/src/interpret/place.rs b/compiler/rustc_mir/src/interpret/place.rs index 392f739e84fd6..7ba79e6f75989 100644 --- a/compiler/rustc_mir/src/interpret/place.rs +++ b/compiler/rustc_mir/src/interpret/place.rs @@ -33,7 +33,7 @@ pub enum MemPlaceMeta { Poison, } -#[cfg(target_arch = "x86_64")] +#[cfg(all(target_arch = "x86_64", target_pointer_width = "64"))] rustc_data_structures::static_assert_size!(MemPlaceMeta, 24); impl MemPlaceMeta { @@ -74,7 +74,7 @@ pub struct MemPlace { pub meta: MemPlaceMeta, } -#[cfg(target_arch = "x86_64")] +#[cfg(all(target_arch = "x86_64", target_pointer_width = "64"))] rustc_data_structures::static_assert_size!(MemPlace, 56); #[derive(Copy, Clone, Debug, Hash, PartialEq, Eq, HashStable)] @@ -87,7 +87,7 @@ pub enum Place { Local { frame: usize, local: mir::Local }, } -#[cfg(target_arch = "x86_64")] +#[cfg(all(target_arch = "x86_64", target_pointer_width = "64"))] rustc_data_structures::static_assert_size!(Place, 64); #[derive(Copy, Clone, Debug)] @@ -96,7 +96,7 @@ pub struct PlaceTy<'tcx, Tag = ()> { pub layout: TyAndLayout<'tcx>, } -#[cfg(target_arch = "x86_64")] +#[cfg(all(target_arch = "x86_64", target_pointer_width = "64"))] rustc_data_structures::static_assert_size!(PlaceTy<'_>, 80); impl<'tcx, Tag> std::ops::Deref for PlaceTy<'tcx, Tag> { @@ -114,7 +114,7 @@ pub struct MPlaceTy<'tcx, Tag = ()> { pub layout: TyAndLayout<'tcx>, } -#[cfg(target_arch = "x86_64")] +#[cfg(all(target_arch = "x86_64", target_pointer_width = "64"))] rustc_data_structures::static_assert_size!(MPlaceTy<'_>, 72); impl<'tcx, Tag> std::ops::Deref for MPlaceTy<'tcx, Tag> { diff --git a/compiler/rustc_mir_build/src/thir/mod.rs b/compiler/rustc_mir_build/src/thir/mod.rs index ed3d3927825af..ce0098fdf860b 100644 --- a/compiler/rustc_mir_build/src/thir/mod.rs +++ b/compiler/rustc_mir_build/src/thir/mod.rs @@ -96,7 +96,7 @@ crate enum StmtKind<'tcx> { } // `Expr` is used a lot. Make sure it doesn't unintentionally get bigger. -#[cfg(target_arch = "x86_64")] +#[cfg(all(target_arch = "x86_64", target_pointer_width = "64"))] rustc_data_structures::static_assert_size!(Expr<'_>, 168); /// The Thir trait implementor lowers their expressions (`&'tcx H::Expr`) diff --git a/compiler/rustc_trait_selection/src/lib.rs b/compiler/rustc_trait_selection/src/lib.rs index e1f8d59991f21..aea3d8eef65d0 100644 --- a/compiler/rustc_trait_selection/src/lib.rs +++ b/compiler/rustc_trait_selection/src/lib.rs @@ -23,7 +23,7 @@ #[macro_use] extern crate rustc_macros; -#[cfg(target_arch = "x86_64")] +#[cfg(all(target_arch = "x86_64", target_pointer_width = "64"))] #[macro_use] extern crate rustc_data_structures; #[macro_use] diff --git a/compiler/rustc_trait_selection/src/traits/fulfill.rs b/compiler/rustc_trait_selection/src/traits/fulfill.rs index 3c447a7d1f9bb..7d451fc234106 100644 --- a/compiler/rustc_trait_selection/src/traits/fulfill.rs +++ b/compiler/rustc_trait_selection/src/traits/fulfill.rs @@ -87,7 +87,7 @@ pub struct PendingPredicateObligation<'tcx> { } // `PendingPredicateObligation` is used a lot. Make sure it doesn't unintentionally get bigger. -#[cfg(target_arch = "x86_64")] +#[cfg(all(target_arch = "x86_64", target_pointer_width = "64"))] static_assert_size!(PendingPredicateObligation<'_>, 56); impl<'a, 'tcx> FulfillmentContext<'tcx> { diff --git a/src/librustdoc/clean/types.rs b/src/librustdoc/clean/types.rs index e560843383ec5..33cc26410363a 100644 --- a/src/librustdoc/clean/types.rs +++ b/src/librustdoc/clean/types.rs @@ -93,7 +93,7 @@ crate struct Item { } // `Item` is used a lot. Make sure it doesn't unintentionally get bigger. -#[cfg(target_arch = "x86_64")] +#[cfg(all(target_arch = "x86_64", target_pointer_width = "64"))] rustc_data_structures::static_assert_size!(Item, 48); impl fmt::Debug for Item { From 402a00a15db6f9da3979839d55d0d8c23d777892 Mon Sep 17 00:00:00 2001 From: mark Date: Mon, 8 Mar 2021 12:41:28 -0600 Subject: [PATCH 17/19] clippy: fix or-pattern in let binding --- src/tools/clippy/clippy_lints/src/loops.rs | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/tools/clippy/clippy_lints/src/loops.rs b/src/tools/clippy/clippy_lints/src/loops.rs index 1c9373a756c88..a87d5b9d37309 100644 --- a/src/tools/clippy/clippy_lints/src/loops.rs +++ b/src/tools/clippy/clippy_lints/src/loops.rs @@ -885,7 +885,9 @@ struct MinifyingSugg<'a>(Sugg<'a>); impl<'a> MinifyingSugg<'a> { fn as_str(&self) -> &str { - let Sugg::NonParen(s) | Sugg::MaybeParen(s) | Sugg::BinOp(_, s) = &self.0; + let s = match &self.0 { + Sugg::NonParen(s) | Sugg::MaybeParen(s) | Sugg::BinOp(_, s) => s, + }; s.as_ref() } From 469c03026075181a62e5bd07afdd11e67002fbb4 Mon Sep 17 00:00:00 2001 From: Henry Boisdequin <65845077+henryboisdequin@users.noreply.github.com> Date: Mon, 8 Mar 2021 11:26:05 +0530 Subject: [PATCH 18/19] Update CONTRIBUTING.md Fixes #77215 As mentioned in #77215, the current CONTRIBUTING.md links to the rustc-dev-guide. Even though the rustc-dev-guide has lots of useful information for contributors, one is already confused by reading the first line of the current CONTRIBUTING.md. > To get started, read the [Getting Started] guide in the [rustc-dev-guide]. This line tells the contributor to go and read the rustc-dev-guide. What is the rustc-dev-guide? What does rustc even mean? These are some of the questions that went into my head when reading this line as a first time contributor. By explaining what the rustc-dev-guide is and some platforms to get help, a new contributor understands what the first step is and the process is much clearer. The `About the [rustc-dev-guide]` section explains what the rustc-dev-guide is, what rustc is, and the purpose out of reading the guide. The `Getting help` section points the user to some places where they can get help, find a mentor, and introduce themsevles. --- CONTRIBUTING.md | 22 +++++++++++++++++++++- 1 file changed, 21 insertions(+), 1 deletion(-) diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index b600074c19770..4bceffd548de6 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -1,9 +1,26 @@ # Contributing to Rust -Thank you for your interest in contributing to Rust! +Thank you for your interest in contributing to Rust! There are many ways to contribute +and we appreciate all of them. To get started, read the [Contributing to Rust] chapter of the [rustc-dev-guide]. +## About the [rustc-dev-guide] + +The [rustc-dev-guide] is meant to help document how rustc –the Rust compiler– works, +as well as to help new contributors get involved in rustc development. It is recommend +to read and understand the [rustc-dev-guide] before making a contribution. This guide +talks about the different bots in the Rust ecosystem, the Rust development tools, +bootstrapping, the compiler architecture, source code representation, and more. + +## [Getting help](https://rustc-dev-guide.rust-lang.org/getting-started.html#asking-questions) + +There are many ways you can get help when you're stuck. Rust has many platforms for this: +[internals], [rust-zulip], and [rust-discord]. It is recommended to ask for help on +the [rust-zulip], but any of these platforms are a great way to seek help and even +find a mentor! You can learn more about asking questions and getting help in the +[Asking Questions](https://rustc-dev-guide.rust-lang.org/getting-started.html#asking-questions) chapter of the [rustc-dev-guide]. + ## Bug reports Did a compiler error message tell you to come here? If you want to create an ICE report, @@ -13,3 +30,6 @@ refer to [this section][contributing-bug-reports] and [open an issue][issue temp [rustc-dev-guide]: https://rustc-dev-guide.rust-lang.org/ [contributing-bug-reports]: https://rustc-dev-guide.rust-lang.org/contributing.html#bug-reports [issue template]: https://github.com/rust-lang/rust/issues/new/choose +[internals]: https://internals.rust-lang.org +[rust-discord]: http://discord.gg/rust-lang +[rust-zulip]: https://rust-lang.zulipchat.com From bf40ac68777870bbb0336d51f06082f288262dcd Mon Sep 17 00:00:00 2001 From: Henry Boisdequin Date: Tue, 9 Mar 2021 08:25:41 +0530 Subject: [PATCH 19/19] Make opening sentence friendlier for new contributors Co-authored-by: Camelid --- CONTRIBUTING.md | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 4bceffd548de6..2827a46ae6f73 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -3,7 +3,13 @@ Thank you for your interest in contributing to Rust! There are many ways to contribute and we appreciate all of them. -To get started, read the [Contributing to Rust] chapter of the [rustc-dev-guide]. +Documentation for contributing to Rust is located in the [Guide to Rustc Development](https://rustc-dev-guide.rust-lang.org/), +commonly known as the [rustc-dev-guide]. Despite the name, this guide documents +not just how to develop rustc (the Rust compiler), but also how to contribute to any part +of the Rust project. + +To get started with contributing, please read the [Contributing to Rust] chapter of the guide. +That chapter explains how to get your development environment set up and how to get help. ## About the [rustc-dev-guide]