fix: implemeted lifetime transformation fot assits
A part of https://github.com/rust-lang/rust-analyzer/issues/13363
I expect to implement transformation of const params in a separate PR
Other assists and a completion affected:
- `generate_function` currently just ignores lifetimes and, consequently, is not affected
- `inline_call` and `replace_derive_with...` don't seem to need lifetime transformation
- `trait_impl` (a completion) is fixed and tested
autopublish: Offset version number
The workflow is currently failing because it's trying to publish 0.0.16, while the last version published was 0.0.149.
Add span to group.
This appears to fix#14959, but I've never contributed to rust-analyzer before and there were some things that confused me:
- I had to add the `fn byte_range` method to get it to build. This was added to rust in [April](https://github.com/rust-lang/rust/pull/109002), so I don't understand why it wasn't needed until now
- When testing, I ran into the fact that rust recently updated its `METADATA_VERSION`, so I had to test this with nightly-2023-05-20. But then I noticed that rust has its own copy of `rust-analyzer`, and the metadata version bump has already been [handled there](60e95e76d0). So I guess I don't really understand the relationship between the code there and the code here.
internal: Migrate some assists to use the structured snippet API
Migrates the following assists:
- `add_missing_impl_members`
- `extract_type_alias`
As an additional requirement, these assists are also migrated to use the mutable AST API, since otherwise there would be overlapping `Indel` spans
Document the sysroot field in JsonProject
rust-analyzer supports both `sysroot` and `sysroot_src` in `rust-project.json`. Document `sysroot` and show example values for both fields.
Uplift `clippy::undropped_manually_drops` lint
This PR aims at uplifting the `clippy::undropped_manually_drops` lint.
## `undropped_manually_drops`
(warn-by-default)
The `undropped_manually_drops` lint check for calls to `std::mem::drop` with a value of `std::mem::ManuallyDrop` which doesn't drop.
### Example
```rust
struct S;
drop(std::mem::ManuallyDrop::new(S));
```
### Explanation
`ManuallyDrop` does not drop it's inner value so calling `std::mem::drop` will not drop the inner value of the `ManuallyDrop` either.
-----
Mostly followed the instructions for uplifting an clippy lint described here: https://github.com/rust-lang/rust/pull/99696#pullrequestreview-1134072751
`@rustbot` label: +I-lang-nominated
r? compiler
-----
For Clippy:
changelog: Moves: Uplifted `clippy::undropped_manually_drops` into rustc
internal: Lazy eager macros
This PR makes eager macros less eager. We now only eagerly expand the input of them, while the actual expansion of the macro itself now happens like other lazy macros.
This change allows unifying a lot of macro handling between the two now, most of the special casing now happens for `include!` specifically as it is a very unique macro (by having two inputs that come from differing files).
Fixes https://github.com/rust-lang/rust-analyzer/issues/14841
Fixes https://github.com/rust-lang/rust-analyzer/issues/14996
Write to stdout if `-` is given as output file
With this PR, if `-o -` or `--emit KIND=-` is provided, output will be written to stdout instead. Binary output (those of type `obj`, `llvm-bc`, `link` and `metadata`) being written this way will result in an error unless stdout is not a tty. Multiple output types going to stdout will trigger an error too, as they will all be mixded together.
This implements https://github.com/rust-lang/compiler-team/issues/431
The idea behind the changes is to introduce an `OutFileName` enum that represents the output - be it a real path or stdout - and to use this enum along the code paths that handle different output types.
Infer return type for async function in `generate_function`
Part of #10122
In `generate_function` assist, when we infer the return type of async function we're generating, we should retrieve the type of parent await expression rather than the call expression itself.
Take MIR dataflow analyses by mutable reference
The main motivation here is any analysis requiring dynamically sized scratch memory to work. One concrete example would be pointer target tracking, where tracking the results of a dereference can result in multiple possible targets. This leads to processing multi-level dereferences requiring the ability to handle a changing number of potential targets per step. A (simplified) function for this would be `fn apply_deref(potential_targets: &mut Vec<Target>)` which would use the scratch space contained in the analysis to send arguments and receive the results.
The alternative to this would be to wrap everything in a `RefCell`, which is what `MaybeRequiresStorage` currently does. This comes with a small perf cost and loses the compiler's guarantee that we don't try to take multiple borrows at the same time.
For the implementation:
* `AnalysisResults` is an unfortunate requirement to avoid an unconstrained type parameter error.
* `CloneAnalysis` could just be `Clone` instead, but that would result in more work than is required to have multiple cursors over the same result set.
* `ResultsVisitor` now takes the results type on in each function as there's no other way to have access to the analysis without cloning it. This could use an associated type rather than a type parameter, but the current approach makes it easier to not care about the type when it's not necessary.
* `MaybeRequiresStorage` now no longer uses a `RefCell`, but the graphviz formatter now does. It could be removed, but that would require even more changes and doesn't really seem necessary.
Removed use of iteration through a HashMap/HashSet in rustc_incremental and replaced with IndexMap/IndexSet
This allows for the `#[allow(rustc::potential_query_instability)]` in rustc_incremental to be removed, moving towards fixing #84447 (although a LOT more modules have to be changed to fully resolve it). Only HashMaps/HashSets that are being iterated through have been modified (although many structs and traits outside of rustc_incremental had to be modified as well, as they had fields/methods that involved a HashMap/HashSet that would be iterated through)
I'm making a PR for just 1 module changed to test for performance regressions and such, for future changes I'll either edit this PR to reflect additional modules being converted, or batch multiple modules of changes together and make a PR for each group of modules.
Avoid unwind across `extern "C"` in `thread_local::fast_local`
This is a minimal fix for #112285, in case we want a simple patch that can be easily to backported if that's desirable.
*(Note: I have another broader cleanup which I've mostly omitted from here to avoid clutter, except for the `Cell` change, which isn't needed to fix UB, but simplifies safety comments).*
The only tier-1 target that this occurs on in a way that seems likely to cause problems in practice linux-gnu, although I believe some folks care about that platform somewhat 😉. I'm unsure how big of an issue this is. I've seen stuff like this behave quite badly, but there's a number of reasons to think this might actually be "fine in practice".
I've hedged my bets and assumed we'll backport this at least to beta but my feeling is that there's not enough evidence this is a problem worth backporting further than that.
### More details
This issue seems to have existed since `thread_local!`'s `const` init functionality was added. It occurs if you have a `const`-initialized thread local for a type that `needs_drop`, the drop panics, and you're on a target with support for static thread locals. In this case, we will end up defining an `extern "C"` function in the user crate rather than in libstd, and because the user crate will not have `#![feature(c_unwind)]` enabled, their panic will not be caught by an auto-inserted abort guard.
In practice, the actual situation where problems are likely[^ub] is somewhat narrower.
On most targets with static thread locals, we manage the TLS dtor list by hand (for reentrancy reasons among others). In these cases, while the users code may panic, we're calling it inside our own `extern "C"` (or `extern "system"`) function, which seems to (at least in practice) catch the panic and convert it to an abort.
However, on a few targets, most notably linux-gnu with recent glibc (but also fuchsia and redox), a tls dtor registration mechanism exists which we can actually use directly, [`__cxa_thread_atexit_impl`](https://github.com/rust-lang/rust/blob/master/library/std/src/sys/unix/thread_local_dtor.rs#L26-L36).
This is the case that seems most likely to be a cause for concern, as now we're passing a function to the system library and panicking out of it in a case where there are may not be Rust frames above it on the call stack (since it's running thread shutdown), and even if there were, it may not be prepared to handle such unwinding. If that's the case, it'd be bad.
Is it? Dunno. The fact that it's a `__cxa_*` function makes me think they probably have considered that the callback could throw but I have no evidence here and it doesn't seem to be written down anywhere, so it's just a guess. (I would not be surprised if someone comes into this thread to tell me how definitely-bad-news it is).
That said, as I said, all this is actually UB! If this isn't a "technically UB but fine in practice", but all bets are off if this is the kind of thing we are telling LLVM about.
[^ub]: This is UB so take that with a grain of salt -- I'm absolutely making assumptions about how the UB will behave "in practice" here, which is almost certainly a mistake.
Force all native libraries to be statically linked when linking a static binary
Previously, `#[link]` without an explicit `kind = "static"` would confuse the linker and end up producing a dynamically linked library because of the `-Bdynamic` flag. However this binary would not work correctly anyways since it was linked with startup code for a static binary.
This PR solves this by forcing all native libraries to be statically linked when the output is a static binary that cannot link to dynamic libraries anyways.
Fixes#108878Fixes#102993
Remember names of `cfg`-ed out items to mention them in diagnostics
# Examples
## `serde::Deserialize` without the `derive` feature (a classic beginner mistake)
I had to slightly modify serde so that it uses explicit re-exports instead of a glob re-export. (Update: a serde PR was merged that adds the manual re-exports)
```
error[E0433]: failed to resolve: could not find `Serialize` in `serde`
--> src/main.rs:1:17
|
1 | #[derive(serde::Serialize)]
| ^^^^^^^^^ could not find `Serialize` in `serde`
|
note: crate `serde` has an item named `Serialize` but it is inactive because its cfg predicate evaluated to false
--> /home/gh-Nilstrieb/.cargo/registry/src/index.crates.io-6f17d22bba15001f/serde-1.0.160/src/lib.rs:343:1
|
343 | #[cfg(feature = "serde_derive")]
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
344 | pub use serde_derive::{Deserialize, Serialize};
| ^^^^^^^^^
= note: the item is gated behind the `serde_derive` feature
= note: see https://doc.rust-lang.org/cargo/reference/features.html for how to activate a crate's feature
```
(the suggestion is not ideal but that's serde's fault)
I already tested the metadata size impact locally by compiling the `windows` crate without any features. `800k` -> `809k`
r? `@ghost`
fix: only generate trait bound for associated types in field types
Given the following definitions:
```rust
trait Trait {
type A;
type B;
type C;
}
#[derive(Clone)]
struct S<T: Trait>
where
T::A: Send,
{
qualified: <T as Trait>::B,
shorthand: T::C,
}
```
we currently expand the derive macro to:
```rust
impl<T> Clone for S<T>
where
T: Trait + Clone,
T::A: Clone,
T::B: Clone,
T::C: Clone,
{ /* ... */ }
```
This does not match how rustc expands it. Specifically, `Clone` bounds for `T::A` and `T::B` should not be generated.
The criteria for associated types to get bound seem to be 1) the associated type appears as part of field types AND 2) it's written in the shorthand form. I have no idea why rustc doesn't consider qualified associated types (there's even a comment that suggests they should be considered; see rust-lang/rust#50730), but it's important to follow rustc.
Improved std support for ps vita target
Fixed a couple of things in std support for ps vita via Vita SDK newlib oss implementation:
- Added missing hardware features to target spec
- Compile in thumb by default (newlib is also compiled in thumb)
- Fixed fs calls. Vita newlib has a not-very-posix dirent. Also vita does not expose inodes, it's stubbed as 0 in stat, and I'm stubbing it here for dirent (because vita newlibs's dirent doesn't even have that field)
- Enabled signal handlers for panic unwinding
- Dropped static link requirement from the platform support md. Also, rearranged sections to better stick with the template.
Run tests on PGO/LTO/BOLT optimized dist artifacts
This PR adds baisc tests for the optimized dist builds on x64 Linux and Windows. A subset of the test suite is run, so it's not perfect, but it's better than the status quo (which is basically no testing at all, apart from the perf bot on Linux).
r? `@ghost`
Use `load`+`store` instead of `memcpy` for small integer arrays
I was inspired by #98892 to see whether, rather than making `mem::swap` do something smart in the library, we could update MIR assignments like `*_1 = *_2` to do something smarter than `memcpy` for sufficiently-small types that doing it inline is going to be better than a `memcpy` call in assembly anyway. After all, special code may help `mem::swap`, but if the "obvious" MIR can just result in the correct thing that helps everything -- other code like `mem::replace`, people doing it manually, and just passing around by value in general -- as well as makes MIR inlining happier since it doesn't need to deal with all the complicated library code if it just sees a couple assignments.
LLVM will turn the short, known-length `memcpy`s into direct instructions in the backend, but that's too late for it to be able to remove `alloca`s. In general, replacing `memcpy`s with typed instructions is hard in the middle-end -- even for `memcpy.inline` where it knows it won't be a function call -- is hard [due to poison propagation issues](https://rust-lang.zulipchat.com/#narrow/stream/187780-t-compiler.2Fwg-llvm/topic/memcpy.20vs.20load-store.20for.20MIR.20assignments/near/360376712). So because we know more about the type invariants -- these are typed copies -- rustc can emit something more specific, allowing LLVM to `mem2reg` away the `alloca`s in some situations.
#52051 previously did something like this in the library for `mem::swap`, but it ended up regressing during enabling mir inlining (cbbf06b0cd), so this has been suboptimal on stable for ≈5 releases now.
The code in this PR is narrowly targeted at just integer arrays in LLVM, but works via a new method on the [`LayoutTypeMethods`](https://doc.rust-lang.org/nightly/nightly-rustc/rustc_codegen_ssa/traits/trait.LayoutTypeMethods.html) trait, so specific backends based on cg_ssa can enable this for more situations over time, as we find them. I don't want to try to bite off too much in this PR, though. (Transparent newtypes and simple things like the 3×usize `String` would be obvious candidates for a follow-up.)
Codegen demonstrations: <https://llvm.godbolt.org/z/fK8hT9aqv>
Before:
```llvm
define void `@swap_rgb48_old(ptr` noalias nocapture noundef align 2 dereferenceable(6) %x, ptr noalias nocapture noundef align 2 dereferenceable(6) %y) unnamed_addr #1 {
%a.i = alloca [3 x i16], align 2
call void `@llvm.lifetime.start.p0(i64` 6, ptr nonnull %a.i)
call void `@llvm.memcpy.p0.p0.i64(ptr` noundef nonnull align 2 dereferenceable(6) %a.i, ptr noundef nonnull align 2 dereferenceable(6) %x, i64 6, i1 false)
tail call void `@llvm.memcpy.p0.p0.i64(ptr` noundef nonnull align 2 dereferenceable(6) %x, ptr noundef nonnull align 2 dereferenceable(6) %y, i64 6, i1 false)
call void `@llvm.memcpy.p0.p0.i64(ptr` noundef nonnull align 2 dereferenceable(6) %y, ptr noundef nonnull align 2 dereferenceable(6) %a.i, i64 6, i1 false)
call void `@llvm.lifetime.end.p0(i64` 6, ptr nonnull %a.i)
ret void
}
```
Note it going to stack:
```nasm
swap_rgb48_old: # `@swap_rgb48_old`
movzx eax, word ptr [rdi + 4]
mov word ptr [rsp - 4], ax
mov eax, dword ptr [rdi]
mov dword ptr [rsp - 8], eax
movzx eax, word ptr [rsi + 4]
mov word ptr [rdi + 4], ax
mov eax, dword ptr [rsi]
mov dword ptr [rdi], eax
movzx eax, word ptr [rsp - 4]
mov word ptr [rsi + 4], ax
mov eax, dword ptr [rsp - 8]
mov dword ptr [rsi], eax
ret
```
Now:
```llvm
define void `@swap_rgb48(ptr` noalias nocapture noundef align 2 dereferenceable(6) %x, ptr noalias nocapture noundef align 2 dereferenceable(6) %y) unnamed_addr #0 {
start:
%0 = load <3 x i16>, ptr %x, align 2
%1 = load <3 x i16>, ptr %y, align 2
store <3 x i16> %1, ptr %x, align 2
store <3 x i16> %0, ptr %y, align 2
ret void
}
```
still lowers to `dword`+`word` operations, but has no stack traffic:
```nasm
swap_rgb48: # `@swap_rgb48`
mov eax, dword ptr [rdi]
movzx ecx, word ptr [rdi + 4]
movzx edx, word ptr [rsi + 4]
mov r8d, dword ptr [rsi]
mov dword ptr [rdi], r8d
mov word ptr [rdi + 4], dx
mov word ptr [rsi + 4], cx
mov dword ptr [rsi], eax
ret
```
And as a demonstration that this isn't just `mem::swap`, a `mem::replace` on a small array (since replace doesn't use swap since #83022), which used to be `memcpy`s in LLVM changes in IR
```llvm
define void `@replace_short_array(ptr` noalias nocapture noundef sret([3 x i32]) dereferenceable(12) %0, ptr noalias noundef align 4 dereferenceable(12) %r, ptr noalias nocapture noundef readonly dereferenceable(12) %v) unnamed_addr #0 {
start:
%1 = load <3 x i32>, ptr %r, align 4
store <3 x i32> %1, ptr %0, align 4
%2 = load <3 x i32>, ptr %v, align 4
store <3 x i32> %2, ptr %r, align 4
ret void
}
```
but that lowers to reasonable `dword`+`qword` instructions still
```nasm
replace_short_array: # `@replace_short_array`
mov rax, rdi
mov rcx, qword ptr [rsi]
mov edi, dword ptr [rsi + 8]
mov dword ptr [rax + 8], edi
mov qword ptr [rax], rcx
mov rcx, qword ptr [rdx]
mov edx, dword ptr [rdx + 8]
mov dword ptr [rsi + 8], edx
mov qword ptr [rsi], rcx
ret
```
Group rfcs tests
This moves all RFC tests to `tests/ui/rfcs/rfc-NNNN-title-title-title/...`
I had to rename some tests due to conflicts, but otherwise this is just a move.
