Auto merge of rust-lang#124856 - matthiaskrgr:rollup-xqxs21x, r=matthiaskrgr

Rollup of 3 pull requests

Successful merges:

 - rust-lang#124223 (coverage: Branch coverage support for let-else and if-let)
 - rust-lang#124571 (coverage: Clean up `mcdc_bitmap_bytes` and `conditions_num`)
 - rust-lang#124838 (next_power_of_two: add a doctest to show what happens on 0)

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

Author: bors

compiler/rustc_codegen_llvm/src/coverageinfo/ffi.rs

@@ -118,7 +118,7 @@ pub mod mcdc {
     #[derive(Clone, Copy, Debug, Default)]
     pub struct DecisionParameters {
         bitmap_idx: u32,
-        conditions_num: u16,
+        num_conditions: u16,
     }
 
     // ConditionId in llvm is `unsigned int` at 18 while `int16_t` at [19](https://github.com/llvm/llvm-project/pull/81257)
@@ -178,7 +178,7 @@ pub mod mcdc {
 
     impl From<DecisionInfo> for DecisionParameters {
         fn from(value: DecisionInfo) -> Self {
-            Self { bitmap_idx: value.bitmap_idx, conditions_num: value.conditions_num }
+            Self { bitmap_idx: value.bitmap_idx, num_conditions: value.num_conditions }
         }
     }
 }

compiler/rustc_codegen_llvm/src/coverageinfo/mod.rs

@@ -207,13 +207,8 @@ impl<'tcx> CoverageInfoBuilderMethods<'tcx> for Builder<'_, '_, 'tcx> {
                 let cond_bitmap = coverage_context
                                     .try_get_mcdc_condition_bitmap(&instance, decision_depth)
                                     .expect("mcdc cond bitmap should have been allocated for merging into the global bitmap");
-                let bitmap_bytes = bx.tcx().coverage_ids_info(instance.def).mcdc_bitmap_bytes;
+                let bitmap_bytes = function_coverage_info.mcdc_bitmap_bytes;
                 assert!(bitmap_idx < bitmap_bytes, "bitmap index of the decision out of range");
-                assert!(
-                    bitmap_bytes <= function_coverage_info.mcdc_bitmap_bytes,
-                    "bitmap length disagreement: query says {bitmap_bytes} but function info only has {}",
-                    function_coverage_info.mcdc_bitmap_bytes
-                );
 
                 let fn_name = bx.get_pgo_func_name_var(instance);
                 let hash = bx.const_u64(function_coverage_info.function_source_hash);

compiler/rustc_middle/src/mir/coverage.rs

@@ -129,17 +129,11 @@ pub enum CoverageKind {
     /// Marks the point in MIR control flow represented by a evaluated condition.
     ///
     /// This is eventually lowered to `llvm.instrprof.mcdc.condbitmap.update` in LLVM IR.
-    ///
-    /// If this statement does not survive MIR optimizations, the condition would never be
-    /// taken as evaluated.
     CondBitmapUpdate { id: ConditionId, value: bool, decision_depth: u16 },
 
     /// Marks the point in MIR control flow represented by a evaluated decision.
     ///
     /// This is eventually lowered to `llvm.instrprof.mcdc.tvbitmap.update` in LLVM IR.
-    ///
-    /// If this statement does not survive MIR optimizations, the decision would never be
-    /// taken as evaluated.
     TestVectorBitmapUpdate { bitmap_idx: u32, decision_depth: u16 },
 }
 
@@ -335,14 +329,14 @@ pub struct MCDCBranchSpan {
 #[derive(TyEncodable, TyDecodable, Hash, HashStable, TypeFoldable, TypeVisitable)]
 pub struct DecisionInfo {
     pub bitmap_idx: u32,
-    pub conditions_num: u16,
+    pub num_conditions: u16,
 }
 
 #[derive(Clone, Debug)]
 #[derive(TyEncodable, TyDecodable, Hash, HashStable, TypeFoldable, TypeVisitable)]
 pub struct MCDCDecisionSpan {
     pub span: Span,
-    pub conditions_num: usize,
+    pub num_conditions: usize,
     pub end_markers: Vec<BlockMarkerId>,
     pub decision_depth: u16,
 }

compiler/rustc_middle/src/mir/pretty.rs

@@ -511,12 +511,12 @@ fn write_coverage_branch_info(
         )?;
     }
 
-    for coverage::MCDCDecisionSpan { span, conditions_num, end_markers, decision_depth } in
+    for coverage::MCDCDecisionSpan { span, num_conditions, end_markers, decision_depth } in
         mcdc_decision_spans
     {
         writeln!(
             w,
-            "{INDENT}coverage mcdc decision {{ conditions_num: {conditions_num:?}, end: {end_markers:?}, depth: {decision_depth:?} }} => {span:?}"
+            "{INDENT}coverage mcdc decision {{ num_conditions: {num_conditions:?}, end: {end_markers:?}, depth: {decision_depth:?} }} => {span:?}"
         )?;
     }
 

compiler/rustc_middle/src/mir/query.rs

@@ -362,8 +362,4 @@ pub struct CoverageIdsInfo {
     /// InstrumentCoverage MIR pass, if the highest-numbered counter increments
     /// were removed by MIR optimizations.
     pub max_counter_id: mir::coverage::CounterId,
-
-    /// Coverage codegen for mcdc needs to know the size of the global bitmap so that it can
-    /// set the `bytemap-bytes` argument of the `llvm.instrprof.mcdc.tvbitmap.update` intrinsic.
-    pub mcdc_bitmap_bytes: u32,
 }

compiler/rustc_mir_build/messages.ftl

@@ -97,7 +97,9 @@ mir_build_deref_raw_pointer_requires_unsafe_unsafe_op_in_unsafe_fn_allowed =
     .note = raw pointers may be null, dangling or unaligned; they can violate aliasing rules and cause data races: all of these are undefined behavior
     .label = dereference of raw pointer
 
-mir_build_exceeds_mcdc_condition_num_limit =  Conditions number of the decision ({$conditions_num}) exceeds limit ({$max_conditions_num}). MCDC analysis will not count this expression.
+mir_build_exceeds_mcdc_condition_limit =
+    number of conditions in decision ({$num_conditions}) exceeds limit ({$limit})
+    .note = this decision will not be instrumented for MC/DC coverage
 
 mir_build_extern_static_requires_unsafe =
     use of extern static is unsafe and requires unsafe block

compiler/rustc_mir_build/src/build/coverageinfo.rs

@@ -1,17 +1,18 @@
-mod mcdc;
 use std::assert_matches::assert_matches;
 use std::collections::hash_map::Entry;
 
 use rustc_data_structures::fx::FxHashMap;
 use rustc_middle::mir::coverage::{BlockMarkerId, BranchSpan, CoverageKind};
 use rustc_middle::mir::{self, BasicBlock, SourceInfo, UnOp};
-use rustc_middle::thir::{ExprId, ExprKind, Thir};
+use rustc_middle::thir::{ExprId, ExprKind, Pat, Thir};
 use rustc_middle::ty::TyCtxt;
 use rustc_span::def_id::LocalDefId;
 
 use crate::build::coverageinfo::mcdc::MCDCInfoBuilder;
 use crate::build::{Builder, CFG};
 
+mod mcdc;
+
 pub(crate) struct BranchInfoBuilder {
     /// Maps condition expressions to their enclosing `!`, for better instrumentation.
     nots: FxHashMap<ExprId, NotInfo>,
@@ -155,7 +156,7 @@ impl BranchInfoBuilder {
     }
 }
 
-impl Builder<'_, '_> {
+impl<'tcx> Builder<'_, 'tcx> {
     /// 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(
@@ -195,4 +196,23 @@ impl Builder<'_, '_> {
 
         branch_info.add_two_way_branch(&mut self.cfg, source_info, then_block, else_block);
     }
+
+    /// If branch coverage is enabled, inject marker statements into `true_block`
+    /// and `false_block`, and record their IDs in the table of branches.
+    ///
+    /// Used to instrument let-else and if-let (including let-chains) for branch coverage.
+    pub(crate) fn visit_coverage_conditional_let(
+        &mut self,
+        pattern: &Pat<'tcx>, // Pattern that has been matched when the true path is taken
+        true_block: BasicBlock,
+        false_block: BasicBlock,
+    ) {
+        // Bail out if branch coverage is not enabled for this function.
+        let Some(branch_info) = self.coverage_branch_info.as_mut() else { return };
+
+        // FIXME(#124144) This may need special handling when MC/DC is enabled.
+
+        let source_info = SourceInfo { span: pattern.span, scope: self.source_scope };
+        branch_info.add_two_way_branch(&mut self.cfg, source_info, true_block, false_block);
+    }
 }

compiler/rustc_mir_build/src/build/coverageinfo/mcdc.rs

@@ -9,12 +9,12 @@ use rustc_middle::ty::TyCtxt;
 use rustc_span::Span;
 
 use crate::build::Builder;
-use crate::errors::MCDCExceedsConditionNumLimit;
+use crate::errors::MCDCExceedsConditionLimit;
 
 /// The MCDC bitmap scales exponentially (2^n) based on the number of conditions seen,
-/// So llvm sets a maximum value prevents the bitmap footprint from growing too large without the user's knowledge.
-/// This limit may be relaxed if the [upstream change](https://github.com/llvm/llvm-project/pull/82448) is merged.
-const MAX_CONDITIONS_NUM_IN_DECISION: usize = 6;
+/// So LLVM imposes a limit to prevent the bitmap footprint from growing too large without the user's knowledge.
+/// This limit may be relaxed if [upstream change #82448](https://github.com/llvm/llvm-project/pull/82448) is merged.
+const MAX_CONDITIONS_IN_DECISION: usize = 6;
 
 #[derive(Default)]
 struct MCDCDecisionCtx {
@@ -99,22 +99,22 @@ impl MCDCState {
             }
             None => decision_ctx.processing_decision.insert(MCDCDecisionSpan {
                 span,
-                conditions_num: 0,
+                num_conditions: 0,
                 end_markers: vec![],
                 decision_depth,
             }),
         };
 
         let parent_condition = decision_ctx.decision_stack.pop_back().unwrap_or_default();
         let lhs_id = if parent_condition.condition_id == ConditionId::NONE {
-            decision.conditions_num += 1;
-            ConditionId::from(decision.conditions_num)
+            decision.num_conditions += 1;
+            ConditionId::from(decision.num_conditions)
         } else {
             parent_condition.condition_id
         };
 
-        decision.conditions_num += 1;
-        let rhs_condition_id = ConditionId::from(decision.conditions_num);
+        decision.num_conditions += 1;
+        let rhs_condition_id = ConditionId::from(decision.num_conditions);
 
         let (lhs, rhs) = match op {
             LogicalOp::And => {
@@ -207,27 +207,27 @@ impl MCDCInfoBuilder {
         // is empty, i.e. when all the conditions of the decision were instrumented,
         // and the decision is "complete".
         if let Some(decision) = decision_result {
-            match decision.conditions_num {
+            match decision.num_conditions {
                 0 => {
                     unreachable!("Decision with no condition is not expected");
                 }
-                1..=MAX_CONDITIONS_NUM_IN_DECISION => {
+                1..=MAX_CONDITIONS_IN_DECISION => {
                     self.decision_spans.push(decision);
                 }
                 _ => {
                     // Do not generate mcdc mappings and statements for decisions with too many conditions.
-                    let rebase_idx = self.branch_spans.len() - decision.conditions_num + 1;
+                    let rebase_idx = self.branch_spans.len() - decision.num_conditions + 1;
                     for branch in &mut self.branch_spans[rebase_idx..] {
                         branch.condition_info = None;
                     }
 
                     // ConditionInfo of this branch shall also be reset.
                     condition_info = None;
 
-                    tcx.dcx().emit_warn(MCDCExceedsConditionNumLimit {
+                    tcx.dcx().emit_warn(MCDCExceedsConditionLimit {
                         span: decision.span,
-                        conditions_num: decision.conditions_num,
-                        max_conditions_num: MAX_CONDITIONS_NUM_IN_DECISION,
+                        num_conditions: decision.num_conditions,
+                        limit: MAX_CONDITIONS_IN_DECISION,
                     });
                 }
             }

compiler/rustc_mir_build/src/build/matches/mod.rs

@@ -2000,6 +2000,9 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
             false,
         );
 
+        // If branch coverage is enabled, record this branch.
+        self.visit_coverage_conditional_let(pat, post_guard_block, otherwise_post_guard_block);
+
         post_guard_block.unit()
     }
 
@@ -2492,6 +2495,10 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
                 None,
                 true,
             );
+
+            // If branch coverage is enabled, record this branch.
+            this.visit_coverage_conditional_let(pattern, matching, failure);
+
             this.break_for_else(failure, this.source_info(initializer_span));
             matching.unit()
         });

compiler/rustc_mir_build/src/errors.rs

@@ -819,12 +819,13 @@ pub struct NontrivialStructuralMatch<'tcx> {
 }
 
 #[derive(Diagnostic)]
-#[diag(mir_build_exceeds_mcdc_condition_num_limit)]
-pub(crate) struct MCDCExceedsConditionNumLimit {
+#[diag(mir_build_exceeds_mcdc_condition_limit)]
+#[note]
+pub(crate) struct MCDCExceedsConditionLimit {
     #[primary_span]
     pub span: Span,
-    pub conditions_num: usize,
-    pub max_conditions_num: usize,
+    pub num_conditions: usize,
+    pub limit: usize,
 }
 
 #[derive(Diagnostic)]

compiler/rustc_mir_transform/src/coverage/mappings.rs

@@ -48,7 +48,7 @@ pub(super) struct MCDCDecision {
     pub(super) span: Span,
     pub(super) end_bcbs: BTreeSet<BasicCoverageBlock>,
     pub(super) bitmap_idx: u32,
-    pub(super) conditions_num: u16,
+    pub(super) num_conditions: u16,
     pub(super) decision_depth: u16,
 }
 
@@ -136,8 +136,8 @@ pub(super) fn generate_coverage_spans(
     // Determine the length of the test vector bitmap.
     let test_vector_bitmap_bytes = mcdc_decisions
         .iter()
-        .map(|&MCDCDecision { bitmap_idx, conditions_num, .. }| {
-            bitmap_idx + (1_u32 << u32::from(conditions_num)).div_ceil(8)
+        .map(|&MCDCDecision { bitmap_idx, num_conditions, .. }| {
+            bitmap_idx + (1_u32 << u32::from(num_conditions)).div_ceil(8)
         })
         .max()
         .unwrap_or(0);
@@ -266,13 +266,13 @@ pub(super) fn extract_mcdc_mappings(
                 .collect::<Option<_>>()?;
 
             let bitmap_idx = next_bitmap_idx;
-            next_bitmap_idx += (1_u32 << decision.conditions_num).div_ceil(8);
+            next_bitmap_idx += (1_u32 << decision.num_conditions).div_ceil(8);
 
             Some(MCDCDecision {
                 span,
                 end_bcbs,
                 bitmap_idx,
-                conditions_num: decision.conditions_num as u16,
+                num_conditions: decision.num_conditions as u16,
                 decision_depth: decision.decision_depth,
             })
         },

compiler/rustc_mir_transform/src/coverage/mod.rs

@@ -182,9 +182,9 @@ fn create_mappings<'tcx>(
     ));
 
     mappings.extend(coverage_spans.mcdc_decisions.iter().filter_map(
-        |&mappings::MCDCDecision { span, bitmap_idx, conditions_num, .. }| {
+        |&mappings::MCDCDecision { span, bitmap_idx, num_conditions, .. }| {
             let code_region = region_for_span(span)?;
-            let kind = MappingKind::MCDCDecision(DecisionInfo { bitmap_idx, conditions_num });
+            let kind = MappingKind::MCDCDecision(DecisionInfo { bitmap_idx, num_conditions });
             Some(Mapping { kind, code_region })
         },
     ));
@@ -260,7 +260,7 @@ fn inject_mcdc_statements<'tcx>(
         span: _,
         ref end_bcbs,
         bitmap_idx,
-        conditions_num: _,
+        num_conditions: _,
         decision_depth,
     } in &coverage_spans.mcdc_decisions
     {

compiler/rustc_mir_transform/src/coverage/query.rs

@@ -61,17 +61,7 @@ fn coverage_ids_info<'tcx>(
         .max()
         .unwrap_or(CounterId::ZERO);
 
-    let mcdc_bitmap_bytes = mir_body
-        .coverage_branch_info
-        .as_deref()
-        .map(|info| {
-            info.mcdc_decision_spans
-                .iter()
-                .fold(0, |acc, decision| acc + (1_u32 << decision.conditions_num).div_ceil(8))
-        })
-        .unwrap_or_default();
-
-    CoverageIdsInfo { max_counter_id, mcdc_bitmap_bytes }
+    CoverageIdsInfo { max_counter_id }
 }
 
 fn all_coverage_in_mir_body<'a, 'tcx>(

library/core/src/num/uint_macros.rs

@@ -2764,6 +2764,7 @@ macro_rules! uint_impl {
         /// ```
         #[doc = concat!("assert_eq!(2", stringify!($SelfT), ".next_power_of_two(), 2);")]
         #[doc = concat!("assert_eq!(3", stringify!($SelfT), ".next_power_of_two(), 4);")]
+        #[doc = concat!("assert_eq!(0", stringify!($SelfT), ".next_power_of_two(), 1);")]
         /// ```
         #[stable(feature = "rust1", since = "1.0.0")]
         #[rustc_const_stable(feature = "const_int_pow", since = "1.50.0")]