Source file allowlist on recorder TargetAllowlist ignored

[ranweiler]

Summary

The source file allowlist most-directly configured on coverage recorders is ignored, and only the source allowlist owned by the debuginfo cache is used for source file filtering. This is more of a confusing API flaw than an active bug, and is due to the debuginfo cache being a later addition.

Found by @Porges.

Context

The DebugInfoCache owns an Arc<Allowlist>. This is used as a source file allowlist, and determines what binary coverage locations should be measured, given the source files they were defined in. However, CoverageRecorder owns a TargetAllowlist, which is composed of two Allowlist values: one for modules, one for source files. But this TargetAllowlist is now only used for its TargetAllowlist.module field, and its source_files field is ignored by the recorder implementations.

For users of the coverage crate, these two nominal source file allowlists can confusingly diverge, via normal use of the debuginfo_cache() and allowlist() builder methods on CoverageRecorder. It is reasonable for a caller to expect that the TargetAllowlist.source_files field is used, but it never will be.

Solutions

The debuginfo cache really is coupled to its source file Allowlist, because the cached analysis is a function of that list. However, the CoverageRecorder still needs to have an additional Allowlist to use for module-level filtering. Furthermore, since debuginfo analysis can be very expensive, we probably don't want to just invalidate the cache (and re-run analysis to create a new one) if the recorder source file allowlist is modified.

The least confusing / best performing change is probably to rename CoverageRecorder.allowlist() (and related fields) to module_allowlist(), and have it accept an Allowlist (instead of a TargetAllowList).

The TargetAllowlist may still be worth keeping in some form after this, but might not.

AB#162997

[Porges]

Removal of TargetAllowList and associated changes are addressed in #3368