Auto merge of #125817 - jieyouxu:rollup-ak8rrn1, r=jieyouxu

Rollup of 6 pull requests

Successful merges:

 - #125652 (Revert propagation of drop-live information from Polonius)
 - #125730 (Apply `x clippy --fix` and `x fmt` on Rustc)
 - #125752 (run-make: enforce `#[must_use]` and arm command wrappers with drop bombs)
 - #125756 (coverage: Optionally instrument the RHS of lazy logical operators)
 - #125796 (Also InstSimplify `&raw*`)
 - #125816 (Don't build the `rust-demangler` binary for coverage tests)

r? `@ghost`
`@rustbot` modify labels: rollup

Author: bors

compiler/rustc_borrowck/src/type_check/liveness/mod.rs

@@ -14,7 +14,6 @@ use std::rc::Rc;
 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<DenseLocationMap>,
     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

compiler/rustc_borrowck/src/type_check/liveness/polonius.rs

@@ -85,13 +85,10 @@ impl<'a, 'tcx> Visitor<'tcx> for UseFactsExtractor<'a, 'tcx> {
 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 not mutated, but expected to be modified as
-    // out param, bug?
-    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 {
@@ -104,10 +101,6 @@ pub(super) fn populate_access_facts<'a, 'tcx>(
         };
         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={:?}",

compiler/rustc_borrowck/src/type_check/liveness/trace.rs

@@ -16,6 +16,7 @@ use rustc_mir_dataflow::impls::MaybeInitializedPlaces;
 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<Local>,
     boring_locals: Vec<Local>,
-    polonius_drop_used: Option<Vec<(Local, Location)>>,
 ) {
     let local_use_map = &LocalUseMap::build(&relevant_live_locals, elements, body);
 
@@ -93,9 +93,7 @@ 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())
-    }
+    results.add_extra_drop_facts(&relevant_live_locals);
 
     results.compute_for_all_locals(relevant_live_locals);
 
@@ -218,21 +216,38 @@ impl<'me, 'typeck, 'flow, 'tcx> LivenessResults<'me, 'typeck, 'flow, 'tcx> {
     ///
     /// 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<Local>,
-    ) {
+    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())?;
+
+        let relevant_live_locals: FxIndexSet<_> = relevant_live_locals.iter().copied().collect();
+
         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);
                 }
             }
         }
+        Some(())
     }
 
     /// Clear the value of fields that are "per local variable".

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();

compiler/rustc_data_structures/src/graph/dominators/mod.rs

@@ -93,7 +93,7 @@ fn dominators_impl<G: ControlFlowGraph>(graph: &G) -> Inner<G::Node> {
     // 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);

compiler/rustc_data_structures/src/graph/iterate/mod.rs

@@ -48,7 +48,7 @@ fn post_order_walk<G: DirectedGraph + Successors>(
         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 @@ where
     /// 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.

compiler/rustc_data_structures/src/graph/scc/mod.rs

@@ -40,7 +40,7 @@ pub struct SccData<S: Idx> {
 }
 
 impl<N: Idx, S: Idx + Ord> Sccs<N, S> {
-    pub fn new(graph: &(impl DirectedGraph<Node = N> + Successors)) -> Self {
+    pub fn new(graph: &impl Successors<Node = N>) -> Self {
         SccsConstruction::construct(graph)
     }
 

compiler/rustc_data_structures/src/profiling.rs

@@ -562,7 +562,7 @@ impl SelfProfiler {
         // 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)?)?;
 

compiler/rustc_data_structures/src/sorted_map.rs

@@ -125,13 +125,13 @@ impl<K: Ord, V> SortedMap<K, V> {
 
     /// Iterate over the keys, sorted
     #[inline]
-    pub fn keys(&self) -> impl Iterator<Item = &K> + ExactSizeIterator + DoubleEndedIterator {
+    pub fn keys(&self) -> impl ExactSizeIterator<Item = &K> + DoubleEndedIterator {
         self.data.iter().map(|(k, _)| k)
     }
 
     /// Iterate over values, sorted by key
     #[inline]
-    pub fn values(&self) -> impl Iterator<Item = &V> + ExactSizeIterator + DoubleEndedIterator {
+    pub fn values(&self) -> impl ExactSizeIterator<Item = &V> + DoubleEndedIterator {
         self.data.iter().map(|(_, v)| v)
     }
 

compiler/rustc_data_structures/src/sync/lock.rs

@@ -69,7 +69,7 @@ mod maybe_sync {
             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.

compiler/rustc_mir_build/src/build/coverageinfo.rs

@@ -157,6 +157,63 @@ impl BranchInfoBuilder {
 }
 
 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(

compiler/rustc_mir_build/src/build/expr/into.rs

@@ -183,9 +183,13 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
                         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()
             }

compiler/rustc_mir_transform/src/instsimplify.rs

@@ -123,7 +123,7 @@ impl<'tcx> InstSimplifyContext<'tcx, '_> {
 
     /// 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;

compiler/rustc_parse_format/src/lib.rs

@@ -286,13 +286,11 @@ impl<'a> Iterator for Parser<'a> {
                                     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;

compiler/rustc_serialize/src/serialize.rs

@@ -252,7 +252,7 @@ impl<S: Encoder> Encodable<S> for () {
 }
 
 impl<D: Decoder> Decodable<D> for () {
-    fn decode(_: &mut D) -> () {}
+    fn decode(_: &mut D) {}
 }
 
 impl<S: Encoder, T> Encodable<S> for PhantomData<T> {

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,
 }