From 077a8219b07b72d8cd0485ffba32e7147e7f7f7d Mon Sep 17 00:00:00 2001 From: Amanda Stjerna Date: Tue, 28 May 2024 12:57:26 +0200 Subject: [PATCH 01/15] Fix back-porting drop-livess from Polonius to tracing --- .../src/type_check/liveness/polonius.rs | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-) diff --git a/compiler/rustc_borrowck/src/type_check/liveness/polonius.rs b/compiler/rustc_borrowck/src/type_check/liveness/polonius.rs index ccfa9f12ef45..808df1d66cb1 100644 --- a/compiler/rustc_borrowck/src/type_check/liveness/polonius.rs +++ b/compiler/rustc_borrowck/src/type_check/liveness/polonius.rs @@ -14,7 +14,7 @@ struct UseFactsExtractor<'me, 'tcx> { var_defined_at: &'me mut VarPointRelation, var_used_at: &'me mut VarPointRelation, location_table: &'me LocationTable, - var_dropped_at: &'me mut VarPointRelation, + var_dropped_at: &'me mut Vec<(Local, Location)>, move_data: &'me MoveData<'tcx>, path_accessed_at_base: &'me mut PathPointRelation, } @@ -37,7 +37,7 @@ fn insert_use(&mut self, local: Local, location: Location) { fn insert_drop_use(&mut self, local: Local, location: Location) { debug!("UseFactsExtractor::insert_drop_use()"); - self.var_dropped_at.push((local, self.location_to_index(location))); + self.var_dropped_at.push((local, location)); } fn insert_path_access(&mut self, path: MovePathIndex, location: Location) { @@ -87,8 +87,12 @@ pub(super) fn populate_access_facts<'a, 'tcx>( body: &Body<'tcx>, location_table: &LocationTable, move_data: &MoveData<'tcx>, - //FIXME: this is not mutated, but expected to be modified as - // out param, bug? + // FIXME: this is an inelegant way of squirreling away a + // copy of `var_dropped_at` in the original `Location` format + // for later use in `trace::trace()`, which updates some liveness- + // internal data based on what Polonius saw. + // Ideally, that part would access the Polonius facts directly, and this + // would be regular facts gathering. dropped_at: &mut Vec<(Local, Location)>, ) { debug!("populate_access_facts()"); @@ -97,7 +101,7 @@ pub(super) fn populate_access_facts<'a, 'tcx>( let mut extractor = UseFactsExtractor { var_defined_at: &mut facts.var_defined_at, var_used_at: &mut facts.var_used_at, - var_dropped_at: &mut facts.var_dropped_at, + var_dropped_at: dropped_at, path_accessed_at_base: &mut facts.path_accessed_at_base, location_table, move_data, From e15564672e4e35a8697b717d49db1bfc6579c612 Mon Sep 17 00:00:00 2001 From: Amanda Stjerna Date: Tue, 28 May 2024 15:49:22 +0200 Subject: [PATCH 02/15] Make drop-use fact collection simpler for `polonius` This shunts all the complexity of siphoning off the drop-use facts into `LivenessResults::add_extra_drop_facts()`, which may or may not be a good approach. --- .../src/type_check/liveness/mod.rs | 11 ++---- .../src/type_check/liveness/polonius.rs | 19 +++------- .../src/type_check/liveness/trace.rs | 35 ++++++++++++++----- compiler/rustc_borrowck/src/type_check/mod.rs | 10 +----- 4 files changed, 34 insertions(+), 41 deletions(-) diff --git a/compiler/rustc_borrowck/src/type_check/liveness/mod.rs b/compiler/rustc_borrowck/src/type_check/liveness/mod.rs index 38ec9f7678eb..8b863efad6ca 100644 --- a/compiler/rustc_borrowck/src/type_check/liveness/mod.rs +++ b/compiler/rustc_borrowck/src/type_check/liveness/mod.rs @@ -14,7 +14,6 @@ use crate::{ constraints::OutlivesConstraintSet, facts::{AllFacts, AllFactsExt}, - location::LocationTable, region_infer::values::LivenessValues, universal_regions::UniversalRegions, }; @@ -39,7 +38,6 @@ pub(super) fn generate<'mir, 'tcx>( elements: &Rc, flow_inits: &mut ResultsCursor<'mir, 'tcx, MaybeInitializedPlaces<'mir, 'tcx>>, move_data: &MoveData<'tcx>, - location_table: &LocationTable, use_polonius: bool, ) { debug!("liveness::generate"); @@ -53,11 +51,9 @@ pub(super) fn generate<'mir, 'tcx>( compute_relevant_live_locals(typeck.tcx(), &free_regions, body); let facts_enabled = use_polonius || AllFacts::enabled(typeck.tcx()); - let polonius_drop_used = facts_enabled.then(|| { - let mut drop_used = Vec::new(); - polonius::populate_access_facts(typeck, body, location_table, move_data, &mut drop_used); - drop_used - }); + if facts_enabled { + polonius::populate_access_facts(typeck, body, move_data); + }; trace::trace( typeck, @@ -67,7 +63,6 @@ pub(super) fn generate<'mir, 'tcx>( move_data, relevant_live_locals, boring_locals, - polonius_drop_used, ); // Mark regions that should be live where they appear within rvalues or within a call: like diff --git a/compiler/rustc_borrowck/src/type_check/liveness/polonius.rs b/compiler/rustc_borrowck/src/type_check/liveness/polonius.rs index 808df1d66cb1..d8f03a07a63c 100644 --- a/compiler/rustc_borrowck/src/type_check/liveness/polonius.rs +++ b/compiler/rustc_borrowck/src/type_check/liveness/polonius.rs @@ -14,7 +14,7 @@ struct UseFactsExtractor<'me, 'tcx> { var_defined_at: &'me mut VarPointRelation, var_used_at: &'me mut VarPointRelation, location_table: &'me LocationTable, - var_dropped_at: &'me mut Vec<(Local, Location)>, + var_dropped_at: &'me mut VarPointRelation, move_data: &'me MoveData<'tcx>, path_accessed_at_base: &'me mut PathPointRelation, } @@ -37,7 +37,7 @@ fn insert_use(&mut self, local: Local, location: Location) { fn insert_drop_use(&mut self, local: Local, location: Location) { debug!("UseFactsExtractor::insert_drop_use()"); - self.var_dropped_at.push((local, location)); + self.var_dropped_at.push((local, self.location_to_index(location))); } fn insert_path_access(&mut self, path: MovePathIndex, location: Location) { @@ -85,33 +85,22 @@ fn visit_place(&mut self, place: &Place<'tcx>, context: PlaceContext, location: pub(super) fn populate_access_facts<'a, 'tcx>( typeck: &mut TypeChecker<'a, 'tcx>, body: &Body<'tcx>, - location_table: &LocationTable, move_data: &MoveData<'tcx>, - // FIXME: this is an inelegant way of squirreling away a - // copy of `var_dropped_at` in the original `Location` format - // for later use in `trace::trace()`, which updates some liveness- - // internal data based on what Polonius saw. - // Ideally, that part would access the Polonius facts directly, and this - // would be regular facts gathering. - dropped_at: &mut Vec<(Local, Location)>, ) { debug!("populate_access_facts()"); + let location_table = typeck.borrowck_context.location_table; if let Some(facts) = typeck.borrowck_context.all_facts.as_mut() { let mut extractor = UseFactsExtractor { var_defined_at: &mut facts.var_defined_at, var_used_at: &mut facts.var_used_at, - var_dropped_at: dropped_at, + var_dropped_at: &mut facts.var_dropped_at, path_accessed_at_base: &mut facts.path_accessed_at_base, location_table, move_data, }; extractor.visit_body(body); - facts.var_dropped_at.extend( - dropped_at.iter().map(|&(local, location)| (local, location_table.mid_index(location))), - ); - for (local, local_decl) in body.local_decls.iter_enumerated() { debug!( "add use_of_var_derefs_origin facts - local={:?}, type={:?}", diff --git a/compiler/rustc_borrowck/src/type_check/liveness/trace.rs b/compiler/rustc_borrowck/src/type_check/liveness/trace.rs index 6cc0e67c0f80..92b70a09a885 100644 --- a/compiler/rustc_borrowck/src/type_check/liveness/trace.rs +++ b/compiler/rustc_borrowck/src/type_check/liveness/trace.rs @@ -16,6 +16,7 @@ use rustc_mir_dataflow::move_paths::{HasMoveData, MoveData, MovePathIndex}; use rustc_mir_dataflow::ResultsCursor; +use crate::location::RichLocation; use crate::{ region_infer::values::{self, LiveLoans}, type_check::liveness::local_use_map::LocalUseMap, @@ -46,7 +47,6 @@ pub(super) fn trace<'mir, 'tcx>( move_data: &MoveData<'tcx>, relevant_live_locals: Vec, boring_locals: Vec, - polonius_drop_used: Option>, ) { let local_use_map = &LocalUseMap::build(&relevant_live_locals, elements, body); @@ -81,6 +81,8 @@ pub(super) fn trace<'mir, 'tcx>( borrowck_context.constraints.liveness_constraints.loans = Some(live_loans); }; + let polonius_facts_gathered = typeck.borrowck_context.all_facts.is_some(); + let cx = LivenessContext { typeck, body, @@ -93,8 +95,8 @@ pub(super) fn trace<'mir, 'tcx>( let mut results = LivenessResults::new(cx); - if let Some(drop_used) = polonius_drop_used { - results.add_extra_drop_facts(drop_used, relevant_live_locals.iter().copied().collect()) + if polonius_facts_gathered { + results.add_extra_drop_facts(relevant_live_locals.iter().copied().collect()); } results.compute_for_all_locals(relevant_live_locals); @@ -218,17 +220,32 @@ fn dropck_boring_locals(&mut self, boring_locals: Vec) { /// /// Add facts for all locals with free regions, since regions may outlive /// the function body only at certain nodes in the CFG. - fn add_extra_drop_facts( - &mut self, - drop_used: Vec<(Local, Location)>, - relevant_live_locals: FxIndexSet, - ) { + fn add_extra_drop_facts(&mut self, relevant_live_locals: FxIndexSet) { + let drop_used = self + .cx + .typeck + .borrowck_context + .all_facts + .as_ref() + .map(|facts| facts.var_dropped_at.clone()) + .into_iter() + .flatten(); let locations = IntervalSet::new(self.cx.elements.num_points()); - for (local, location) in drop_used { + for (local, location_index) in drop_used { if !relevant_live_locals.contains(&local) { let local_ty = self.cx.body.local_decls[local].ty; if local_ty.has_free_regions() { + let location = match self + .cx + .typeck + .borrowck_context + .location_table + .to_location(location_index) + { + RichLocation::Start(l) => l, + RichLocation::Mid(l) => l, + }; self.cx.add_drop_live_facts_for(local, local_ty, &[location], &locations); } } diff --git a/compiler/rustc_borrowck/src/type_check/mod.rs b/compiler/rustc_borrowck/src/type_check/mod.rs index 4e46a0c62c7c..a05fa967af03 100644 --- a/compiler/rustc_borrowck/src/type_check/mod.rs +++ b/compiler/rustc_borrowck/src/type_check/mod.rs @@ -188,15 +188,7 @@ pub(crate) fn type_check<'mir, 'tcx>( checker.equate_inputs_and_outputs(body, universal_regions, &normalized_inputs_and_output); checker.check_signature_annotation(body); - liveness::generate( - &mut checker, - body, - elements, - flow_inits, - move_data, - location_table, - use_polonius, - ); + liveness::generate(&mut checker, body, elements, flow_inits, move_data, use_polonius); translate_outlives_facts(&mut checker); let opaque_type_values = infcx.take_opaque_types(); From 8066ebc294f110a758fb2b44a68138db10d38ce0 Mon Sep 17 00:00:00 2001 From: Amanda Stjerna Date: Tue, 28 May 2024 16:02:09 +0200 Subject: [PATCH 03/15] Move the rest of the logic into `add_extra_drop_facts()` --- .../src/type_check/liveness/trace.rs | 16 +++++++--------- 1 file changed, 7 insertions(+), 9 deletions(-) diff --git a/compiler/rustc_borrowck/src/type_check/liveness/trace.rs b/compiler/rustc_borrowck/src/type_check/liveness/trace.rs index 92b70a09a885..50843c602cc8 100644 --- a/compiler/rustc_borrowck/src/type_check/liveness/trace.rs +++ b/compiler/rustc_borrowck/src/type_check/liveness/trace.rs @@ -81,8 +81,6 @@ pub(super) fn trace<'mir, 'tcx>( borrowck_context.constraints.liveness_constraints.loans = Some(live_loans); }; - let polonius_facts_gathered = typeck.borrowck_context.all_facts.is_some(); - let cx = LivenessContext { typeck, body, @@ -95,9 +93,7 @@ pub(super) fn trace<'mir, 'tcx>( let mut results = LivenessResults::new(cx); - if polonius_facts_gathered { - results.add_extra_drop_facts(relevant_live_locals.iter().copied().collect()); - } + results.add_extra_drop_facts(&relevant_live_locals); results.compute_for_all_locals(relevant_live_locals); @@ -220,16 +216,17 @@ fn dropck_boring_locals(&mut self, boring_locals: Vec) { /// /// Add facts for all locals with free regions, since regions may outlive /// the function body only at certain nodes in the CFG. - fn add_extra_drop_facts(&mut self, relevant_live_locals: FxIndexSet) { + fn add_extra_drop_facts(&mut self, relevant_live_locals: &[Local]) -> Option<()> { let drop_used = self .cx .typeck .borrowck_context .all_facts .as_ref() - .map(|facts| facts.var_dropped_at.clone()) - .into_iter() - .flatten(); + .map(|facts| facts.var_dropped_at.clone())?; + + let relevant_live_locals: FxIndexSet<_> = relevant_live_locals.iter().copied().collect(); + let locations = IntervalSet::new(self.cx.elements.num_points()); for (local, location_index) in drop_used { @@ -250,6 +247,7 @@ fn add_extra_drop_facts(&mut self, relevant_live_locals: FxIndexSet) { } } } + Some(()) } /// Clear the value of fields that are "per local variable". From dabd05bbab9df465a580858c3e03dc1797b12deb Mon Sep 17 00:00:00 2001 From: r0cky Date: Thu, 30 May 2024 09:51:27 +0800 Subject: [PATCH 04/15] Apply x clippy --fix and x fmt --- .../src/graph/dominators/mod.rs | 2 +- .../src/graph/iterate/mod.rs | 4 ++-- .../rustc_data_structures/src/graph/scc/mod.rs | 2 +- .../rustc_data_structures/src/profiling.rs | 2 +- .../rustc_data_structures/src/sorted_map.rs | 4 ++-- .../rustc_data_structures/src/sync/lock.rs | 2 +- compiler/rustc_parse_format/src/lib.rs | 18 ++++++++---------- compiler/rustc_serialize/src/serialize.rs | 2 +- compiler/stable_mir/src/mir/pretty.rs | 4 ++-- compiler/stable_mir/src/ty.rs | 6 +++--- library/proc_macro/src/bridge/fxhash.rs | 2 +- library/proc_macro/src/bridge/rpc.rs | 6 +++--- library/test/src/cli.rs | 2 +- library/test/src/term/terminfo/parm.rs | 2 +- 14 files changed, 28 insertions(+), 30 deletions(-) diff --git a/compiler/rustc_data_structures/src/graph/dominators/mod.rs b/compiler/rustc_data_structures/src/graph/dominators/mod.rs index 30e240cf85b8..d1d2de670b82 100644 --- a/compiler/rustc_data_structures/src/graph/dominators/mod.rs +++ b/compiler/rustc_data_structures/src/graph/dominators/mod.rs @@ -93,7 +93,7 @@ fn dominators_impl(graph: &G) -> Inner { // These are all done here rather than through one of the 'standard' // graph traversals to help make this fast. 'recurse: while let Some(frame) = stack.last_mut() { - while let Some(successor) = frame.iter.next() { + for successor in frame.iter.by_ref() { if real_to_pre_order[successor].is_none() { let pre_order_idx = pre_order_to_real.push(successor); real_to_pre_order[successor] = Some(pre_order_idx); diff --git a/compiler/rustc_data_structures/src/graph/iterate/mod.rs b/compiler/rustc_data_structures/src/graph/iterate/mod.rs index 78d05a6e1951..6fca57d32f77 100644 --- a/compiler/rustc_data_structures/src/graph/iterate/mod.rs +++ b/compiler/rustc_data_structures/src/graph/iterate/mod.rs @@ -48,7 +48,7 @@ struct PostOrderFrame { let node = frame.node; visited[node] = true; - while let Some(successor) = frame.iter.next() { + for successor in frame.iter.by_ref() { if !visited[successor] { stack.push(PostOrderFrame { node: successor, iter: graph.successors(successor) }); continue 'recurse; @@ -112,7 +112,7 @@ pub fn push_start_node(&mut self, start_node: G::Node) { /// This is equivalent to just invoke `next` repeatedly until /// you get a `None` result. pub fn complete_search(&mut self) { - while let Some(_) = self.next() {} + for _ in self.by_ref() {} } /// Returns true if node has been visited thus far. diff --git a/compiler/rustc_data_structures/src/graph/scc/mod.rs b/compiler/rustc_data_structures/src/graph/scc/mod.rs index 914a6a16348c..7f36e4ca16df 100644 --- a/compiler/rustc_data_structures/src/graph/scc/mod.rs +++ b/compiler/rustc_data_structures/src/graph/scc/mod.rs @@ -40,7 +40,7 @@ pub struct SccData { } impl Sccs { - pub fn new(graph: &(impl DirectedGraph + Successors)) -> Self { + pub fn new(graph: &impl Successors) -> Self { SccsConstruction::construct(graph) } diff --git a/compiler/rustc_data_structures/src/profiling.rs b/compiler/rustc_data_structures/src/profiling.rs index c6d51a5d6b4f..240f2671c3b2 100644 --- a/compiler/rustc_data_structures/src/profiling.rs +++ b/compiler/rustc_data_structures/src/profiling.rs @@ -562,7 +562,7 @@ pub fn new( // ASLR is disabled and the heap is otherwise deterministic. let pid: u32 = process::id(); let filename = format!("{crate_name}-{pid:07}.rustc_profile"); - let path = output_directory.join(&filename); + let path = output_directory.join(filename); let profiler = Profiler::with_counter(&path, measureme::counters::Counter::by_name(counter_name)?)?; diff --git a/compiler/rustc_data_structures/src/sorted_map.rs b/compiler/rustc_data_structures/src/sorted_map.rs index 21d7c91ec482..885f023122ab 100644 --- a/compiler/rustc_data_structures/src/sorted_map.rs +++ b/compiler/rustc_data_structures/src/sorted_map.rs @@ -125,13 +125,13 @@ pub fn iter(&self) -> std::slice::Iter<'_, (K, V)> { /// Iterate over the keys, sorted #[inline] - pub fn keys(&self) -> impl Iterator + ExactSizeIterator + DoubleEndedIterator { + pub fn keys(&self) -> impl ExactSizeIterator + DoubleEndedIterator { self.data.iter().map(|(k, _)| k) } /// Iterate over values, sorted by key #[inline] - pub fn values(&self) -> impl Iterator + ExactSizeIterator + DoubleEndedIterator { + pub fn values(&self) -> impl ExactSizeIterator + DoubleEndedIterator { self.data.iter().map(|(_, v)| v) } diff --git a/compiler/rustc_data_structures/src/sync/lock.rs b/compiler/rustc_data_structures/src/sync/lock.rs index 756984642c74..780be7739458 100644 --- a/compiler/rustc_data_structures/src/sync/lock.rs +++ b/compiler/rustc_data_structures/src/sync/lock.rs @@ -69,7 +69,7 @@ fn drop(&mut self) { match self.mode { Mode::NoSync => { let cell = unsafe { &self.lock.mode_union.no_sync }; - debug_assert_eq!(cell.get(), true); + debug_assert!(cell.get()); cell.set(false); } // SAFETY (unlock): We know that the lock is locked as this type is a proof of that. diff --git a/compiler/rustc_parse_format/src/lib.rs b/compiler/rustc_parse_format/src/lib.rs index faf6ca784670..ef71333d1c39 100644 --- a/compiler/rustc_parse_format/src/lib.rs +++ b/compiler/rustc_parse_format/src/lib.rs @@ -286,13 +286,11 @@ fn next(&mut self) -> Option> { lbrace_byte_pos.to(InnerOffset(rbrace_byte_pos.0 + width)), ); } - } else { - if let Some(&(_, maybe)) = self.cur.peek() { - match maybe { - '?' => self.suggest_format_debug(), - '<' | '^' | '>' => self.suggest_format_align(maybe), - _ => self.suggest_positional_arg_instead_of_captured_arg(arg), - } + } else if let Some(&(_, maybe)) = self.cur.peek() { + match maybe { + '?' => self.suggest_format_debug(), + '<' | '^' | '>' => self.suggest_format_align(maybe), + _ => self.suggest_positional_arg_instead_of_captured_arg(arg), } } Some(NextArgument(Box::new(arg))) @@ -1028,7 +1026,7 @@ fn find_width_map_from_snippet( if next_c == '{' { // consume up to 6 hexanumeric chars let digits_len = - s.clone().take(6).take_while(|(_, c)| c.is_digit(16)).count(); + s.clone().take(6).take_while(|(_, c)| c.is_ascii_hexdigit()).count(); let len_utf8 = s .as_str() @@ -1047,14 +1045,14 @@ fn find_width_map_from_snippet( width += required_skips + 2; s.nth(digits_len); - } else if next_c.is_digit(16) { + } else if next_c.is_ascii_hexdigit() { width += 1; // We suggest adding `{` and `}` when appropriate, accept it here as if // it were correct let mut i = 0; // consume up to 6 hexanumeric chars while let (Some((_, c)), _) = (s.next(), i < 6) { - if c.is_digit(16) { + if c.is_ascii_hexdigit() { width += 1; } else { break; diff --git a/compiler/rustc_serialize/src/serialize.rs b/compiler/rustc_serialize/src/serialize.rs index 412f7eced433..c84bb26735c5 100644 --- a/compiler/rustc_serialize/src/serialize.rs +++ b/compiler/rustc_serialize/src/serialize.rs @@ -252,7 +252,7 @@ fn encode(&self, _s: &mut S) {} } impl Decodable for () { - fn decode(_: &mut D) -> () {} + fn decode(_: &mut D) {} } impl Encodable for PhantomData { diff --git a/compiler/stable_mir/src/mir/pretty.rs b/compiler/stable_mir/src/mir/pretty.rs index bbca3965852a..580dc1a2b88f 100644 --- a/compiler/stable_mir/src/mir/pretty.rs +++ b/compiler/stable_mir/src/mir/pretty.rs @@ -156,7 +156,7 @@ fn pretty_terminator(writer: &mut W, terminator: &TerminatorKind) -> i fn pretty_terminator_head(writer: &mut W, terminator: &TerminatorKind) -> io::Result<()> { use self::TerminatorKind::*; - const INDENT: &'static str = " "; + const INDENT: &str = " "; match terminator { Goto { .. } => write!(writer, "{INDENT}goto"), SwitchInt { discr, .. } => { @@ -315,7 +315,7 @@ fn pretty_operand(operand: &Operand) -> String { } fn pretty_const(literal: &Const) -> String { - with(|cx| cx.const_pretty(&literal)) + with(|cx| cx.const_pretty(literal)) } fn pretty_rvalue(writer: &mut W, rval: &Rvalue) -> io::Result<()> { diff --git a/compiler/stable_mir/src/ty.rs b/compiler/stable_mir/src/ty.rs index d62054eff609..50bf0a5d74ef 100644 --- a/compiler/stable_mir/src/ty.rs +++ b/compiler/stable_mir/src/ty.rs @@ -526,7 +526,7 @@ pub enum IntTy { impl IntTy { pub fn num_bytes(self) -> usize { match self { - IntTy::Isize => crate::target::MachineInfo::target_pointer_width().bytes().into(), + IntTy::Isize => crate::target::MachineInfo::target_pointer_width().bytes(), IntTy::I8 => 1, IntTy::I16 => 2, IntTy::I32 => 4, @@ -549,7 +549,7 @@ pub enum UintTy { impl UintTy { pub fn num_bytes(self) -> usize { match self { - UintTy::Usize => crate::target::MachineInfo::target_pointer_width().bytes().into(), + UintTy::Usize => crate::target::MachineInfo::target_pointer_width().bytes(), UintTy::U8 => 1, UintTy::U16 => 2, UintTy::U32 => 4, @@ -1185,7 +1185,7 @@ pub fn read_bool(&self) -> Result { match self.read_int()? { 0 => Ok(false), 1 => Ok(true), - val @ _ => Err(error!("Unexpected value for bool: `{val}`")), + val => Err(error!("Unexpected value for bool: `{val}`")), } } diff --git a/library/proc_macro/src/bridge/fxhash.rs b/library/proc_macro/src/bridge/fxhash.rs index f4e905441972..f9f74c63fc4f 100644 --- a/library/proc_macro/src/bridge/fxhash.rs +++ b/library/proc_macro/src/bridge/fxhash.rs @@ -69,7 +69,7 @@ fn write(&mut self, mut bytes: &[u8]) { hash.add_to_hash(u16::from_ne_bytes(bytes[..2].try_into().unwrap()) as usize); bytes = &bytes[2..]; } - if (size_of::() > 1) && bytes.len() >= 1 { + if (size_of::() > 1) && !bytes.is_empty() { hash.add_to_hash(bytes[0] as usize); } self.hash = hash.hash; diff --git a/library/proc_macro/src/bridge/rpc.rs b/library/proc_macro/src/bridge/rpc.rs index 6d75a5a627c8..202a8e04543b 100644 --- a/library/proc_macro/src/bridge/rpc.rs +++ b/library/proc_macro/src/bridge/rpc.rs @@ -264,9 +264,9 @@ fn from(payload: Box) -> Self { } } -impl Into> for PanicMessage { - fn into(self) -> Box { - match self { +impl From for Box { + fn from(val: PanicMessage) -> Self { + match val { PanicMessage::StaticStr(s) => Box::new(s), PanicMessage::String(s) => Box::new(s), PanicMessage::Unknown => { diff --git a/library/test/src/cli.rs b/library/test/src/cli.rs index 6ac3b3eaa797..b7d24405b775 100644 --- a/library/test/src/cli.rs +++ b/library/test/src/cli.rs @@ -200,7 +200,7 @@ fn usage(binary: &str, options: &getopts::Options) { pub fn parse_opts(args: &[String]) -> Option { // Parse matches. let opts = optgroups(); - let binary = args.get(0).map(|c| &**c).unwrap_or("..."); + let binary = args.first().map(|c| &**c).unwrap_or("..."); let args = args.get(1..).unwrap_or(args); let matches = match opts.parse(args) { Ok(m) => m, diff --git a/library/test/src/term/terminfo/parm.rs b/library/test/src/term/terminfo/parm.rs index 2815f6cfc77f..c5b4ef01893c 100644 --- a/library/test/src/term/terminfo/parm.rs +++ b/library/test/src/term/terminfo/parm.rs @@ -524,7 +524,7 @@ fn format(val: Param, op: FormatOp, flags: Flags) -> Result, String> { } else { let mut s_ = Vec::with_capacity(flags.width); s_.extend(repeat(b' ').take(n)); - s_.extend(s.into_iter()); + s_.extend(s); s = s_; } } From fa563c138433583ebbd64c963666f6c9ddb53739 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Dorian=20P=C3=A9ron?= Date: Fri, 26 Apr 2024 13:01:06 +0000 Subject: [PATCH 05/15] coverage: Add CLI support for `-Zcoverage-options=condition` --- compiler/rustc_session/src/config.rs | 18 +++++++++++++++++- compiler/rustc_session/src/options.rs | 3 ++- compiler/rustc_session/src/session.rs | 5 +++++ .../src/compiler-flags/coverage-options.md | 7 +++++-- .../coverage-options.bad.stderr | 2 +- .../ui/instrument-coverage/coverage-options.rs | 5 ++++- 6 files changed, 34 insertions(+), 6 deletions(-) diff --git a/compiler/rustc_session/src/config.rs b/compiler/rustc_session/src/config.rs index 0287085ad60e..a622f1b577df 100644 --- a/compiler/rustc_session/src/config.rs +++ b/compiler/rustc_session/src/config.rs @@ -159,7 +159,23 @@ pub enum CoverageLevel { Block, /// Also instrument branch points (includes block coverage). Branch, - /// Instrument for MC/DC. Mostly a superset of branch coverage, but might + /// Same as branch coverage, but also adds branch instrumentation for + /// certain boolean expressions that are not directly used for branching. + /// + /// For example, in the following code, `b` does not directly participate + /// in a branch, but condition coverage will instrument it as its own + /// artificial branch: + /// ``` + /// # let (a, b) = (false, true); + /// let x = a && b; + /// // ^ last operand + /// ``` + /// + /// This level is mainly intended to be a stepping-stone towards full MC/DC + /// instrumentation, so it might be removed in the future when MC/DC is + /// sufficiently complete, or if it is making MC/DC changes difficult. + Condition, + /// Instrument for MC/DC. Mostly a superset of condition coverage, but might /// differ in some corner cases. Mcdc, } diff --git a/compiler/rustc_session/src/options.rs b/compiler/rustc_session/src/options.rs index cca9f1eb9978..fd4a3a9e6ceb 100644 --- a/compiler/rustc_session/src/options.rs +++ b/compiler/rustc_session/src/options.rs @@ -395,7 +395,7 @@ mod desc { pub const parse_optimization_fuel: &str = "crate=integer"; pub const parse_dump_mono_stats: &str = "`markdown` (default) or `json`"; pub const parse_instrument_coverage: &str = parse_bool; - pub const parse_coverage_options: &str = "`block` | `branch` | `mcdc`"; + pub const parse_coverage_options: &str = "`block` | `branch` | `condition` | `mcdc`"; pub const parse_instrument_xray: &str = "either a boolean (`yes`, `no`, `on`, `off`, etc), or a comma separated list of settings: `always` or `never` (mutually exclusive), `ignore-loops`, `instruction-threshold=N`, `skip-entry`, `skip-exit`"; pub const parse_unpretty: &str = "`string` or `string=string`"; pub const parse_treat_err_as_bug: &str = "either no value or a non-negative number"; @@ -961,6 +961,7 @@ pub(crate) fn parse_coverage_options(slot: &mut CoverageOptions, v: Option<&str> match option { "block" => slot.level = CoverageLevel::Block, "branch" => slot.level = CoverageLevel::Branch, + "condition" => slot.level = CoverageLevel::Condition, "mcdc" => slot.level = CoverageLevel::Mcdc, _ => return false, } diff --git a/compiler/rustc_session/src/session.rs b/compiler/rustc_session/src/session.rs index a5cf136db6a6..87bbfcf07c84 100644 --- a/compiler/rustc_session/src/session.rs +++ b/compiler/rustc_session/src/session.rs @@ -353,6 +353,11 @@ pub fn instrument_coverage_branch(&self) -> bool { && self.opts.unstable_opts.coverage_options.level >= CoverageLevel::Branch } + pub fn instrument_coverage_condition(&self) -> bool { + self.instrument_coverage() + && self.opts.unstable_opts.coverage_options.level >= CoverageLevel::Condition + } + pub fn instrument_coverage_mcdc(&self) -> bool { self.instrument_coverage() && self.opts.unstable_opts.coverage_options.level >= CoverageLevel::Mcdc diff --git a/src/doc/unstable-book/src/compiler-flags/coverage-options.md b/src/doc/unstable-book/src/compiler-flags/coverage-options.md index 212788335502..87a9a00b3c0f 100644 --- a/src/doc/unstable-book/src/compiler-flags/coverage-options.md +++ b/src/doc/unstable-book/src/compiler-flags/coverage-options.md @@ -5,13 +5,16 @@ This option controls details of the coverage instrumentation performed by Multiple options can be passed, separated by commas. Valid options are: -- `block`, `branch`, `mcdc`: +- `block`, `branch`, `condition`, `mcdc`: Sets the level of coverage instrumentation. Setting the level will override any previously-specified level. - `block` (default): Blocks in the control-flow graph will be instrumented for coverage. - `branch`: In addition to block coverage, also enables branch coverage instrumentation. + - `condition`: + In addition to branch coverage, also instruments some boolean expressions + as branches, even if they are not directly used as branch conditions. - `mcdc`: - In addition to block and branch coverage, also enables MC/DC instrumentation. + In addition to condition coverage, also enables MC/DC instrumentation. (Branch coverage instrumentation may differ in some cases.) diff --git a/tests/ui/instrument-coverage/coverage-options.bad.stderr b/tests/ui/instrument-coverage/coverage-options.bad.stderr index 9e2c19e90d59..4a272cf97fb0 100644 --- a/tests/ui/instrument-coverage/coverage-options.bad.stderr +++ b/tests/ui/instrument-coverage/coverage-options.bad.stderr @@ -1,2 +1,2 @@ -error: incorrect value `bad` for unstable option `coverage-options` - `block` | `branch` | `mcdc` was expected +error: incorrect value `bad` for unstable option `coverage-options` - `block` | `branch` | `condition` | `mcdc` was expected diff --git a/tests/ui/instrument-coverage/coverage-options.rs b/tests/ui/instrument-coverage/coverage-options.rs index 2a80ce4ab2e9..8f523a5fd11a 100644 --- a/tests/ui/instrument-coverage/coverage-options.rs +++ b/tests/ui/instrument-coverage/coverage-options.rs @@ -1,5 +1,5 @@ //@ needs-profiler-support -//@ revisions: block branch mcdc bad +//@ revisions: block branch condition mcdc bad //@ compile-flags -Cinstrument-coverage //@ [block] check-pass @@ -8,6 +8,9 @@ //@ [branch] check-pass //@ [branch] compile-flags: -Zcoverage-options=branch +//@ [condition] check-pass +//@ [condition] compile-flags: -Zcoverage-options=condition + //@ [mcdc] check-pass //@ [mcdc] compile-flags: -Zcoverage-options=mcdc From 20174e6638eb0d5ac0195706ce6cc7185949a407 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Dorian=20P=C3=A9ron?= Date: Mon, 29 Apr 2024 15:58:14 +0000 Subject: [PATCH 06/15] coverage: Add a test for `-Zcoverage-options=condition` --- tests/coverage/condition/conditions.cov-map | 117 +++++++++++++++++++ tests/coverage/condition/conditions.coverage | 90 ++++++++++++++ tests/coverage/condition/conditions.rs | 67 +++++++++++ 3 files changed, 274 insertions(+) create mode 100644 tests/coverage/condition/conditions.cov-map create mode 100644 tests/coverage/condition/conditions.coverage create mode 100644 tests/coverage/condition/conditions.rs diff --git a/tests/coverage/condition/conditions.cov-map b/tests/coverage/condition/conditions.cov-map new file mode 100644 index 000000000000..6c81f7bda985 --- /dev/null +++ b/tests/coverage/condition/conditions.cov-map @@ -0,0 +1,117 @@ +Function name: conditions::assign_3_and_or +Raw bytes (60): 0x[01, 01, 06, 0d, 13, 09, 16, 01, 05, 01, 05, 09, 16, 01, 05, 08, 01, 1c, 01, 00, 2f, 03, 01, 09, 00, 0a, 01, 00, 0d, 00, 0e, 20, 05, 16, 00, 0d, 00, 0e, 05, 00, 12, 00, 13, 20, 0d, 09, 00, 12, 00, 13, 13, 00, 17, 00, 18, 03, 01, 05, 01, 02] +Number of files: 1 +- file 0 => global file 1 +Number of expressions: 6 +- expression 0 operands: lhs = Counter(3), rhs = Expression(4, Add) +- expression 1 operands: lhs = Counter(2), rhs = Expression(5, Sub) +- expression 2 operands: lhs = Counter(0), rhs = Counter(1) +- expression 3 operands: lhs = Counter(0), rhs = Counter(1) +- expression 4 operands: lhs = Counter(2), rhs = Expression(5, Sub) +- expression 5 operands: lhs = Counter(0), rhs = Counter(1) +Number of file 0 mappings: 8 +- Code(Counter(0)) at (prev + 28, 1) to (start + 0, 47) +- Code(Expression(0, Add)) at (prev + 1, 9) to (start + 0, 10) + = (c3 + (c2 + (c0 - c1))) +- Code(Counter(0)) at (prev + 0, 13) to (start + 0, 14) +- Branch { true: Counter(1), false: Expression(5, Sub) } at (prev + 0, 13) to (start + 0, 14) + true = c1 + false = (c0 - c1) +- Code(Counter(1)) at (prev + 0, 18) to (start + 0, 19) +- Branch { true: Counter(3), false: Counter(2) } at (prev + 0, 18) to (start + 0, 19) + true = c3 + false = c2 +- Code(Expression(4, Add)) at (prev + 0, 23) to (start + 0, 24) + = (c2 + (c0 - c1)) +- Code(Expression(0, Add)) at (prev + 1, 5) to (start + 1, 2) + = (c3 + (c2 + (c0 - c1))) + +Function name: conditions::assign_3_or_and +Raw bytes (56): 0x[01, 01, 04, 05, 07, 09, 0d, 01, 05, 01, 05, 08, 01, 17, 01, 00, 2f, 03, 01, 09, 00, 0a, 01, 00, 0d, 00, 0e, 20, 05, 0e, 00, 0d, 00, 0e, 0e, 00, 12, 00, 13, 20, 09, 0d, 00, 12, 00, 13, 09, 00, 17, 00, 18, 03, 01, 05, 01, 02] +Number of files: 1 +- file 0 => global file 1 +Number of expressions: 4 +- expression 0 operands: lhs = Counter(1), rhs = Expression(1, Add) +- expression 1 operands: lhs = Counter(2), rhs = Counter(3) +- expression 2 operands: lhs = Counter(0), rhs = Counter(1) +- expression 3 operands: lhs = Counter(0), rhs = Counter(1) +Number of file 0 mappings: 8 +- Code(Counter(0)) at (prev + 23, 1) to (start + 0, 47) +- Code(Expression(0, Add)) at (prev + 1, 9) to (start + 0, 10) + = (c1 + (c2 + c3)) +- Code(Counter(0)) at (prev + 0, 13) to (start + 0, 14) +- Branch { true: Counter(1), false: Expression(3, Sub) } at (prev + 0, 13) to (start + 0, 14) + true = c1 + false = (c0 - c1) +- Code(Expression(3, Sub)) at (prev + 0, 18) to (start + 0, 19) + = (c0 - c1) +- Branch { true: Counter(2), false: Counter(3) } at (prev + 0, 18) to (start + 0, 19) + true = c2 + false = c3 +- Code(Counter(2)) at (prev + 0, 23) to (start + 0, 24) +- Code(Expression(0, Add)) at (prev + 1, 5) to (start + 1, 2) + = (c1 + (c2 + c3)) + +Function name: conditions::assign_and +Raw bytes (38): 0x[01, 01, 01, 01, 05, 06, 01, 0d, 01, 00, 21, 01, 01, 09, 00, 0a, 01, 00, 0d, 00, 0e, 20, 05, 02, 00, 0d, 00, 0e, 05, 00, 12, 00, 13, 01, 01, 05, 01, 02] +Number of files: 1 +- file 0 => global file 1 +Number of expressions: 1 +- expression 0 operands: lhs = Counter(0), rhs = Counter(1) +Number of file 0 mappings: 6 +- Code(Counter(0)) at (prev + 13, 1) to (start + 0, 33) +- Code(Counter(0)) at (prev + 1, 9) to (start + 0, 10) +- Code(Counter(0)) at (prev + 0, 13) to (start + 0, 14) +- Branch { true: Counter(1), false: Expression(0, Sub) } at (prev + 0, 13) to (start + 0, 14) + true = c1 + false = (c0 - c1) +- Code(Counter(1)) at (prev + 0, 18) to (start + 0, 19) +- Code(Counter(0)) at (prev + 1, 5) to (start + 1, 2) + +Function name: conditions::assign_or +Raw bytes (38): 0x[01, 01, 01, 01, 05, 06, 01, 12, 01, 00, 20, 01, 01, 09, 00, 0a, 01, 00, 0d, 00, 0e, 20, 05, 02, 00, 0d, 00, 0e, 02, 00, 12, 00, 13, 01, 01, 05, 01, 02] +Number of files: 1 +- file 0 => global file 1 +Number of expressions: 1 +- expression 0 operands: lhs = Counter(0), rhs = Counter(1) +Number of file 0 mappings: 6 +- Code(Counter(0)) at (prev + 18, 1) to (start + 0, 32) +- Code(Counter(0)) at (prev + 1, 9) to (start + 0, 10) +- Code(Counter(0)) at (prev + 0, 13) to (start + 0, 14) +- Branch { true: Counter(1), false: Expression(0, Sub) } at (prev + 0, 13) to (start + 0, 14) + true = c1 + false = (c0 - c1) +- Code(Expression(0, Sub)) at (prev + 0, 18) to (start + 0, 19) + = (c0 - c1) +- Code(Counter(0)) at (prev + 1, 5) to (start + 1, 2) + +Function name: conditions::foo +Raw bytes (9): 0x[01, 01, 00, 01, 01, 21, 01, 02, 02] +Number of files: 1 +- file 0 => global file 1 +Number of expressions: 0 +Number of file 0 mappings: 1 +- Code(Counter(0)) at (prev + 33, 1) to (start + 2, 2) + +Function name: conditions::func_call +Raw bytes (28): 0x[01, 01, 01, 01, 05, 04, 01, 25, 01, 01, 0a, 20, 05, 02, 01, 09, 00, 0a, 05, 00, 0e, 00, 0f, 01, 01, 01, 00, 02] +Number of files: 1 +- file 0 => global file 1 +Number of expressions: 1 +- expression 0 operands: lhs = Counter(0), rhs = Counter(1) +Number of file 0 mappings: 4 +- Code(Counter(0)) at (prev + 37, 1) to (start + 1, 10) +- Branch { true: Counter(1), false: Expression(0, Sub) } at (prev + 1, 9) to (start + 0, 10) + true = c1 + false = (c0 - c1) +- Code(Counter(1)) at (prev + 0, 14) to (start + 0, 15) +- Code(Counter(0)) at (prev + 1, 1) to (start + 0, 2) + +Function name: conditions::simple_assign +Raw bytes (9): 0x[01, 01, 00, 01, 01, 08, 01, 03, 02] +Number of files: 1 +- file 0 => global file 1 +Number of expressions: 0 +Number of file 0 mappings: 1 +- Code(Counter(0)) at (prev + 8, 1) to (start + 3, 2) + diff --git a/tests/coverage/condition/conditions.coverage b/tests/coverage/condition/conditions.coverage new file mode 100644 index 000000000000..a71ab8e5d894 --- /dev/null +++ b/tests/coverage/condition/conditions.coverage @@ -0,0 +1,90 @@ + LL| |#![feature(coverage_attribute)] + LL| |//@ edition: 2021 + LL| |//@ compile-flags: -Zcoverage-options=condition + LL| |//@ llvm-cov-flags: --show-branches=count + LL| | + LL| |use core::hint::black_box; + LL| | + LL| 2|fn simple_assign(a: bool) { + LL| 2| let x = a; + LL| 2| black_box(x); + LL| 2|} + LL| | + LL| 3|fn assign_and(a: bool, b: bool) { + LL| 3| let x = a && b; + ^2 + ------------------ + | Branch (LL:13): [True: 2, False: 1] + ------------------ + LL| 3| black_box(x); + LL| 3|} + LL| | + LL| 3|fn assign_or(a: bool, b: bool) { + LL| 3| let x = a || b; + ^1 + ------------------ + | Branch (LL:13): [True: 2, False: 1] + ------------------ + LL| 3| black_box(x); + LL| 3|} + LL| | + LL| 4|fn assign_3_or_and(a: bool, b: bool, c: bool) { + LL| 4| let x = a || b && c; + ^2 ^1 + ------------------ + | Branch (LL:13): [True: 2, False: 2] + | Branch (LL:18): [True: 1, False: 1] + ------------------ + LL| 4| black_box(x); + LL| 4|} + LL| | + LL| 4|fn assign_3_and_or(a: bool, b: bool, c: bool) { + LL| 4| let x = a && b || c; + ^2 ^3 + ------------------ + | Branch (LL:13): [True: 2, False: 2] + | Branch (LL:18): [True: 1, False: 1] + ------------------ + LL| 4| black_box(x); + LL| 4|} + LL| | + LL| 3|fn foo(a: bool) -> bool { + LL| 3| black_box(a) + LL| 3|} + LL| | + LL| 3|fn func_call(a: bool, b: bool) { + LL| 3| foo(a && b); + ^2 + ------------------ + | Branch (LL:9): [True: 2, False: 1] + ------------------ + LL| 3|} + LL| | + LL| |#[coverage(off)] + LL| |fn main() { + LL| | simple_assign(true); + LL| | simple_assign(false); + LL| | + LL| | assign_and(true, false); + LL| | assign_and(true, true); + LL| | assign_and(false, false); + LL| | + LL| | assign_or(true, false); + LL| | assign_or(true, true); + LL| | assign_or(false, false); + LL| | + LL| | assign_3_or_and(true, false, false); + LL| | assign_3_or_and(true, true, false); + LL| | assign_3_or_and(false, false, true); + LL| | assign_3_or_and(false, true, true); + LL| | + LL| | assign_3_and_or(true, false, false); + LL| | assign_3_and_or(true, true, false); + LL| | assign_3_and_or(false, false, true); + LL| | assign_3_and_or(false, true, true); + LL| | + LL| | func_call(true, false); + LL| | func_call(true, true); + LL| | func_call(false, false); + LL| |} + diff --git a/tests/coverage/condition/conditions.rs b/tests/coverage/condition/conditions.rs new file mode 100644 index 000000000000..3d658dc93e0c --- /dev/null +++ b/tests/coverage/condition/conditions.rs @@ -0,0 +1,67 @@ +#![feature(coverage_attribute)] +//@ edition: 2021 +//@ compile-flags: -Zcoverage-options=condition +//@ llvm-cov-flags: --show-branches=count + +use core::hint::black_box; + +fn simple_assign(a: bool) { + let x = a; + black_box(x); +} + +fn assign_and(a: bool, b: bool) { + let x = a && b; + black_box(x); +} + +fn assign_or(a: bool, b: bool) { + let x = a || b; + black_box(x); +} + +fn assign_3_or_and(a: bool, b: bool, c: bool) { + let x = a || b && c; + black_box(x); +} + +fn assign_3_and_or(a: bool, b: bool, c: bool) { + let x = a && b || c; + black_box(x); +} + +fn foo(a: bool) -> bool { + black_box(a) +} + +fn func_call(a: bool, b: bool) { + foo(a && b); +} + +#[coverage(off)] +fn main() { + simple_assign(true); + simple_assign(false); + + assign_and(true, false); + assign_and(true, true); + assign_and(false, false); + + assign_or(true, false); + assign_or(true, true); + assign_or(false, false); + + assign_3_or_and(true, false, false); + assign_3_or_and(true, true, false); + assign_3_or_and(false, false, true); + assign_3_or_and(false, true, true); + + assign_3_and_or(true, false, false); + assign_3_and_or(true, true, false); + assign_3_and_or(false, false, true); + assign_3_and_or(false, true, true); + + func_call(true, false); + func_call(true, true); + func_call(false, false); +} From 35a8746832a4d9103f317c5cac56cfa1e0316d8c Mon Sep 17 00:00:00 2001 From: Zalathar Date: Wed, 1 May 2024 17:33:33 +1000 Subject: [PATCH 07/15] coverage: Instrument the RHS value of lazy logical operators When a lazy logical operator (`&&` or `||`) occurs outside of an `if` condition, it normally doesn't have any associated control-flow branch, so we don't have an existing way to track whether it was true or false. This patch adds special code to handle this case, by inserting extra MIR blocks in a diamond shape after evaluating the RHS. This gives us a place to insert the appropriate marker statements, which can then be given their own counters. --- .../rustc_mir_build/src/build/coverageinfo.rs | 57 +++++++ .../rustc_mir_build/src/build/expr/into.rs | 8 +- tests/coverage/condition/conditions.cov-map | 153 +++++++++++------- tests/coverage/condition/conditions.coverage | 5 + 4 files changed, 162 insertions(+), 61 deletions(-) diff --git a/compiler/rustc_mir_build/src/build/coverageinfo.rs b/compiler/rustc_mir_build/src/build/coverageinfo.rs index ee9eeb62990d..855dcbbcb346 100644 --- a/compiler/rustc_mir_build/src/build/coverageinfo.rs +++ b/compiler/rustc_mir_build/src/build/coverageinfo.rs @@ -157,6 +157,63 @@ pub(crate) fn into_done(self) -> Option> { } impl<'tcx> Builder<'_, 'tcx> { + /// If condition coverage is enabled, inject extra blocks and marker statements + /// that will let us track the value of the condition in `place`. + pub(crate) fn visit_coverage_standalone_condition( + &mut self, + mut expr_id: ExprId, // Expression giving the span of the condition + place: mir::Place<'tcx>, // Already holds the boolean condition value + block: &mut BasicBlock, + ) { + // Bail out if condition coverage is not enabled for this function. + let Some(branch_info) = self.coverage_branch_info.as_mut() else { return }; + if !self.tcx.sess.instrument_coverage_condition() { + return; + }; + + // Remove any wrappers, so that we can inspect the real underlying expression. + while let ExprKind::Use { source: inner } | ExprKind::Scope { value: inner, .. } = + self.thir[expr_id].kind + { + expr_id = inner; + } + // If the expression is a lazy logical op, it will naturally get branch + // coverage as part of its normal lowering, so we can disregard it here. + if let ExprKind::LogicalOp { .. } = self.thir[expr_id].kind { + return; + } + + let source_info = SourceInfo { span: self.thir[expr_id].span, scope: self.source_scope }; + + // Using the boolean value that has already been stored in `place`, set up + // control flow in the shape of a diamond, so that we can place separate + // marker statements in the true and false blocks. The coverage MIR pass + // will use those markers to inject coverage counters as appropriate. + // + // block + // / \ + // true_block false_block + // (marker) (marker) + // \ / + // join_block + + let true_block = self.cfg.start_new_block(); + let false_block = self.cfg.start_new_block(); + self.cfg.terminate( + *block, + source_info, + mir::TerminatorKind::if_(mir::Operand::Copy(place), true_block, false_block), + ); + + branch_info.add_two_way_branch(&mut self.cfg, source_info, true_block, false_block); + + let join_block = self.cfg.start_new_block(); + self.cfg.goto(true_block, source_info, join_block); + self.cfg.goto(false_block, source_info, join_block); + // Any subsequent codegen in the caller should use the new join block. + *block = join_block; + } + /// If branch coverage is enabled, inject marker statements into `then_block` /// and `else_block`, and record their IDs in the table of branch spans. pub(crate) fn visit_coverage_branch_condition( diff --git a/compiler/rustc_mir_build/src/build/expr/into.rs b/compiler/rustc_mir_build/src/build/expr/into.rs index 9349133d2dbc..c08a3b6691af 100644 --- a/compiler/rustc_mir_build/src/build/expr/into.rs +++ b/compiler/rustc_mir_build/src/build/expr/into.rs @@ -183,9 +183,13 @@ pub(crate) fn expr_into_dest( const_: Const::from_bool(this.tcx, constant), }, ); - let rhs = unpack!(this.expr_into_dest(destination, continuation, rhs)); + let mut rhs_block = unpack!(this.expr_into_dest(destination, continuation, rhs)); + // Instrument the lowered RHS's value for condition coverage. + // (Does nothing if condition coverage is not enabled.) + this.visit_coverage_standalone_condition(rhs, destination, &mut rhs_block); + let target = this.cfg.start_new_block(); - this.cfg.goto(rhs, source_info, target); + this.cfg.goto(rhs_block, source_info, target); this.cfg.goto(short_circuit, source_info, target); target.unit() } diff --git a/tests/coverage/condition/conditions.cov-map b/tests/coverage/condition/conditions.cov-map index 6c81f7bda985..74726e269fc6 100644 --- a/tests/coverage/condition/conditions.cov-map +++ b/tests/coverage/condition/conditions.cov-map @@ -1,44 +1,107 @@ Function name: conditions::assign_3_and_or -Raw bytes (60): 0x[01, 01, 06, 0d, 13, 09, 16, 01, 05, 01, 05, 09, 16, 01, 05, 08, 01, 1c, 01, 00, 2f, 03, 01, 09, 00, 0a, 01, 00, 0d, 00, 0e, 20, 05, 16, 00, 0d, 00, 0e, 05, 00, 12, 00, 13, 20, 0d, 09, 00, 12, 00, 13, 13, 00, 17, 00, 18, 03, 01, 05, 01, 02] +Raw bytes (69): 0x[01, 01, 07, 07, 11, 09, 0d, 01, 05, 05, 09, 16, 1a, 05, 09, 01, 05, 09, 01, 1c, 01, 00, 2f, 03, 01, 09, 00, 0a, 01, 00, 0d, 00, 0e, 20, 05, 1a, 00, 0d, 00, 0e, 05, 00, 12, 00, 13, 20, 09, 16, 00, 12, 00, 13, 13, 00, 17, 00, 18, 20, 0d, 11, 00, 17, 00, 18, 03, 01, 05, 01, 02] Number of files: 1 - file 0 => global file 1 -Number of expressions: 6 -- expression 0 operands: lhs = Counter(3), rhs = Expression(4, Add) -- expression 1 operands: lhs = Counter(2), rhs = Expression(5, Sub) +Number of expressions: 7 +- expression 0 operands: lhs = Expression(1, Add), rhs = Counter(4) +- expression 1 operands: lhs = Counter(2), rhs = Counter(3) - expression 2 operands: lhs = Counter(0), rhs = Counter(1) -- expression 3 operands: lhs = Counter(0), rhs = Counter(1) -- expression 4 operands: lhs = Counter(2), rhs = Expression(5, Sub) -- expression 5 operands: lhs = Counter(0), rhs = Counter(1) -Number of file 0 mappings: 8 +- expression 3 operands: lhs = Counter(1), rhs = Counter(2) +- expression 4 operands: lhs = Expression(5, Sub), rhs = Expression(6, Sub) +- expression 5 operands: lhs = Counter(1), rhs = Counter(2) +- expression 6 operands: lhs = Counter(0), rhs = Counter(1) +Number of file 0 mappings: 9 - Code(Counter(0)) at (prev + 28, 1) to (start + 0, 47) - Code(Expression(0, Add)) at (prev + 1, 9) to (start + 0, 10) - = (c3 + (c2 + (c0 - c1))) + = ((c2 + c3) + c4) - Code(Counter(0)) at (prev + 0, 13) to (start + 0, 14) -- Branch { true: Counter(1), false: Expression(5, Sub) } at (prev + 0, 13) to (start + 0, 14) +- Branch { true: Counter(1), false: Expression(6, Sub) } at (prev + 0, 13) to (start + 0, 14) true = c1 false = (c0 - c1) - Code(Counter(1)) at (prev + 0, 18) to (start + 0, 19) -- Branch { true: Counter(3), false: Counter(2) } at (prev + 0, 18) to (start + 0, 19) - true = c3 - false = c2 +- Branch { true: Counter(2), false: Expression(5, Sub) } at (prev + 0, 18) to (start + 0, 19) + true = c2 + false = (c1 - c2) - Code(Expression(4, Add)) at (prev + 0, 23) to (start + 0, 24) - = (c2 + (c0 - c1)) + = ((c1 - c2) + (c0 - c1)) +- Branch { true: Counter(3), false: Counter(4) } at (prev + 0, 23) to (start + 0, 24) + true = c3 + false = c4 - Code(Expression(0, Add)) at (prev + 1, 5) to (start + 1, 2) - = (c3 + (c2 + (c0 - c1))) + = ((c2 + c3) + c4) Function name: conditions::assign_3_or_and -Raw bytes (56): 0x[01, 01, 04, 05, 07, 09, 0d, 01, 05, 01, 05, 08, 01, 17, 01, 00, 2f, 03, 01, 09, 00, 0a, 01, 00, 0d, 00, 0e, 20, 05, 0e, 00, 0d, 00, 0e, 0e, 00, 12, 00, 13, 20, 09, 0d, 00, 12, 00, 13, 09, 00, 17, 00, 18, 03, 01, 05, 01, 02] +Raw bytes (73): 0x[01, 01, 09, 05, 07, 0b, 11, 09, 0d, 01, 05, 01, 05, 22, 11, 01, 05, 22, 11, 01, 05, 09, 01, 17, 01, 00, 2f, 03, 01, 09, 00, 0a, 01, 00, 0d, 00, 0e, 20, 05, 22, 00, 0d, 00, 0e, 22, 00, 12, 00, 13, 20, 1e, 11, 00, 12, 00, 13, 1e, 00, 17, 00, 18, 20, 09, 0d, 00, 17, 00, 18, 03, 01, 05, 01, 02] +Number of files: 1 +- file 0 => global file 1 +Number of expressions: 9 +- expression 0 operands: lhs = Counter(1), rhs = Expression(1, Add) +- expression 1 operands: lhs = Expression(2, Add), rhs = Counter(4) +- expression 2 operands: lhs = Counter(2), rhs = Counter(3) +- expression 3 operands: lhs = Counter(0), rhs = Counter(1) +- expression 4 operands: lhs = Counter(0), rhs = Counter(1) +- expression 5 operands: lhs = Expression(8, Sub), rhs = Counter(4) +- expression 6 operands: lhs = Counter(0), rhs = Counter(1) +- expression 7 operands: lhs = Expression(8, Sub), rhs = Counter(4) +- expression 8 operands: lhs = Counter(0), rhs = Counter(1) +Number of file 0 mappings: 9 +- Code(Counter(0)) at (prev + 23, 1) to (start + 0, 47) +- Code(Expression(0, Add)) at (prev + 1, 9) to (start + 0, 10) + = (c1 + ((c2 + c3) + c4)) +- Code(Counter(0)) at (prev + 0, 13) to (start + 0, 14) +- Branch { true: Counter(1), false: Expression(8, Sub) } at (prev + 0, 13) to (start + 0, 14) + true = c1 + false = (c0 - c1) +- Code(Expression(8, Sub)) at (prev + 0, 18) to (start + 0, 19) + = (c0 - c1) +- Branch { true: Expression(7, Sub), false: Counter(4) } at (prev + 0, 18) to (start + 0, 19) + true = ((c0 - c1) - c4) + false = c4 +- Code(Expression(7, Sub)) at (prev + 0, 23) to (start + 0, 24) + = ((c0 - c1) - c4) +- Branch { true: Counter(2), false: Counter(3) } at (prev + 0, 23) to (start + 0, 24) + true = c2 + false = c3 +- Code(Expression(0, Add)) at (prev + 1, 5) to (start + 1, 2) + = (c1 + ((c2 + c3) + c4)) + +Function name: conditions::assign_and +Raw bytes (51): 0x[01, 01, 04, 07, 0e, 09, 0d, 01, 05, 01, 05, 07, 01, 0d, 01, 00, 21, 03, 01, 09, 00, 0a, 01, 00, 0d, 00, 0e, 20, 05, 0e, 00, 0d, 00, 0e, 05, 00, 12, 00, 13, 20, 09, 0d, 00, 12, 00, 13, 03, 01, 05, 01, 02] Number of files: 1 - file 0 => global file 1 Number of expressions: 4 -- expression 0 operands: lhs = Counter(1), rhs = Expression(1, Add) +- expression 0 operands: lhs = Expression(1, Add), rhs = Expression(3, Sub) - expression 1 operands: lhs = Counter(2), rhs = Counter(3) - expression 2 operands: lhs = Counter(0), rhs = Counter(1) - expression 3 operands: lhs = Counter(0), rhs = Counter(1) -Number of file 0 mappings: 8 -- Code(Counter(0)) at (prev + 23, 1) to (start + 0, 47) +Number of file 0 mappings: 7 +- Code(Counter(0)) at (prev + 13, 1) to (start + 0, 33) - Code(Expression(0, Add)) at (prev + 1, 9) to (start + 0, 10) - = (c1 + (c2 + c3)) + = ((c2 + c3) + (c0 - c1)) +- Code(Counter(0)) at (prev + 0, 13) to (start + 0, 14) +- Branch { true: Counter(1), false: Expression(3, Sub) } at (prev + 0, 13) to (start + 0, 14) + true = c1 + false = (c0 - c1) +- Code(Counter(1)) at (prev + 0, 18) to (start + 0, 19) +- Branch { true: Counter(2), false: Counter(3) } at (prev + 0, 18) to (start + 0, 19) + true = c2 + false = c3 +- Code(Expression(0, Add)) at (prev + 1, 5) to (start + 1, 2) + = ((c2 + c3) + (c0 - c1)) + +Function name: conditions::assign_or +Raw bytes (51): 0x[01, 01, 04, 07, 0d, 05, 09, 01, 05, 01, 05, 07, 01, 12, 01, 00, 20, 03, 01, 09, 00, 0a, 01, 00, 0d, 00, 0e, 20, 05, 0e, 00, 0d, 00, 0e, 0e, 00, 12, 00, 13, 20, 09, 0d, 00, 12, 00, 13, 03, 01, 05, 01, 02] +Number of files: 1 +- file 0 => global file 1 +Number of expressions: 4 +- expression 0 operands: lhs = Expression(1, Add), rhs = Counter(3) +- expression 1 operands: lhs = Counter(1), rhs = Counter(2) +- expression 2 operands: lhs = Counter(0), rhs = Counter(1) +- expression 3 operands: lhs = Counter(0), rhs = Counter(1) +Number of file 0 mappings: 7 +- Code(Counter(0)) at (prev + 18, 1) to (start + 0, 32) +- Code(Expression(0, Add)) at (prev + 1, 9) to (start + 0, 10) + = ((c1 + c2) + c3) - Code(Counter(0)) at (prev + 0, 13) to (start + 0, 14) - Branch { true: Counter(1), false: Expression(3, Sub) } at (prev + 0, 13) to (start + 0, 14) true = c1 @@ -48,42 +111,8 @@ Number of file 0 mappings: 8 - Branch { true: Counter(2), false: Counter(3) } at (prev + 0, 18) to (start + 0, 19) true = c2 false = c3 -- Code(Counter(2)) at (prev + 0, 23) to (start + 0, 24) - Code(Expression(0, Add)) at (prev + 1, 5) to (start + 1, 2) - = (c1 + (c2 + c3)) - -Function name: conditions::assign_and -Raw bytes (38): 0x[01, 01, 01, 01, 05, 06, 01, 0d, 01, 00, 21, 01, 01, 09, 00, 0a, 01, 00, 0d, 00, 0e, 20, 05, 02, 00, 0d, 00, 0e, 05, 00, 12, 00, 13, 01, 01, 05, 01, 02] -Number of files: 1 -- file 0 => global file 1 -Number of expressions: 1 -- expression 0 operands: lhs = Counter(0), rhs = Counter(1) -Number of file 0 mappings: 6 -- Code(Counter(0)) at (prev + 13, 1) to (start + 0, 33) -- Code(Counter(0)) at (prev + 1, 9) to (start + 0, 10) -- Code(Counter(0)) at (prev + 0, 13) to (start + 0, 14) -- Branch { true: Counter(1), false: Expression(0, Sub) } at (prev + 0, 13) to (start + 0, 14) - true = c1 - false = (c0 - c1) -- Code(Counter(1)) at (prev + 0, 18) to (start + 0, 19) -- Code(Counter(0)) at (prev + 1, 5) to (start + 1, 2) - -Function name: conditions::assign_or -Raw bytes (38): 0x[01, 01, 01, 01, 05, 06, 01, 12, 01, 00, 20, 01, 01, 09, 00, 0a, 01, 00, 0d, 00, 0e, 20, 05, 02, 00, 0d, 00, 0e, 02, 00, 12, 00, 13, 01, 01, 05, 01, 02] -Number of files: 1 -- file 0 => global file 1 -Number of expressions: 1 -- expression 0 operands: lhs = Counter(0), rhs = Counter(1) -Number of file 0 mappings: 6 -- Code(Counter(0)) at (prev + 18, 1) to (start + 0, 32) -- Code(Counter(0)) at (prev + 1, 9) to (start + 0, 10) -- Code(Counter(0)) at (prev + 0, 13) to (start + 0, 14) -- Branch { true: Counter(1), false: Expression(0, Sub) } at (prev + 0, 13) to (start + 0, 14) - true = c1 - false = (c0 - c1) -- Code(Expression(0, Sub)) at (prev + 0, 18) to (start + 0, 19) - = (c0 - c1) -- Code(Counter(0)) at (prev + 1, 5) to (start + 1, 2) + = ((c1 + c2) + c3) Function name: conditions::foo Raw bytes (9): 0x[01, 01, 00, 01, 01, 21, 01, 02, 02] @@ -94,18 +123,24 @@ Number of file 0 mappings: 1 - Code(Counter(0)) at (prev + 33, 1) to (start + 2, 2) Function name: conditions::func_call -Raw bytes (28): 0x[01, 01, 01, 01, 05, 04, 01, 25, 01, 01, 0a, 20, 05, 02, 01, 09, 00, 0a, 05, 00, 0e, 00, 0f, 01, 01, 01, 00, 02] +Raw bytes (39): 0x[01, 01, 03, 01, 05, 0b, 02, 09, 0d, 05, 01, 25, 01, 01, 0a, 20, 05, 02, 01, 09, 00, 0a, 05, 00, 0e, 00, 0f, 20, 09, 0d, 00, 0e, 00, 0f, 07, 01, 01, 00, 02] Number of files: 1 - file 0 => global file 1 -Number of expressions: 1 +Number of expressions: 3 - expression 0 operands: lhs = Counter(0), rhs = Counter(1) -Number of file 0 mappings: 4 +- expression 1 operands: lhs = Expression(2, Add), rhs = Expression(0, Sub) +- expression 2 operands: lhs = Counter(2), rhs = Counter(3) +Number of file 0 mappings: 5 - Code(Counter(0)) at (prev + 37, 1) to (start + 1, 10) - Branch { true: Counter(1), false: Expression(0, Sub) } at (prev + 1, 9) to (start + 0, 10) true = c1 false = (c0 - c1) - Code(Counter(1)) at (prev + 0, 14) to (start + 0, 15) -- Code(Counter(0)) at (prev + 1, 1) to (start + 0, 2) +- Branch { true: Counter(2), false: Counter(3) } at (prev + 0, 14) to (start + 0, 15) + true = c2 + false = c3 +- Code(Expression(1, Add)) at (prev + 1, 1) to (start + 0, 2) + = ((c2 + c3) + (c0 - c1)) Function name: conditions::simple_assign Raw bytes (9): 0x[01, 01, 00, 01, 01, 08, 01, 03, 02] diff --git a/tests/coverage/condition/conditions.coverage b/tests/coverage/condition/conditions.coverage index a71ab8e5d894..3215b391d622 100644 --- a/tests/coverage/condition/conditions.coverage +++ b/tests/coverage/condition/conditions.coverage @@ -15,6 +15,7 @@ ^2 ------------------ | Branch (LL:13): [True: 2, False: 1] + | Branch (LL:18): [True: 1, False: 1] ------------------ LL| 3| black_box(x); LL| 3|} @@ -24,6 +25,7 @@ ^1 ------------------ | Branch (LL:13): [True: 2, False: 1] + | Branch (LL:18): [True: 0, False: 1] ------------------ LL| 3| black_box(x); LL| 3|} @@ -34,6 +36,7 @@ ------------------ | Branch (LL:13): [True: 2, False: 2] | Branch (LL:18): [True: 1, False: 1] + | Branch (LL:23): [True: 1, False: 0] ------------------ LL| 4| black_box(x); LL| 4|} @@ -44,6 +47,7 @@ ------------------ | Branch (LL:13): [True: 2, False: 2] | Branch (LL:18): [True: 1, False: 1] + | Branch (LL:23): [True: 2, False: 1] ------------------ LL| 4| black_box(x); LL| 4|} @@ -57,6 +61,7 @@ ^2 ------------------ | Branch (LL:9): [True: 2, False: 1] + | Branch (LL:14): [True: 1, False: 1] ------------------ LL| 3|} LL| | From 4b96e44ebbf42dc8b6f3e5ac4894739adc17d826 Mon Sep 17 00:00:00 2001 From: Scott McMurray Date: Thu, 30 May 2024 22:05:30 -0700 Subject: [PATCH 08/15] Also InstSimplify `&raw*` We do this for `&*` and `&mut*` already; might as well do it for raw pointers too. --- .../rustc_mir_transform/src/instsimplify.rs | 2 +- .../ref_of_deref.pointers.InstSimplify.diff | 58 +++++++++++++++++++ .../ref_of_deref.references.InstSimplify.diff | 58 +++++++++++++++++++ tests/mir-opt/instsimplify/ref_of_deref.rs | 40 +++++++++++++ 4 files changed, 157 insertions(+), 1 deletion(-) create mode 100644 tests/mir-opt/instsimplify/ref_of_deref.pointers.InstSimplify.diff create mode 100644 tests/mir-opt/instsimplify/ref_of_deref.references.InstSimplify.diff create mode 100644 tests/mir-opt/instsimplify/ref_of_deref.rs diff --git a/compiler/rustc_mir_transform/src/instsimplify.rs b/compiler/rustc_mir_transform/src/instsimplify.rs index a54332b6f257..40db3e38fd32 100644 --- a/compiler/rustc_mir_transform/src/instsimplify.rs +++ b/compiler/rustc_mir_transform/src/instsimplify.rs @@ -123,7 +123,7 @@ fn try_eval_bool(&self, a: &Operand<'_>) -> Option { /// Transform `&(*a)` ==> `a`. fn simplify_ref_deref(&self, source_info: &SourceInfo, rvalue: &mut Rvalue<'tcx>) { - if let Rvalue::Ref(_, _, place) = rvalue { + if let Rvalue::Ref(_, _, place) | Rvalue::AddressOf(_, place) = rvalue { if let Some((base, ProjectionElem::Deref)) = place.as_ref().last_projection() { if rvalue.ty(self.local_decls, self.tcx) != base.ty(self.local_decls, self.tcx).ty { return; diff --git a/tests/mir-opt/instsimplify/ref_of_deref.pointers.InstSimplify.diff b/tests/mir-opt/instsimplify/ref_of_deref.pointers.InstSimplify.diff new file mode 100644 index 000000000000..52b3d1e1d40b --- /dev/null +++ b/tests/mir-opt/instsimplify/ref_of_deref.pointers.InstSimplify.diff @@ -0,0 +1,58 @@ +- // MIR for `pointers` before InstSimplify ++ // MIR for `pointers` after InstSimplify + + fn pointers(_1: *const [i32], _2: *mut i32) -> () { + debug const_ptr => _1; + debug mut_ptr => _2; + let mut _0: (); + let _3: &[i32]; + scope 1 { + debug _a => _3; + let _4: &i32; + scope 2 { + debug _b => _4; + let _5: &mut i32; + scope 3 { + debug _c => _5; + let _6: *const [i32]; + scope 4 { + debug _d => _6; + let _7: *const i32; + scope 5 { + debug _e => _7; + let _8: *mut i32; + scope 6 { + debug _f => _8; + } + } + } + } + } + } + + bb0: { + StorageLive(_3); + _3 = &(*_1); + StorageLive(_4); + _4 = &(*_2); + StorageLive(_5); + _5 = &mut (*_2); + StorageLive(_6); +- _6 = &raw const (*_1); ++ _6 = _1; + StorageLive(_7); + _7 = &raw const (*_2); + StorageLive(_8); +- _8 = &raw mut (*_2); ++ _8 = _2; + _0 = const (); + StorageDead(_8); + StorageDead(_7); + StorageDead(_6); + StorageDead(_5); + StorageDead(_4); + StorageDead(_3); + return; + } + } + diff --git a/tests/mir-opt/instsimplify/ref_of_deref.references.InstSimplify.diff b/tests/mir-opt/instsimplify/ref_of_deref.references.InstSimplify.diff new file mode 100644 index 000000000000..ca0828a225a0 --- /dev/null +++ b/tests/mir-opt/instsimplify/ref_of_deref.references.InstSimplify.diff @@ -0,0 +1,58 @@ +- // MIR for `references` before InstSimplify ++ // MIR for `references` after InstSimplify + + fn references(_1: &i32, _2: &mut [i32]) -> () { + debug const_ref => _1; + debug mut_ref => _2; + let mut _0: (); + let _3: &i32; + scope 1 { + debug _a => _3; + let _4: &[i32]; + scope 2 { + debug _b => _4; + let _5: &mut [i32]; + scope 3 { + debug _c => _5; + let _6: *const i32; + scope 4 { + debug _d => _6; + let _7: *const [i32]; + scope 5 { + debug _e => _7; + let _8: *mut [i32]; + scope 6 { + debug _f => _8; + } + } + } + } + } + } + + bb0: { + StorageLive(_3); +- _3 = &(*_1); ++ _3 = _1; + StorageLive(_4); + _4 = &(*_2); + StorageLive(_5); +- _5 = &mut (*_2); ++ _5 = _2; + StorageLive(_6); + _6 = &raw const (*_1); + StorageLive(_7); + _7 = &raw const (*_2); + StorageLive(_8); + _8 = &raw mut (*_2); + _0 = const (); + StorageDead(_8); + StorageDead(_7); + StorageDead(_6); + StorageDead(_5); + StorageDead(_4); + StorageDead(_3); + return; + } + } + diff --git a/tests/mir-opt/instsimplify/ref_of_deref.rs b/tests/mir-opt/instsimplify/ref_of_deref.rs new file mode 100644 index 000000000000..37e164bc17f1 --- /dev/null +++ b/tests/mir-opt/instsimplify/ref_of_deref.rs @@ -0,0 +1,40 @@ +//@ test-mir-pass: InstSimplify +#![crate_type = "lib"] +#![feature(raw_ref_op)] + +// For each of these, only 2 of the 6 should simplify, +// as the others have the wrong types. + +// EMIT_MIR ref_of_deref.references.InstSimplify.diff +// CHECK-LABEL: references +pub fn references(const_ref: &i32, mut_ref: &mut [i32]) { + // CHECK: _3 = _1; + let _a = &*const_ref; + // CHECK: _4 = &(*_2); + let _b = &*mut_ref; + // CHECK: _5 = _2; + let _c = &mut *mut_ref; + // CHECK: _6 = &raw const (*_1); + let _d = &raw const *const_ref; + // CHECK: _7 = &raw const (*_2); + let _e = &raw const *mut_ref; + // CHECK: _8 = &raw mut (*_2); + let _f = &raw mut *mut_ref; +} + +// EMIT_MIR ref_of_deref.pointers.InstSimplify.diff +// CHECK-LABEL: pointers +pub unsafe fn pointers(const_ptr: *const [i32], mut_ptr: *mut i32) { + // CHECK: _3 = &(*_1); + let _a = &*const_ptr; + // CHECK: _4 = &(*_2); + let _b = &*mut_ptr; + // CHECK: _5 = &mut (*_2); + let _c = &mut *mut_ptr; + // CHECK: _6 = _1; + let _d = &raw const *const_ptr; + // CHECK: _7 = &raw const (*_2); + let _e = &raw const *mut_ptr; + // CHECK: _8 = _2; + let _f = &raw mut *mut_ptr; +} From 06c4cc44b61cab331d0db1f0592ec7d172abf881 Mon Sep 17 00:00:00 2001 From: Oli Scherer Date: Fri, 31 May 2024 08:56:38 +0000 Subject: [PATCH 09/15] Also resolve the type of constants, even if we already turned it into an error constant --- compiler/rustc_hir_typeck/src/writeback.rs | 1 + .../const_generic_type.rs | 10 ++++++++++ .../const_generic_type.stderr | 19 +++++++++++++++++++ 3 files changed, 30 insertions(+) create mode 100644 tests/ui/type-alias-impl-trait/const_generic_type.rs create mode 100644 tests/ui/type-alias-impl-trait/const_generic_type.stderr diff --git a/compiler/rustc_hir_typeck/src/writeback.rs b/compiler/rustc_hir_typeck/src/writeback.rs index 31caa52d2671..e337105f0110 100644 --- a/compiler/rustc_hir_typeck/src/writeback.rs +++ b/compiler/rustc_hir_typeck/src/writeback.rs @@ -865,6 +865,7 @@ fn fold_const(&mut self, ct: ty::Const<'tcx>) -> ty::Const<'tcx> { self.handle_term(ct, ty::Const::outer_exclusive_binder, |tcx, guar| { ty::Const::new_error(tcx, guar, ct.ty()) }) + .super_fold_with(self) } fn fold_predicate(&mut self, predicate: ty::Predicate<'tcx>) -> ty::Predicate<'tcx> { diff --git a/tests/ui/type-alias-impl-trait/const_generic_type.rs b/tests/ui/type-alias-impl-trait/const_generic_type.rs new file mode 100644 index 000000000000..3af122fc4e33 --- /dev/null +++ b/tests/ui/type-alias-impl-trait/const_generic_type.rs @@ -0,0 +1,10 @@ +//@edition: 2021 + +#![feature(type_alias_impl_trait)] +type Bar = impl std::fmt::Display; + +async fn test() {} +//~^ ERROR: type annotations needed +//~| ERROR: `Bar` is forbidden as the type of a const generic parameter + +fn main() {} diff --git a/tests/ui/type-alias-impl-trait/const_generic_type.stderr b/tests/ui/type-alias-impl-trait/const_generic_type.stderr new file mode 100644 index 000000000000..5501f23c8bb1 --- /dev/null +++ b/tests/ui/type-alias-impl-trait/const_generic_type.stderr @@ -0,0 +1,19 @@ +error[E0283]: type annotations needed + --> $DIR/const_generic_type.rs:6:1 + | +LL | async fn test() {} + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ cannot infer type + | + = note: cannot satisfy `_: std::fmt::Display` + +error: `Bar` is forbidden as the type of a const generic parameter + --> $DIR/const_generic_type.rs:6:24 + | +LL | async fn test() {} + | ^^^^^^^^^^ + | + = note: the only supported types are integers, `bool` and `char` + +error: aborting due to 2 previous errors + +For more information about this error, try `rustc --explain E0283`. From befcdec7778bc901f47fa8ebd4d5e322a8bd187e Mon Sep 17 00:00:00 2001 From: Oli Scherer Date: Fri, 31 May 2024 08:58:56 +0000 Subject: [PATCH 10/15] Check that we can constrain the hidden tpye of a TAIT used in a const generic type --- .../const_generic_type.infer.stderr | 10 ++++++++++ ..._type.stderr => const_generic_type.no_infer.stderr} | 8 ++++---- tests/ui/type-alias-impl-trait/const_generic_type.rs | 10 +++++++--- 3 files changed, 21 insertions(+), 7 deletions(-) create mode 100644 tests/ui/type-alias-impl-trait/const_generic_type.infer.stderr rename tests/ui/type-alias-impl-trait/{const_generic_type.stderr => const_generic_type.no_infer.stderr} (73%) diff --git a/tests/ui/type-alias-impl-trait/const_generic_type.infer.stderr b/tests/ui/type-alias-impl-trait/const_generic_type.infer.stderr new file mode 100644 index 000000000000..6a1a770228da --- /dev/null +++ b/tests/ui/type-alias-impl-trait/const_generic_type.infer.stderr @@ -0,0 +1,10 @@ +error: `Bar` is forbidden as the type of a const generic parameter + --> $DIR/const_generic_type.rs:7:24 + | +LL | async fn test() { + | ^^^^^^^^^^ + | + = note: the only supported types are integers, `bool` and `char` + +error: aborting due to 1 previous error + diff --git a/tests/ui/type-alias-impl-trait/const_generic_type.stderr b/tests/ui/type-alias-impl-trait/const_generic_type.no_infer.stderr similarity index 73% rename from tests/ui/type-alias-impl-trait/const_generic_type.stderr rename to tests/ui/type-alias-impl-trait/const_generic_type.no_infer.stderr index 5501f23c8bb1..a1a69bfaca37 100644 --- a/tests/ui/type-alias-impl-trait/const_generic_type.stderr +++ b/tests/ui/type-alias-impl-trait/const_generic_type.no_infer.stderr @@ -1,15 +1,15 @@ error[E0283]: type annotations needed - --> $DIR/const_generic_type.rs:6:1 + --> $DIR/const_generic_type.rs:7:1 | -LL | async fn test() {} +LL | async fn test() { | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ cannot infer type | = note: cannot satisfy `_: std::fmt::Display` error: `Bar` is forbidden as the type of a const generic parameter - --> $DIR/const_generic_type.rs:6:24 + --> $DIR/const_generic_type.rs:7:24 | -LL | async fn test() {} +LL | async fn test() { | ^^^^^^^^^^ | = note: the only supported types are integers, `bool` and `char` diff --git a/tests/ui/type-alias-impl-trait/const_generic_type.rs b/tests/ui/type-alias-impl-trait/const_generic_type.rs index 3af122fc4e33..95a5e1c62861 100644 --- a/tests/ui/type-alias-impl-trait/const_generic_type.rs +++ b/tests/ui/type-alias-impl-trait/const_generic_type.rs @@ -1,10 +1,14 @@ //@edition: 2021 +//@revisions: infer no_infer #![feature(type_alias_impl_trait)] type Bar = impl std::fmt::Display; -async fn test() {} -//~^ ERROR: type annotations needed -//~| ERROR: `Bar` is forbidden as the type of a const generic parameter +async fn test() { + //[no_infer]~^ ERROR: type annotations needed + //~^^ ERROR: `Bar` is forbidden as the type of a const generic parameter + #[cfg(infer)] + let x: u32 = N; +} fn main() {} From 9abfebdf1e3eb821d160673d0fbd1d58b310b4e6 Mon Sep 17 00:00:00 2001 From: Zalathar Date: Thu, 30 May 2024 16:06:14 +1000 Subject: [PATCH 11/15] Add an alternate `--demangle` mode to coverage-dump The coverage-dump tool already needs `rustc_demangle` for its own purposes, so the amount of extra code needed for a demangle mode is very small. --- src/tools/coverage-dump/README.md | 5 +++++ src/tools/coverage-dump/src/main.rs | 19 +++++++++++++++++++ 2 files changed, 24 insertions(+) diff --git a/src/tools/coverage-dump/README.md b/src/tools/coverage-dump/README.md index e2625d5adf27..49d8e14c7bcc 100644 --- a/src/tools/coverage-dump/README.md +++ b/src/tools/coverage-dump/README.md @@ -6,3 +6,8 @@ The output format is mostly arbitrary, so it's OK to change the output as long as any affected tests are also re-blessed. However, the output should be consistent across different executions on different platforms, so avoid printing any information that is platform-specific or non-deterministic. + +## Demangle mode + +When run as `coverage-dump --demangle`, this tool instead functions as a +command-line demangler that can be invoked by `llvm-cov`. diff --git a/src/tools/coverage-dump/src/main.rs b/src/tools/coverage-dump/src/main.rs index 93fed1799e04..b21e3e292f2b 100644 --- a/src/tools/coverage-dump/src/main.rs +++ b/src/tools/coverage-dump/src/main.rs @@ -7,6 +7,13 @@ fn main() -> anyhow::Result<()> { let args = std::env::args().collect::>(); + // The coverage-dump tool already needs `rustc_demangle` in order to read + // coverage metadata, so it's very easy to also have a separate mode that + // turns it into a command-line demangler for use by coverage-run tests. + if &args[1..] == &["--demangle"] { + return demangle(); + } + let llvm_ir_path = args.get(1).context("LLVM IR file not specified")?; let llvm_ir = std::fs::read_to_string(llvm_ir_path).context("couldn't read LLVM IR file")?; @@ -15,3 +22,15 @@ fn main() -> anyhow::Result<()> { Ok(()) } + +fn demangle() -> anyhow::Result<()> { + use std::fmt::Write as _; + + let stdin = std::io::read_to_string(std::io::stdin())?; + let mut output = String::with_capacity(stdin.len()); + for line in stdin.lines() { + writeln!(output, "{:#}", rustc_demangle::demangle(line))?; + } + print!("{output}"); + Ok(()) +} From 10ffc228a851489b1678d830517541bd00d4da49 Mon Sep 17 00:00:00 2001 From: Zalathar Date: Thu, 30 May 2024 16:21:42 +1000 Subject: [PATCH 12/15] Use `coverage-dump --demangle` as the demangler for coverage-run tests This avoids the need to build `rust-demangler` when running coverage tests, since we typically need to build `coverage-dump` anyway. --- src/bootstrap/src/core/build_steps/test.rs | 13 +------------ src/tools/compiletest/src/runtest/coverage.rs | 17 ++++++++++++----- 2 files changed, 13 insertions(+), 17 deletions(-) diff --git a/src/bootstrap/src/core/build_steps/test.rs b/src/bootstrap/src/core/build_steps/test.rs index 215886863624..76302a7900de 100644 --- a/src/bootstrap/src/core/build_steps/test.rs +++ b/src/bootstrap/src/core/build_steps/test.rs @@ -1781,7 +1781,7 @@ fn run(self, builder: &Builder<'_>) { .arg(builder.ensure(tool::JsonDocLint { compiler: json_compiler, target })); } - if mode == "coverage-map" { + if matches!(mode, "coverage-map" | "coverage-run") { let coverage_dump = builder.ensure(tool::CoverageDump { compiler: compiler.with_stage(0), target: compiler.host, @@ -1789,17 +1789,6 @@ fn run(self, builder: &Builder<'_>) { cmd.arg("--coverage-dump-path").arg(coverage_dump); } - if mode == "coverage-run" { - // The demangler doesn't need the current compiler, so we can avoid - // unnecessary rebuilds by using the bootstrap compiler instead. - let rust_demangler = builder.ensure(tool::RustDemangler { - compiler: compiler.with_stage(0), - target: compiler.host, - extra_features: Vec::new(), - }); - cmd.arg("--rust-demangler-path").arg(rust_demangler); - } - cmd.arg("--src-base").arg(builder.src.join("tests").join(suite)); cmd.arg("--build-base").arg(testdir(builder, compiler.host).join(suite)); diff --git a/src/tools/compiletest/src/runtest/coverage.rs b/src/tools/compiletest/src/runtest/coverage.rs index dad3fb301333..8bd7c7e808d3 100644 --- a/src/tools/compiletest/src/runtest/coverage.rs +++ b/src/tools/compiletest/src/runtest/coverage.rs @@ -10,10 +10,15 @@ use crate::runtest::{static_regex, Emit, ProcRes, TestCx, WillExecute}; impl<'test> TestCx<'test> { + fn coverage_dump_path(&self) -> &Path { + self.config + .coverage_dump_path + .as_deref() + .unwrap_or_else(|| self.fatal("missing --coverage-dump")) + } + pub(crate) fn run_coverage_map_test(&self) { - let Some(coverage_dump_path) = &self.config.coverage_dump_path else { - self.fatal("missing --coverage-dump"); - }; + let coverage_dump_path = self.coverage_dump_path(); let (proc_res, llvm_ir_path) = self.compile_test_and_save_ir(); if !proc_res.status.success() { @@ -102,8 +107,10 @@ pub(crate) fn run_coverage_run_test(&self) { let proc_res = self.run_llvm_tool("llvm-cov", |cmd| { cmd.args(["show", "--format=text", "--show-line-counts-or-regions"]); - cmd.arg("--Xdemangler"); - cmd.arg(self.config.rust_demangler_path.as_ref().unwrap()); + // Specify the demangler binary and its arguments. + let coverage_dump_path = self.coverage_dump_path(); + cmd.arg("--Xdemangler").arg(coverage_dump_path); + cmd.arg("--Xdemangler").arg("--demangle"); cmd.arg("--instr-profile"); cmd.arg(&profdata_path); From feb8f3cc5d0728db5038de151f02bac540dc713a Mon Sep 17 00:00:00 2001 From: Zalathar Date: Thu, 30 May 2024 17:15:36 +1000 Subject: [PATCH 13/15] Use `Builder::tool_exe` to build the coverage-dump tool This appears to be the canonical way to build a tool with the stage 0 compiler. --- src/bootstrap/src/core/build_steps/test.rs | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/src/bootstrap/src/core/build_steps/test.rs b/src/bootstrap/src/core/build_steps/test.rs index 76302a7900de..29b3d1669b4b 100644 --- a/src/bootstrap/src/core/build_steps/test.rs +++ b/src/bootstrap/src/core/build_steps/test.rs @@ -1782,10 +1782,7 @@ fn run(self, builder: &Builder<'_>) { } if matches!(mode, "coverage-map" | "coverage-run") { - let coverage_dump = builder.ensure(tool::CoverageDump { - compiler: compiler.with_stage(0), - target: compiler.host, - }); + let coverage_dump = builder.tool_exe(Tool::CoverageDump); cmd.arg("--coverage-dump-path").arg(coverage_dump); } From 54b6849e06cc4847625cdf6a7312eb017786867f Mon Sep 17 00:00:00 2001 From: Zalathar Date: Thu, 30 May 2024 17:22:02 +1000 Subject: [PATCH 14/15] Remove unused rust-demangler support from compiletest --- src/tools/compiletest/src/common.rs | 3 --- src/tools/compiletest/src/lib.rs | 3 --- src/tools/compiletest/src/runtest.rs | 4 ---- 3 files changed, 10 deletions(-) diff --git a/src/tools/compiletest/src/common.rs b/src/tools/compiletest/src/common.rs index 7ff45edd4b26..b0047770564c 100644 --- a/src/tools/compiletest/src/common.rs +++ b/src/tools/compiletest/src/common.rs @@ -187,9 +187,6 @@ pub struct Config { /// The rustdoc executable. pub rustdoc_path: Option, - /// The rust-demangler executable. - pub rust_demangler_path: Option, - /// The coverage-dump executable. pub coverage_dump_path: Option, diff --git a/src/tools/compiletest/src/lib.rs b/src/tools/compiletest/src/lib.rs index 99bde107f3a4..62e71e9b59dd 100644 --- a/src/tools/compiletest/src/lib.rs +++ b/src/tools/compiletest/src/lib.rs @@ -46,7 +46,6 @@ pub fn parse_config(args: Vec) -> Config { .reqopt("", "run-lib-path", "path to target shared libraries", "PATH") .reqopt("", "rustc-path", "path to rustc to use for compiling", "PATH") .optopt("", "rustdoc-path", "path to rustdoc to use for compiling", "PATH") - .optopt("", "rust-demangler-path", "path to rust-demangler to use in tests", "PATH") .optopt("", "coverage-dump-path", "path to coverage-dump to use in tests", "PATH") .reqopt("", "python", "path to python to use for doc tests", "PATH") .optopt("", "jsondocck-path", "path to jsondocck to use for doc tests", "PATH") @@ -232,7 +231,6 @@ fn make_absolute(path: PathBuf) -> PathBuf { run_lib_path: make_absolute(opt_path(matches, "run-lib-path")), rustc_path: opt_path(matches, "rustc-path"), rustdoc_path: matches.opt_str("rustdoc-path").map(PathBuf::from), - rust_demangler_path: matches.opt_str("rust-demangler-path").map(PathBuf::from), coverage_dump_path: matches.opt_str("coverage-dump-path").map(PathBuf::from), python: matches.opt_str("python").unwrap(), jsondocck_path: matches.opt_str("jsondocck-path"), @@ -337,7 +335,6 @@ pub fn log_config(config: &Config) { logv(c, format!("run_lib_path: {:?}", config.run_lib_path)); logv(c, format!("rustc_path: {:?}", config.rustc_path.display())); logv(c, format!("rustdoc_path: {:?}", config.rustdoc_path)); - logv(c, format!("rust_demangler_path: {:?}", config.rust_demangler_path)); logv(c, format!("src_base: {:?}", config.src_base.display())); logv(c, format!("build_base: {:?}", config.build_base.display())); logv(c, format!("stage_id: {}", config.stage_id)); diff --git a/src/tools/compiletest/src/runtest.rs b/src/tools/compiletest/src/runtest.rs index 4ea12a0f9e48..79e158992d47 100644 --- a/src/tools/compiletest/src/runtest.rs +++ b/src/tools/compiletest/src/runtest.rs @@ -3561,10 +3561,6 @@ fn run_rmake_v2_test(&self) { cmd.env("RUSTDOC", cwd.join(rustdoc)); } - if let Some(ref rust_demangler) = self.config.rust_demangler_path { - cmd.env("RUST_DEMANGLER", cwd.join(rust_demangler)); - } - if let Some(ref node) = self.config.nodejs { cmd.env("NODE", node); } From 20699fe6b23615cac13e4661b755fde2b4f36822 Mon Sep 17 00:00:00 2001 From: Michael Goulet Date: Thu, 30 May 2024 11:51:06 -0400 Subject: [PATCH 15/15] Stop using translate_args in the new solver --- .../src/solve/eval_ctxt/mod.rs | 11 -- .../src/solve/normalizes_to/mod.rs | 123 +++++++++++------- ...-constraining-predicates-ambig.next.stderr | 12 ++ ...-requires-constraining-predicates-ambig.rs | 29 +++++ ...res-constraining-predicates.current.stderr | 12 ++ ...quires-constraining-predicates.next.stderr | 12 ++ ...e-impl-requires-constraining-predicates.rs | 24 ++++ 7 files changed, 167 insertions(+), 56 deletions(-) create mode 100644 tests/ui/specialization/source-impl-requires-constraining-predicates-ambig.next.stderr create mode 100644 tests/ui/specialization/source-impl-requires-constraining-predicates-ambig.rs create mode 100644 tests/ui/specialization/source-impl-requires-constraining-predicates.current.stderr create mode 100644 tests/ui/specialization/source-impl-requires-constraining-predicates.next.stderr create mode 100644 tests/ui/specialization/source-impl-requires-constraining-predicates.rs diff --git a/compiler/rustc_trait_selection/src/solve/eval_ctxt/mod.rs b/compiler/rustc_trait_selection/src/solve/eval_ctxt/mod.rs index b18b59d9a752..f18f1f4f8f0d 100644 --- a/compiler/rustc_trait_selection/src/solve/eval_ctxt/mod.rs +++ b/compiler/rustc_trait_selection/src/solve/eval_ctxt/mod.rs @@ -12,7 +12,6 @@ use rustc_middle::traits::solve::{ inspect, CanonicalInput, CanonicalResponse, Certainty, PredefinedOpaquesData, QueryResult, }; -use rustc_middle::traits::specialization_graph; use rustc_middle::ty::AliasRelationDirection; use rustc_middle::ty::TypeFolder; use rustc_middle::ty::{ @@ -900,16 +899,6 @@ pub(super) fn fresh_args_for_item(&mut self, def_id: DefId) -> ty::GenericArgsRe args } - pub(super) fn translate_args( - &self, - param_env: ty::ParamEnv<'tcx>, - source_impl: DefId, - source_args: ty::GenericArgsRef<'tcx>, - target_node: specialization_graph::Node, - ) -> ty::GenericArgsRef<'tcx> { - crate::traits::translate_args(self.infcx, param_env, source_impl, source_args, target_node) - } - pub(super) fn register_ty_outlives(&self, ty: Ty<'tcx>, lt: ty::Region<'tcx>) { self.infcx.register_region_obligation_with_cause(ty, lt, &ObligationCause::dummy()); } diff --git a/compiler/rustc_trait_selection/src/solve/normalizes_to/mod.rs b/compiler/rustc_trait_selection/src/solve/normalizes_to/mod.rs index 22a77e2ecb8d..8c63bd824bc0 100644 --- a/compiler/rustc_trait_selection/src/solve/normalizes_to/mod.rs +++ b/compiler/rustc_trait_selection/src/solve/normalizes_to/mod.rs @@ -1,4 +1,4 @@ -use crate::traits::specialization_graph; +use crate::traits::specialization_graph::{self, LeafDef, Node}; use super::assembly::structural_traits::AsyncCallableRelevantTypes; use super::assembly::{self, structural_traits, Candidate}; @@ -9,7 +9,6 @@ use rustc_infer::traits::query::NoSolution; use rustc_infer::traits::solve::inspect::ProbeKind; use rustc_infer::traits::solve::MaybeCause; -use rustc_infer::traits::specialization_graph::LeafDef; use rustc_infer::traits::Reveal; use rustc_middle::traits::solve::{CandidateSource, Certainty, Goal, QueryResult}; use rustc_middle::traits::BuiltinImplSource; @@ -189,8 +188,7 @@ fn consider_impl_candidate( // In case the associated item is hidden due to specialization, we have to // return ambiguity this would otherwise be incomplete, resulting in // unsoundness during coherence (#105782). - let Some(assoc_def) = fetch_eligible_assoc_item_def( - ecx, + let Some(assoc_def) = ecx.fetch_eligible_assoc_item_def( goal.param_env, goal_trait_ref, goal.predicate.def_id(), @@ -235,16 +233,10 @@ fn consider_impl_candidate( // // And then map these args to the args of the defining impl of `Assoc`, going // from `[u32, u64]` to `[u32, i32, u64]`. - let impl_args_with_gat = - goal.predicate.alias.args.rebase_onto(tcx, goal_trait_ref.def_id, impl_args); - let args = ecx.translate_args( - goal.param_env, - impl_def_id, - impl_args_with_gat, - assoc_def.defining_node, - ); + let associated_item_args = + ecx.translate_args(&assoc_def, goal, impl_def_id, impl_args, impl_trait_ref)?; - if !tcx.check_args_compatible(assoc_def.item.def_id, args) { + if !tcx.check_args_compatible(assoc_def.item.def_id, associated_item_args) { return error_response( ecx, "associated item has mismatched generic item arguments", @@ -272,7 +264,7 @@ fn consider_impl_candidate( ty::AssocKind::Fn => unreachable!("we should never project to a fn"), }; - ecx.instantiate_normalizes_to_term(goal, term.instantiate(tcx, args)); + ecx.instantiate_normalizes_to_term(goal, term.instantiate(tcx, associated_item_args)); ecx.evaluate_added_goals_and_make_canonical_response(Certainty::Yes) }) } @@ -889,38 +881,79 @@ fn consider_builtin_transmute_candidate( } } -/// This behavior is also implemented in `rustc_ty_utils` and in the old `project` code. -/// -/// FIXME: We should merge these 3 implementations as it's likely that they otherwise -/// diverge. -#[instrument(level = "trace", skip(ecx, param_env), ret)] -fn fetch_eligible_assoc_item_def<'tcx>( - ecx: &EvalCtxt<'_, InferCtxt<'tcx>>, - param_env: ty::ParamEnv<'tcx>, - goal_trait_ref: ty::TraitRef<'tcx>, - trait_assoc_def_id: DefId, - impl_def_id: DefId, -) -> Result, NoSolution> { - let node_item = - specialization_graph::assoc_def(ecx.interner(), impl_def_id, trait_assoc_def_id) - .map_err(|ErrorGuaranteed { .. }| NoSolution)?; +impl<'tcx> EvalCtxt<'_, InferCtxt<'tcx>> { + fn translate_args( + &mut self, + assoc_def: &LeafDef, + goal: Goal<'tcx, ty::NormalizesTo<'tcx>>, + impl_def_id: DefId, + impl_args: ty::GenericArgsRef<'tcx>, + impl_trait_ref: rustc_type_ir::TraitRef>, + ) -> Result, NoSolution> { + let tcx = self.interner(); + Ok(match assoc_def.defining_node { + Node::Trait(_) => goal.predicate.alias.args, + Node::Impl(target_impl_def_id) => { + if target_impl_def_id == impl_def_id { + // Same impl, no need to fully translate, just a rebase from + // the trait is sufficient. + goal.predicate.alias.args.rebase_onto(tcx, impl_trait_ref.def_id, impl_args) + } else { + let target_args = self.fresh_args_for_item(target_impl_def_id); + let target_trait_ref = tcx + .impl_trait_ref(target_impl_def_id) + .unwrap() + .instantiate(tcx, target_args); + // Relate source impl to target impl by equating trait refs. + self.eq(goal.param_env, impl_trait_ref, target_trait_ref)?; + // Also add predicates since they may be needed to constrain the + // target impl's params. + self.add_goals( + GoalSource::Misc, + tcx.predicates_of(target_impl_def_id) + .instantiate(tcx, target_args) + .into_iter() + .map(|(pred, _)| goal.with(tcx, pred)), + ); + goal.predicate.alias.args.rebase_onto(tcx, impl_trait_ref.def_id, target_args) + } + } + }) + } - let eligible = if node_item.is_final() { - // Non-specializable items are always projectable. - true - } else { - // Only reveal a specializable default if we're past type-checking - // and the obligation is monomorphic, otherwise passes such as - // transmute checking and polymorphic MIR optimizations could - // get a result which isn't correct for all monomorphizations. - if param_env.reveal() == Reveal::All { - let poly_trait_ref = ecx.resolve_vars_if_possible(goal_trait_ref); - !poly_trait_ref.still_further_specializable() + /// This behavior is also implemented in `rustc_ty_utils` and in the old `project` code. + /// + /// FIXME: We should merge these 3 implementations as it's likely that they otherwise + /// diverge. + #[instrument(level = "trace", skip(self, param_env), ret)] + fn fetch_eligible_assoc_item_def( + &self, + param_env: ty::ParamEnv<'tcx>, + goal_trait_ref: ty::TraitRef<'tcx>, + trait_assoc_def_id: DefId, + impl_def_id: DefId, + ) -> Result, NoSolution> { + let node_item = + specialization_graph::assoc_def(self.interner(), impl_def_id, trait_assoc_def_id) + .map_err(|ErrorGuaranteed { .. }| NoSolution)?; + + let eligible = if node_item.is_final() { + // Non-specializable items are always projectable. + true } else { - trace!(?node_item.item.def_id, "not eligible due to default"); - false - } - }; + // Only reveal a specializable default if we're past type-checking + // and the obligation is monomorphic, otherwise passes such as + // transmute checking and polymorphic MIR optimizations could + // get a result which isn't correct for all monomorphizations. + if param_env.reveal() == Reveal::All { + let poly_trait_ref = self.resolve_vars_if_possible(goal_trait_ref); + !poly_trait_ref.still_further_specializable() + } else { + trace!(?node_item.item.def_id, "not eligible due to default"); + false + } + }; - if eligible { Ok(Some(node_item)) } else { Ok(None) } + if eligible { Ok(Some(node_item)) } else { Ok(None) } + } } diff --git a/tests/ui/specialization/source-impl-requires-constraining-predicates-ambig.next.stderr b/tests/ui/specialization/source-impl-requires-constraining-predicates-ambig.next.stderr new file mode 100644 index 000000000000..17918a77821d --- /dev/null +++ b/tests/ui/specialization/source-impl-requires-constraining-predicates-ambig.next.stderr @@ -0,0 +1,12 @@ +warning: the feature `specialization` is incomplete and may not be safe to use and/or cause compiler crashes + --> $DIR/source-impl-requires-constraining-predicates-ambig.rs:14:12 + | +LL | #![feature(specialization)] + | ^^^^^^^^^^^^^^ + | + = note: see issue #31844 for more information + = help: consider using `min_specialization` instead, which is more stable and complete + = note: `#[warn(incomplete_features)]` on by default + +warning: 1 warning emitted + diff --git a/tests/ui/specialization/source-impl-requires-constraining-predicates-ambig.rs b/tests/ui/specialization/source-impl-requires-constraining-predicates-ambig.rs new file mode 100644 index 000000000000..977493c885b2 --- /dev/null +++ b/tests/ui/specialization/source-impl-requires-constraining-predicates-ambig.rs @@ -0,0 +1,29 @@ +//@ revisions: current next +//@ ignore-compare-mode-next-solver (explicit revisions) +//@[next] compile-flags: -Znext-solver +//@[next] check-pass +//@[current] known-bug: unknown +//@[current] failure-status: 101 +//@[current] dont-check-compiler-stderr + +// Tests that rebasing from the concrete impl to the default impl also processes the +// `[u32; 0]: IntoIterator` predicate to constrain the `?U` impl arg. +// This test also makes sure that we don't do anything weird when rebasing the args +// is ambiguous. + +#![feature(specialization)] +//[next]~^ WARN the feature `specialization` is incomplete + +trait Spec { + type Assoc; +} + +default impl Spec for T where T: IntoIterator { + type Assoc = U; +} + +impl Spec for [T; 0] {} + +fn main() { + let x: <[_; 0] as Spec>::Assoc = 1; +} diff --git a/tests/ui/specialization/source-impl-requires-constraining-predicates.current.stderr b/tests/ui/specialization/source-impl-requires-constraining-predicates.current.stderr new file mode 100644 index 000000000000..3442e5b5ca61 --- /dev/null +++ b/tests/ui/specialization/source-impl-requires-constraining-predicates.current.stderr @@ -0,0 +1,12 @@ +warning: the feature `specialization` is incomplete and may not be safe to use and/or cause compiler crashes + --> $DIR/source-impl-requires-constraining-predicates.rs:9:12 + | +LL | #![feature(specialization)] + | ^^^^^^^^^^^^^^ + | + = note: see issue #31844 for more information + = help: consider using `min_specialization` instead, which is more stable and complete + = note: `#[warn(incomplete_features)]` on by default + +warning: 1 warning emitted + diff --git a/tests/ui/specialization/source-impl-requires-constraining-predicates.next.stderr b/tests/ui/specialization/source-impl-requires-constraining-predicates.next.stderr new file mode 100644 index 000000000000..3442e5b5ca61 --- /dev/null +++ b/tests/ui/specialization/source-impl-requires-constraining-predicates.next.stderr @@ -0,0 +1,12 @@ +warning: the feature `specialization` is incomplete and may not be safe to use and/or cause compiler crashes + --> $DIR/source-impl-requires-constraining-predicates.rs:9:12 + | +LL | #![feature(specialization)] + | ^^^^^^^^^^^^^^ + | + = note: see issue #31844 for more information + = help: consider using `min_specialization` instead, which is more stable and complete + = note: `#[warn(incomplete_features)]` on by default + +warning: 1 warning emitted + diff --git a/tests/ui/specialization/source-impl-requires-constraining-predicates.rs b/tests/ui/specialization/source-impl-requires-constraining-predicates.rs new file mode 100644 index 000000000000..532fc3676406 --- /dev/null +++ b/tests/ui/specialization/source-impl-requires-constraining-predicates.rs @@ -0,0 +1,24 @@ +//@ check-pass +//@ revisions: current next +//@ ignore-compare-mode-next-solver (explicit revisions) +//@[next] compile-flags: -Znext-solver + +// Tests that rebasing from the concrete impl to the default impl also processes the +// `[u32; 0]: IntoIterator` predicate to constrain the `?U` impl arg. + +#![feature(specialization)] +//~^ WARN the feature `specialization` is incomplete + +trait Spec { + type Assoc; +} + +default impl Spec for T where T: IntoIterator { + type Assoc = U; +} + +impl Spec for [T; 0] {} + +fn main() { + let x: <[u32; 0] as Spec>::Assoc = 1; +}