Skip to content

Change is_eligible_for_coverage from a hook to a query - #161813

Merged
rust-bors[bot] merged 1 commit into
rust-lang:mainfrom
Zalathar:is-eligible
Aug 27, 2026
Merged

Change is_eligible_for_coverage from a hook to a query#161813
rust-bors[bot] merged 1 commit into
rust-lang:mainfrom
Zalathar:is-eligible

Conversation

@Zalathar

Copy link
Copy Markdown
Member

This check is called from a few different places when coverage is enabled, so we should probably let the query system take care of memoizing results and tracking dependencies.

(It was made a hook in #122322, but I didn't have strong reasons for making it a hook and not a query, other than it being relatively small and simple.)

There should be no user-visible change to compiler behaviour.

This check is called from a few different places when coverage is enabled, so
we should probably let the query system take care of memoizing results and
tracking dependencies.
@Zalathar Zalathar added the A-code-coverage Area: Source-based code coverage (-Cinstrument-coverage) label Aug 26, 2026
@rustbot

rustbot commented Aug 26, 2026

Copy link
Copy Markdown
Collaborator

Some changes occurred to MIR optimizations

cc @rust-lang/wg-mir-opt

@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. labels Aug 26, 2026
@rustbot

rustbot commented Aug 26, 2026

Copy link
Copy Markdown
Collaborator

r? @JonathanBrouwer

rustbot has assigned @JonathanBrouwer.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

Why was this reviewer chosen?

The reviewer was selected based on:

  • Owners of files modified in this PR: compiler
  • compiler expanded to 75 candidates
  • Random selection from 18 candidates

@JonathanBrouwer

Copy link
Copy Markdown
Member

@bors try @rust-timer queue

@rust-timer

This comment has been minimized.

@rustbot rustbot added the S-waiting-on-perf Status: Waiting on a perf run to be completed. label Aug 26, 2026
@rust-bors

This comment has been minimized.

rust-bors Bot pushed a commit that referenced this pull request Aug 26, 2026
Change `is_eligible_for_coverage` from a hook to a query
@JonathanBrouwer

JonathanBrouwer commented Aug 26, 2026

Copy link
Copy Markdown
Member

Hmmm I guess coverage is not enabled in rust-timer so this should be neutral. Maybe having more queries has some overhead? Better to check anyways

@JonathanBrouwer JonathanBrouwer left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

r=me if perf is neutral or better

View changes since this review

@rust-bors

rust-bors Bot commented Aug 26, 2026

Copy link
Copy Markdown
Contributor

☀️ Try build successful (CI)
Build commit: a872013 (a8720131acf959cfb2feabffdc3a7bec026da4b6)
Base parent: 3b8ee6c (3b8ee6c0ca55afb08e2e130003227a3195394425)

@rust-timer

This comment has been minimized.

@rust-timer

Copy link
Copy Markdown
Collaborator

Finished benchmarking commit (a872013): comparison URL.

Overall result: no relevant changes - no action needed

Benchmarking means the PR may be perf-sensitive. Consider adding rollup=never if this change is not fit for rolling up.

@rustbot label: -S-waiting-on-perf -perf-regression

Instruction count

This perf run didn't have relevant results for this metric.

Max RSS (memory usage)

Results (primary 1.2%, secondary -2.8%)

A less reliable metric. May be of interest, but not used to determine the overall result above.

mean range count
Regressions ❌
(primary)
3.9% [2.5%, 5.2%] 2
Regressions ❌
(secondary)
- - 0
Improvements ✅
(primary)
-4.2% [-4.2%, -4.2%] 1
Improvements ✅
(secondary)
-2.8% [-2.8%, -2.8%] 1
All ❌✅ (primary) 1.2% [-4.2%, 5.2%] 3

Cycles

Results (secondary 2.6%)

A less reliable metric. May be of interest, but not used to determine the overall result above.

mean range count
Regressions ❌
(primary)
- - 0
Regressions ❌
(secondary)
2.6% [2.6%, 2.6%] 1
Improvements ✅
(primary)
- - 0
Improvements ✅
(secondary)
- - 0
All ❌✅ (primary) - - 0

Binary size

Results (secondary -0.0%)

A less reliable metric. May be of interest, but not used to determine the overall result above.

mean range count
Regressions ❌
(primary)
- - 0
Regressions ❌
(secondary)
- - 0
Improvements ✅
(primary)
- - 0
Improvements ✅
(secondary)
-0.0% [-0.0%, -0.0%] 3
All ❌✅ (primary) - - 0

Bootstrap: 473.778s -> 474.228s (0.09%)
Artifact size: 402.59 MiB -> 402.62 MiB (0.01%)

@rustbot rustbot removed the S-waiting-on-perf Status: Waiting on a perf run to be completed. label Aug 26, 2026
@Zalathar

Copy link
Copy Markdown
Member Author

Perf is neutral.

@bors r=JonathanBrouwer rollup

@rust-bors

rust-bors Bot commented Aug 27, 2026

Copy link
Copy Markdown
Contributor

📌 Commit 2304aed has been approved by JonathanBrouwer

It is now in the queue for this repository.

@rust-bors rust-bors Bot 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 Aug 27, 2026
jhpratt added a commit to jhpratt/rust that referenced this pull request Aug 27, 2026
…ouwer

Change `is_eligible_for_coverage` from a hook to a query

- Inspired by seeing rust-lang#161808 add more eligibility conditions
---

This check is called from a few different places when coverage is enabled, so we should probably let the query system take care of memoizing results and tracking dependencies.

(It was made a hook in rust-lang#122322, but I didn't have strong reasons for making it a hook and not a query, other than it being relatively small and simple.)

There should be no user-visible change to compiler behaviour.
rust-bors Bot pushed a commit that referenced this pull request Aug 27, 2026
Rollup of 7 pull requests

Successful merges:

 - #150075 (Implement clamp_to)
 - #161034 (Add SVE-accelerated Vec::retain_mut for aarch64)
 - #161628 (interpret: ensure that calls via no-unwind ABIs do not unwind)
 - #161012 (borrowck: Normalize non-rigid aliases in NLL type relating)
 - #161702 (Use `drop_guard` in some places in {core,alloc,std})
 - #161813 (Change `is_eligible_for_coverage` from a hook to a query)
 - #161842 (chore: fix cargo lints)
jhpratt added a commit to jhpratt/rust that referenced this pull request Aug 27, 2026
…ouwer

Change `is_eligible_for_coverage` from a hook to a query

- Inspired by seeing rust-lang#161808 add more eligibility conditions
---

This check is called from a few different places when coverage is enabled, so we should probably let the query system take care of memoizing results and tracking dependencies.

(It was made a hook in rust-lang#122322, but I didn't have strong reasons for making it a hook and not a query, other than it being relatively small and simple.)

There should be no user-visible change to compiler behaviour.
rust-bors Bot pushed a commit that referenced this pull request Aug 27, 2026
Rollup of 6 pull requests

Successful merges:

 - #161034 (Add SVE-accelerated Vec::retain_mut for aarch64)
 - #161628 (interpret: ensure that calls via no-unwind ABIs do not unwind)
 - #161012 (borrowck: Normalize non-rigid aliases in NLL type relating)
 - #161702 (Use `drop_guard` in some places in {core,alloc,std})
 - #161813 (Change `is_eligible_for_coverage` from a hook to a query)
 - #161842 (chore: fix cargo lints)
rust-bors Bot pushed a commit that referenced this pull request Aug 27, 2026
Rollup of 7 pull requests

Successful merges:

 - #161034 (Add SVE-accelerated Vec::retain_mut for aarch64)
 - #161628 (interpret: ensure that calls via no-unwind ABIs do not unwind)
 - #161012 (borrowck: Normalize non-rigid aliases in NLL type relating)
 - #161691 (Assorted bootstrap config refactors (part 1/N))
 - #161813 (Change `is_eligible_for_coverage` from a hook to a query)
 - #161842 (chore: fix cargo lints)
 - #161843 (rustdoc: fix lint `cargo::non_kebab_case_bins`)
@rust-bors
rust-bors Bot merged commit 6cfd543 into rust-lang:main Aug 27, 2026
14 checks passed
@rustbot rustbot added this to the 1.100.0 milestone Aug 27, 2026
rust-bors Bot pushed a commit that referenced this pull request Aug 27, 2026
Rollup merge of #161813 - Zalathar:is-eligible, r=JonathanBrouwer

Change `is_eligible_for_coverage` from a hook to a query

- Inspired by seeing #161808 add more eligibility conditions
---

This check is called from a few different places when coverage is enabled, so we should probably let the query system take care of memoizing results and tracking dependencies.

(It was made a hook in #122322, but I didn't have strong reasons for making it a hook and not a query, other than it being relatively small and simple.)

There should be no user-visible change to compiler behaviour.
@Zalathar
Zalathar deleted the is-eligible branch August 27, 2026 07:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

A-code-coverage Area: Source-based code coverage (-Cinstrument-coverage) S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants