Don’t insert coverage counters for unreachable!() code

[maboesanman]

With the introduction of source based code coverage, I ran into this quirk:

This seems like it should be a fully covered function but an unreachable counter has been placed on the invocation of the unreachable!() macro.

Would it be possible to make calls to unreachable skip the insert of the counter?

[nagisa]

unreachable! is entirely a library thing and is just a special case of panic!. Seems to me like it'd be presumptuous in a way to handle panic specially for the purposes of coverage, and making unreachable! itself special in the compiler is probably too heavy-weight...

Perhaps what we want here is some attribute or something of that sort that prevents insertion of counters in a more general manner...

[maboesanman]

I agree that unreachable!() shouldn't itself be special, but I do think if such an attribute were introduced it should be added to unreachable, as to me the same reasons for marking code as unreachable apply to whether or not you want to collect coverage on it.

[samuelcolvin]

This would be very useful, a generic way to disable coverage for unreachable!() and similar would be very useful.

[Zalathar]

Coverage instrumentation already has “special handling” of panic! in the sense that it assumes panics never occur. Given that assumption, it wouldn't be unreasonable to teach the instrumentor to ignore code paths that always diverge.

I was playing around with having the coverage graph ignore MIR blocks that end with TerminatorKind::Call { target: None, .. }, and the results seemed promising.

(I don't have a PR because my quick experiments clearly needed a lot more polish before they could ever be merged, but it does seem like a viable approach.)

[clarfonthey]

Just poking through some old coverage issues, but this feels like a good use case for the #[coverage(ignore)] attribute I mention here: #84605 (comment)

Effectively, it would be nice to be able to mark areas of code as "I don't expect this to be covered," where they would still have coverage instrumentation but wouldn't count toward overall coverage metrics.

[Zalathar]

A while back I was thinking about implementing my own Rust-specific tool for parsing coverage data and presenting coverage reports, as an alternative to the LLVM-provided tool that we use now (llvm-cov).

(I didn't make much progress because I ended up shifting my focus to instrumentation, but it's an idea I'd like to return to some day.)

Supporting #[coverage(ignore)] would require some kind of tool like that, because LLVM's tool isn't flexible enough on its own. But if we did have such a tool, we could do a lot of interesting things.

[clarfonthey]

Very fair! I do wonder if it'd be possible to inject arbitrary metadata into the LLVM coverage data, though, so that at least any potential tool wouldn't have to manually generate everything on their own, and could still use the same instrumentation.

That way, we could mark certain lines/points as being "ignored" in such a way that a tool like llvm-cov would ignore them, but other tools could make use of them.