It turns out that this visitor doesn't actually need `nested_filter::All` to
handle nested items; it just needs to override `visit_nested_item` and look up
the item's span.
First of all, note that `Map` has three different relevant meanings.
- The `intravisit::Map` trait.
- The `map::Map` struct.
- The `NestedFilter::Map` associated type.
The `intravisit::Map` trait is impl'd twice.
- For `!`, where the methods are all unreachable.
- For `map::Map`, which gets HIR stuff from the `TyCtxt`.
As part of getting rid of `map::Map`, this commit changes `impl
intravisit::Map for map::Map` to `impl intravisit::Map for TyCtxt`. It's
fairly straightforward except various things are renamed, because the
existing names would no longer have made sense.
- `trait intravisit::Map` becomes `trait intravisit::HirTyCtxt`, so named
because it gets some HIR stuff from a `TyCtxt`.
- `NestedFilter::Map` assoc type becomes `NestedFilter::MaybeTyCtxt`,
because it's always `!` or `TyCtxt`.
- `Visitor::nested_visit_map` becomes `Visitor::maybe_tcx`.
I deliberately made the new trait and associated type names different to
avoid the old `type Map: Map` situation, which I found confusing. We now
have `type MaybeTyCtxt: HirTyCtxt`.
The end goal is to eliminate `Map` altogether.
I added a `hir_` prefix to all of them, that seemed simplest. The
exceptions are `module_items` which became `hir_module_free_items` because
there was already a `hir_module_items`, and `items` which became
`hir_free_items` for consistency with `hir_module_free_items`.
When preparing a function's coverage counters and metadata during codegen, any
part of the original coverage graph that was removed by MIR optimizations can
be treated as having an execution count of zero.
Somewhat counter-intuitively, if we give those unreachable nodes a _higher_
priority for receiving physical counters (instead of counter expressions), that
ends up reducing the total number of physical counters needed.
This works because if a node is unreachable, we don't actually create a
physical counter for it. Instead that node gets a fixed zero counter, and any
other node that would have relied on that physical counter in its counter
expression can just ignore that term completely.
coverage: Defer part of counter-creation until codegen
Follow-up to #135481 and #135873.
One of the pleasant properties of the new counter-assignment algorithm is that we can stop partway through the process, store the intermediate state in MIR, and then resume the rest of the algorithm during codegen. This lets it take into account which parts of the control-flow graph were eliminated by MIR opts, resulting in fewer physical counters and simpler counter expressions.
Those improvements end up completely obsoleting much larger chunks of code that were previously responsible for cleaning up the coverage metadata after MIR opts, while also doing a more thorough cleanup job.
(That change also unlocks some further simplifications that I've kept out of this PR to limit its scope.)
All hooks receive a `TyCtxtAt` argument.
Currently hooks can be called through `TyCtxtAt` or `TyCtxt`. In the
latter case, a `TyCtxtAt` is constructed with a dummy span and passed to
the hook.
However, in practice hooks are never called through `TyCtxtAt`, and
always receive a dummy span. (I confirmed this via code inspection, and
double-checked it by temporarily making the `TyCtxtAt` code path panic
and running all the tests.)
This commit removes all the `TyCtxtAt` machinery for hooks. All hooks
now receive `TyCtxt` instead of `TyCtxtAt`. There are two existing hooks
that use `TyCtxtAt::span`: `const_caller_location_provider` and
`try_destructure_mir_constant_for_user_output`. For both hooks the span
is always a dummy span, probably unintentionally. This dummy span use is
now explicit. If a non-dummy span is needed for these two hooks it would
be easy to add it as an extra argument because hooks are less
constrained than queries.
This assumes that the set of valid node IDs is exactly `0..num_nodes`.
In practice, we have a lot of graph-algorithm code that already assumes that
nodes are densely numbered, by using `num_nodes` to allocate per-node indexed
data structures.
Add `#[optimize(none)]`
cc #54882
This extends the `optimize` attribute to add `none`, which corresponds to the LLVM `OptimizeNone` attribute.
Not sure if an MCP is required for this, happy to file one if so.
By removing all methods from this struct and treating it as a collection of
data fields, we make it easier for a future PR to store that data in a query
result, without having to move all of its methods into `rustc_middle`.
This dedicated type seemed like a good idea at the time, but if we want to
store this information in a query result then a plainer data type is more
convenient.
Using `SmallVec` here was fine when it was a module-private detail, but if we
want to pass these terms across query boundaries then it's not worth the extra
hassle.
Replacing a method call with direct field access is slightly simpler.
Using the name `counter_terms` is more consistent with other code that tries to
maintain a distinction between (physical) "counters" and "expressions".
This reflects the fact that we can't compute meaningful info for a function
that wasn't instrumented and therefore doesn't have `function_coverage_info`.
Making these separate types from `CovTerm` and `Expression` was historically
very helpful, but now that most of the counter-creation work is handled by
`node_flow` they are no longer needed.
- Move `make_bcb_counters` out of `CoverageCounters`
- Split out `make_node_counter_priority_list`
- Flatten `Transcriber` into the function `transcribe_counters`
During coverage instrumentation, this variable always holds the coverage graph,
which is a simplified view of the MIR control-flow graph. The new name is
clearer in context, and also shorter.
coverage: Use a query to find counters/expressions that must be zero
As of #133446, this query (`coverage_ids_info`) determines which counter/expression IDs are unused. So with only a little extra work, we can take the code that was using that information to determine which coverage counters/expressions must be zero, and move that inside the query as well.
There should be no change in compiler output.
This query (`coverage_ids_info`) already determines which counter/expression
IDs are unused, so it only takes a little extra effort to also determine which
counters/expressions must have a value of zero.
take 2
open up coroutines
tweak the wordings
the lint works up until 2021
We were missing one case, for ADTs, which was
causing `Result` to yield incorrect results.
only include field spans with significant types
deduplicate and eliminate field spans
switch to emit spans to impl Drops
Co-authored-by: Niko Matsakis <nikomat@amazon.com>
collect drops instead of taking liveness diff
apply some suggestions and add explantory notes
small fix on the cache
let the query recurse through coroutine
new suggestion format with extracted variable name
fine-tune the drop span and messages
bugfix on runtime borrows
tweak message wording
filter out ecosystem types earlier
apply suggestions
clippy
check lint level at session level
further restrict applicability of the lint
translate bid into nop for stable mir
detect cycle in type structure
coverage: Restrict empty-span expansion to only cover `{` and `}`
Coverage instrumentation has some tricky code for converting a coverage-relevant `Span` into a set of start/end line/byte-column coordinates that will be embedded in the CGU's coverage metadata.
A big part of this complexity is special code for handling empty spans, which are expanded into non-empty spans (if possible) because LLVM's coverage reporter does not handle empty spans well.
This PR simplifies that code by restricting it to only apply in two specific situations: when the character after the empty span is `{`, or the character before the empty span is `}`.
(As an added benefit, this means that the expanded spans no longer extend awkwardly beyond the end of a physical line, which was common under the previous implementation.)
Along the way, this PR also removes some unhelpful code for dealing with function source code spread across multiple files. Functions currently can't have coverage spans in multiple files, and if that ever changes (e.g. to properly support expansion regions) then this code will need to be completely overhauled anyway.
Functions currently can't have mappings in multiple files, and if that ever
changes (e.g. to properly support expansion regions), this code will need to be
completely overhauled anyway.
