Strengthen invalid_value lint to forbid uninit primitives, adjust docs to say that's UB
For context: https://github.com/rust-lang/rust/issues/66151#issuecomment-1174477404=
This does not make it a FCW, but it does explicitly state in the docs that uninit integers are UB.
This also doesn't affect any runtime behavior, uninit u32's will still successfully be created through mem::uninitialized.
Thought of doing this by having a struct and an enum with Default and Alt cases, but not sure if we wanted to have the text in code instead of having “demangling()” and “demangling-alt()” in the ftl file.
Don’t like the current way of having structs representing the same-ish and using long names to distinguish their expectations, instead of putting this in an enum and handling the different cases inside the type.
I am fine with whichever option the team prefers; also understand having them as separate structs keeps it simple.
Initial implementation `result_large_err`
This is a shot at #6560, #4652, and #3884. The lint checks for `Result` being returned from functions/methods where the `Err` variant is larger than a configurable threshold (the default of which is 128 bytes). There has been some discussion around this, which I'll try to quickly summarize:
* A large `Err`-variant may force an equally large `Result` if `Err` is actually bigger than `Ok`.
* There is a cost involved in large `Result`, as LLVM may choose to `memcpy` them around above a certain size.
* We usually expect the `Err` variant to be seldomly used, but pay the cost every time.
* `Result` returned from library code has a high chance of bubbling up the call stack, getting stuffed into `MyLibError { IoError(std::io::Error), ParseError(parselib::Error), ...}`, exacerbating the problem.
This PR deliberately does not take into account comparing the `Ok` to the `Err` variant (e.g. a ratio, or one being larger than the other). Rather we choose an absolute threshold for `Err`'s size, above which we warn. The reason for this is that `Err`s probably get `map_err`'ed further up the call stack, and we can't draw conclusions from the ratio at the point where the `Result` is returned. A relative threshold would also be less predictable, while not accounting for the cost of LLVM being forced to generate less efficient code if the `Err`-variant is _large_ in absolute terms.
We lint private functions as well as public functions, as the perf-cost applies to in-crate code as well.
In order to account for type-parameters, I conjured up `fn approx_ty_size`. The function relies on `LateContext::layout_of` to compute the actual size, and in case of failure (e.g. due to generics) tries to come up with an "at least size". In the latter case, the size of obviously wrong, but the inspected size certainly can't be smaller than that. Please give the approach a heavy dose of review, as I'm not actually familiar with the type-system at all (read: I have no idea what I'm doing).
The approach does, however flimsy it is, allow us to successfully lint situations like
```rust
pub union UnionError<T: Copy> {
_maybe: T,
_or_perhaps_even: (T, [u8; 512]),
}
// We know `UnionError<T>` will be at least 512 bytes, no matter what `T` is
pub fn param_large_union<T: Copy>() -> Result<(), UnionError<T>> {
Ok(())
}
```
I've given some refactoring to `functions/result_unit_err.rs` to re-use some bits. This is also the groundwork for #6409
The default threshold is 128 because of https://github.com/rust-lang/rust-clippy/issues/4652#issue-505670554
`lintcheck` does not trigger this lint for a threshold of 128. It does warn for 64, though.
The suggestion currently is the following, which is just a placeholder for discussion to be had. I did have the computed size in a `span_label`. However, that might cause both ui-tests here and lints elsewhere to become flaky wrt to their output (as the size is platform dependent).
```
error: the `Err`-variant returned via this `Result` is very large
--> $DIR/result_large_err.rs:36:34
|
LL | pub fn param_large_error<R>() -> Result<(), (u128, R, FullyDefinedLargeError)> {
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ The `Err` variant is unusually large, at least 128 bytes
```
changelog: Add [`result_large_err`] lint
Avoid bootstrap from updating rls submodule
Our CI performing tests of the `x86_64-fortanix-unknown-sgx` failed with:
```
21:00:53 + ./configure --enable-lld --disable-rpath --set llvm.ninja=false --set rust.verbose-tests=true
21:00:53 + ./x.py test --stage=1 --target=x86_64-fortanix-unknown-sgx library/std --host= --no-doc --exclude src/tools/linkchecker
21:00:53 Building rustbuild
21:00:53 Finished dev [unoptimized] target(s) in 0.11s
21:00:53 Updating submodule src/tools/rls
21:00:54 Building stage0 std artifacts (x86_64-unknown-linux-gnu -> x86_64-unknown-linux-gnu)
21:00:54 Updating crates.io index
21:00:55 Updating git repository `[https://github.com/rust-lang/cargo`](https://github.com/rust-lang/cargo%60)
...
21:00:55 Updating git repository `[https://github.com/rust-lang/rust-clippy`](https://github.com/rust-lang/rust-clippy%60)
21:00:56 Updating git repository `[https://github.com/rust-lang/rustfmt`](https://github.com/rust-lang/rustfmt%60)
21:00:56 error: failed to select a version for `libgit2-sys`.
21:00:56 ... required by package `cargo v0.65.0 (https://github.com/rust-lang/cargo?rev=5514f1e0e1b3650ed8a78306198e90b66b292693#5514f1e0)`
21:00:56 ... which satisfies git dependency `cargo` of package `rls v1.41.0 (/home/jenkins/workspace/rust-sgx-ci/rust/src/tools/rls)`
21:00:56 versions that meet the requirements `^0.13.2` are: 0.13.4+1.4.2, 0.13.3+1.4.2, 0.13.2+1.4.2
21:00:56
21:00:56 the package `libgit2-sys` links to the native library `git2`, but it conflicts with a previous package which links to `git2` as well:
21:00:56 package `libgit2-sys v0.14.0+1.5.0`
21:00:56 ... which satisfies dependency `libgit2-sys = "^0.14.0"` of package `cargo v0.66.0 (/home/jenkins/workspace/rust-sgx-ci/rust/src/tools/cargo)`
21:00:56 Only one package in the dependency graph may specify the same links value. This helps ensure that only one copy of a native library is linked in the final binary. Try to adjust your dependencies so that only one package uses the links ='libgit2-sys' value. For more information, see https://doc.rust-lang.org/cargo/reference/resolver.html#links.
21:00:56
21:00:56 failed to select a version for `libgit2-sys` which could resolve this conflict
```
This is related to the version bump of `libgit2-sys` in #11004, but the root cause is the RLS is sunset (#100863). When the bootstrapper manages the git submodules, the wrong repo commit is checked out. This PR removes rls from the list of rust submodules.
Rollup of 5 pull requests
Successful merges:
- #99517 (Display raw pointer as *{mut,const} T instead of *-ptr in errors)
- #99928 (Do not leak type variables from opaque type relation)
- #100473 (Attempt to normalize `FnDef` signature in `InferCtxt::cmp`)
- #100653 (Move the cast_float_to_int fallback code to GCC)
- #100941 (Point at the string inside literal and mention if we need string inte…)
Failed merges:
r? `@ghost`
`@rustbot` modify labels: rollup
Move the cast_float_to_int fallback code to GCC
Now that we require at least LLVM 13, that codegen backend is always
using its intrinsic `fptosi.sat` and `fptoui.sat` conversions, so it
doesn't need the manual implementation. However, the GCC backend still
needs it, so we can move all of that code down there.
Attempt to normalize `FnDef` signature in `InferCtxt::cmp`
Stashes a normalization callback in `InferCtxt` so that the signature we get from `tcx.fn_sig(..).subst(..)` in `InferCtxt::cmp` can be properly normalized, since we cannot expect for it to have normalized types since it comes straight from astconv.
This is kind of a hack, but I will say that `@jyn514` found the fact that we present unnormalized types to be very confusing in real life code, and I agree with that feeling. Though altogether I am still a bit unsure about whether this PR is worth the effort, so I'm open to alternatives and/or just closing it outright.
On the other hand, this isn't a ridiculously heavy implementation anyways -- it's less than a hundred lines of changes, and half of that is just miscellaneous cleanup.
This is stacked onto #100471 which is basically unrelated, and it can be rebased off of that when that lands or if needed.
---
The code:
```rust
trait Foo { type Bar; }
impl<T> Foo for T {
type Bar = i32;
}
fn foo<T>(_: <T as Foo>::Bar) {}
fn needs_i32_ref_fn(f: fn(&'static i32)) {}
fn main() {
needs_i32_ref_fn(foo::<()>);
}
```
Before:
```
= note: expected fn pointer `fn(&'static i32)`
found fn item `fn(<() as Foo>::Bar) {foo::<()>}`
```
After:
```
= note: expected fn pointer `fn(&'static i32)`
found fn item `fn(i32) {foo::<()>}`
```
Do not leak type variables from opaque type relation
The "root cause" is that we call `InferCtxt::resolve_vars_if_possible` (3d9dd681f5) on the types we get back in `TypeError::Sorts` since I added a call to it in `InferCtxt::same_type_modulo_infer`. However if this `TypeError` comes from a `InferCtxt::commit_if_ok`, then it may reference type variables that do not exist anymore, which is problematic.
We avoid this by substituting the `TypeError` with the types we had before being generalized while handling opaques.
This is kinda gross, and I feel like we can get the same issue from other places where we generalize type/const inference variables. Maybe not? I don't know.
Fixes#99914Fixes#99970Fixes#100463
Display raw pointer as *{mut,const} T instead of *-ptr in errors
The `*-ptr` is rather confusing, and we have the full information for properly displaying the information.
Use object instead of LLVM for reading bitcode from rlibs
Together with changes I plan to make as part of https://github.com/rust-lang/rust/pull/97485 this will allow entirely removing usage of LLVM's archive reader and thus allow removing `archive_ro.rs` and `ArchiveWrapper.cpp`.
Rollup of 9 pull requests
Successful merges:
- #95376 (Add `vec::Drain{,Filter}::keep_rest`)
- #100092 (Fall back when relating two opaques by substs in MIR typeck)
- #101019 (Suggest returning closure as `impl Fn`)
- #101022 (Erase late bound regions before comparing types in `suggest_dereferences`)
- #101101 (interpret: make read-pointer-as-bytes a CTFE-only error with extra information)
- #101123 (Remove `register_attr` feature)
- #101175 (Don't --bless in pre-push hook)
- #101176 (rustdoc: remove unused CSS selectors for `.table-display`)
- #101180 (Add another MaybeUninit array test with const)
Failed merges:
r? `@ghost`
`@rustbot` modify labels: rollup
rustdoc: remove unused CSS selectors for `.table-display`
This class was added to support the function signature [src] lockup. That lockup was changed in 34bd2b845b to use flexbox instead, leaving these selectors unused.
Continuation of #101046
Don't --bless in pre-push hook
Running with --bless causes the push to succeed if there are fixable
formatting changes, but the changes don't make it into the push.
We should have the user rerun with --bless (or x.py fmt) and commit the
changes themselves (they might want to amend a particular commit, for
instance).
interpret: make read-pointer-as-bytes a CTFE-only error with extra information
Next step in the reaction to https://github.com/rust-lang/rust/issues/99923. Also teaches Miri to implicitly strip provenance in more situations when transmuting pointers to integers, which fixes https://github.com/rust-lang/miri/issues/2456.
Pointer-to-int transmutation during CTFE now produces a message like this:
```
= help: this code performed an operation that depends on the underlying bytes representing a pointer
= help: the absolute address of a pointer is not known at compile-time, so such operations are not supported
```
r? ``@oli-obk``
