Handle stashing of delayed bugs
By just emitting them immediately, because it does happen in practice, when errors are downgraded to delayed bugs.
We already had one case in `lint.rs` where we handled this at the callsite. This commit changes things so it's handled within `stash_diagnostic` instead, because #121812 identified a second case, and it's possible there are more.
Fixes#121812.
r? ````@oli-obk````
Make the success arms of `if lhs || rhs` meet up in a separate block
Extracted from #118305, where this is necessary to avoid introducing a bug when injecting marker statements into the then/else arms.
---
In the previous code (#111752), the success block of `lhs` would jump directly to the success block of `rhs`. However, `rhs_success_block` could already contain statements that are specific to the RHS, and the direct goto causes them to be executed in the LHS success path as well.
This patch therefore creates a fresh block that the LHS and RHS success blocks can both jump to.
---
I think the reason we currently get away with this is that `rhs_success_block` usually doesn't contain anything other than StorageDead statements for locals used by the RHS, and those statements don't seem to cause problems in the LHS success path (which never makes those locals live).
But if we start adding meaningful statements for branch coverage (or MC/DC coverage), it's important to keep the LHS and RHS blocks separate.
make unused_imports less assertive in test modules
closes#121502
This is a fairly small change and I used the fix suggested in the example expected error message.
Not sure if I should've rather used the alternatives but this one seems the most descriptive.
Some alternatives:
- if this is meant to be a test module, add `#[cfg(test)]` to the containing module
- try adding #[cfg(test)] to this test module
- consider adding #[allow(unused_imports)] if you want to silent the lint on the unused import
- consider removing the unused import
Improve error messages for generics with default parameters
Fixes#120785
Issue: Previously, all type parameters with default types were deliberately ignored to simplify error messages. For example, an error message for Box type would display `Box<T>` instead of `Box<T, _>`. But, this resulted in unclear error message when a concrete type was used instead of the default type.
Fix: This PR fixes it by checking if a concrete type is specified after a default type to display the entire type name or the simplified type name.
test: enable `unpacked-lto` tests
This enables the correct `unpacked-lto` tests.
Not sure whether `.o` should be removed.
They are bitcode for linker-plugin-lto, though there might be some `.o` for `#[no_builtins]`?
Combine `Sub` and `Equate`
Combine `Sub` and `Equate` into a new relation called `TypeRelating` (that name sounds familiar...)
Tracks the difference between `Sub` and `Equate` via `ambient_variance: ty::Variance` much like the `NllTypeRelating` relation, but implemented slightly jankier because it's a more general purpose relation.
r? lcnr
Add stubs in IR and ABI for `f16` and `f128`
This is the very first step toward the changes in https://github.com/rust-lang/rust/pull/114607 and the [`f16` and `f128` RFC](https://rust-lang.github.io/rfcs/3453-f16-and-f128.html). It adds the types to `rustc_type_ir::FloatTy` and `rustc_abi::Primitive`, and just propagates those out as `unimplemented!` stubs where necessary.
These types do not parse yet so there is no feature gate, and it should be okay to use `unimplemented!`.
The next steps will probably be AST support with parsing and the feature gate.
r? `@compiler-errors`
cc `@Nilstrieb` suggested breaking the PR up in https://github.com/rust-lang/rust/pull/120645#issuecomment-1925900572
Delete architecture-specific memchr code in std::sys
Currently all architecture-specific memchr code is only used in `std::io`. Most of the actual `memchr` capacity exposed to the user through the slice API is instead implemented in `core::slice::memchr`.
Hence this commit deletes `memchr` from `std::sys[_common]` and replace calls to it by calls to `core::slice::memchr` functions. This deletes `(r)memchr` from the list of symbols linked to libc.
The interest of putting architecture specific code back in core is linked to the discussion to be had in #113654
By just emitting them immediately, because it does happen in practice,
when errors are downgraded to delayed bugs.
We already had one case in `lint.rs` where we handled this at the
callsite. This commit changes things so it's handled within
`stash_diagnostic` instead, because #121812 identified a second case,
and it's possible there are more.
Fixes#121812.
Remove doc aliases to PATH
Remove aliases for `split_paths` and `join_paths` as should have been done in <https://github.com/rust-lang/rust/pull/119748> (Bors merged the wrong commit).
allow statics pointing to mutable statics
Fixes https://github.com/rust-lang/rust/issues/120450 for good. We can even simplify our checks: no need to specifically go looking for mutable references in const, we can just reject any reference that points to something mutable.
r? `@oli-obk`
Add proper cfg to keep only one AlignmentEnum definition for different target_pointer_widths
Detected by #121752
Only one AlignmentEnum would be used with a specified target_pointer_width
Safe Transmute: Revise safety analysis
This PR migrates `BikeshedIntrinsicFrom` to a simplified safety analysis (described [here](https://github.com/rust-lang/project-safe-transmute/issues/15)) that does not rely on analyzing the visibility of types and fields.
The revised analysis treats primitive types as safe, and user-defined types as potentially carrying safety invariants. If Rust gains explicit (un)safe fields, this PR is structured so that it will be fairly easy to thread support for those annotations into the analysis.
Notably, this PR removes the `Context` type parameter from `BikeshedIntrinsicFrom`. Most of the files changed by this PR are just UI tests tweaked to accommodate the removed parameter.
r? `@compiler-errors`
rustc: Fix wasm64 metadata object files
It looks like LLD will detect object files being either 32 or 64-bit depending on any memory present. LLD will additionally reject 32-bit objects during a 64-bit link. Previously metadata objects did not have any memories in them which led LLD to conclude they were 32-bit objects which broke 64-bit targets for wasm.
This commit fixes this by ensuring that for 64-bit targets there's a memory object present to get LLD to detect it's a 64-bit target. Additionally this commit moves away from a hand-crafted wasm encoder to the `wasm-encoder` crate on crates.io as the complexity grows for the generated object file.
Closes#121460
Detect empty leading where clauses on type aliases
1. commit: refactor the AST of type alias where clauses
* I could no longer bear the look of `.0.1` and `.1.0`
* Arguably moving `split` out of `TyAlias` into a substruct might not make that much sense from a semantic standpoint since it reprs an index into `TyAlias.predicates` but it's alright and it cleans up the usage sites of `TyAlias`
2. commit: fix an oversight: An empty leading where clause is still a leading where clause
* semantically reject empty leading where clauses on lazy type aliases
* e.g., on `#![feature(lazy_type_alias)] type X where = ();`
* make empty leading where clauses on assoc types trigger lint `deprecated_where_clause_location`
* e.g., `impl Trait for () { type X where = (); }`
change equate for binders to not rely on subtyping
*summary by `@spastorino` and `@lcnr*`
### Context
The following code:
```rust
type One = for<'a> fn(&'a (), &'a ());
type Two = for<'a, 'b> fn(&'a (), &'b ());
mod my_api {
use std::any::Any;
use std::marker::PhantomData;
pub struct Foo<T: 'static> {
a: &'static dyn Any,
_p: PhantomData<*mut T>, // invariant, the type of the `dyn Any`
}
impl<T: 'static> Foo<T> {
pub fn deref(&self) -> &'static T {
match self.a.downcast_ref::<T>() {
None => unsafe { std::hint::unreachable_unchecked() },
Some(a) => a,
}
}
pub fn new(a: T) -> Foo<T> {
Foo::<T> {
a: Box::leak(Box::new(a)),
_p: PhantomData,
}
}
}
}
use my_api::*;
fn main() {
let foo = Foo::<One>::new((|_, _| ()) as One);
foo.deref();
let foo: Foo<Two> = foo;
foo.deref();
}
```
has UB from hitting the `unreachable_unchecked`. This happens because `TypeId::of::<One>()` is not the same as `TypeId::of::<Two>()` despite them being considered the same types by the type checker.
Currently the type checker considers binders to be equal if subtyping succeeds in both directions: `for<'a> T<'a> eq for<'b> U<'b>` holds if `for<'a> exists<'b> T<'b> <: T'<a> AND for<'b> exists<'a> T<'a> <: T<'b>` holds. This results in `for<'a> fn(&'a (), &'a ())` and `for<'a, 'b> fn(&'a (), &'b ())` being equal in the type system.
`TypeId` is computed by looking at the *structure* of a type. Even though these types are semantically equal, they have a different *structure* resulting in them having different `TypeId`. This can break invariants of unsafe code at runtime and is unsound when happening at compile time, e.g. when using const generics.
So as seen in `main`, we can assign a value of type `Foo::<One>` to a binding of type `Foo<Two>` given those are considered the same type but then when we call `deref`, it calls `downcast_ref` that relies on `TypeId` and we would hit the `None` arm as these have different `TypeId`s.
As stated in https://github.com/rust-lang/rust/issues/97156#issuecomment-1879030033, this causes the API of existing crates to be unsound.
## What should we do about this
The same type resulting in different `TypeId`s is a significant footgun, breaking a very reasonable assumptions by authors of unsafe code. It will also be unsound by itself once they are usable in generic contexts with const generics.
There are two options going forward here:
- change how the *structure* of a type is computed before relying on it. i.e. continue considering `for<'a> fn(&'a (), &'a ())` and `for<'a, 'b> fn(&'a (), &'b ())` to be equal, but normalize them to a common representation so that their `TypeId` are also the same.
- change how the semantic equality of binders to match the way we compute the structure of types. i.e. `for<'a> fn(&'a (), &'a ())` and `for<'a, 'b> fn(&'a (), &'b ())` still have different `TypeId`s but are now also considered to not be semantically equal.
---
Advantages of the first approach:
- with the second approach some higher ranked types stop being equal, even though they are subtypes of each other
General thoughts:
- changing the approach in the future will be breaking
- going from first to second may break ordinary type checking, as types which were previously equal are now distinct
- going from second to first may break coherence, because previously disjoint impls overlap as the used types are now equal
- both of these are quite unlikely. This PR did not result in any crater failures, so this should not matter too much
Advantages of the second approach:
- the soundness of the first approach requires more non-local reasoning. We have to make sure that changes to subtyping do not cause the representative computation to diverge from semantic equality
- e.g. we intend to consider higher ranked implied bounds when subtyping to [fix] https://github.com/rust-lang/rust/issues/25860, I don't know how this will interact and don't feel confident making any prediction here.
- computing a representative type is non-trivial and soundness critical, therefore adding complexity to the "core type system"
---
This PR goes with the second approach. A crater run did not result in any regressions. I am personally very hesitant about trying the first approach due to the above reasons. It feels like there are more unknowns when going that route.
### Changing the way we equate binders
Relating bound variables from different depths already results in a universe error in equate. We therefore only need to make sure that there is 1-to-1 correspondence between bound variables when relating binders. This results in concrete types being structurally equal after anonymizing their bound variables.
We implement this by instantiating one of the binder with placeholders and the other with inference variables and then equating the instantiated types. We do so in both directions.
More formally, we change the typing rules as follows:
```
for<'r0, .., 'rn> exists<'l0, .., 'ln> LHS<'l0, .., 'ln> <: RHS<'r0, .., 'rn>
for<'l0, .., 'ln> exists<'r0, .., 'rn> RHS<'r0, .., 'rn> <: LHS<'l0, .., 'ln>
--------------------------------------------------------------------------
for<'l0, .., 'ln> LHS<'l0, .., 'ln> eq for<'r0, .., 'rn> RHS<'r0, .., 'rn>
```
to
```
for<'r0, .., 'rn> exists<'l0, .., 'ln> LHS<'l0, .., 'ln> eq RHS<'r0, .., 'rn>
for<'l0, .., 'ln> exists<'r0, .., 'rn> RHS<'r0, .., 'rn> eq LHS<'l0, .., 'ln>
--------------------------------------------------------------------------
for<'l0, .., 'ln> LHS<'l0, .., 'ln> eq for<'r0, .., 'rn> RHS<'r0, .., 'rn>
```
---
Fixes#97156
r? `@lcnr`
Emitter cleanups
Some cleanups I made when reading emitter code. In particular, `HumanEmitter` and `JsonEmitter` have gone from three constructors to one.
r? `@oli-obk`
Count stashed errors again
Stashed diagnostics are such a pain. Their "might be emitted, might not" semantics messes with lots of things.
#120828 and #121206 made some big changes to how they work, improving some things, but still leaving some problems, as seen by the issues caused by #121206. This PR aims to fix all of them by restricting them in a way that eliminates the "might be emitted, might not" semantics while still allowing 98% of their benefit. Details in the individual commit logs.
r? `@oli-obk`
Use volatile access instead of `#[used]` for `on_tls_callback`
The first commit adds a volatile load of `p_thread_callback` when registering a dtor so that the compiler knows if the callback is used or not. I don't believe the added volatile instruction is otherwise significant in the context. In my testing using the volatile load allowed the compiler to correctly reason about whether `on_tls_callback` is used or not, allowing it to be omitted entirely in some cases. Admittedly it usually is used due to `Thread` but that can be avoided (e.g. in DLLs or with custom entry points that avoid the offending APIs). Ideally this would be something the compiler could help a bit more with so we didn't have to use tricks like `#[used]` or volatile. But alas.
I also used this as an opportunity to clean up the `unused` lints which I don't think serve a purpose any more.
The second commit removes the volatile load of `_tls_used` with `#cfg[target_thread_local]` because `#[thread_local]` already implies it. And if it ever didn't then `#[thread_local]` would be broken when there aren't any dtors.
Skip unnecessary comparison with half-open range patterns
This is the last remaining detail in the implementation of half-open range patterns. Until now, a half-open range pattern like `10..` was converted to `10..T::MAX` before lowering to MIR, which generated an extra pointless comparison. With this PR we don't generate it.
