[`identity_op`]: Fix duplicate diagnostics
Relates to #12379
In the `identity_op` lint, the following diagnostic was emitted two times
```
--> tests/ui/identity_op.rs:156:5
|
LL | 1 * 1;
| ^^^^^ help: consider reducing it to: `1`
|
```
because both of the left operand and the right operand are the identity element of the multiplication.
This PR fixes the issue so that if a diagnostic is created for an operand, the check of the other operand will be skipped. It's fine because the result is always the same in the affected operators.
---
changelog: [`identity_op`]: Fix duplicate diagnostics
Check for try blocks in `question_mark` more consistently
Fixes#12337
I split this PR up into two commits since this moves a method out of an `impl`, which makes for a pretty bad diff (the `&self` parameter is now unused, and there isn't a reason for that function to be part of the `impl` now).
The first commit is the actual relevant change and the 2nd commit just moves stuff (github's "hide whitespace" makes the diff easier to look at)
------------
Now for the actual issue:
`?` within `try {}` blocks desugars to a `break` to the block, rather than a `return`, so that changes behavior in those cases.
The lint has multiple patterns to look for and in *some* of them it already does correctly check whether we're in a try block, but this isn't done for all of its patterns.
We could add another `self.inside_try_block()` check to the function that looks for `let-else-return`, but I chose to actually just move those checks out and instead have them in `LintPass::check_{stmt,expr}`. This has the advantage that we can't (easily) accidentally forget to add that check in new patterns that might be added in the future.
(There's also a bit of a subtle interaction between two lints, where `question_mark`'s LintPass calls into `manual_let_else`, so I added a check to make sure we don't avoid linting for something that doesn't have anything to do with `?`)
changelog: [`question_mark`]: avoid linting on try blocks in more cases
fix [`derive_partial_eq_without_eq`] FP on trait projection
fixes: #9413#9319
---
changelog: fix [`derive_partial_eq_without_eq`] FP on trait projection
Well, this is awkward, it works but I don't understand why, why `clippy_utils::ty::implements_trait` couldn't detects the existance of `Eq` trait, even thought it's obviously present in the derive attribute.
Pointers cannot be converted to integers at compile time
Fix#12402
changelog: [`transmutes_expressible_as_ptr_casts`]: do not suggest invalid const casts
Dedup std_instead_of_core by using first segment span for uniqueness
Relates to #12379.
Instead of checking that the paths have an identical span, it checks that the relevant `std` part of the path segment's span is identical. Added a multiline test, because my first implementation was worse and failed that, then I realized that you could grab the span off the first_segment `Ident`.
I did find another bug that isn't addressed by this, and that exists on master as well.
The path:
```Rust
use std::{io::Write, fmt::Display};
```
Will get fixed into:
```Rust
use core::{io::Write, fmt::Display};
```
Which doesn't compile since `io::Write` isn't in `core`, if any of those paths are present in `core` it'll do the replace and cause a miscompilation. Do you think I should file a separate bug for that? Since `rustfmt` default splits those up it isn't that big of a deal.
Rustfmt:
```Rust
// Pre
use std::{io::Write, fmt::Display};
// Post
use std::fmt::Display;
use std::io::Write;
```
---
changelog: [`std_instead_of_core`]: Fix duplicated output on multiple imports
`f16` and `f128` step 2: intrinsics
Continuation of https://github.com/rust-lang/rust/pull/121728, another portion of https://github.com/rust-lang/rust/pull/114607.
This PR adds `f16` and `f128` intrinsics, and hooks them up to both HIR and LLVM. This is all still unexposed to the frontend, which will probably be the next step. Also update itanium mangling per `@rcvalle's` in https://github.com/rust-lang/rust/pull/121728/files#r1506570300, and fix a typo from step 1.
Once these types are usable in code, I will add the codegen tests from #114607 (codegen is passing on that branch)
This does add more `unimplemented!`s to Clippy, but I still don't think we can do better until library support is added.
r? `@compiler-errors`
cc `@Nilstrieb`
`@rustbot` label +T-compiler +F-f16_and_f128
Document manifest options
All built-in cargo commands have this output included when run with `cargo foo --help`. Clippy is the only exception, so I have copied the text here. As far as I can tell, the flags come from Cargo itself and not clippy, but the user almost certainly does not care about that.
```text
Manifest Options:
--manifest-path <PATH> Path to Cargo.toml
--frozen Require Cargo.lock and cache are up to date
--locked Require Cargo.lock is up to date
--offline Run without accessing the network
```
changelog: Add the standard Manifest Options section to the `--help` output
fix: `manual_memcpy` wrong indexing for multi dimensional arrays
fixes: #9334
This PR fixes an invalid suggestion for multi-dimensional arrays.
For example,
```rust
let src = vec![vec![0; 5]; 5];
let mut dst = vec![0; 5];
for i in 0..5 {
dst[i] = src[i][i];
}
```
For the above code, Clippy suggests `dst.copy_from_slice(&src[i]);`, but it is not compilable because `i` is only used to loop the array.
I adjusted it so that Clippy `manual_memcpy` works properly for multi-dimensional arrays.
changelog: [`manual_memcpy`]: Fixes invalid indexing suggestions for multi-dimensional arrays
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
Fix FP in `threadlocal!` when falling back to `os_local`
**Issue**: The lint always triggers for some targets.
**Why**: The lint assumes an `__init` function is not present for const initializers.
This does not hold for all targets. Some targets always have an initializer function.
**Fix**: If an initializer function exists, we check that it is not a `const` fn.
Lint overview before the patch:
1. Outside `threadlocal!`, then exit early.
2. In `threadlocal!` and `__init` does not exist => is const already, exit early.
3. In `threadlocal!` and `__init` exists and `__init` body can be `const` => we lint.
Lint overview after the patch:
1. Outside `threadlocal!`, then exit early.
2. In `threadlocal!` and `__init` does not exist => is const already, exit early.
3. In `threadlocal!` and `__init` exists and is `const fn` => is const already, exit early.
4. In `threadlocal!` and `__init` exists and is not `const fn` and `__init` body can be `const` => we lint.
The patch adds step 3.
changelog: Fixed fp in [`thread_local_initializer_can_be_made_const`] where fallback implementation of `threadlocal!` was always linted.
## Verifying the changes
For each of these platforms [ mac, x86_64-windows-gnu, x86_64-windows-msvc ]:
1. switched to master, ran bless. Found a failure for windows-gnu.
2. switched to patch, ran bless. Success for all three platforms.
## How platform affects the lint:
The compiler performs a [conditional compilation depending on platform features](https://doc.rust-lang.org/src/std/sys/common/thread_local/mod.rs.html). The `os_local` fallback includes a call to `__init`, without step 3, we lint in all cases.
```rust
cfg_if::cfg_if! {
if #[cfg(any(all(target_family = "wasm", not(target_feature = "atomics")), target_os = "uefi"))] {
#[doc(hidden)]
mod static_local;
#[doc(hidden)]
pub use static_local::{Key, thread_local_inner};
} else if #[cfg(target_thread_local)] {
#[doc(hidden)]
mod fast_local;
#[doc(hidden)]
pub use fast_local::{Key, thread_local_inner};
} else {
#[doc(hidden)]
mod os_local;
#[doc(hidden)]
pub use os_local::{Key, thread_local_inner};
}
}
```
r? `@llogiq`
`os_local` impl of `thread_local` — regardless of whether it is const and
unlike other implementations — includes an `fn __init(): EXPR`.
Existing implementation of the lint checked for the presence of said
function and whether the expr can be made const. Because for `os_local`
we always have an `__init()`, it triggers for const implementations.
The solution is to check whether the `__init()` function is already const.
If it is `const`, there is nothing to do. Otherwise, we verify that we can
make it const.
Co-authored-by: Alejandra González <blyxyas@gmail.com>
Bump macOS CI version to 13
I looked into the random failures we've been getting on macOS CI, not entirely sure what is going on but it seems to be an issue out of our department. A [plain driver](314425001d/src/driver.rs) being ran on the test files [without `ui_test` or any special flags](314425001d/src/driver.rs) hit the same issue
It didn't occur on `macos-13` the few times I tried it though so let's upgrade to that. [The current `macos-latest` refers to `macos-12`](https://github.com/actions/runner-images?tab=readme-ov-file#available-images), later this year it will refer to `macos-14` which runs on `aarch64` so specifying the version for our x64 macOS tests will also save a future headache
r? `@flip1995`
changelog: none
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`
Add new `mixed_attributes_style` lint
Add a new lint to detect cases where both inner and outer attributes are used on a same item.
r? `@llogiq`
----
changelog: Add new [`mixed_attributes_style`] lint
Alphabetize configuration options and lints in Clippy documentation
changelog: alphabetize configuration options and affected lints listed in section 3.1. of the Clippy Documentation
r? blyxyas
Stashed errors used to be counted as errors, but could then be
cancelled, leading to `ErrorGuaranteed` soundness holes. #120828 changed
that, closing the soundness hole. But it introduced other difficulties
because you sometimes have to account for pending stashed errors when
making decisions about whether errors have occured/will occur and it's
easy to overlook these.
This commit aims for a middle ground.
- Stashed errors (not warnings) are counted immediately as emitted
errors, avoiding the possibility of forgetting to consider them.
- The ability to cancel (or downgrade) stashed errors is eliminated, by
disallowing the use of `steal_diagnostic` with errors, and introducing
the more restrictive methods `try_steal_{modify,replace}_and_emit_err`
that can be used instead.
Other things:
- `DiagnosticBuilder::stash` and `DiagCtxt::stash_diagnostic` now both
return `Option<ErrorGuaranteed>`, which enables the removal of two
`delayed_bug` calls and one `Ty::new_error_with_message` call. This is
possible because we store error guarantees in
`DiagCtxt::stashed_diagnostics`.
- Storing the guarantees also saves us having to maintain a counter.
- Calls to the `stashed_err_count` method are no longer necessary
alongside calls to `has_errors`, which is a nice simplification, and
eliminates two more `span_delayed_bug` calls and one FIXME comment.
- Tests are added for three of the four fixed PRs mentioned below.
- `issue-121108.rs`'s output improved slightly, omitting a non-useful
error message.
Fixes#121451.
Fixes#121477.
Fixes#121504.
Fixes#121508.
Use `LitKind::Err` for malformed floats
#121120 changed `StringReader::cook_lexer_literal` to return `LitKind::Err` for malformed integer literals. This commit does the same for float literals, for consistency.
r? ``@fmease``
Diagnostic renaming
Renaming various diagnostic types from `Diagnostic*` to `Diag*`. Part of https://github.com/rust-lang/compiler-team/issues/722. There are more to do but this is enough for one PR.
r? `@davidtwco`