Commit Graph

1946 Commits

Author SHA1 Message Date
bors
584f183dc0 Auto merge of #124194 - matthiaskrgr:rollup-40s0c4q, r=matthiaskrgr
Rollup of 3 pull requests

Successful merges:

 - #123409 (Implement Modified Condition/Decision  Coverage)
 - #124104 (Fix capturing duplicated lifetimes via parent in `precise_captures` (`impl use<'...>`))
 - #124137 (Match hyphen in multi-revision comment matchers)

r? `@ghost`
`@rustbot` modify labels: rollup
2024-04-20 10:50:39 +00:00
Matthias Krüger
efb264fa78
Rollup merge of #123409 - ZhuUx:master, r=oli-obk
Implement Modified Condition/Decision  Coverage

This is an implementation based on llvm backend support (>= 18) by `@evodius96` and branch coverage support by `@Zalathar.`

### Major changes:

* Add -Zcoverage-options=mcdc as switch. Now coverage options accept either `no-branch`, `branch`, or `mcdc`. `mcdc` also enables `branch` because it is essential to work.
* Add coverage mapping for MCDCBranch and MCDCDecision. Note that MCDCParameter evolves from  llvm 18 to llvm 19. The mapping in rust side mainly references to 19 and is casted to 18 types in llvm wrapper.
* Add wrapper for mcdc instrinc functions from llvm. And inject associated statements to mir.
* Add BcbMappingKind::Decision, I'm not sure is it proper but can't find a better way temporarily.
* Let coverage-dump support parsing MCDCBranch and MCDCDecision from llvm ir.
* Add simple tests to check whether mcdc works.
* Same as clang, currently rustc does not generate instrument for decision with more than 6 condtions or only 1 condition due to considerations of resource.

### Implementation Details

1. To get information about conditions and decisions, `MCDCState` in `BranchInfoBuilder` is used during hir lowering to mir. For expressions with logical op we call `Builder::visit_coverage_branch_operation` to record its sub conditions, generate condition ids for them and save their spans (to construct the span of whole decision). This process mainly references to the implementation in clang and is described in comments over `MCDCState::record_conditions`. Also true marks and false marks introduced by branch coverage are used to detect where the decision evaluation ends: the next id  of the condition == 0.
2. Once the `MCDCState::decision_stack` popped all recorded conditions, we can ensure that the decision is checked over and push it into `decision_spans`. We do not manually insert decision span to avoid complexity from then_else_break in nested if scopes.
3. When constructing CoverageSpans, add condition info to BcbMappingKind::Branch and decision info to BcbMappingKind::Decision. If the branch mapping has non-zero condition id it will be transformed to MCDCBranch mapping and insert `CondBitmapUpdate` statements to its evaluated blocks. While decision bcb mapping will insert `TestVectorBitmapUpdate` in all its end blocks.

### Usage
```bash
 echo "[build]\nprofiler=true" >> config.toml
./x build --stage 1
./x test tests/coverage/mcdc_if.rs
```
to build the compiler and run tests.

```shell
export PATH=path/to/llvm-build:$PATH
rustup toolchain link mcdc build/host/stage1
cargo +mcdc rustc --bin foo -- -Cinstrument-coverage -Zcoverage-options=mcdc
cd target/debug
LLVM_PROFILE_FILE="foo.profraw" ./foo
llvm-profdata merge -sparse foo.profraw -o foo.profdata
llvm-cov show ./foo -instr-profile=foo.profdata --show-mcdc
```
to check "foo" code.

### Problems to solve

For now decision mapping will insert statements to its all end blocks, which may be optimized by inserting a final block of the decision. To do this we must also trace the evaluated value at each end of the decision and join them separately.

This implementation is not heavily tested so there should be some unrevealed issues. We are going to check our rust products in the next.  Please let me know if you had any suggestions or comments.
2024-04-20 11:10:31 +02:00
bors
a61b14d15e Auto merge of #124156 - DianQK:disable-match_branches, r=RalfJung
Disable SimplifyToExp in MatchBranchSimplification

Due to the miscompilation mentioned in #124150, We need to disable MatchBranchSimplification temporarily.

To fully resolve this issue, my plan is:
1. Disable SimplifyToExp in MatchBranchSimplification (this PR).
2. Remove all potentially unclear transforms in #124122.
3. Gradually add back the removed transforms (possibly multiple PRs).

r? `@Nilstrieb` or `@oli-obk`
2024-04-20 08:47:07 +00:00
DianQK
b52be28fda
Disable SimplifyToExp in MatchBranchSimplification 2024-04-20 05:37:53 +08:00
bors
ce3263e60e Auto merge of #124113 - RalfJung:interpret-scalar-ops, r=oli-obk
interpret: use ScalarInt for bin-ops; avoid PartialOrd for ScalarInt

Best reviewed commit-by-commit

r? `@oli-obk`
2024-04-19 17:00:28 +00:00
zhuyunxing
439dbfa1ec coverage. Lowering MC/DC statements to llvm-ir 2024-04-20 00:34:40 +08:00
Ralf Jung
42220f0930 ScalarInt: add methods to assert being a (u)int of given size 2024-04-19 13:51:52 +02:00
zhuyunxing
cf6b6cb2b4 coverage. Generate Mappings of decisions and conditions for MC/DC 2024-04-19 17:09:26 +08:00
Scott McMurray
90b4c86335 Ensure [rust] debuginfo-level-std doesn't change core's MIR 2024-04-18 09:35:36 -07:00
Scott McMurray
d05545c05d At debuginfo=0, don't inline debuginfo when inlining 2024-04-18 09:35:35 -07:00
Nicholas Nethercote
27374a0214 Avoid unnecessary rustc_span::DUMMY_SP usage.
In some cases `DUMMY_SP` is already imported. In other cases this commit
adds the necessary import, in files where `DUMMY_SP` is used more than
once.
2024-04-16 15:55:24 +10:00
许杰友 Jieyou Xu (Joe)
5580ae9795
Rollup merge of #123934 - WaffleLapkin:graph-mini-refactor, r=fmease
`rustc_data_structures::graph` mini refactor

Who doesn't love to breathe dust from the ancient times?
2024-04-15 16:56:18 +01:00
Maybe Waffle
435db9b9bd Use RPITIT for Successors and Predecessors traits
Now with RPITIT instead of GAT!
2024-04-15 13:34:08 +00:00
Maybe Waffle
f5144938bd Rename WithNumEdges => NumEdges and WithStartNode => StartNode 2024-04-14 15:51:29 +00:00
Maybe Waffle
0d5fc9bf58 Merge {With,Graph}{Successors,Predecessors} into {Successors,Predecessors}
Now with GAT!
2024-04-14 15:48:53 +00:00
Maybe Waffle
398da593a5 Merge WithNumNodes into DirectedGraph 2024-04-14 15:46:40 +00:00
Michael Goulet
568703c4bd Use a helper to zip together parent and child captures for coroutine-closures 2024-04-10 13:39:52 -04:00
Michael Goulet
69b690f0f6 Only assert for child/parent projection compatibility AFTER checking that theyre coming from the same place 2024-04-10 10:13:24 -04:00
Matthias Krüger
df068dba0a
Rollup merge of #123668 - oli-obk:by_move_body_golfing, r=compiler-errors
async closure coroutine by move body MirPass refactoring

Unsure about the last commit, but I think the other changes help in simplifying the control flow
2024-04-10 04:27:41 +02:00
Oli Scherer
e14e7954ae Iterate over parent captures first, as there is a 1:N mapping of parent captures to child captures 2024-04-09 19:51:11 +00:00
Oli Scherer
bf497b8347 Add a FIXME 2024-04-09 19:51:11 +00:00
Guillaume Gomez
cfe1faa75d
Rollup merge of #123658 - compiler-errors:stop-assuming, r=oli-obk
Stop making any assumption about the projections applied to the upvars in the `ByMoveBody` pass

