Use the same DISubprogram for each instance of the same inlined function within a caller
# Issue Details:
The call to `panic` within a function like `Option::unwrap` is translated to LLVM as a `tail call` (as it will never return), when multiple calls to the same function like this is inlined LLVM will notice the common `tail call` block (i.e., loading the same panic string + location info and then calling `panic`) and merge them together.
When merging these instructions together, LLVM will also attempt to merge the debug locations as well, but this fails (i.e., debug info is dropped) as Rust emits a new `DISubprogram` at each inline site thus LLVM doesn't recognize that these are actually the same function and so thinks that there isn't a common debug location.
As an example of this when building for x86_64 Windows (note the lack of `.cv_loc` before the call to `panic`, thus it will be attributed to the same line at the `addq` instruction):
```
.cv_loc 0 1 23 0 # src\lib.rs:23:0
addq $40, %rsp
retq
leaq .Lalloc_f570dea0a53168780ce9a91e67646421(%rip), %rcx
leaq .Lalloc_629ace53b7e5b76aaa810d549cc84ea3(%rip), %r8
movl $43, %edx
callq _ZN4core9panicking5panic17h12e60b9063f6dee8E
int3
```
# Fix Details:
Cache the `DISubprogram` emitted for each inlined function instance within a caller so that this can be reused if that instance is encountered again, this also requires caching the `DILexicalBlock` and `DIVariable` objects to avoid creating duplicates.
After this change the above assembly now looks like:
```
.cv_loc 0 1 23 0 # src\lib.rs:23:0
addq $40, %rsp
retq
.cv_inline_site_id 5 within 0 inlined_at 1 0 0
.cv_inline_site_id 6 within 5 inlined_at 1 12 0
.cv_loc 6 2 935 0 # library\core\src\option.rs:935:0
leaq .Lalloc_5f55955de67e57c79064b537689facea(%rip), %rcx
leaq .Lalloc_e741d4de8cb5801e1fd7a6c6795c1559(%rip), %r8
movl $43, %edx
callq _ZN4core9panicking5panic17hde1558f32d5b1c04E
int3
```
Add disclaimer on size assertion macro
Sometimes people are inspired by rustc to add size assertions to their code and copy the macro. This is bad because it causes hard build errors. rustc happens to be special where it makes this okay.
For example, see #115028 (not sure whether they were directly inspired by this function), but I think I've also seen other cases.
Warn on elided lifetimes in associated constants (`ELIDED_LIFETIMES_IN_ASSOCIATED_CONSTANT`)
Elided lifetimes in associated constants (in impls) erroneously resolve to fresh lifetime parameters on the impl since #97313. This is not correct behavior (see #38831).
I originally opened #114716 to fix this, but given the time that has passed, the crater results seem pretty bad: https://github.com/rust-lang/rust/pull/114716#issuecomment-1682091952
This PR alternatively implements a lint against this behavior, and I'm hoping to bump this to deny in a few versions.
Redefine the pluralize macro's arm
Redefine the unintuitive pluralize macro's arm because of the negation. The initial code starts with check if count is not 1, which is confusing and unintuitive.
The arm shoud start with checking,
- if "count" `is 1` then, append `""` (empty string) - indicate as singular
- Then check if "count" `is not 1` (more than 1), append `"s"` - indicate as plural
Before:
```rs
// This arm is abit confusing since it start with checking, if "count" is more than 1, append "s".
($x:expr) => {
if $x != 1 { "s" } else { "" }
};
```
After:
```rs
// Pluralize based on count (e.g., apples)
($x:expr) => {
if $x == 1 { "" } else { "s" }
};
```
Sometimes people are inspired by rustc to add size assertions to their
code and copy the macro. This is bad because it causes hard build
errors. rustc happens to be special where it makes this okay.
coverage: Give the instrumentor its own counter type, separate from MIR
Within the MIR representation of coverage data, `CoverageKind` is an important part of `StatementKind::Coverage`, but the `InstrumentCoverage` pass also uses it heavily as an internal data structure. This means that any change to `CoverageKind` also needs to update all of the internal parts of `InstrumentCoverage` that manipulate it directly, making the MIR representation difficult to modify.
---
This change fixes that by giving the instrumentor its own `BcbCounter` type for internal use, which is then converted to a `CoverageKind` when injecting coverage information into MIR.
The main change is mostly mechanical, because the initial `BcbCounter` is drop-in compatible with `CoverageKind`, minus the unnecessary `CoverageKind::Unreachable` variant.
I've then removed the `function_source_hash` field from `BcbCounter::Counter`, as a small example of how the two types can now usefully differ from each other. Every counter in a MIR-level function should have the same source hash, so we can supply the hash during the conversion to `CoverageKind::Counter` instead.
---
*Background:* BCB stands for “basic coverage block”, which is a node in the simplified control-flow graph used by coverage instrumentation. The instrumentor pass uses the function's actual MIR control-flow graph to build a simplified BCB graph, then assigns coverage counters and counter expressions to various nodes/edges in that simplified graph, and then finally injects corresponding coverage information into the underlying MIR.
Fix a stack overflow with long else if chains
This fixes stack overflows when running the `issue-74564-if-expr-stack-overflow.rs` test with the parallel compiler.
Add MIR validation for unwind out from nounwind functions + fixes to make validation pass
`@Nilstrieb` This is the MIR validation you asked in https://github.com/rust-lang/rust/pull/112403#discussion_r1222739722.
Two passes need to be fixed to get the validation to pass:
* `RemoveNoopLandingPads` currently unconditionally introduce a resume block (even there is none to begin with!), changed to not do that
* Generator state transform introduces a `assert` which may unwind, and its drop elaboration also introduces many new `UnwindAction`s, so in this case run the AbortUnwindingCalls after the transformation.
I believe this PR should also fixRust-for-Linux/linux#1016, cc `@ojeda`
r? `@Nilstrieb`
custom_mir: change Call() terminator syntax to something more readable
I find our current syntax very hard to read -- I cannot even remember the order of arguments, and having the "next block" *before* the actual function call is very counter-intuitive IMO. So I suggest we use `Call(ret_val = function(v), next_block)` instead.
r? `@JakobDegen`
Ignore unexpected incr-comp session dirs
Clearly the code path can be hit without the presence of a compiler bug.
All it takes is mischief. See #71698.
Ignore problematic directories instead of ICE:ing. `continue`ing is
already done for problematic dirs in the code block above us.
Closes#71698.
With this fix, the output is this instead of ICE:
```
$ cargo +stage1 new gz-ice && cd gz-ice
$ cargo +stage1 build
$ find target -type f -exec gzip {} \;
$ cargo +stage1 run
Created binary (application) `gz-ice` package
Compiling gz-ice v0.1.0 (/tmp/gz-ice)
Finished dev [unoptimized + debuginfo] target(s) in 0.13s
gzip: target/debug/gz-ice has 1 other link -- unchanged
gzip: target/debug/deps/gz_ice-de919414dd9926b9 has 1 other link -- unchanged
Compiling gz-ice v0.1.0 (/tmp/gz-ice)
warning: failed to garbage collect invalid incremental compilation session directory `/tmp/gz-ice/target/debug/incremental/gz_ice-23qx9z9j9vghe/s-gnwd8daity-kp10sj.lock.gz`: Not a directory (os error 20)
warning: `gz-ice` (bin "gz-ice") generated 1 warning
Finished dev [unoptimized + debuginfo] target(s) in 0.13s
Running `target/debug/gz-ice`
Hello, world!
```
Avoid side-effects from `try_coerce` when suggesting borrowing LHS of cast
The name `try_coerce` is a bit misleading -- it has side-effects, so when it's used in diagnostics code, it sometimes causes spurious obligations to be registered which cause other errors to occur that really make no sense in context.
Addendum: let's just rename `try_coerce` to `coerce` -- the `try_` part doesn't really add much, imo.
Normalize return type of `deduce_future_output_from_obligations`
Fixes#114909
Also confirmed to fix#114727 manually
Now that we have weak/lazy type aliases, we need to normalize those in future signatures to ensure that `replace_opaque_types_with_inference_vars` actually sees TAITs behind them. This isn't needed in the new solver, but added a test to make sure it doesn't regress there either.
r? types cc `@oli-obk` (who's gone, worst case can delay this PR until he's back)
Fix ABI flags in RISC-V/LoongArch ELF file generated by rustc
Fix#114153
It turns out the current way to set these flags are completely wrong. In LLVM the target ABI is used instead of target features to determine these flags.
Not sure how to write a test though. Or maybe a test isn't necessary because this affects only those touching target json?
r? `@Nilstrieb`
This shows one small benefit of separating `BcbCounter` from `CoverageKind`.
The function source hash will be the same for all counters within a function,
so instead of passing it through `CoverageCounters` and storing it in every
counter, we can just supply it during the final conversion to `CoverageKind`.
rustdoc: Add lint `redundant_explicit_links`
Closes#87799.
- Lint warns by default
- Reworks link parser to cache original link's display text
r? `@jyn514`
Clearly the code path can be hit without the presence of a compiler bug.
All it takes is mischief. See 71698.
Ignore problematic directories instead of ICE:ing. `continue`ing is
already done for problematic dirs in the code block above us.
`Nonterminal`-related cleanups
In #114647 I am trying to remove `Nonterminal`. It has a number of preliminary cleanups that are worth merging even if #114647 doesn't merge, so let's do them in this PR.
r? `@petrochenkov`
Otherwise the file name generated for generator_drop will become
core.ptr-drop_in_place.[generator@<FILEPATH>_<NUMBERS>].generator_drop.0.mir
instead of main-{closure#0}.generator_drop.0.mir which breaks a mir-opt
test.
Replace the \01__gnu_mcount_nc to LLVM intrinsic for ARM
Current `-Zinstrument-mcount` for ARM32 use the `\01__gnu_mcount_nc` directly for its instrumentation function.
However, the LLVM does not use this mcount function directly, but it wraps it to intrinsic, `llvm.arm.gnu.eabi.mcount` and the transform pass also only handle the intrinsic.
As a result, current `-Zinstrument-mcount` not work on ARM32. Refer: https://github.com/namhyung/uftrace/issues/1764
This commit replaces the mcount name from native function to the LLVM intrinsic so that the transform pass can handle it.
[RFC-3086] Restrict the parsing of `count`
Fix#111904
The original RFC didn't mention the possibility of using `${count(t,)}` and such thing isn't very semantically accurate which can lead to confusion.
Normalize before checking if local is freeze in `deduced_param_attrs`
Not normalizing the local type eagerly results in possibly exponential amounts of normalization happening downstream in `is_freeze_raw`.
Fixes#113372
It's much more complicated than it needs to be, and it doesn't modify
the expression. We can do the `Result` handling outside of it, and
change it to just return a span.
Also fix an errant comma that makes the comment hard to read.
Speed up compilation of `type-system-chess`
[`type-system-chess`](https://github.com/rust-lang/rustc-perf/pull/1680) is an unusual program that implements a compile-time chess position solver in the trait system(!) This PR is about making it compile faster.
r? `@ghost`
Revert PR #114052 to fix invalid suggestion
This PR reverts https://github.com/rust-lang/rust/pull/114052 to fix the invalid suggestion produced by the PR.
Unfortunately the invalid suggestion cannot be improved from the current position where it's emitted since we lack enough information (is an assignment?, left or right?, ...) to be able to fix it here. Furthermore the previous wasn't wrong, just suboptimal, contrary to the current one which is just wrong.
Added a regression test and commented out some code instead of removing it so we can use it later.
Reopens https://github.com/rust-lang/rust/issues/114050
Fixes https://github.com/rust-lang/rust/issues/114925
Fix suggestion for attempting to define a string with single quotes
Currently attempting to compile `fn main() { let _ = '\\"'; }` will result in the following error message:
```
error: character literal may only contain one codepoint
--> src/main.rs:1:21
|
1 | fn main() { let _ = '\\"'; }
| ^^^^^
|
help: if you meant to write a `str` literal, use double quotes
|
1 | fn main() { let _ = "\\""; }
| ~~~~~
```
The suggestion is invalid as it fails to escape the `"`. This PR fixes the suggestion so that it now reads:
```
help: if you meant to write a `str` literal, use double quotes
|
1 | fn main() { let _ = "\\\""; }
| ~~~~~~
```
The relevant test is also updated to ensure that this does not regress in future.
Current `-Zinstrument-mcount` for ARM32 use the `\01__gnu_mcount_nc`
directly for its instrumentation function.
However, the LLVM does not use this mcount function directly, but it wraps
it to intrinsic, `llvm.arm.gnu.eabi.mcount` and the transform pass also
only handle the intrinsic.
As a result, current `-Zinstrument-mcount` not work on ARM32.
Refer: https://github.com/namhyung/uftrace/issues/1764
This commit replaces the mcount name from native function to the
LLVM intrinsic so that the transform pass can handle it.
Signed-off-by: ChoKyuWon <kyuwoncho18@gmail.com>
