lint/ctypes: fix `()` return type checks
Fixes#113436.
`()` is normally FFI-unsafe, but is FFI-safe when used as a return type. It is also desirable that a transparent newtype for `()` is FFI-safe when used as a return type.
In order to support this, when a type was deemed FFI-unsafe, because of a `()` type, and was used in return type - then the type was considered FFI-safe. However, this was the wrong approach - it didn't check that the `()` was part of a transparent newtype! The consequence of this is that the presence of a `()` type in a more complex return type would make it the entire type be considered safe (as long as the `()` type was the first that the lint found) - which is obviously incorrect.
Instead, this logic is removed, and after [consultation with t-lang](https://github.com/rust-lang/rust/issues/113436#issuecomment-1640756721), I've fixed the bugs and inconsistencies and made `()` FFI-safe within types.
I also refactor a function, but that's not too exciting.
The warning can be reproduced with 1.63 but not with 1.64.
$ rustc +1.63 tests/ui/lint/unused/const-local-var.rs
warning: constant `F` is never used
--> tests/ui/lint/unused/const-local-var.rs:14:9
|
14 | const F: i32 = 2;
| ^^^^^^^^^^^^^^^^^
|
= note: `#[warn(dead_code)]` on by default
$ rustc +1.64 tests/ui/lint/unused/const-local-var.rs
Add a regression test to prevent the problem from re-appearing.
Now that this lint runs on any external-ABI fn-ptr, normalization won't
always succeed, so use `try_normalize_erasing_regions` instead.
Signed-off-by: David Wood <david@davidtw.co>
Consider `()` within types to be FFI-safe, and `()` to be FFI-safe as a
return type (incl. when in a transparent newtype).
Signed-off-by: David Wood <david@davidtw.co>
`()` is normally FFI-unsafe, but is FFI-safe when used as a return type.
It is also desirable that a transparent newtype for `()` is FFI-safe when
used as a return type.
In order to support this, when an type was deemed FFI-unsafe, because of
a `()` type, and was used in return type - then the type was considered
FFI-safe. However, this was the wrong approach - it didn't check that the
`()` was part of a transparent newtype! The consequence of this is that
the presence of a `()` type in a more complex return type would make it
the entire type be considered safe (as long as the `()` type was the
first that the lint found) - which is obviously incorrect.
Instead, this logic is removed, and a unit return type or a transparent
wrapper around a unit is checked for directly for functions and fn-ptrs.
Signed-off-by: David Wood <david@davidtw.co>
(re-)tighten sourceinfo span of adjustments in MIR
Diagnostics rely on the spans of MIR statements being (approximately) correct in order to give suggestions relative to that span (i.e. `shrink_to_hi` and `shrink_to_lo`).
I discovered that we're *intentionally* lowering THIR exprs with their parent expr's span if they come from adjustments that are due to a parent expression. While I understand why that may be desirable to demonstrate the relationship of an adjustment and the expression that requires it, it leads to
1. very verbose borrowck output
2. incorrect spans for suggestions
Some diagnostics get around that by giving suggestions relative to other spans we've collected during MIR lowering, such as the span of the method's identifier (e.g. `name` in `.name()`), but this doesn't work too well when things come from desugaring.
I assume it also has lead to numerous tweaks and complications to diagnostics code down the road, which this PR doesn't necessarily aim to fix but may open the gates to fixing later... The last three commits are simplifications due to the fact that we can assume that the move span actually points to what is being moved (and a test).
This regressed in #89110, which was debated somewhat in #90286. cc `@Aaron1011` who originally made this change.
r? diagnostics
Fixes#113547Fixes#111016
Uplift `clippy::fn_null_check` lint
This PR aims at uplifting the `clippy::fn_null_check` lint into rustc.
## `incorrect_fn_null_checks`
(warn-by-default)
The `incorrect_fn_null_checks` lint checks for expression that checks if a function pointer is null.
### Example
```rust
let fn_ptr: fn() = /* somehow obtained nullable function pointer */
if (fn_ptr as *const ()).is_null() { /* ... */ }
```
### Explanation
Function pointers are assumed to be non-null, checking for their nullity is incorrect.
-----
Mostly followed the instructions for uplifting a clippy lint described here: https://github.com/rust-lang/rust/pull/99696#pullrequestreview-1134072751
`@rustbot` label: +I-lang-nominated
r? compiler
Extend previous checks for external ABI fn-ptrs to use in internal
statics, constants, type aliases and algebraic data types.
Signed-off-by: David Wood <david.wood@huawei.com>
Extend previous commit's support for checking for external fn-ptrs in
internal fn types to report errors for multiple found fn-ptrs.
Signed-off-by: David Wood <david.wood@huawei.com>
Instead of skipping functions with internal ABIs, check that the
signature doesn't contain any fn-ptr types with external ABIs that
aren't FFI-safe.
Signed-off-by: David Wood <david.wood@huawei.com>
Extend `unused_must_use` to cover block exprs
Given code like
```rust
#[must_use]
fn foo() -> i32 {
42
}
fn warns() {
{
foo();
}
}
fn does_not_warn() {
{
foo()
};
}
fn main() {
warns();
does_not_warn();
}
```
### Before This PR
```
warning: unused return value of `foo` that must be used
--> test.rs:8:9
|
8 | foo();
| ^^^^^
|
= note: `#[warn(unused_must_use)]` on by default
help: use `let _ = ...` to ignore the resulting value
|
8 | let _ = foo();
| +++++++
warning: 1 warning emitted
```
### After This PR
```
warning: unused return value of `foo` that must be used
--> test.rs:8:9
|
8 | foo();
| ^^^^^
|
= note: `#[warn(unused_must_use)]` on by default
help: use `let _ = ...` to ignore the resulting value
|
8 | let _ = foo();
| +++++++
warning: unused return value of `foo` that must be used
--> test.rs:14:9
|
14 | foo()
| ^^^^^
|
help: use `let _ = ...` to ignore the resulting value
|
14 | let _ = foo();
| +++++++ +
warning: 2 warnings emitted
```
Fixes#104253.
- Either explicitly annotate `let x: () = expr;` where `x` has unit
type, or remove the unit binding to leave only `expr;` instead.
- Fix disjoint-capture-in-same-closure test
Uplift `clippy::undropped_manually_drops` lint
This PR aims at uplifting the `clippy::undropped_manually_drops` lint.
## `undropped_manually_drops`
(warn-by-default)
The `undropped_manually_drops` lint check for calls to `std::mem::drop` with a value of `std::mem::ManuallyDrop` which doesn't drop.
### Example
```rust
struct S;
drop(std::mem::ManuallyDrop::new(S));
```
### Explanation
`ManuallyDrop` does not drop it's inner value so calling `std::mem::drop` will not drop the inner value of the `ManuallyDrop` either.
-----
Mostly followed the instructions for uplifting an clippy lint described here: https://github.com/rust-lang/rust/pull/99696#pullrequestreview-1134072751
`@rustbot` label: +I-lang-nominated
r? compiler
-----
For Clippy:
changelog: Moves: Uplifted `clippy::undropped_manually_drops` into rustc
Uplift `clippy::invalid_utf8_in_unchecked` lint
This PR aims at uplifting the `clippy::invalid_utf8_in_unchecked` lint into two lints.
## `invalid_from_utf8_unchecked`
(deny-by-default)
The `invalid_from_utf8_unchecked` lint checks for calls to `std::str::from_utf8_unchecked` and `std::str::from_utf8_unchecked_mut` with an invalid UTF-8 literal.
### Example
```rust
unsafe {
std::str::from_utf8_unchecked(b"cl\x82ippy");
}
```
### Explanation
Creating such a `str` would result in undefined behavior as per documentation for `std::str::from_utf8_unchecked` and `std::str::from_utf8_unchecked_mut`.
## `invalid_from_utf8`
(warn-by-default)
The `invalid_from_utf8` lint checks for calls to `std::str::from_utf8` and `std::str::from_utf8_mut` with an invalid UTF-8 literal.
### Example
```rust
std::str::from_utf8(b"ru\x82st");
```
### Explanation
Trying to create such a `str` would always return an error as per documentation for `std::str::from_utf8` and `std::str::from_utf8_mut`.
-----
Mostly followed the instructions for uplifting a clippy lint described here: https://github.com/rust-lang/rust/pull/99696#pullrequestreview-1134072751
````@rustbot```` label: +I-lang-nominated
r? compiler
-----
For Clippy:
changelog: Moves: Uplifted `clippy::invalid_utf8_in_unchecked` into rustc
Consider lint check attributes on match arms
Currently, lint check attributes on match arms have no effect for some lints. This PR makes some lint passes to take those attributes into account.
- `LateContextAndPass` for late lint doesn't update `last_node_with_lint_attrs` when it visits match arms. This leads to lint check attributes on match arms taking no effects on late lints that operate on the arms' pattern:
```rust
match value {
#[deny(non_snake_case)]
PAT => {} // `non_snake_case` only warned due to default lint level
}
```
To be honest, I'm not sure whether this is intentional or just an oversight. I've dug the implementation history and searched up issues/PRs but couldn't find any discussion on this.
- `MatchVisitor` doesn't update its lint level when it visits match arms. This leads to check lint attributes on match arms taking no effect on some lints handled by this visitor, namely: `bindings_with_variant_name` and `irrefutable_let_patterns`.
This seems to be a fallout from #108504. Before 05082f57af, when the visitor operated on HIR rather than THIR, check lint attributes for the said lints were effective. [This playground][play] compiles successfully on current stable (1.69) but fails on current beta and nightly.
I wasn't sure where best to place the test for this. Let me know if there's a better place.
[play]: https://play.rust-lang.org/?version=stable&mode=debug&edition=2021&gist=38432b79e535cb175f8f7d6d236d29c3
[play-match]: https://play.rust-lang.org/?version=beta&mode=debug&edition=2021&gist=629aa71b7c84b269beadeba664e2221d
Dont check `must_use` on nested `impl Future` from fn
Fixes (but does not close, per beta policy) #111484
Also fixes a `FIXME` left in the code about (presumably) false-positives on non-async `#[must_use] fn() -> impl Future` cases, though if that's not desirable to include in the beta backport then I can certainly revert it.
Beta nominating as it fixes a beta ICE.
Rename InstCombine to InstSimplify
```
╭ ➜ ben@archlinux:~/rust
╰ ➤ rg -i instcombine
src/doc/rustc-dev-guide/src/mir/optimizations.md
134:may have been misapplied. Examples of this are `InstCombine` and `ConstantPropagation`.
src/ci/docker/host-x86_64/disabled/dist-x86_64-haiku/llvm-config.sh
38: instcombine instrumentation interpreter ipo irreader lanai \
tests/codegen/slice_as_from_ptr_range.rs
4:// min-llvm-version: 15.0 (because this is a relatively new instcombine)
```
r? `@scottmcm`
added TraitAlias to check_item() for missing_docs
As in issue #111025 the `missing_docs` was not being triggered for trait aliases. I added `TraitAlias` to the pattern match for check_item(), and the lint seems to be behaving appropriately
Tweak await span to not contain dot
Fixes a discrepancy between method calls and await expressions where the latter are desugared to have a span that *contains* the dot (i.e. `.await`) but method call identifiers don't contain the dot. This leads to weird suggestions suggestions in borrowck -- see linked issue.
Fixes#110761
This mostly touches a bunch of tests to tighten their `await` span.
Run various queries from other queries instead of explicitly in phases
These are just legacy leftovers from when rustc didn't have a query system. While there are more cleanups of this sort that can be done here, I want to land them in smaller steps.
This phased order of query invocations was already a lie, as any query that looks at types (e.g. the wf checks run before) can invoke e.g. const eval which invokes borrowck, which invokes typeck, ...
This test was ignored long ago in
https://github.com/rust-lang/rust/pull/20578/ when the syntax for
closures was changed.
The current status is that a closure with an explicit `!` return type
will trigger the `unreachable_code` lint which appears to be the
original intent of the test
(https://github.com/rust-lang/rust/pull/16836). A closure without a
return type won't trigger the lint since the `!` type isn't inferred
(AFAIK). This restores the test to its original form.
Rollup of 8 pull requests
Successful merges:
- #110153 (Fix typos in compiler)
- #110165 (rustdoc: use CSS `overscroll-behavior` instead of JavaScript)
- #110175 (Symbol cleanups)
- #110203 (Remove `..` from return type notation)
- #110205 (rustdoc: make settings radio and checks thicker, less contrast)
- #110222 (Improve the error message when forwarding a matched fragment to another macro)
- #110237 (Split out a separate feature gate for impl trait in associated types)
- #110241 (tidy: Issue an error when UI test limits are too high)
Failed merges:
- #110218 (Remove `ToRegionVid`)
r? `@ghost`
`@rustbot` modify labels: rollup
Add `useless_anonymous_reexport` lint
This is a follow-up of https://github.com/rust-lang/rust/pull/108936. We once again show all anonymous re-exports in rustdoc, however we also wanted to add a lint to let users know that it very likely doesn't have the effect they think it has.
Make `unused_allocation` lint against `Box::new` too
Previously it only linted against `box` syntax, which likely won't ever be stabilized, which is pretty useless. Even now I'm not sure if it's a meaningful lint, but it's at least something 🤷
This means that code like the following will be linted against:
```rust
Box::new([1, 2, 3]).len();
f(&Box::new(1)); // where f : &i32 -> ()
```
The lint works by checking if a `Box::new` (or `box`) expression has an a borrow adjustment, meaning that the code that first stores the box in a variable won't be linted against:
```rust
let boxed = Box::new([1, 2, 3]); // no lint
boxed.len();
```
lint: don't suggest MaybeUninit::assume_init for uninhabited types
Creating a zeroed uninhabited type such as `!` or an empty enum with `mem::zeroed()` (or transmuting `()` to `!`) currently triggers this lint:
```rs
warning: the type `!` does not permit zero-initialization
--> test.rs:5:23
|
5 | let _val: ! = mem::zeroed();
| ^^^^^^^^^^^^^
| |
| this code causes undefined behavior when executed
| help: use `MaybeUninit<T>` instead, and only call `assume_init` after initialization is done
|
= note: the `!` type has no valid value
```
The `MaybeUninit` suggestion in the help message seems confusing/useless for uninhabited types, as such a type cannot be fully initialized in the first place (as the note implies).
This PR limits this help message to inhabited types which can be initialized
Emit warnings on unused parens in index expressions
Fixes: #96606.
I am not sure what the best term for "index expression" is. Is there a better term we could use?
