-
Notifications
You must be signed in to change notification settings - Fork 12.9k
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
Unexplained errors applying -Z instrument-coverage
to some rustc ui
tests
#80045
Comments
I created this issue for the 4 tests that created errors or output that I can't explain. |
I don't know if any of these issues are actually bugs in |
Backtrace:
The error happens during drop elaboration. I'm guessing |
This indicates a possible performance issue with that query; maybe there's a way we can implement it that doesn't demand optimized MIR. EDIT: The fact that this is causing new errors might also mean that it isn't correct to rely on that query, but that is less clear to me. Also, I feel like this function should be getting called by cc @wesleywiser |
..skipping the layout one for now..
Here we're generating MIR for a function which can never be called, due to its |
I haven't looked into the layout issue yet but it looks like the coverage code is causing a layout to be computed that wasn't before. Unclear to me if that's actually a problem or not; the test is relying on the side effectful outputs of a query computed on demand so that's not going to be super robust. |
I can't think of a reason the "unreachable function handling" would mark anything as used. It also seems "late" in the process (well after MIR processing is done). But in any case, it doesn't (at least not intentionally) actually use the code. It just gets the spans for unused functions, and adds code regions for those spans to the coverage map for a function that is used (being codegenned). But outside of that, if an unused function (as reported by these errors) is still going through codegen anyway, the coverage map variables (generated by LLVM) may add references to those functions' symbols that could mark functions "used" (perhaps only indirectly) in LLVM IR. Are the errors generated based on an assessment of used or not used, from an LLVM perspective? Or just from MIR? If only from MIR, then I don't think the coverage map generation would play a role in marking them used; and I would look at the |
LLVM [IR] won't affect any compiler messages emitted by rustc. I also can't think of how the unreachable code handling would affect this, just throwing out ideas. It does seem that without coverage enabled the functions are marked unused, and with coverage they are considered used, and I'm not sure why. |
The regression in the first case is legitimate and not related to unused functions. If you look at the MIR after rust/compiler/rustc_mir/src/dataflow/impls/mod.rs Lines 680 to 717 in 4031f7b
We used to have a bespoke terminator for that operation, but it was refactored away. To fix the test, modify that function to skip over code coverage statements when looking for a discriminant read or put the code coverage statements elsewhere. |
@tmandry - I currently inject the I think I will try injecting the statement at the front of the statements vec, and in the first ( I'll see if that resolves it. Who knows, maybe it will help with other issues. |
See rust-lang#80045 (comment) Coverage statements are moved to the beginning of the BCB. This does also affect what's counted before a panic, changing some results, but I think these results may even be preferred? In any case, there are no guarantees about what's counted when a panic occurs (by design).
$ ./x.py test --rustc-args="-Zinstrument-coverage" src/test/ui/consts/unstable-precise-live-drops-in-libcore.rs Now passes, with #80072 |
1 down, 3 to go. The fix in #80072 did not fix tests 2, 3, and 4 above. |
…2.1, r=tmandry Fixed conflict with drop elaboration and coverage See rust-lang#80045 (comment) Coverage statements are moved to the beginning of the BCB. This does also affect what's counted before a panic, changing some results, but I think these results may even be preferred? In any case, there are no guarantees about what's counted when a panic occurs (by design). r? `@tmandry` FYI `@wesleywiser` `@ecstatic-morse`
When experimentally adding
-Zinstrument-coverage
to the set ofui
tests in the Rust source tree, most tests still pass; of those that don't, almost all failures can be explained, and some improvements have been made to surface known incompatibilities between-Zinstrument-coverage
and other compiler options (see #79958 (comment)).However, there are
43 tests that still fail with-Zinstrument-coverage
that I don't understand.I'm hoping someone(s) that understand these tests and/or compiler options can take a look, explain why they are expected to fail, or clarify what the bug might be, if they should not fail.
Fixed via #80072
$ ./x.py test --rustc-args="-Zinstrument-coverage" src/test/ui/consts/unstable-precise-live-drops-in-libcore.rsFailure due to:error[E0493]: destructors cannot be evaluated at compile-time
--> /usr/local/google/home/richkadel/rust/src/test/ui/consts/unstable-precise-live-drops-in-libcore.rs:15:25
|
LL | pub const fn unwrap(self) -> T {
| ^^^^ constant functions cannot evaluate destructors
(this error is printed twice)
The text was updated successfully, but these errors were encountered: