Lint unused pub items in binary crates
~~This PR adds a new unstable flag -Ztreat-pub-as-pub-crate as [@Kobzol](https://github.com/Kobzol) suggested.~~
~~When compiling binary crates with this flag, the seed worklist will only contain the entry fn and won't contain other reachable items. Then we can do the dead code analysis for pub items just like they are pub(crate).~~
Related zulip thread [#general > pub/pub(crate) within a binary is a footgun](https://rust-lang.zulipchat.com/#narrow/channel/122651-general/topic/pub.2Fpub.28crate.29.20within.20a.20binary.20is.20a.20footgun/with/558931034).
---
Updated:
Adds a new lint `unused_pub_items_in_binary` (crate-level, default allow for now) instead of the previous unstable flag to lint unused `pub` items for binary crates.
See more details of implementation in https://github.com/rust-lang/rust/pull/149509#issuecomment-4102337689.
This lint is allowed by default, but I believe this has been better than the unstable flag. Making it warn-by-default will lead to a lot of noise for this PR (like bless many tests). So I'd like to make it warn-by-default in a separate PR in the future.
Add `const_param_ty_unchecked` gate
Add `const_param_ty_unchecked` internal feature gate to skip `ConstParamTy_` trait enforcement on type. Provides an escape hatch for writing tests and examples that use const generics without needing to ensure all fields implement `ConstParamTy_`.
r? BoxyUwU
Make retags an implicit part of typed copies
Ever since Stacked Borrows was first implemented in Miri, that was done with `Retag` statements: given a place (usually a local variable), those statements find all references stored inside the place and refresh their tags to ensure the aliasing requirements are upheld. However, this is a somewhat unsatisfying approach for multiple reasons:
- It leaves open the [question](https://github.com/rust-lang/unsafe-code-guidelines/issues/371) of where to even put `Retag` statements. Over time, the AddRetag pass settled on one possible answer to this, but it wasn't very canonical.
- For assignments of the form `*ptr = expr`, if the assignment involves copying a reference, we probably want to do a retag -- but if we do a `Retag(*ptr)` as the next instruction, it can be non-trivial to argue that this even retags the right value, so we refrained from doing retags in that case. This has [come up](https://github.com/llvm/llvm-project/pull/160913#issuecomment-3341908717) as a potential issue for Rust making better use of LLVM "captures" annotations. (That said, there might be [other ways](https://github.com/rust-lang/unsafe-code-guidelines/issues/593#issuecomment-4328112031) to obtain this desired optimization.)
- Normal compilation avoids generating retags, but we still generate LLVM IR with `noalias`. What does that even mean? How do MIR optimization passes interact with retags? These are questions we have to figure out to make better use of aliasing information, but currently we can't even really ask such questions.
I think we should resolve all that by making retags part of what happens during a typed copy (a concept and interpreter infrastructure that did not exist yet when retags were initially introduced). Under this proposal, when executing a MIR assignment statement, what conceptually happens is as follows:
- We evaluate the LHS to a place.
- We evaluate the RHS to a value. This does a typed load from memory if needed, raising UB if memory does not contain a valid representation of the assignment's type.
- We walk that value, identify all references inside of it, and retag them. If this happens as part of passing a function argument, this is a protecting retag.
- We store (a representation of) the value into the place.
However, this semantics doesn't fully work: there's a mandatory MIR pass that turns expressions like `&mut ***ptr` into intermediate deref's. Those must *not* do any retags. So far this happened because the AddRetag pass did not add retags for assignments to deref temporaries, but that information is not recorded in cross-crate MIR. Therefore I instead added a field to `Rvalue::Use` to indicate whether this value should be retagged or not. A non-retagging copy seems like a sufficiently canonical primitive that we should be able to express it. Dealing with the fallout from that is a large chunk of the overall diff. (I also considered adding this field to `StatementKind::Assign` instead, but decided against that as we only actually need it for `Rvalue::Use`. I am not sure if this was the right call...)
This neatly answers the question of when retags should occur, and handles cases like `*ptr = expr`. It avoids traversing values twice in Miri. It makes codegen's use of `noalias` sound wrt the actual MIR that it is working on. It also gives us a target semantics to evaluate MIR opts against. However, I did not carefully check all MIR opts -- in particular, GVN needs a thorough look under the new semantics; it currently can turn alias-correct code into alias-incorrect code. (But this PR doesn't make things any worse for normal compilation where the retag indicator is anyway ignored.)
Another side-effect of this PR is that `-Zmiri-disable-validation` now also disables alias checking. It'd be nicer to keep them orthogonal but I find this an acceptable price to pay.
- [rustc benchmark results](https://github.com/rust-lang/rust/pull/154341#issuecomment-4125313290)
- [miri benchmark results](https://github.com/rust-lang/rust/pull/154341#issuecomment-4129840926)
Add extra symbol check for `.to_owned()`
Follow up of rust-lang/rust#154646
Previously when adding a suggestion for using `Cow::into_owned()` instead of `ToOwned::to_owned()`, the compiler would just convert the methods `Span` into a `String` and do checks on that `String`. This PR adds an extra guard to that suggestion by checking if the method is `sym::to_owned_method`.
r? @davidtwco since you added the review to the previous PR
Previously when adding a suggestion for using `Cow::into_owned()`
instead of `ToOwned::to_owned()`, the compiler would just convert the
methods `Span` into a `String` and do checks on that `String`. This PR
adds an extra guard to that suggestion by checking if the method is
`sym::to_owned_method`.
Specifically:
- `HashStable` -> `StableHash` (trait)
- `HashStable` -> `StableHash` (derive)
- `HashStable_NoContext` -> `StableHash_NoContext` (derive)
Note: there are some names in `compiler/rustc_macros/src/hash_stable.rs`
that are still to be renamed, e.g. `HashStableMode`.
Part of MCP 983.
`std::hash::Hash` looks like this:
```
pub trait Hash {
fn hash<H>(&self, state: &mut H)
where H: Hasher;
...
}
```
The method is generic.
In contrast, `HashStable` looks like this:
```
pub trait HashStable<Hcx> {
fn hash_stable(&self, hcx: &mut Hcx, hasher: &mut StableHasher);
}
```
and impls look like this (in crates upstream of `rustc_middle`):
```
impl<Hcx: HashStableContext> HashStable<Hcx> for Path {
fn hash_stable(&self, hcx: &mut Hcx, hasher: &mut StableHasher) {
...
}
}
```
or this (in `rustc_middle` and crates downstream of `rustc_middle`):
```
impl<'tcx> HashStable<StableHashingContext<'tcx>> for rustc_feature::Features {
fn hash_stable(&self, hcx: &mut StableHashingContext<'tcx>, hasher: &mut StableHasher) {
...
}
}
```
Differences to `std::hash::Hash`:
- The trait is generic, rather than the method.
- The way impls are written depends their position in the crate graph.
- This explains why we have both `derive(HashStable)` and
`derive(HashStable_Generic)`. The former is for the
downstream-of-`rustc_middle` case, the latter is for the upstream of
`rustc_middle` case.
Why the differences? It all boils down to `HashStable` and
`HashStableContext` being in different crates. But the previous commit
fixed that, which means `HashStable` can be simplified to this, with a
generic method:
```
pub trait HashStable {
fn hash_stable<Hcx: HashStableContext>(&self, hcx: &mut Hcx, hasher: &mut StableHasher);
}
```
and all impls look like this:
```
impl HashStable for Path {
fn hash_stable<Hcx: HashStableContext>(&self, hcx: &mut Hcx, hasher: &mut StableHasher) {
...
}
}
```
Other consequences:
- `derive(HashStable_Generic)` is no longer needed; `derive(HashStable)`
can be used instead.
- In this commit, `derive(HashStable_Generic` is made a synonym of
`derive(HashStable)`. The next commit will remove this synonym,
because it's a change that touches many lines.
- `#[stable_hash_generic]` is no longer needed (for `newtype_index`);
`#[stable_hash]` can be used instead.
- `#[stable_hash_no_context]` was already a synonym of
`#[stable_hash_generic]`, so it's also removed in favour of just
`#[stable_hash]`.
- The difference between `derive(HashStable)` and
`derive(HashStable_NoContext)` now comes down to the difference
between `synstructure::AddBounds::Generics` and
`synstructure::AddBounds::Fields`, which is basically "vanilla derive"
vs "(near) perfect derive".
- I have improved the comments on `HashStableMode` to better
explaining this subtle difference.
- `rustc_middle/src/ich/impls_syntax.rs` is no longer needed; the
relevant impls can be defined in the crate that defines the relevant
type.
- Occurrences of `for<'a> HashStable<StableHashingContext<'a>>` are
replaced with with `HashStable`, hooray.
- The commit adds a `HashStableContext::hashing_controls` method, which
is no big deal. (It's necessary for `AdtDefData::hash_stable`, which
calls `hashing_controls` and used to have an `hcx` that was a
concrete `StableHashingContext` but now has an `hcx` that is just
`Hcx: HashStableContext`.)
Overall this is a big simplification, removing a lot of confusing
complexity in stable hashing traits.
This puts it in the same crate as the `HashStable` and `ToStableHasher`
traits. This requires introducing three types `RawSpan`, `RawDefId` and
`RawDefPathHash` to work around the fact that `rustc_data_structures`
is upstream of `rustc_span` and so doesn't have access to `Span`,
`DefId`, and `DefPathHash`. This is a bit ugly but is worth it because
moving `HashStableContext` enables big cleanups across many crates in
subsequent commits.
`def_id.to_stable_hash_key(hcx)` calls `hcx.def_path_hash(def_id)`. This
commit replaces various other occurrences of the latter form with the
former form. It's all inlined so there should be no perf effects.
This will make things easier for the next commit, which will change
`DefId::to_stable_hash_key`, among other things.
* Implement core::arch::return_address and tests
Fix typo
Apply suggestions from code review
Wording/docs changes.
Co-authored-by: Ralf Jung <post@ralfj.de>
Change signature according to Ralf's comment
Fix call to `core::intrinsics::return_address()` according to the new signature
Add cranelift implementation for intrinsic
Change wording on `return_address!()` to be clear that returning a null pointer is best-effort.
Fix formatting of doc comment
Fix mistake in cranelift codegen
* Do not generate llvm.returnaddress on wasm
* Supress return_address test on miri
Add intrinsic for launch-sized workgroup memory on GPUs
Workgroup memory is a memory region that is shared between all
threads in a workgroup on GPUs. Workgroup memory can be allocated
statically or after compilation, when launching a gpu-kernel.
The intrinsic added here returns the pointer to the memory that is
allocated at launch-time.
# Interface
With this change, workgroup memory can be accessed in Rust by
calling the new `gpu_launch_sized_workgroup_mem<T>() -> *mut T`
intrinsic.
It returns the pointer to workgroup memory guaranteeing that it is
aligned to at least the alignment of `T`.
The pointer is dereferencable for the size specified when launching the
current gpu-kernel (which may be the size of `T` but can also be larger
or smaller or zero).
All calls to this intrinsic return a pointer to the same address.
See the intrinsic documentation for more details.
## Alternative Interfaces
It was also considered to expose dynamic workgroup memory as extern
static variables in Rust, like they are represented in LLVM IR.
However, due to the pointer not being guaranteed to be dereferencable
(that depends on the allocated size at runtime), such a global must be
zero-sized, which makes global variables a bad fit.
# Implementation Details
Workgroup memory in amdgpu and nvptx lives in address space 3.
Workgroup memory from a launch is implemented by creating an
external global variable in address space 3. The global is declared with
size 0, as the actual size is only known at runtime. It is defined
behavior in LLVM to access an external global outside the defined size.
There is no similar way to get the allocated size of launch-sized
workgroup memory on amdgpu an nvptx, so users have to pass this
out-of-band or rely on target specific ways for now.
Tracking issue: rust-lang/rust#135516
Workgroup memory is a memory region that is shared between all
threads in a workgroup on GPUs. Workgroup memory can be allocated
statically or after compilation, when launching a gpu-kernel.
The intrinsic added here returns the pointer to the memory that is
allocated at launch-time.
# Interface
With this change, workgroup memory can be accessed in Rust by
calling the new `gpu_launch_sized_workgroup_mem<T>() -> *mut T`
intrinsic.
It returns the pointer to workgroup memory guaranteeing that it is
aligned to at least the alignment of `T`.
The pointer is dereferencable for the size specified when launching the
current gpu-kernel (which may be the size of `T` but can also be larger
or smaller or zero).
All calls to this intrinsic return a pointer to the same address.
See the intrinsic documentation for more details.
## Alternative Interfaces
It was also considered to expose dynamic workgroup memory as extern
static variables in Rust, like they are represented in LLVM IR.
However, due to the pointer not being guaranteed to be dereferencable
(that depends on the allocated size at runtime), such a global must be
zero-sized, which makes global variables a bad fit.
# Implementation Details
Workgroup memory in amdgpu and nvptx lives in address space 3.
Workgroup memory from a launch is implemented by creating an
external global variable in address space 3. The global is declared with
size 0, as the actual size is only known at runtime. It is defined
behavior in LLVM to access an external global outside the defined size.
There is no similar way to get the allocated size of launch-sized
workgroup memory on amdgpu an nvptx, so users have to pass this
out-of-band or rely on target specific ways for now.
c-variadic: fix implementation on `avr`
tracking issue: https://github.com/rust-lang/rust/issues/44930
cc target maintainer @Patryk27
I ran into multiple issues, and although with this PR and a little harness I can run the test with qemu on avr, the implementation is perhaps not ideal.
The problem we found is that on `avr` the `c_int/c_uint` types are `i16/u16`, and this was not handled in the c-variadic checks. Luckily there is a field in the target configuration that contains the targets `c_int_width`. However, this field is not actually used in `core` at all, there the 16-bit targets are just hardcoded.
https://github.com/rust-lang/rust/blob/1500f0f47f5fe8ddcd6528f6c6c031b210b4eac5/library/core/src/ffi/primitives.rs#L174-L185
Perhaps we should expose this like endianness and pointer width?
---
Finally there are some changes to the test to make it compile with `no_std`.
Add #![unstable_removed(..)] attribute to track removed features
Adds the #![unstable_removed(..)] attribute to enable tracking removed library features.
Produce an error when a removed attribute is used.
Add a test that it works.
For https://github.com/rust-lang/rust/issues/141617
r? @jyn514
Fix ICE in `span_extend_prev_while` with multibyte characters
Fixes https://github.com/rust-lang/rust/issues/155037
The function assumed that the character found by `rfind` was always one byte wide, using a hardcoded `1` instead of `c.len_utf8()`. When a multibyte character appeared immediately before the span, this caused the resulting span to point into the middle of a UTF-8 sequence, triggering an assertion failure in `bytepos_to_file_charpos`.
Jonathan Brouwer