Skip to content

coverage: Attribute macro-generated function bodies to their invocation site - #158833

Open
alex wants to merge 2 commits into
rust-lang:mainfrom
alex:coverage-macro-invocation-site
Open

coverage: Attribute macro-generated function bodies to their invocation site#158833
alex wants to merge 2 commits into
rust-lang:mainfrom
alex:coverage-macro-invocation-site

Conversation

@alex

@alex alex commented Jul 6, 2026

Copy link
Copy Markdown
Member

Functions whose bodies are entirely proc-macro-generated (e.g. the impls from a #[derive(...)] macro) have every statement span pointing at the macro invocation site, because proc-macro output tokens get call-site spans.

Before Rust 1.84, the empty invocation-site spans that survived span refinement were widened by one column, so the invocation line reported the generated code's execution count via the entry counter. PR #132675 (1.84) restricted that widening to spans adjacent to '{' or '}', which as a side effect left such functions either completely uninstrumented, or with a single leftover sliver region wired to an arbitrary non-entry counter that reads 0 even when the function runs.

Restore the pre-1.84 semantics deliberately: when a function body is entirely macro-expanded and its spans point back into the invocation site (distinguishing proc-macro output from macro_rules! definition-site tokens, which normal refinement still handles), emit a single code region over the visible invocation site - the derive-macro path, the attribute, or name! - wired to the function's entry counter.

Never-called generated functions get the same region as an unused-function mapping (count 0); executed siblings from the same invocation share the span, so llvm-cov's aggregation covers the line. #[automatically_derived] impls remain uninstrumented, as before.

…on site

Functions whose bodies are entirely proc-macro-generated (e.g. the impls
from a `#[derive(...)]` macro) have every statement span pointing at the
macro invocation site, because proc-macro output tokens get call-site spans.

Before Rust 1.84, the empty invocation-site spans that survived span
refinement were widened by one column, so the invocation line reported the
generated code's execution count via the entry counter. PR rust-lang#132675 (1.84)
restricted that widening to spans adjacent to '{' or '}', which as a side
effect left such functions either completely uninstrumented, or with a
single leftover sliver region wired to an arbitrary non-entry counter that
reads 0 even when the function runs.

Restore the pre-1.84 semantics deliberately: when a function body is
entirely macro-expanded and its spans point back into the invocation site
(distinguishing proc-macro output from `macro_rules!` definition-site
tokens, which normal refinement still handles), emit a single code region
over the visible invocation site - the derive-macro path, the attribute, or
`name!` - wired to the function's entry counter.

Never-called generated functions get the same region as an unused-function
mapping (count 0); executed siblings from the same invocation share the
span, so llvm-cov's aggregation covers the line. `#[automatically_derived]`
impls remain uninstrumented, as before.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@rustbot

rustbot commented Jul 6, 2026

Copy link
Copy Markdown
Collaborator

Some changes occurred to MIR optimizations

cc @rust-lang/wg-mir-opt

Some changes occurred in coverage tests.

cc @Zalathar

Some changes occurred in coverage instrumentation.

cc @Zalathar

@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 Jul 6, 2026
@rustbot

rustbot commented Jul 6, 2026

Copy link
Copy Markdown
Collaborator

r? @wesleywiser

rustbot has assigned @wesleywiser.
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 21 candidates

@rustbot

rustbot commented Jul 6, 2026

Copy link
Copy Markdown
Collaborator

⚠️ Warning ⚠️

  • There are issue links (such as #123) in the commit messages of the following commits.
    Please move them to the PR description, to avoid spamming the issues with references to the commit, and so this bot can automatically canonicalize them to avoid issues with subtree.

@alex

alex commented Jul 6, 2026

Copy link
Copy Markdown
Member Author

@Zalathar FYI, this undoes one of the consequences of #132675. I don't think it regresses though :-)

@alex

alex commented Jul 27, 2026

Copy link
Copy Markdown
Member Author

@rustbot reroll

@rustbot rustbot assigned JohnTitor and unassigned wesleywiser Jul 27, 2026
Comment thread compiler/rustc_mir_transform/src/coverage/spans.rs Outdated
Review feedback: `extract_refined_covspans` already has an `ExpnTree`
whose nodes cache each expansion's kind, call site, and parent context, so
walk that tree instead of re-deriving the same data from
`outer_expn_data()`. Also drops the explicit dummy-span check (the tree
already stores dummy call sites as `None`) and unwraps `macro_kind`,
since the loop cannot exit without seeing at least one macro expansion.

No functional change intended; coverage test snapshots are unchanged.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@alex

alex commented Aug 5, 2026

Copy link
Copy Markdown
Member Author

If I understand the Rust LLM contribution policy, does this need to be closed (it was developed by/in conjunction with Claude)?

@oli-obk

oli-obk commented Aug 6, 2026

Copy link
Copy Markdown
Contributor

We're explicitly not retconning the rules. To avoid it landing in reroll-hell, maybe still check in the #llm-mentoring stream if there is a reviewer who knows about this area willing to review it

@Zalathar

Zalathar commented Aug 6, 2026

Copy link
Copy Markdown
Member

It’s difficult to accept ad-hoc special cases in this area of code, even if they improve coverage output, because they impose steep costs on more fundamental improvements to how macro expansions are handled.

@alex

alex commented Aug 6, 2026

Copy link
Copy Markdown
Member Author

@Zalathar That's a comment about the nature of the PR itself (special casing macro-generated code), not LLMs, is that right?

Assuming I'm understanding correctly, it sounds like this is wontfix until the more fundamental improvements occur? If that's right, I guess the thing for me to do is close this, and turn it into an issue so this doesn't get lost -- is there a place to follow (or even help) with the blocking work?

@JohnTitor

Copy link
Copy Markdown
Member

r? Zalathar
I guess you're familiar with this

@rustbot rustbot assigned Zalathar and unassigned JohnTitor Aug 8, 2026
@rustbot

rustbot commented Aug 8, 2026

Copy link
Copy Markdown
Collaborator

Zalathar is not on the review rotation at the moment.
They may take a while to respond.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

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.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants