Simplify some nested `if` statements
Applies some but not all instances of `clippy::collapsible_if`. Some ended up looking worse afterwards, though, so I left those out. Also applies instances of `clippy::collapsible_else_if`
Review with whitespace disabled please.
Add an internal lint that warns when accessing untracked data
Some methods access data that is not tracked by the query system and should be used with caution. As suggested in https://github.com/rust-lang/rust/pull/128815#issuecomment-2275488683, in this PR I propose a lint (modeled on the `potential_query_instability` lint) that warns when using some specially-annotatted functions.
I can't tell myself if this lint would be that useful, compared to renaming `Steal::is_stolen` to `is_stolen_untracked`. This would depend on whether there are other functions we'd want to lint like this. So far it seems they're called `*_untracked`, which may be clear enough.
r? ``@oli-obk``
Don't Suggest Labeling `const` and `unsafe` Blocks
Fixes#128604
Previously, both anonymous constant blocks (E.g. The labeled block
inside `['_'; 'block: { break 'block 1 + 2; }]`) and inline const
blocks (E.g. `const { ... }`) were considered to be the same
kind of blocks. This caused the compiler to incorrectly suggest
labeling both the blocks when only anonymous constant blocks can be
labeled.
This PR adds an other enum variant to `Context` so that both the
blocks can be handled appropriately.
Also, adds some doc comments and removes unnecessary `&mut` in a
couple of places.
Implement a first version of RFC 3525: struct target features
This PR is an attempt at implementing https://github.com/rust-lang/rfcs/pull/3525, behind a feature gate `struct_target_features`.
There's obviously a few tasks that ought to be done before this is merged; in no particular order:
- add proper error messages
- add tests
- create a tracking issue for the RFC
- properly serialize/deserialize the new target_features field in `rmeta` (assuming I even understood that correctly :-))
That said, as I am definitely not a `rustc` expert, I'd like to get some early feedback on the overall approach before fixing those things (and perhaps some pointers for `rmeta`...), hence this early PR :-)
Here's an example piece of code that I have been using for testing - with the new code, the calls to intrinsics get correctly inlined:
```rust
#![feature(struct_target_features)]
use std::arch::x86_64::*;
/*
// fails to compile
#[target_feature(enable = "avx")]
struct Invalid(u32);
*/
#[target_feature(enable = "avx")]
struct Avx {}
#[target_feature(enable = "sse")]
struct Sse();
/*
// fails to compile
extern "C" fn bad_fun(_: Avx) {}
*/
/*
// fails to compile
#[inline(always)]
fn inline_fun(_: Avx) {}
*/
trait Simd {
fn do_something(&self);
}
impl Simd for Avx {
fn do_something(&self) {
unsafe {
println!("{:?}", _mm256_setzero_ps());
}
}
}
impl Simd for Sse {
fn do_something(&self) {
unsafe {
println!("{:?}", _mm_setzero_ps());
}
}
}
struct WithAvx {
#[allow(dead_code)]
avx: Avx,
}
impl Simd for WithAvx {
fn do_something(&self) {
unsafe {
println!("{:?}", _mm256_setzero_ps());
}
}
}
#[inline(never)]
fn dosomething<S: Simd>(simd: &S) {
simd.do_something();
}
fn main() {
/*
// fails to compile
Avx {};
*/
if is_x86_feature_detected!("avx") {
let avx = unsafe { Avx {} };
dosomething(&avx);
dosomething(&WithAvx { avx });
}
if is_x86_feature_detected!("sse") {
dosomething(&unsafe { Sse {} })
}
}
```
Tracking:
- https://github.com/rust-lang/rust/issues/129107
add repr to the allowlist for naked functions
Fixes#129412 (combining unstable features #90957 (`#![feature(naked_functions)]`) and #82232 (`#![feature(fn_align)]`)
rustdoc: clean up tuple <-> primitive conversion docs
This adds a minor missing feature to `fake_variadic`, so that it can render `impl From<(T,)> for [T; 1]` correctly.
Use cnum for extern crate data key
Noticed this when fixing #129184. I still have yet to put up a fix for that (mostly because I'm too lazy to minimize a test, that will come soon though).
Emit an error for invalid use of the linkage attribute
fixes#128486
Currently, the use of the linkage attribute for Mod, Impl,... is incorrectly permitted. This PR will correct this issue by generating errors, and I've also added some UI test cases for it.
Related: #128552.
Shrink `TyKind::FnPtr`.
By splitting the `FnSig` within `TyKind::FnPtr` into `FnSigTys` and `FnHeader`, which can be packed more efficiently. This reduces the size of the hot `TyKind` type from 32 bytes to 24 bytes on 64-bit platforms. This reduces peak memory usage by a few percent on some benchmarks. It also reduces cache misses and page faults similarly, though this doesn't translate to clear cycles or wall-time improvements on CI.
r? `@compiler-errors`
Use more slice patterns inside the compiler
Nothing super noteworthy. Just replacing the common 'fragile' pattern of "length check followed by indexing or unwrap" with slice patterns for legibility and 'robustness'.
r? ghost
By splitting the `FnSig` within `TyKind::FnPtr` into `FnSigTys` and
`FnHeader`, which can be packed more efficiently. This reduces the size
of the hot `TyKind` type from 32 bytes to 24 bytes on 64-bit platforms.
This reduces peak memory usage by a few percent on some benchmarks. It
also reduces cache misses and page faults similarly, though this doesn't
translate to clear cycles or wall-time improvements on CI.
Emit an error for invalid use of the `#[no_sanitize]` attribute
fixes#128487.
Currently, the use of the `#[no_sanitize]` attribute for Mod, Impl,... is incorrectly permitted. This PR will correct this issue by generating errors, and I've also added some UI test cases for it.
Referenced #128458. As far as I know, the `#[no_sanitize]` attribute can only be used with functions, so I changed that part to `Fn` and `Method` using `check_applied_to_fn_or_method`. However, I couldn't find explicit documentation on this, so I could be mistaken...
PR #128581 introduced an assertion that all builtin attributes are
actually checked via `CheckAttrVisitor` and aren't accidentally usable
on completely unrelated HIR nodes. Unfortunately, the check had
correctness problems.
The match on attribute path segments looked like
```rust,ignore
[sym::should_panic] => /* check is implemented */
match BUILTIN_ATTRIBUTE_MAP.get(name) {
// checked below
Some(BuiltinAttribute { type_: AttributeType::CrateLevel, .. }) => {}
Some(_) => {
if !name.as_str().starts_with("rustc_") {
span_bug!(
attr.span,
"builtin attribute {name:?} not handled by `CheckAttrVisitor`"
)
}
}
None => (),
}
```
However, it failed to account for edge cases such as an attribute whose:
1. path segments *starts* with a builtin attribute such as
`should_panic`
2. which does not start with `rustc_`, and
3. is also an `AttributeType::Normal` attribute upon registration with
the builtin attribute map
These conditions when all satisfied cause the span bug to be issued for e.g.
`#[should_panic::skip]` because the `[sym::should_panic]` arm is not matched (since it's
`[sym::should_panic, sym::skip]`).
See <https://github.com/rust-lang/rust/issues/128622>.
Assert that all attributes are actually checked via `CheckAttrVisitor` and aren't accidentally usable on completely unrelated HIR nodes
``@oli-obk's`` #128444 with unreachable case removed to avoid that PR bitrotting away.
Based on #128402.
This PR will make adding a new attribute ICE on any use of that attribute unless it gets a handler added in `rustc_passes::CheckAttrVisitor`.
r? ``@nnethercote`` (since you were the reviewer of the original PR)
Attribute checking simplifications
remove an unused boolean and then merge two big matches into one
I was reviewing some attributes and realized we don't really check this list against the list of builtin attributes, so we "may" totally be missing some attributes that we should be checking (like the `coroutine` attribute, which you can just apply to random stuff)
```rust
#![feature(coroutines)]
#[coroutine]
struct Foo;
```
just compiles for example. Unless we check that the fallthrough match arm is never reached for builtin attributes, we're just going to keep forgetting to add them here, too. I can do that without the changes in this PR, but it seemed like a nice cleanup
`#[naked]`: report incompatible attributes
tracking issue: https://github.com/rust-lang/rust/issues/90957
this is a re-implementation of https://github.com/rust-lang/rust/pull/93809 by ``@bstrie`` which was closed 2 years ago due to inactivity.
This PR takes some of the final comments into account, specifically providing a little more context in error messages, and using an allow list to determine which attributes are compatible with `#[naked]`.
Notable attributes that are incompatible with `#[naked]` are:
* `#[inline]`
* `#[track_caller]`
* ~~`#[target_feature]`~~ (this is now allowed, see PR discussion)
* `#[test]`, `#[ignore]`, `#[should_panic]`
These attributes just directly conflict with what `#[naked]` should do.
Naked functions are still important for systems programming, embedded, and operating systems, so I'd like to move them forward.
improve error message when `global_asm!` uses `asm!` options
specifically, what was
error: expected one of `)`, `att_syntax`, or `raw`, found `preserves_flags`
--> $DIR/bad-options.rs:45:25
|
LL | global_asm!("", options(preserves_flags));
| ^^^^^^^^^^^^^^^ expected one of `)`, `att_syntax`, or `raw`
is now
error: the `preserves_flags` option cannot be used with `global_asm!`
--> $DIR/bad-options.rs:45:25
|
LL | global_asm!("", options(preserves_flags));
| ^^^^^^^^^^^^^^^ the `preserves_flags` option is not meaningful for global-scoped inline assembly
mirroring the phrasing of the [reference](https://doc.rust-lang.org/reference/inline-assembly.html#options).
This is also a bit of a refactor for a future `naked_asm!` macro (for use in `#[naked]` functions). Currently this sort of error can come up when switching from inline to global asm, or when a user just isn't that experienced with assembly. With `naked_asm!` added to the mix hitting this error is more likely.
- merge error codes
- use attribute name that is incompatible in error message
- add test for conditional incompatible attribute
- add `linkage` to the allowlist
Extend rules of dead code analysis for impls for adts to impls for types refer to adts
The rules of dead code analysis for impl blocks can be extended to self types which refer to adts.
So that we can lint the following unused struct and trait:
```rust
struct Foo; //~ ERROR struct `Foo` is never constructed
trait Trait { //~ ERROR trait `Trait` is never used
fn foo(&self) {}
}
impl Trait for &Foo {}
```
r? `@pnkfelix`
`#[naked]`: use an allowlist for allowed options on `asm!` in naked functions
tracking issue: https://github.com/rust-lang/rust/issues/90957
this is mostly just a refactor, but using an allowlist (rather than a denylist) for which asm options are allowed in naked functions is a little safer.
These options are disallowed because naked functions are effectively global asm, but defined using inline asm.
Improve dead code analysis
Fixes#120770
1. check impl items later if self ty is private although the trait method is public, cause we must use the ty firstly if it's private
2. mark the adt live if it appears in pattern, like generic argument, this implies the use of the adt
3. based on the above, we can handle the case that private adts impl Default, so that we don't need adding rustc_trivial_field_reads on Default, and the logic in should_ignore_item
r? ``@pnkfelix``
Miri function identity hack: account for possible inlining
Having a non-lifetime generic is not the only reason a function can be duplicated. Another possibility is that the function may be eligible for cross-crate inlining. So also take into account the inlining attribute in this Miri hack for function pointer identity.
That said, `cross_crate_inlinable` will still sometimes return true even for `inline(never)` functions:
- when they are `DefKind::Ctor(..) | DefKind::Closure` -- I assume those cannot be `InlineAttr::Never` anyway?
- when `cross_crate_inline_threshold == InliningThreshold::Always`
so maybe this is still not quite the right criterion to use for function pointer identity.
Re-implement a type-size based limit
r? lcnr
This PR reintroduces the type length limit added in #37789, which was accidentally made practically useless by the caching changes to `Ty::walk` in #72412, which caused the `walk` function to no longer walk over identical elements.
Hitting this length limit is not fatal unless we are in codegen -- so it shouldn't affect passes like the mir inliner which creates potentially very large types (which we observed, for example, when the new trait solver compiles `itertools` in `--release` mode).
This also increases the type length limit from `1048576 == 2 ** 20` to `2 ** 24`, which covers all of the code that can be reached with craterbot-check. Individual crates can increase the length limit further if desired.
Perf regression is mild and I think we should accept it -- reinstating this limit is important for the new trait solver and to make sure we don't accidentally hit more type-size related regressions in the future.
Fixes#125460
Show `used attribute`'s kind for user when find it isn't applied to a `static` variable.
For example :
```rust
extern "C" {
#[used] //~ ERROR attribute must be applied to a `static` variable
static FOO: i32; // show the kind of this item to help user understand why the error is reported.
}
```
fixes#126789
coverage: Make `#[coverage(..)]` apply recursively to nested functions
This PR makes the (currently-unstable) `#[coverage(off)]` and `#[coverage(on)]` attributes apply recursively to all nested functions/closures, instead of just the function they are directly attached to.
Those attributes can now also be applied to modules and to impl/impl-trait blocks, where they have no direct effect, but will be inherited by all enclosed functions/closures/methods that don't override the inherited value.
---
Fixes#126625.
miri: make sure we can find link_section statics even for the local crate
Miri needs some way to iterate all the exported functions and "used" statics of all crates. For dependency crates, this already works fine since we can overwrite the query resonsible for computing `exported_symbols`, but it turns out for local binary crates this does not work: for binaries, `reachable_set` skips a lot of its logic and only checks `contains_extern_indicator()` and `RUSTC_STD_INTERNAL_SYMBOL`. Other flags like `CodegenFnAttrFlags::USED` are entirely ignored.
This PR proposes to use the same check, `has_custom_linkage`, in binaries that we already use to drive the main workqueue of the reachability recursive traversal. I have no idea why binaries used a slightly different check that ignores `USED` -- was that deliberate or does it just not matter most of the time?
Rollup of 7 pull requests
Successful merges:
- #126618 (Mark assoc tys live only if the corresponding trait is live)
- #126746 (Deny `use<>` for RPITITs)
- #126868 (not use offset when there is not ends with brace)
- #126884 (Do not ICE when suggesting dereferencing closure arg)
- #126893 (Eliminate the distinction between PREC_POSTFIX and PREC_PAREN precedence level)
- #126915 (Don't suggest awaiting in closure patterns)
- #126943 (De-duplicate all consecutive native libs regardless of their options)
r? `@ghost`
`@rustbot` modify labels: rollup
Detect unused structs which derived Default
<!--
If this PR is related to an unstable feature or an otherwise tracked effort,
please link to the relevant tracking issue here. If you don't know of a related
tracking issue or there are none, feel free to ignore this.
This PR will get automatically assigned to a reviewer. In case you would like
a specific user to review your work, you can assign it to them by using
r? <reviewer name>
-->
Fixes#98871
coverage: Overhaul validation of the `#[coverage(..)]` attribute
This PR makes sweeping changes to how the (currently-unstable) coverage attribute is validated:
- Multiple coverage attributes on the same item/expression are now treated as an error.
- The attribute must always be `#[coverage(off)]` or `#[coverage(on)]`, and the error messages for this are more consistent.
- A trailing comma is still allowed after off/on, since that's part of the normal attribute syntax.
- Some places that silently ignored a coverage attribute now produce an error instead.
- These cases were all clearly bugs.
- Some places that ignored a coverage attribute (with a warning) now produce an error instead.
- These were originally added as lints, but I don't think it makes much sense to knowingly allow new attributes to be used in meaningless places.
- Some of these errors might soon disappear, if it's easy to extend recursive coverage attributes to things like modules and impl blocks.
---
One of the goals of this PR is to lay a more solid foundation for making the coverage attribute recursive, so that it applies to all nested functions/closures instead of just the one it is directly attached to.
Fixes#126658.
This PR incorporates #126659, which adds more tests for validation of the coverage attribute.
`@rustbot` label +A-code-coverage
Show notice about "never used" of Debug for enum
Close#123068
If an ADT implements `Debug` trait and it is not used, the compiler says a note that indicates intentionally ignored during dead code analysis as [this note](2207179a59/tests/ui/lint/dead-code/unused-variant.stderr (L9)).
However this node is not shown for variants that have fields in enum. This PR fixes to show the note.
