-Zinstrument-coverage showing missed coverage on _part_ of function signature (with proc-macro?) #86972
Comments
jonas-schievink
added
the
A-code-coverage
|
FYI: @richkadel |
|
|
|
I tried minimizing with the following macro: #[proc_macro_attribute]
pub fn pyfunction(attr: proc_macro::TokenStream, input: proc_macro::TokenStream) -> proc_macro::TokenStream {
let ast = syn::parse_macro_input!(input as syn::ItemFn);
quote::quote!(
#ast
).into()
}This produces the following extremely confusing coverage report, where it seems that not only does the function not get covered, but also the test function isn't covered at all!
|
|
@davidhewitt this happened when we upgraded to pyo3 0.14. I skimmed that diff and couldn't see what might have led to a change in a coverage. Do you have any ideas? |
|
Upgraded from which earlier version? |
|
0.13.1 -- pyca/cryptography@120e804 is the commit that cause the loss of coverage |
Obviously I don't know how you invoked this, but based on the output you're showing here, it looks like you didn't invoke the test, when running the binary that produced those coverage results; in which case, the results are correct. The compiler compiled the test and the proc_macro, and shows the correct coverage spans, and if they were not executed, then I know that some proc_macros do work, but I'm sure the coverage results can be "fooled" by other proc macros. Things that factor into it are: Does the generated code (e.g., from Since coverage has no insight into the semantic intent of a proc_macro, it may be impossible to infer a mapping that looks reasonable to the human eye. In the case of your original example, since half of the function signature appears to have been executed, but the full function signature appears to have coverage, with the second half showing My guess is there is a span produced for the full function signature for a function or closure that is not counted (covered by not called?) and another span produced for only part of the function signature (not covering the last argument or the return type) for some reason. It could be that there is logic to why the proc_macro returns quoted result spans this way, but if not, then it may be just a less than ideal computation of the mapped span (a bug only noticeable in coverage results or perhaps in an diagnostic messages from the compiler) that, if corrected, would fix your coverage. For example, I recently fixed a problem with a few instances of the following "mistake": let item = parse_macro_input!(item as syn::ItemFn);
let syn::ItemFn { attrs, sig, vis: _, block } = item;
...
let ret_type = sig.output;
let inputs = sig.inputs;
let span = sig.ident.span();
let ident = sig.ident;
let output = quote_spanned! {span=>
fn #ident () #ret_type {
async fn func(#inputs) #ret_type { <-- These braces are redundant, and
#block
} <-- create an inconsistent span map of the function body
...
}
}The This is pretty hard to figure out, and your problem is not that similar to this one, as far as I can tell (looking at the pyo library proc_macro code just a bit). No obvious fix here. I'm not actively working on the coverage code right now (I have had to shift to other projects at work), but at some point I'd like to get some time to try to address problems like these by solving #85000. I don't know if this will work, but it's a possible general solution for some macro issues, and at the moment the only thing I can think of for your issue, since a root cause is not jumping out at me. Maybe the proc_macro author can see how the macro spans could be improved? |
|
Is there a good way for us to debug the spans generated by a macro? I'm
more than happy to dive into that and attempt to just improve things on the
pyo3 side.
…On Mon, Jul 12, 2021 at 2:10 PM Rich Kadel ***@***.***> wrote:
This produces the following extremely confusing coverage report, where it
seems that not only does the function not get covered, but also the test
function isn't covered at all!
Obviously I don't know how you invoked this, but based on the output
you're showing here, it looks like you didn't invoke the test, when running
the binary that produced those coverage results; in which case, the results
are correct. The compiler compiled the test and the proc_macro, and shows
the correct coverage spans, and if they were not executed, then 0 is
correct.
I know that some proc_macros do work, but I'm sure the coverage results
can be "fooled" by other proc macros. Things that factor into it are: Does
the generated code (e.g., from quote!() or quote_spanned! produce a
syntacticly similar result or not (for example, in this case, from
something that looks like a function to something that is a function), and
are the spans mapped correctly (or at least sensibly).
Since coverage has no insight into the semantic intent of a proc_macro, it
may be impossible to infer a mapping that looks reasonable to the human eye.
In the case of your original example, since half of the function signature
appears to have been executed, but the full function signature appears to
have coverage, with the second half showing 0 executions, that likely
means there are at least two execution paths (could be 2 or more functions,
or could be branching logic in a single function, but my guess is the
former).
My guess is there is a span produced for the full function signature for a
function or closure that is not counted (covered by not called?) and
another span produced for only part of the function signature (not covering
the last argument or the return type) for some reason.
It could be that there is logic to why the proc_macro returns quoted
result spans this way, but if not, then it may be just a less than ideal
computation of the mapped span (a bug only noticeable in coverage results
or perhaps in an diagnostic messages from the compiler) that, if corrected,
would fix your coverage.
For example, I recently fixed a problem with a few instances of the
following "mistake":
let item = parse_macro_input!(item as syn::ItemFn);
let syn::ItemFn { attrs, sig, vis: _, block } = item;
...
let ret_type = sig.output;
let inputs = sig.inputs;
let span = sig.ident.span();
let ident = sig.ident;
let output = quote_spanned! {span=>
fn #ident () #ret_type {
async fn func(#inputs) #ret_type { <-- These braces are redundant, and
#block
} <-- create an inconsistent span map of the function body
...
}
}
The #block includes the function body braces in the original source. The
redundant braces in the proc_macro produce a function body that is in a
different syntax context from the function signature, so coverage can't
associate the function signature span with the function body span in the
same way it does without a proc_macro, breaking the ability to show
coverage results of anything in the original body!
This is pretty hard to figure out, and your problem is not that similar to
this one, as far as I can tell (looking at the pyo library proc_macro code
just a bit). No obvious fix here.
I'm not actively working on the coverage code right now (I have had to
shift to other projects at work), but at some point I'd like to get some
time to try to address problems *like* these by solving #85000
<#85000>. I don't know if this
will work, but it's a possible general solution for some macro issues, and
at the moment the only thing I can think of for your issue, since a root
cause is not jumping out at me.
Maybe the proc_macro author can see how the macro spans could be improved?
—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
<#86972 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AAAAGBGGKDNB6QDBD2KVLZLTXMV2VANCNFSM5AAUY2HQ>
.
--
All that is necessary for evil to succeed is for good people to do nothing.
|
I'm not sure if there's a "good" way, but I believe there is a compiler |
|
Ok, I've put a full reproducer in: https://github.com/alex/temp-rust-coverage With teh current contents of the repo So somehow the no-op proc-macro is causing a loss of coverage, even in spans it doesn't touch. I can't imagine what could cause that. But also I can't imagine what I did wrong in this reproducer. |
|
Thank you. This is helpful! |
|
OK, I tried your sample, and I think the problem you're having is related to how you are running and extracting the results. Your script works for me and does produce the results with missing coverage, but when I changed the first Dig into the documentation in the Rust Unstable Book to get more insights on this, but I think there are a couple of issues with your script.
This should fix your test program, but that doesn't mean it will fix your original issue unless your coverage results from your original program are also being generated incorrectly. Let me know if you can reproduce that. Thanks! |
|
Fascinating... I can confirm that with I don't really understand why having a proc-macro or not impacts this. Unfortunately this doesn't impact the underlying issue that caused me to file the bug. But at least now I have a reproduction environment where I can continue to pursue the real issue here. |
|
Ok, that repo has now been updated to reproduce the original issue here (partial coverage of arguments). If you change I suspect, but can't prove yet, that what's happening behind the scenes is that pyo3 is generating other code that's uses the spans of these arguments (which is common in proc-macros) so that if there's type-checking errors, the original line will be attributed. |
|
For future reference, the results from your new sample are shown below as a screenshot, so the specific coverage spans show up (by character). Also, FYI, I inserted a blank line at the top to reduce confusion. At least in my environment (using the
|
|
I haven't attempted to officially prove it by reverting the patch (since like 900 follow up patches rely on it)... but I'll bet a nickle that PyO3/pyo3#1440 is what introduced this issue, since it started generating code with spans on the I don't know exactly how the spans work for rust's coverage, but I'll bet something is going on like:
The coverage machinery sees the big span as covered from (1) but the sub-span of that from (2) as not covered and draws something what we see in that screenshot. Am I anywhere within 10 country miles of something plausible? |
|
Yes plausible. Not sure but it fits what we see. Did you notice this issue? A fix was submitted. Is that fix in the version you upgraded to? If not, just maybe that fix could help, but maybe a long shot. |
|
Yes, that fix is present in the version tested by the test repo (and the
real pyca/cryptography)
…On Wed, Jul 14, 2021 at 10:56 PM Rich Kadel ***@***.***> wrote:
Yes plausible. Not sure but it fits what we see.
Did you notice this issue?
PyO3/pyo3#1506 <PyO3/pyo3#1506>
A fix was submitted. Is that fix in the version you upgraded to? If not,
just maybe that fix could help, but maybe a long shot.
—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
<#86972 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AAAAGBFUDI7GOBHUEBETHODTXZE5HANCNFSM5AAUY2HQ>
.
--
All that is necessary for evil to succeed is for good people to do nothing.
|
|
A git bisect tells me that in fact PyO3/pyo3@ce851ad is the commit that introduce the coverage issues. Going to stare at the commit intently and see which part triggers it. |
…hey are uncovered rust-lang/rust#86972 describes this issue in some detail and links to a minimal reproducer making this change, and this change alone, resolves the coverage issues
|
I've now sent PyO3/pyo3#1726 as a workaround. |
|
Sorry I haven't participated in the discussion. Thanks both for all the digging, this is really interesting. |
|
At this point I have no idea if this is current, I'm going to close this. |
Starting with an apology that I have not yet figured out how to reduce this to something smaller than "our entire project".
We use
-Zinstrument-coveragein pyca/cryptography for capturing coverage. With a recent upgrade to pyo3, we're seeing missing coverage for part of the function signature. A screenshot from codecov (https://codecov.io/gh/pyca/cryptography/src/120e804d8a36b4adfa71b2e5700a773ff9cfd19a/src/rust/src/ocsp.rs):Having missing coverage on a function argument doesn't make a ton of sense.
The process by which we run this can be found here: https://github.com/pyca/cryptography/blob/main/.github/workflows/ci.yml#L207-L267
I am fearful that this is a challenging intersection between coverage and proc-macros, but as I said I regrettably haven't minimized this test case yet.
The text was updated successfully, but these errors were encountered: