Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Rollup of 3 pull requests #124856

Closed
wants to merge 10 commits into from

Conversation

matthiaskrgr
Copy link
Member

Successful merges:

r? @ghost
@rustbot modify labels: rollup

Create a similar rollup

Zalathar and others added 10 commits April 30, 2024 22:35
This code for recalculating `mcdc_bitmap_bytes` doesn't provide any benefit,
because its result won't have changed from the value in `FunctionCoverageInfo`
that was computed during the MIR instrumentation pass.
These comments appear to be inspired by the similar comments on
`CounterIncrement` and `ExpressionUsed`. But those comments refer to specific
simplification steps performed during coverage codegen, and there is no
corresponding step for the MC/DC coverage statements.
This field counts the number of conditions that contribute to a particular
decision, but the name "conditions num" sounds like an ID instead of a count,
so "num conditions" is clearer.
…er-errors

coverage: Branch coverage support for let-else and if-let

This PR adds branch coverage instrumentation for let-else and if-let, including let-chains.

This lifts two of the limitations listed at rust-lang#124118.
…r-errors

coverage: Clean up `mcdc_bitmap_bytes` and `conditions_num`

This is a combination of two mostly-separate MC/DC coverage cleanups that would conflict with each other, plus some extra tests that appeared along the way.

The first change is to stop recomputing `mcdc_bitmap_bytes` in the query that produces `CoverageIdsInfo`. This appears to have been inspired by how we were already computing `max_counter_id`, but there's an important difference between the two cases.

When computing `max_counter_id`, the highest counter ID seen might be less than the highest ID used during MIR instrumentation, because some counter-increment statements might have been removed by MIR optimizations.

But for the recomputation used for `mcdc_bitmap_bytes`, that's impossible, because both computations are based on pre-optimization info. So there's no need to recompute the exact same value, when it can't have changed

---

The second change is to rename `conditions_num` to `num_conditions`, to make it clearer that this refers to a *number of conditions*, not some kind of ID number.

Because this change touched the compiler warning for a decision containing too many conditions, I also noticed that we didn't have any tests for that warning.

(It now seems a bit strange to me that this is a compiler warning, not a lint, because it can't be silenced or denied by the usual mechanisms for controlling lints. But I consider that change to be beyond the scope of this PR.)
…tmcm

next_power_of_two: add a doctest to show what happens on 0
@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. T-libs Relevant to the library team, which will review and decide on the PR/issue. rollup A PR which is a rollup labels May 7, 2024
@matthiaskrgr
Copy link
Member Author

@bors r+ rollup=never p=3

@bors
Copy link
Contributor

bors commented May 7, 2024

📌 Commit ec9020d has been approved by matthiaskrgr

It is now in the queue for this repository.

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels May 7, 2024
@bors
Copy link
Contributor

bors commented May 7, 2024

⌛ Testing commit ec9020d with merge 4883951...

bors added a commit to rust-lang-ci/rust that referenced this pull request May 7, 2024
…iaskrgr

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
@rust-log-analyzer
Copy link
Collaborator

The job x86_64-gnu-llvm-18 failed! Check out the build log: (web) (plain)

Click to see the possible cause of the failure (guessed by this bot)
failures:

---- [ui] tests/ui/instrument-coverage/mcdc-condition-limit.rs#bad stdout ----

error in revision `bad`: test compilation failed although it shouldn't!
status: exit status: 101
command: env -u RUSTC_LOG_COLOR RUSTC_ICE="0" RUST_BACKTRACE="short" "/checkout/obj/build/x86_64-unknown-linux-gnu/stage1/bin/rustc" "/checkout/tests/ui/instrument-coverage/mcdc-condition-limit.rs" "-Zthreads=1" "-Zsimulate-remapped-rust-src-base=/rustc/FAKE_PREFIX" "-Ztranslate-remapped-path-to-local-path=no" "-Z" "ignore-directory-in-diagnostics-source-blocks=/cargo" "-Z" "ignore-directory-in-diagnostics-source-blocks=/checkout/vendor" "--sysroot" "/checkout/obj/build/x86_64-unknown-linux-gnu/stage1" "--target=x86_64-unknown-linux-gnu" "--cfg" "bad" "--check-cfg" "cfg(FALSE,good,bad)" "--error-format" "json" "--json" "future-incompat" "-Ccodegen-units=1" "-Zui-testing" "-Zdeduplicate-diagnostics=no" "-Zwrite-long-types-to-disk=no" "-Cstrip=debuginfo" "--emit" "metadata" "-C" "prefer-dynamic" "--out-dir" "/checkout/obj/build/x86_64-unknown-linux-gnu/test/ui/instrument-coverage/mcdc-condition-limit.bad" "-A" "unused" "-A" "internal_features" "-Crpath" "-Cdebuginfo=0" "-Lnative=/checkout/obj/build/x86_64-unknown-linux-gnu/native/rust-test-helpers" "-L" "/checkout/obj/build/x86_64-unknown-linux-gnu/test/ui/instrument-coverage/mcdc-condition-limit.bad/auxiliary" "--edition=2021" "-Cinstrument-coverage" "-Zcoverage-options=mcdc" "-Zno-profiler-runtime"
--- stderr -------------------------------
thread 'rustc' panicked at compiler/rustc_mir_build/src/build/coverageinfo/mcdc.rs:219:38:
attempt to subtract with overflow
stack backtrace:
stack backtrace:
   0: rust_begin_unwind
   1: core::panicking::panic_fmt
   2: core::panicking::panic_const::panic_const_sub_overflow
   3: <rustc_mir_build::build::coverageinfo::mcdc::MCDCInfoBuilder>::visit_evaluated_condition::<<rustc_mir_build::build::Builder>::visit_coverage_branch_condition::{closure#0}>
   4: <rustc_mir_build::build::Builder>::then_else_break_inner
   5: <rustc_mir_build::build::Builder>::then_else_break_inner
   6: <rustc_mir_build::build::Builder>::then_else_break_inner
   7: <rustc_mir_build::build::Builder>::then_else_break_inner
   9: <rustc_mir_build::build::Builder>::in_scope::<<rustc_mir_build::build::Builder>::expr_into_dest::{closure#0}::{closure#0}, ()>
  10: <rustc_mir_build::build::Builder>::expr_into_dest::{closure#0}
  11: <rustc_mir_build::build::Builder>::expr_into_dest
  12: <rustc_mir_build::build::Builder>::ast_block_stmts
  12: <rustc_mir_build::build::Builder>::ast_block_stmts
  13: <rustc_mir_build::build::Builder>::in_scope::<<rustc_mir_build::build::Builder>::ast_block::{closure#0}, ()>
  14: <rustc_mir_build::build::Builder>::expr_into_dest
  15: <rustc_mir_build::build::Builder>::in_scope::<<rustc_mir_build::build::Builder>::expr_into_dest::{closure#0}::{closure#0}, ()>
  16: <rustc_mir_build::build::Builder>::expr_into_dest::{closure#0}
  17: <rustc_mir_build::build::Builder>::expr_into_dest
  18: rustc_mir_build::build::mir_build
  19: rustc_mir_transform::mir_built
      [... omitted 2 frames ...]
  20: rustc_mir_build::check_unsafety::check_unsafety
      [... omitted 2 frames ...]
  21: <rustc_middle::hir::map::Map>::par_body_owners::<rustc_interface::passes::run_required_analyses::{closure#1}::{closure#0}>::{closure#0}
  22: std::panicking::try::<(), core::panic::unwind_safe::AssertUnwindSafe<rustc_data_structures::sync::parallel::enabled::par_for_each_in<&rustc_span::def_id::LocalDefId, &[rustc_span::def_id::LocalDefId], <rustc_middle::hir::map::Map>::par_body_owners<rustc_interface::passes::run_required_analyses::{closure#1}::{closure#0}>::{closure#0}>::{closure#0}::{closure#1}::{closure#0}>>
  23: <rustc_data_structures::sync::parallel::ParallelGuard>::run::<(), rustc_data_structures::sync::parallel::enabled::par_for_each_in<&rustc_span::def_id::LocalDefId, &[rustc_span::def_id::LocalDefId], <rustc_middle::hir::map::Map>::par_body_owners<rustc_interface::passes::run_required_analyses::{closure#1}::{closure#0}>::{closure#0}>::{closure#0}::{closure#1}::{closure#0}>
  24: <rustc_session::session::Session>::time::<(), rustc_interface::passes::run_required_analyses::{closure#1}>
  25: rustc_interface::passes::analysis
      [... omitted 2 frames ...]
  27: <rustc_interface::interface::Compiler>::enter::<rustc_driver_impl::run_compiler::{closure#0}::{closure#1}, core::result::Result<core::option::Option<rustc_interface::queries::Linker>, rustc_span::ErrorGuaranteed>>
  28: <scoped_tls::ScopedKey<rustc_span::SessionGlobals>>::set::<rustc_interface::util::run_in_thread_with_globals<rustc_interface::util::run_in_thread_pool_with_globals<rustc_interface::interface::run_compiler<core::result::Result<(), rustc_span::ErrorGuaranteed>, rustc_driver_impl::run_compiler::{closure#0}>::{closure#1}, core::result::Result<(), rustc_span::ErrorGuaranteed>>::{closure#0}, core::result::Result<(), rustc_span::ErrorGuaranteed>>::{closure#0}::{closure#0}::{closure#0}, core::result::Result<(), rustc_span::ErrorGuaranteed>>
  29: rustc_span::create_session_globals_then::<core::result::Result<(), rustc_span::ErrorGuaranteed>, rustc_interface::util::run_in_thread_with_globals<rustc_interface::util::run_in_thread_pool_with_globals<rustc_interface::interface::run_compiler<core::result::Result<(), rustc_span::ErrorGuaranteed>, rustc_driver_impl::run_compiler::{closure#0}>::{closure#1}, core::result::Result<(), rustc_span::ErrorGuaranteed>>::{closure#0}, core::result::Result<(), rustc_span::ErrorGuaranteed>>::{closure#0}::{closure#0}::{closure#0}>
note: Some details are omitted, run with `RUST_BACKTRACE=full` for a verbose backtrace.
---
note: please make sure that you have updated to the latest nightly

note: rustc 1.80.0-nightly (488395147 2024-05-07) running on x86_64-unknown-linux-gnu

note: compiler flags: -Z threads=1 -Z simulate-remapped-rust-src-base=/rustc/FAKE_PREFIX -Z translate-remapped-path-to-local-path=no -Z ignore-directory-in-diagnostics-source-blocks=/cargo -Z ignore-directory-in-diagnostics-source-blocks=/checkout/vendor -C codegen-units=1 -Z ui-testing -Z deduplicate-diagnostics=no -Z write-long-types-to-disk=no -C strip=debuginfo -C prefer-dynamic -C rpath -C debuginfo=0 -C instrument-coverage -Z coverage-options=mcdc -Z no-profiler-runtime
query stack during panic:
query stack during panic:
#0 [mir_built] building MIR for `main`
#1 [check_unsafety] unsafety-checking `main`
end of query stack
------------------------------------------


@bors
Copy link
Contributor

bors commented May 7, 2024

💔 Test failed - checks-actions

@bors bors added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels May 7, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
rollup A PR which is a rollup S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. T-libs Relevant to the library team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants