Fixed conflict with drop elaboration and coverage
See
https://github.com/rust-lang/rust/issues/80045#issuecomment-745733339
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`
Fix issue #78496
EarlyOtherwiseBranch finds MIR structures like:
```
bb0: {
...
_2 = discriminant(X)
...
switchInt(_2) -> [1_isize: bb1, otherwise: bb3]
}
bb1: {
...
_3 = discriminant(Y)
...
switchInt(_3) -> [1_isize: bb2, otherwise: bb3]
}
bb2: {...}
bb3: {...}
```
And transforms them into something like:
```
bb0: {
...
_2 = discriminant(X)
_3 = discriminant(Y)
_4 = Eq(_2, _3)
switchInt(_4) -> [true: bb4, otherwise: bb3]
}
bb2: {...} // unchanged
bb3: {...} // unchanged
bb4: {
switchInt(_2) -> [1_isize: bb2, otherwise: bb3]
}
```
But that is not always a safe thing to do -- sometimes the early `otherwise` branch is necessary so the later block could assume the value of `discriminant(X)`.
I am not totally sure what's the best way to detect that, but fixing #78496 should be easy -- we just check if `X` is a sub-expression of `Y`. A more precise test might be to check if `Y` contains a `Downcast(1)` of `X`, but I think this might be good enough.
Fix#78496
See
https://github.com/rust-lang/rust/issues/80045#issuecomment-745733339
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).
Fixes reported bugs in Rust Coverage
Fixes: #79569Fixes: #79566Fixes: #79565
For the first issue (#79569), I got hit a `debug_assert!()` before
encountering the reported error message (because I have `debug = true`
enabled in my config.toml).
The assertion showed me that some `SwitchInt`s can have more than one
target pointing to the same `BasicBlock`.
I had thought that was invalid, but since it seems to be possible, I'm
allowing this now.
I added a new test for this.
----
In the last two cases above, both tests (intentionally) fail to compile,
but the `InstrumentCoverage` pass is invoked anyway.
The MIR starts with an `Unreachable` `BasicBlock`, which I hadn't
encountered before. (I had assumed the `InstrumentCoverage` pass
would only be invoked with MIRs from successful compilations.)
I don't have test infrastructure set up to test coverage on files that
fail to compile, so I didn't add a new test.
r? `@tmandry`
FYI: `@wesleywiser`
consider assignments of union field of ManuallyDrop type safe
Assigning to `Copy` union fields is safe because that assignment will never drop anything. However, with https://github.com/rust-lang/rust/pull/77547, unions may also have `ManuallyDrop` fields, and their assignments are currently still unsafe. That seems unnecessary though, as assigning `ManuallyDrop` does not drop anything either, and is thus safe even for union fields.
I assume this will at least require FCP.
[mir-opt] Allow debuginfo to be generated for a constant or a Place
Prior to this commit, debuginfo was always generated by mapping a name
to a Place. This has the side-effect that `SimplifyLocals` cannot remove
locals that are only used for debuginfo because their other uses have
been const-propagated.
To allow these locals to be removed, we now allow debuginfo to point to
a constant value. The `ConstProp` pass detects when debuginfo points to
a local with a known constant value and replaces it with the value. This
allows the later `SimplifyLocals` pass to remove the local.
Adds checks for:
* `no_core` attribute
* explicitly-enabled `legacy` symbol mangling
* mir_opt_level > 1 (which enables inlining)
I removed code from the `Inline` MIR pass that forcibly disabled
inlining if `-Zinstrument-coverage` was set. The default `mir_opt_level`
does not enable inlining anyway. But if the level is explicitly set and
is greater than 1, I issue a warning.
The new warnings show up in tests, which is much better for diagnosing
potential option conflicts in these cases.
Remove an unused dependency that made `rustdoc` crash
Whilst struggling with https://github.com/rust-lang/rust/issues/79980 I discovered that this dependency was unused, and that made rustdoc crash. This PR removes it.
Fixes: #79569Fixes: #79566Fixes: #79565
For the first issue (#79569), I got hit a `debug_assert!()` before
encountering the reported error message (because I have `debug = true`
enabled in my config.toml).
The assertion showed me that some `SwitchInt`s can have more than one
target pointing to the same `BasicBlock`.
I had thought that was invalid, but since it seems to be possible, I'm
allowing this now.
I added a new test for this.
----
In the last two cases above, both tests (intentionally) fail to compile,
but the `InstrumentCoverage` pass is invoked anyway.
The MIR starts with an `Unreachable` `BasicBlock`, which I hadn't
encountered before. (I had assumed the `InstrumentCoverage` pass
would only be invoked with MIRs from successful compilations.)
I don't have test infrastructure set up to test coverage on files that
fail to compile, so I didn't add a new test.
Dogfood `str_split_once()`
Part of https://github.com/rust-lang/rust/issues/74773.
Beyond increased clarity, this fixes some instances of a common confusion with how `splitn(2)` behaves: the first element will always be `Some()`, regardless of the delimiter, and even if the value is empty.
Given this code:
```rust
fn main() {
let val = "...";
let mut iter = val.splitn(2, '=');
println!("Input: {:?}, first: {:?}, second: {:?}", val, iter.next(), iter.next());
}
```
We get:
```
Input: "no_delimiter", first: Some("no_delimiter"), second: None
Input: "k=v", first: Some("k"), second: Some("v")
Input: "=", first: Some(""), second: Some("")
```
Using `str_split_once()` makes more clear what happens when the delimiter is not found.
Fixes to Rust coverage
Fixes: #79725
Some macros can create a situation where `fn_sig_span` and `body_span`
map to different files.
New documentation on coverage tests incorrectly assumed multiple test
binaries could just be listed at the end of the `llvm-cov` command,
but it turns out each binary needs a `--object` prefix.
This PR fixes the bug and updates the documentation to correct that
issue. It also fixes a few other minor issues in internal implementation
comments, and adds documentation on getting coverage results for doc
tests.
Fixes: #79725
Some macros can create a situation where `fn_sig_span` and `body_span`
map to different files.
New documentation on coverage tests incorrectly assumed multiple test
binaries could just be listed at the end of the `llvm-cov` command,
but it turns out each binary needs a `--object` prefix.
This PR fixes the bug and updates the documentation to correct that
issue. It also fixes a few other minor issues in internal implementation
comments, and adds documentation on getting coverage results for doc
tests.
Prior to this commit, debuginfo was always generated by mapping a name
to a Place. This has the side-effect that `SimplifyLocals` cannot remove
locals that are only used for debuginfo because their other uses have
been const-propagated.
To allow these locals to be removed, we now allow debuginfo to point to
a constant value. The `ConstProp` pass detects when debuginfo points to
a local with a known constant value and replaces it with the value. This
allows the later `SimplifyLocals` pass to remove the local.
Added one more test (two files) showing coverage of generics and unused
functions across crates.
Created and referenced new Issues, as requested.
Added comments.
Added a note about the possible effects of compiler options on LLVM
coverage maps.
Fixes multiple issue with counters, with simplification
Includes a change to the implicit else span in ast_lowering, so coverage
of the implicit else no longer spans the `then` block.
Adds coverage for unused closures and async function bodies.
Fixes: #78542
Adding unreachable regions for known MIR missing from coverage map
Cleaned up PR commits, and removed link-dead-code requirement and tests
Coverage no longer depends on Issue #76038 (`-C link-dead-code` is
no longer needed or enforced, so MSVC can use the same tests as
Linux and MacOS now)
Restrict adding unreachable regions to covered files
Improved the code that adds coverage for uncalled functions (with MIR
but not-codegenned) to avoid generating coverage in files not already
included in the files with covered functions.
Resolved last known issue requiring --emit llvm-ir workaround
Fixed bugs in how unreachable code spans were added.
Allow using generic trait methods in `const fn`
Next step for https://github.com/rust-lang/rust/issues/67792, this now also allows code like the following:
```rust
struct S;
impl const PartialEq for S {
fn eq(&self, _: &S) -> bool {
true
}
}
const fn equals_self<T: PartialEq>(t: &T) -> bool {
*t == *t
}
pub const EQ: bool = equals_self(&S);
```
This works by threading const-ness of trait predicates through trait selection, in particular through `ParamCandidate`, and exposing it in the resulting `ImplSource`.
Since this change makes two bounds `T: Trait` and `T: ?const Trait` that only differ in their const-ness be treated like different bounds, candidate winnowing has been changed to drop the `?const` candidate in favor of the const candidate, to avoid ambiguities when both a const and a non-const bound is present.
Never inline naked functions
The `#[naked]` attribute disabled prologue / epilogue emission for the
function and it is responsibility of a developer to provide them. The
compiler is no position to inline such functions correctly.
Disable inlining of naked functions at LLVM and MIR level.
Closes#60919.
The `#[naked]` attribute disabled prologue / epilogue emission for the
function and it is responsibility of a developer to provide them. The
compiler is no position to inline such functions correctly.
Disable inlining of naked functions at LLVM and MIR level.
add optimization fuel checks to some mir passes
Fixes#77402
Inserts a bunch of calls to `consider_optimizing`. Note that `consider_optimizing` is the method that actually decrements the fuel count, so the point at which it's called is when the optimization takes place, from a fuel perspective. This means that where we call it has some thought behind it:
1. We probably don't want to decrement the fuel count before other simple checks, otherwise we count an optimization as being performed even if nothing was mutated (ie. it returned early).
2. In cases like `InstCombine`, where we gather optimizations in a pass and then mutate values, we probably would rather skip the gathering pass for performance reasons rather than skip the mutations afterwards.