So it turns out that because of subtle optimizations like [`truncate_capture_for_optimization`](ab5bda1aa7/compiler/rustc_hir_typeck/src/upvar.rs (L2351)), we simply cannot make any assumptions about the shape of the projections applied to the upvar locals in a coroutine body.

So stop doing that -- the code is resilient to such projections, so the assertion really existed only to "protect against the unknown".

r? oli-obk
Fixes #123650
2024-04-09 13:39:23 +02:00
Oli Scherer
f014e91a38 Shrink a loop to its looping part and move out the part that runs after the loop 2024-04-09 07:53:12 +00:00
Oli Scherer
626fd4b258 prefer expect over let else bug! 2024-04-09 07:48:55 +00:00
Oli Scherer
688e531925 Split out a complex if condition into a named function 2024-04-09 07:47:40 +00:00
bors
59c808fcd9 Auto merge of #122387 - DianQK:re-enable-early-otherwise-branch, r=cjgillot
Re-enable the early otherwise branch optimization

Closes #95162. Fixes #119014.

This is the first part of #121397.

An invalid enum discriminant can come from anywhere. We have to check to see if all successors contain the discriminant statement. This should have a pass to hoist instructions.

r? cjgillot
2024-04-09 01:02:29 +00:00
Michael Goulet
54a93ab11e Actually, stop making any assumption about the projections applied to the upvar 2024-04-08 19:47:52 -04:00
bors
ab5bda1aa7 Auto merge of #123645 - matthiaskrgr:rollup-yd8d7f1, r=matthiaskrgr
Rollup of 9 pull requests

Successful merges:

 - #122781 (Fix argument ABI for overaligned structs on ppc64le)
 - #123367 (Safe Transmute: Compute transmutability from `rustc_target::abi::Layout`)
 - #123518 (Fix `ByMove` coroutine-closure shim (for 2021 precise closure capturing behavior))
 - #123547 (bootstrap: remove unused pub fns)
 - #123564 (Don't emit divide-by-zero panic paths in `StepBy::len`)
 - #123578 (Restore `pred_known_to_hold_modulo_regions`)
 - #123591 (Remove unnecessary cast from `LLVMRustGetInstrProfIncrementIntrinsic`)
 - #123632 (parser: reduce visibility of unnecessary public `UnmatchedDelim`)
 - #123635 (CFI: Fix ICE in KCFI non-associated function pointers)

r? `@ghost`
`@rustbot` modify labels: rollup
2024-04-08 20:31:08 +00:00
Matthias Krüger
36d1915449
Rollup merge of #123518 - compiler-errors:by-move-fixes, r=oli-obk
Fix `ByMove` coroutine-closure shim (for 2021 precise closure capturing behavior)

This PR reworks the way that we perform the `ByMove` coroutine-closure shim to account for the fact that the upvars of the outer coroutine-closure and the inner coroutine might not line up due to edition-2021 closure capture rules changes.

Specifically, the number of upvars may differ *and/or* the inner coroutine may have additional projections applied to an upvar. This PR reworks the information we pass into the `ByMoveBody` MIR visitor to account for both of these facts.

I tried to leave comments explaining exactly what everything is doing, but let me know if you have questions.

r? oli-obk
2024-04-08 22:06:21 +02:00
bors
211518e5fb Auto merge of #120614 - DianQK:simplify-switch-int, r=cjgillot
Transforms match into an assignment statement

Fixes #106459.

We should be able to do some similar transformations, like `enum` to `enum`.

r? mir-opt
2024-04-08 18:28:50 +00:00
Oli Scherer
84acfe86de Actually create ranged int types in the type system. 2024-04-08 12:02:19 +00:00
DianQK
166bb1bd46
Don't change the otherwise of the switch 2024-04-08 19:20:07 +08:00
DianQK
f4407370db
Change the return type of can_simplify to Option<()> 2024-04-08 19:09:57 +08:00
DianQK
032bb742ab
Add comments for CompareType 2024-04-08 19:07:53 +08:00
DianQK
254289a16e
Updating the MIR with MirPatch 2024-04-08 19:04:11 +08:00
DianQK
e752af765e
Transforms a match containing negative numbers into an assignment statement as well 2024-04-08 19:00:57 +08:00
DianQK
1f061f47e2
Transforms match into an assignment statement 2024-04-08 19:00:53 +08:00
DianQK
7af7458453
Refactor MatchBranchSimplification 2024-04-08 18:54:42 +08:00
DianQK
a334848ba3
Re-enable the early otherwise branch optimization 2024-04-07 21:14:29 +08:00
DianQK
31e74771f0
Resolve unsound hoisting of discriminant in EarlyOtherwiseBranch 2024-04-07 21:14:26 +08:00
Ben Kimock
a7912cb421 Put checks that detect UB under their own flag below debug_assertions 2024-04-06 11:21:47 -04:00
Michael Goulet
ad0fcac72b Account for an additional reborrow inserted by UniqueImmBorrow and MutBorrow 2024-04-05 17:35:03 -04:00
Michael Goulet
49c4ebcc40 Check the base of the place too! 2024-04-05 16:48:45 -04:00
Michael Goulet
0f13bd436b Add some helpful comments 2024-04-05 16:29:49 -04:00
Michael Goulet
3674032eb2 Rework the ByMoveBody shim to actually work correctly 2024-04-05 15:28:13 -04:00
Guillaume Gomez
02ee8a8cee
Rollup merge of #123350 - compiler-errors:async-closure-by-move, r=oli-obk
Actually use the inferred `ClosureKind` from signature inference in coroutine-closures

A follow-up to https://github.com/rust-lang/rust/pull/123349, which fixes another subtle bug: We were not taking into account the async closure kind we infer during closure signature inference.

When I pass a closure directly to an arg like `fn(x: impl async FnOnce())`, that should have the side-effect of artificially restricting the kind of the async closure to `ClosureKind::FnOnce`. We weren't doing this -- that's a quick fix; however, it uncovers a second, more subtle bug with the way that `move`, async closures, and `FnOnce` interact.

Specifically, when we have an async closure like:
```
let x = Struct;
let c = infer_as_fnonce(async move || {
  println!("{x:?}");
}
```

The outer closure captures `x` by move, but the inner coroutine still immutably borrows `x` from the outer closure. Since we've forced the closure to by `async FnOnce()`, we can't actually *do* a self borrow, since the signature of `AsyncFnOnce::call_once` doesn't have a borrowed lifetime. This means that all `async move` closures that are constrained to `FnOnce` will fail borrowck.

We can fix that by detecting this case specifically, and making the *inner* async closure `move` as well. This is always beneficial to closure analysis, since if we have an `async FnOnce()` that's `move`, there's no reason to ever borrow anything, so `move` isn't artificially restrictive.
2024-04-05 16:38:51 +02:00
bors
c0ddaef075 Auto merge of #123444 - saethlin:const-eval-inline-cycles, r=tmiasko
Teach MIR inliner query cycle avoidance about const_eval_select

Fixes https://github.com/rust-lang/rust/issues/122659

r? tmiasko
2024-04-05 04:34:05 +00:00
Jacob Pratt
58eb6e5803
Rollup merge of #123464 - fmease:rn-has-proj-to-has-aliases, r=compiler-errors
Cleanup: Rename `HAS_PROJECTIONS` to `HAS_ALIASES` etc.

The name of the bitflag `HAS_PROJECTIONS` and of its corresponding method `has_projections` is quite historical dating back to a time when projections were the only kind of alias type.

I think it's time to update it to clear up any potential confusion for newcomers and to reduce unnecessary friction during contributor onboarding.

r? types
2024-04-04 21:16:58 -04:00
Michael Goulet
55e46612c1 Force move async-closures that are FnOnce to make their inner coroutines also move 2024-04-04 19:44:51 -04:00
León Orell Valerian Liehr
6f17b7f0ab
Rename HAS_PROJECTIONS to HAS_ALIASES etc. 2024-04-04 19:26:17 +02:00