Moved coverage counter injection from BasicBlock to Statement.

[richkadel]

As discussed on Zulip: https://rust-lang.zulipchat.com/#narrow/stream/233931-t-compiler.2Fmajor-changes/topic/Implement.20LLVM-compatible.20source-based.20cod.20compiler-team.23278

[rust-highfive]

r? @ecstatic-morse

(rust_highfive has picked a reviewer for you, use r? to override)

[richkadel]

r? @wesleywiser
FYI @tmandry

[richkadel]

This code builds for me without errors, even after I did another ./x.py clean. So I don't see why it's failing in CI with errors related to the derive...RustcEncodable, RustcDecodable....

I made a couple of what seem to be insignificant changes just now, and pushed them after successful testing locally. We'll see if something changes.

Note the way that I'm deriving these traits appears to be identical to how they are derived for other structs and enums, even in the same crate librustc_middle/mir/mod.rs, but the only failures are related to the new structs and enums that I just added.

Maybe I need a bootstrap config? I'm not sure.

Here's a snippet from the errors in the CI log, for reference:

    Checking rustc_builtin_macros v0.0.0 (/checkout/src/librustc_builtin_macros)
error[E0107]: wrong number of type arguments: expected 1, found 0
    --> src/librustc_middle/mir/mod.rs:1510:51
     |
1510 |   #[derive(Clone, Debug, PartialEq, RustcEncodable, RustcDecodable, HashStable, TypeFoldable)]
     |                                                     ^^^^^^^^^^^^^^
     |                                                     |
     |                                                     expected 1 type argument
     |                                                     in this macro invocation
     | 
    ::: /checkout/library/core/src/macros/mod.rs:1432:5
     |
1432 | /     pub macro RustcDecodable($item:item) {
1433 | |         /* compiler built-in */
1434 | |     }
     | |_____- in this expansion of `#[derive(RustcDecodable)]`

error[E0107]: wrong number of type arguments: expected 1, found 0
    --> src/librustc_middle/mir/mod.rs:1510:35
     |
1510 |   #[derive(Clone, Debug, PartialEq, RustcEncodable, RustcDecodable, HashStable, TypeFoldable)]
     |                                     ^^^^^^^^^^^^^^
     |                                     |
     |                                     expected 1 type argument

[richkadel]

I just tested this on my local Linux workstation, after completely deleting my build directory first, and I can't reproduce this error.

@wesleywiser @tmandry - Are you familiar with this kind of issue?

The only thing different is I synched up my fork from rust-lang/rust yesterday around noon.

I can try synching again, but it seems too high of a coincidence that someone changed something in RustcEncoder/Decoder in the last 24 hours. Plus the error is only affecting my new structs and enums, and none of the others in the same package.

So I assume there's a different explanation.

[Mark-Simulacrum]

#73851 merged 16 hours ago and is a pretty big rework of the serialization infra, I would rebase atop that.

[richkadel]

Ah. Thanks Mark. I'll do that.

[richkadel]

OK, yes, I can repro this locally now.

[richkadel]

@Mark-Simulacrum - I'd like to "@bors try" this PR with tests enabled, for Linux, Mac, and Windows-Gnu. (MSVC is not currently supported).

Can you provide current instructions to set that up?

Thanks!

[Mark-Simulacrum]

You should enable at most 2 builders on try at the same time (and wait to kick off a build until the previous one finishes), with at most 1 apple builder in any try run.

The steps are roughly:
For linux and windows: edit the try builder list here to include whichever builders you want, pulling from the rest of that file, e.g., in this case you'll want to look for name: x86_64-apple for the apple tests, name: x86_64-mingw-1 or name: x86_64-mingw-2 (2 runs UI and compile-fail tests, 1 runs everything else), and name: x86_64-gnu for main linux testing.

After editing run ./x.py run src/tools/expand-yaml-anchors and push

[bors]

☔ The latest upstream changes (presumably #75187) made this pull request unmergeable. Please resolve the merge conflicts.

[richkadel]

@Mark-Simulacrum - Thanks! I'm starting with Linux and MacOS (1 builder each).

See below. If I've done this wrong, let me know.

When you say _"...and wait to kick off a build until the previous one finishes", do you mean, if there is another try job currently in progress, manually watch the queue, wait for it to finish, and then submit @bors try request, hopefully getting it in before someone else?

Or is there a way I can automate this?

(FYI, I'm not sure I have the github permissions to start my own @bors try. I may need @wesleywiser or @tmandry to do it.)

    strategy:
      matrix:
        include:
          - name: x86_64-gnu
            <<: *job-linux-xl
          - name: x86_64-apple
            env:
              SCRIPT: ./x.py --stage 2 test
              RUST_CONFIGURE_ARGS: --build=x86_64-apple-darwin --enable-sanitizers --enable-profiler --set rust.jemalloc
              RUSTC_RETRY_LINKER_ON_SEGFAULT: 1
              MACOSX_DEPLOYMENT_TARGET: 10.8
              MACOSX_STD_DEPLOYMENT_TARGET: 10.7
              NO_LLVM_ASSERTIONS: 1
              NO_DEBUG_ASSERTIONS: 1
            <<: *job-macos-xl

[richkadel]

(FYI, if the snippet looks right, the PR has been updated, and is ready for the @bors try

[tmandry]

@bors try

[bors]

⌛ Trying commit 6381f8a4dcf3caf223b4289d465e932ae4f46de5 with merge 67ed07aa158e60bc073ab0ad5616071768ee2a5b...

[Mark-Simulacrum]

Snippet looks right. Mostly it should be fine to start these on this PR not in parallel (i.e., don't request another try build on new push until this one finishes); we don't actually have metrics to see how much capacity we have but we try to avoid spawning too many builders on non-auto branch

[bors]

☔ The latest upstream changes (presumably #75145) made this pull request unmergeable. Please resolve the merge conflicts.

[richkadel]

@Mark-Simulacrum - The try has been running for quite a while.

I'm not able to find the running logs for my try jobs. Is there a way for me to see what's going on?

Thanks

[bors]

⌛ Trying commit 36a96633aa7982177668f7bc607364cf3c666f22 with merge 56bc179652e0615fe28be47135c9b5b7c6589ab5...

[bors]

☀️ Try build successful - checks-actions, checks-azure
Build commit: 56bc179652e0615fe28be47135c9b5b7c6589ab5 (56bc179652e0615fe28be47135c9b5b7c6589ab5)

[richkadel]

@wesleywiser I made changes per your feedback, and also removed the changes to ci.yml files that expanded the bors try tests.

[bors]

☔ The latest upstream changes (presumably #75566) made this pull request unmergeable. Please resolve the merge conflicts.

[wesleywiser]

@richkadel

Do you want me to leave this as is, with StatementKind::Coverage(Box), or do you like the idea of StatementKind::Injected(Box)? (or something else)

I think for now, I'd prefer to leave it as StatementKind::Coverage. If we find a strong need to generalize this later, we can do that refactoring then.

[richkadel]

I just resolved the new conflicts in the test .diff files.

@wesleywiser - If you're ready, can we submit to bors with norollup? There have been a lot of conflicts over the past 24 hours.

Thanks

[richkadel]

Hmm, I must have missed the formatting issue last night. Fixed.

[wesleywiser]

cc @rust-lang/compiler

This PR adds a new MIR StatementKind which is used by the LLVM code coverage feature. Prior to this change, the approach used a few intrinsics to set up the code coverage instrumentation correctly but this required relatively complex modifications to generated MIR during the code coverage pass.

The new approach here is much simpler overall and most of the rest of rustc can treat this new statement as a nop.

[richkadel]

(Minor change. I just removed an unused parameter, is_cleanup, not needed now that I'm using "Statement".)

[wesleywiser]

@bors r+

[bors]

📌 Commit d129ac2 has been approved by wesleywiser

[bors]

⌛ Testing commit d129ac2 with merge 5f6fcad...

[bors]

☀️ Test successful - checks-actions, checks-azure
Approved by: wesleywiser
Pushing 5f6fcad to master...

[richkadel]

🎉 Thanks @wesleywiser and @tmandry ! This will make everything else much better and easier.

I'm making good progress on injecting counters with good coverage spans, for branches (starting with specific cases). I'll share these soon, along with tests.

[tmiasko]

@richkadel is PlaceContext::NonUse(NonUseContext::Coverage) reserved for a future use? It's never constructed currently.

[wesleywiser]

@tmiasko I think this got left in accidentally from a prior version of these changes where it was used. I believe it can be removed.