`tcx.def_kind()` could theoretically invoke another query, which could
cause an infinite query loop. Accessing the HIR map directly makes that
less likely to happen.
I also changed it to use `as_local()` (`tcx.def_kind()` seems to
implicitly call `expect_local()`) and `opt_def_kind()` to reduce the
chance of panicking on valid code.
rustc_target: `TyAndLayout::field` should never error.
This refactor (making `TyAndLayout::field` return `TyAndLayout` without any `Result` around it) is based on a simple observation, regarding `TyAndLayout::field`:
If `cx.layout_of(ty)` succeeds (for some `cx` and `ty`), then `.field(cx, i)` on the resulting `TyAndLayout` should *always* succeed in computing `cx.layout_of(field_ty)` (where `field_ty` is the type of the `i`th field of `ty`).
The reason for this is that no matter which field is chosen, `cx.layout_of(field_ty)` *will have already been computed*, as part of computing `cx.layout_of(ty)`, as we cannot determine the layout of *any* type without considering the layouts of *all* of its fields.
And so it should be fine to turn any errors into ICEs, since they likely indicate a `cx` mismatch, or some other edge case that is due to a compiler bug (as opposed to ever being an user-facing error).
<hr/>
Each commit should probably be reviewed separately, though note that there's some `where` clauses (in `rustc_target::abi::call::*`) that change in most commits.
cc `@nagisa` `@oli-obk`
Handle match statements with non exhaustive variants in closures
This PR ensures that the behavior for match statements with non exhaustive variants is the same inside and outside closures.
If we have a non-exhaustive SingleVariant which is defined in a different crate, then we should handle the case the same way we would handle a MultiVariant: borrow the match discriminant.
Closes https://github.com/rust-lang/project-rfc-2229/issues/59
r? `@nikomatsakis`
Remove `Session.if_let_suggestions`
We can instead if either the LHS or RHS types contain
`TyKind::Error`. In addition to covering the case where
we would have previously updated `if_let_suggestions`, this might
also prevent redundant errors in other cases as well.
Suggestion for call on immutable binding of mutable type
When calling a method requiring a mutable self borrow on an inmutable
to a mutable borrow of the type, suggest making the binding mutable.
Fix#83241.
ast_lowering: Introduce `lower_span` for catching all spans entering HIR
This PR cherry-picks the `fn lower_span` change from https://github.com/rust-lang/rust/pull/84373.
I also introduced `fn lower_ident` for lowering spans in identifiers, and audited places where HIR structures with spans or identifiers are constructed and added a few missing `lower_span`s/`lower_ident`s.
Having a hook for spans entering HIR can be useful for things other than https://github.com/rust-lang/rust/pull/84373, e.g. https://github.com/rust-lang/rust/issues/35148.
I also want to check whether this change causes perf regressions due to some accidental inlining issues.
r? `@cjgillot`
Make `-Z gcc-ld=lld` work for Apple targets
`-Z gcc-ld=lld` was introduced in #85961. It does not work on Macos because lld needs be either named `ld64` or passed `-flavor darwin` as the first two arguments in order to select the Mach-O flavor. Rust invokes cc (=clang) on Macos for linking which calls `ld` as linker binary and not `ld64`, so just creating an `ld64` binary and modifying the search path with `-B` does not work.
In order to solve this patch does:
* Set the `lld_flavor` for all Apple-derived targets to `LldFlavor::Ld64`. As far as I can see this actually works towards fixing `-Xlinker=rust-lld` as all those targets use the Mach-O object format.
* Copy/hardlink rust-lld to the gcc-ld subdirectory as ld64 next to ld.
* If `-Z gcc-ld=lld` is used and the target lld flavor is Ld64 add `-fuse-ld=/path/to/ld64` to the linker invocation.
Fixes#86945.
Old error output:
3 | let _: usize = $f;
| ----- ^ expected `usize`, found struct `Baz`
| |
| expected due to this
New error output:
3 | let _: usize = $f;
| ----- ^^ expected `usize`, found struct `Baz`
| |
| expected due to this
Old error output:
= note: this warning originates in the macro `foo` (in Nightly builds, run with -Z macro-backtrace for more info)
help: wrap this expression in parentheses
|
4 | break '_l $f(;)
| ^ ^
New error output:
= note: this warning originates in the macro `foo` (in Nightly builds, run with -Z macro-backtrace for more info)
help: wrap this expression in parentheses
|
4 | break '_l ($f);
| ^ ^
Macros 2.0-ify rustc_arena
For the purpose of battle-testing macros 2.0 I'm looking to dogfood it in rustc, one crate at a time.
(Note that there are only 12 changed lines if you ignore whitespace.)
Before:
help: consider adding semicolon after the expression so its temporaries are dropped sooner, before the local variables declared by the block are dropped
|
LL | for x in DroppingSlice(&*v).iter(); {
| +
After:
help: consider adding semicolon after the expression so its temporaries are dropped sooner, before the local variables declared by the block are dropped
|
LL | };
| +
This seems like a reasonable fix: since the desugared "expr_drop_temps_mut"
contains the entire desugared loop construct, its span should contain the
entire loop construct as well.
Add missing const edge case
We don't "process" const so we need to check for additional cases when the PatKind is a Path. We need to make sure that if there is only one variant that there is no field. If there is one or more field, we will want to borrow the match scrutinee
Closes https://github.com/rust-lang/rust/issues/88331
r? `@nikomatsakis`
Treat macros as HIR items
Macros have historically been treated differently from other items at the HIR level. This PR makes them just like normal items. There are a few special cases left over, which I've attempted to lay out below. By normalizing the treatment of macro items, this PR simplifies a fair bit of code and fixes some bugs at the same time. For more information, see #87406.
r? `@cjgillot`
## Backwards incompatibility
This is backwards incompatible in one small way. Due to a mistake, it was previously possible to apply stability attributes to an exported macro, even without enabling the `staged_api` feature. This never should have worked. After this PR, it will error, as it should. We could make it a warning instead, but that would require a special case for a feature that shouldn't ever have worked and is likely used by no or very few crates, so I'm not thrilled about the idea.
## Notes for reviewers
### Commit seperation
I'd recommend reviewing this PR commit by commit. The commit chunking wasn't perfect, but it's better than looking at the combined diff, which is quite overwhelming. The compiler and standard library build after each commit, although tests do not necessarily pass and tools do not necessarily build till the end of the series.
### Special cases
There are a few special cases that remain after this change. Here are the notable ones I remember:
1. Visibility works a bit differently for `macro_rules!` macros than other items, since they aren't generally marked with `pub` but instead with `#[macro_export]`.
2. Since `#[macro_export]` macros always have paths at the top level of the crate, some additional handling needs to be done on the reexport to top level.
### Performance impact
I don't know for sure, but theses changes may slightly hurt performance. They create more work for the compiler in a few places. For instance, some operations that were previously run only for exported macros are now run for all macros. A perf run is probably advisable. For all I know we'll see performance improvements instead. :)
## Issues resolved
This resolves#87406 (the tracking issue for this change). It also fixes several bugs:
Fixes#59306.
Fixes#73754.
Fixes#87257.
S390x inline asm
This adds register definitions and constraint codes for the s390x general and floating point registers necessary for fixing #85931; as well as a few tests.
Further testing is needed, but I am a little unsure of what specific tests should be added to `src/test/assembly/asm/s390x.rs` to address this.
Previously, the result of `predicates_of` for a foreign trait
would depend on the *current* state of the corresponding source
file in the foreign crate. This could lead to ICEs during incremental
compilation, since the on-disk contents of the upstream source file
could potentially change without the upstream crate being recompiled.
Additionally, this ensure that that the metadata we produce for a crate
only depends on its *compiled* upstream dependencies (e.g an rlib or
rmeta file), *not* the current on-disk state of the upstream crate
source files.
We can instead if either the LHS or RHS types contain
`TyKind::Error`. In addition to covering the case where
we would have previously updated `if_let_suggestions`, this might
also prevent redundant errors in other cases as well.
Introduce `~const`
- [x] Removed `?const` and change uses of `?const`
- [x] Added `~const` to the AST. It is gated behind const_trait_impl.
- [x] Validate `~const` in ast_validation.
- [x] Update UI Tests
- [x] Add enum `BoundConstness` (With variants `NotConst` and
`ConstIfConst` allowing future extensions)
- [x] Adjust trait selection and pre-existing code to use `BoundConstness`.
- [ ] Optional steps for this PR
- [x] Fix#88155
- [x] ~~Do something with constness bounds in chalk~~ Must be done to rust-lang/chalk (just tried to refactor, there are a lot of errors to resolve :( )
- [ ] Adjust Error messages for `~const` bounds that can't be satisfied.
r? `@oli-obk`
Both the place where the parser encounters a needed closed delimiter and
the unclosed opening delimiter are important, so they should get the
same level of highlighting in the output.
Adjust linking order of static nobundle libraries
Link the static libraries with "-bundle" modifier from upstream rust crate right after linking this rust crate.
Some linker such as GNU linker `ld.bdf` treat order of linking as order of dependency.
After this change, static libraries with "-bundle" modifier is linked in the same order as "+bundle" modifier.
So we can change the value of "bundle" modifier without causing linking error.
fix: https://github.com/rust-lang/rust/issues/87541
r? `@petrochenkov`
`#[inline]` non-generic `pub fn`s in `rustc_target::abi` and `ty::layout`.
Mostly doing this as a perf curiosity, having spotted that `#[inline]` usage is a bit spotty.
- [x] Removed `?const` and change uses of `?const`
- [x] Added `~const` to the AST. It is gated behind const_trait_impl.
- [x] Validate `~const` in ast_validation.
- [ ] Add enum `BoundConstness` to the HIR. (With variants `NotConst` and
`ConstIfConst` allowing future extensions)
- [ ] Adjust trait selection and pre-existing code to use `BoundConstness`.
- [ ] Optional steps (*for this PR, obviously*)
- [ ] Fix#88155
- [ ] Do something with constness bounds in chalk
lazily "compute" anon const default substs
Continuing the work of #83086, this implements the discussed solution for the [unused substs problem](https://github.com/rust-lang/project-const-generics/blob/master/design-docs/anon-const-substs.md#unused-substs). As of now, anonymous constants inherit all of their parents generics, even if they do not use them, e.g. in `fn foo<T, const N: usize>() -> [T; N + 1]`, the array length has `T` as a generic parameter even though it doesn't use it. These *unused substs* cause some backwards incompatible, and imo incorrect behavior, e.g. #78369.
---
We do not actually filter any generic parameters here and the `default_anon_const_substs` query still a dummy which only checks that
- we now prevent the previously existing query cycles and are able to call `predicates_of(parent)` when computing the substs of anonymous constants
- the default anon consts substs only include the typeflags we assume it does.
Implementing that filtering will be left as future work.
---
The idea of this PR is to delay the creation of the anon const substs until after we've computed `predicates_of` for the parent of the anon const. As the predicates of the parent can however contain the anon const we still have to create a `ty::Const` for it.
We do this by changing the substs field of `ty::Unevaluated` to an option and modifying accesses to instead call the method `unevaluated.substs(tcx)` which returns the substs as before. If the substs - now `substs_` - of `ty::Unevaluated` are `None`, it means that the anon const currently has its default substs, i.e. the substs it has when first constructed, which are the generic parameters it has available. To be able to call `unevaluated.substs(tcx)` in a `TypeVisitor`, we add the non-defaulted method `fn tcx_for_anon_const_substs(&self) -> Option<TyCtxt<'tcx>>`. In case `tcx_for_anon_const_substs` returns `None`, unknown anon const default substs are skipped entirely.
Even when `substs_` is `None` we still have to treat the constant as if it has its default substs. To do this, `TypeFlags` are modified so that it is clear whether they can still change when *exposing* any anon const default substs. A new flag, `HAS_UNKNOWN_DEFAULT_CONST_SUBSTS`, is added in case some default flags are missing.
The rest of this PR are some smaller changes to either not cause cycles by trying to access the default anon const substs too early or to be able to access the `tcx` in previously unused locations.
cc `@rust-lang/project-const-generics`
r? `@nikomatsakis`
Fixes for LLVM change 0f45c16f2caa7c035e5c3edd40af9e0d51ad6ba7
More details in the individual commit messages, but the summary is: LLVM deleted an unused-to-them method that we used, we worked around it to avoid annoying cleanup/restructuring in the Rust-side code.
Fix debugger stepping behavior with `match` expressions
Previously, we would set up the source lines for `match` expressions so
that the code generated to perform the test of the scrutinee was matched
to the line of the arm that required the test and then jump from the arm
block to the "next" block was matched to all of the lines in the `match`
expression.
While that makes sense, it has the side effect of causing strange
stepping behavior in debuggers.
I've changed the source information so that all of the generated tests
are sourced to `match {scrutinee}` and the jumps are sourced to the last
line of the block they are inside. This resolves the weird stepping
behavior in all debuggers and resolves some instances of "ambiguous
symbol" errors in WinDbg preventing the user from setting breakpoints at
`match` expressions.
Before:
https://user-images.githubusercontent.com/831192/128577421-ee0c9c03-da28-4d16-997a-d57988a7bb7f.mp4
After:
https://user-images.githubusercontent.com/831192/128577433-2ceab04d-953e-4e31-9387-93f049c71ff3.mp4Fixes#87817
Morph `layout_raw` query into `layout_of`.
Before this PR, `LayoutCx::layout_of` wrapped the `layout_raw` query, to:
* normalize the type, before attempting to compute the layout
* pass the layout to `record_layout_for_printing`, for `-Zprint-type-sizes`
Moving those two responsibilities into the query may reduce overhead (due to cached calls skipping those steps), but I want to do a perf run to know.
One of the changes I had to make was changing the return type of the query, to be able to both get out the type produced by normalizing inside the query *and* to match the signature of the old `TyCtxt::layout_of`. This change may be worse, perf-wise, so that's another reason I want to check.
r? `@nagisa` cc `@oli-obk`
The above-mentioned commit (part of the LLVM 14 development cycle)
removes a method that rustc uses somewhat extensively. We mostly switch
to lower-level methods that exist in all versions of LLVM we use, so no
new ifdef logic is required in most cases.
Link the static libraries with "-bundle" modifier from upstream rust crate
right after linking this rust crate. Some linker such as GNU linker
`ld.bdf` treat order of linking as order of dependency. After this change,
static libraries with "-bundle" modifier is linked in the same order as
"+bundle" modifier. So we can change the value of "bundle" modifier without
causing linking error.
Use undef for uninitialized bytes in constants
Fixes#83657
This generates good code when the const is fully uninit, e.g.
```rust
#[no_mangle]
pub const fn fully_uninit() -> MaybeUninit<[u8; 10]> {
const M: MaybeUninit<[u8; 10]> = MaybeUninit::uninit();
M
}
```
generates
```asm
fully_uninit:
ret
```
as you would expect.
There is no improvement, however, when it's partially uninit, e.g.
```rust
pub struct PartiallyUninit {
x: u64,
y: MaybeUninit<[u8; 10]>
}
#[no_mangle]
pub const fn partially_uninit() -> PartiallyUninit {
const X: PartiallyUninit = PartiallyUninit { x: 0xdeadbeefcafe, y: MaybeUninit::uninit() };
X
}
```
generates
```asm
partially_uninit:
mov rax, rdi
mov rcx, qword ptr [rip + .L__unnamed_1+16]
mov qword ptr [rdi + 16], rcx
movups xmm0, xmmword ptr [rip + .L__unnamed_1]
movups xmmword ptr [rdi], xmm0
ret
.L__unnamed_1:
.asciz "\376\312\357\276\255\336\000"
.zero 16
.size .L__unnamed_1, 24
```
which copies a bunch of zeros in place of the undef bytes, the same as before this change.
Edit: generating partially-undef constants isn't viable at the moment anyways due to #84565, so it's disabled
Use if-let guards in the codebase and various other pattern cleanups
Dogfooding if-let guards as experimentation for the feature.
Tracking issue #51114. Conflicts with #87937.
* Highlight the whole pattern if it has no fields
* Highlight the whole definition if it has no fields
* Only highlight the pattern name if the pattern is multi-line
* Determine whether a pattern is multi-line based on distance from name
to last field, rather than first field
Normalize projections under binders
Fixes#70243Fixes#70120Fixes#62529Fixes#87219
Issues to followup on after (probably fixed, but no test added here):
#76956#56556#79207#85636
r? `@nikomatsakis`
Previously, we would set up the source lines for `match` expressions so
that the code generated to perform the test of the scrutinee was matched
to the line of the arm that required the test and then jump from the arm
block to the "next" block was matched to all of the lines in the `match`
expression.
While that makes sense, it has the side effect of causing strange
stepping behavior in debuggers.
I've changed the source information so that all of the generated tests
are sourced to `match {scrutinee}` and the jumps are sourced to the last
line of the block they are inside. This resolves the weird stepping
behavior in all debuggers and resolves some instances of "ambiguous
symbol" errors in WinDbg preventing the user from setting breakpoints at
`match` expressions.
Fix typo “a Rc” → “an Rc” (and a few more)
After stumbling about it in the dev-guide, I’ve devided to eliminate all mentions of “a Rc”, replacing it with “an Rc”. E.g.
```plain
$ rg "(^|[^'])\ba\b[^\w=:]*\bRc"
compiler/rustc_data_structures/src/owning_ref/mod.rs
1149:/// Typedef of a owning reference that uses a `Rc` as the owner.
library/std/src/ffi/os_str.rs
919: /// Converts a [`OsString`] into a [`Rc`]`<OsStr>` without copying or allocating.
library/std/src/ffi/c_str.rs
961: /// Converts a [`CString`] into a [`Rc`]`<CStr>` without copying or allocating.
src/doc/rustc-dev-guide/src/query.md
61:are cheaply cloneable; insert a `Rc` if necessary).
src/doc/book/src/ch15-06-reference-cycles.md
72:decreases the reference count of the `a` `Rc<List>` instance from 2 to 1 as
library/alloc/src/rc.rs
1746: /// Converts a generic type `T` into a `Rc<T>`
```
_(the match in the book is a false positive)_
Since the dev-guide is a submodule, it’s getting a separate PR: rust-lang/rustc-dev-guide#1191
I’ve also gone ahead and done the same search for `RwLock` and hit a few cases in the `OwningRef` adaption. Then, I couldn’t keep the countless cases of “a owning …” or “a owner” unaddressed, which concludes this PR.
`@rustbot` label C-cleanup
Remove `Session.trait_methods_not_found`
Instead, avoid registering the problematic well-formed obligation
to begin with. This removes global untracked mutable state,
and avoids potential issues with incremental compilation.
Refactor `named_asm_labels` to a HIR lint
As discussed on #88169, the `named_asm_labels` lint could be moved to a HIR lint. That allows future lints or custom plugins or clippy lints to more easily access the `asm!` macro's data and create better error messages with the lints.
Don't mark `if_let_guard` as an incomplete feature
I don't think there is any reason for `if_let_guard` to be an incomplete feature, and I think the reason they were marked in the first place was simply because they weren't implemented at all.
r? `@pnkfelix`
cc tracking issue #51114
Correctly handle remapping from path containing the current directory with trailing paths
If we have a `auxiliary/lib.rs`, and we generate the metadata with `--remap-path-prefix $PWD/auxiliary=xyz`, the path to `$PWD/auxiliary/lib.rs` won't be correctly remapped in the metadata. This is because internally, path to the working directory itself and relative paths to files under the working directory are remapped separately (hence neither are affected since neither has `$PWD/auxiliary` as prefix), but the concatenation between the working directory and the relative path is not remapped. This PR fixes that.
Improve detection of generics on lang items
Adds detection for the required generics for all lang items. Many lang items require an exact or minimum amount of generic arguments and if they don't exist, the compiler will ICE. This does not add any additional validation about bounds on generics or any other lang item restrictions.
Fixes one of the ICEs in #87573
cc `@FabianWolff`
Improve liveness analysis for generators
Liveness analysis for generators assumes that execution always continues
normally after a yield point, not accounting for the fact that generator
could be dropped before completion.
If generators captures any variables by reference, those variables could
be used within a generator, or when the generator completes, but also
after each yield point in the case the generator is dropped.
Account for the case when generator is dropped after yielding, but
before running to the completion. This effectively considers all
variables captured by reference to be used after a yield point.
Fixes#84292.
Use custom wrap-around type instead of RangeInclusive
Two reasons:
1. More memory is allocated than necessary for `valid_range` in `Scalar`. The range is not used as an iterator and `exhausted` is never used.
2. `contains`, `count` etc. methods in `RangeInclusive` are doing very unhelpful(and dangerous!) things when used as a wrap-around range. - In general this PR wants to limit potentially confusing methods, that have a low probability of working.
Doing a local perf run, every metric shows improvement except for instructions.
Max-rss seem to have a very consistent improvement.
Sorry - newbie here, probably doing something wrong.
Stabilize and document `--force-warn`
This PR will stabilize and document the `--force-warn` command line option. It is currently a draft, pending an FCP.
I've taken the liberty of tidying up the lint level command line options a bit as part of this. The changes are quite minor and should only affect rustc's help output. I'm making them here because they're trivial and, in one case, necessary to unify the way `--force-warn` with the way the other options are displayed.
I also want to mention that `@rylev` has done a ton of work on moving this along and deserves most of the credit. I'm just the one who landed up writing this particular PR.
Resolves#86516.
Allow specifying an deployment target version for all iOS llvm targets
Closes: https://github.com/rust-lang/rust/issues/79408
This pull requests adds the same procedure to define the iOS-version for the LLVM-target as was used for the simulator target and the desktop target.
This then closes the original problem mentioned in the above issue. The problem with incompatible bitcode remains, but is probably not easy fixable.
I realised that something is still not right. Try to fix that.
r? `@petrochenkov`
Liveness analysis for generators assumes that execution always continues
normally after a yield point, not accounting for the fact that generator
could be dropped before completion.
If generators captures any variables by reference, those variables could
be used within a generator, or when the generator completes, but also
after each yield point in the case the generator is dropped.
Account for the case when generator is dropped after yielding, but
before running to the completion. This effectively considers all
variables captured by reference to be used after a yield point.
Remove `Session.used_attrs` and move logic to `CheckAttrVisitor`
Instead of updating global state to mark attributes as used,
we now explicitly emit a warning when an attribute is used in
an unsupported position. As a side effect, we are to emit more
detailed warning messages (instead of just a generic "unused" message).
`Session.check_name` is removed, since its only purpose was to mark
the attribute as used. All of the callers are modified to use
`Attribute.has_name`
Additionally, `AttributeType::AssumedUsed` is removed - an 'assumed
used' attribute is implemented by simply not performing any checks
in `CheckAttrVisitor` for a particular attribute.
We no longer emit unused attribute warnings for the `#[rustc_dummy]`
attribute - it's an internal attribute used for tests, so it doesn't
mark sense to treat it as 'unused'.
With this commit, a large source of global untracked state is removed.
Warn about unreachable code following an expression with an uninhabited type
This pull request fixes#85071. The issue is that liveness analysis currently is "smarter" than reachability analysis when it comes to detecting uninhabited types: Unreachable code is detected during type checking, where full type information is not yet available. Therefore, the check for type inhabitedness is quite crude:
fc81ad22c4/compiler/rustc_typeck/src/check/expr.rs (L202-L205)
i.e. it only checks for `!`, but not other, non-trivially uninhabited types, such as empty enums, structs containing an uninhabited type, etc. By contrast, liveness analysis, which runs after type checking, can benefit from the more sophisticated `tcx.is_ty_uninhabited_from()`:
fc81ad22c4/compiler/rustc_passes/src/liveness.rs (L981)fc81ad22c4/compiler/rustc_passes/src/liveness.rs (L996)
This can lead to confusing warnings when a variable is reported as unused, but the use of the variable is not reported as unreachable. For instance:
```rust
enum Foo {}
fn f() -> Foo {todo!()}
fn main() {
let x = f();
let _ = x;
}
```
currently leads to
```
warning: unused variable: `x`
--> t1.rs:5:9
|
5 | let x = f();
| ^ help: if this is intentional, prefix it with an underscore: `_x`
|
= note: `#[warn(unused_variables)]` on by default
warning: 1 warning emitted
```
which is confusing, because `x` _appears_ to be used in line 6. With my changes, I get:
```
warning: unreachable expression
--> t1.rs:6:13
|
5 | let x = f();
| --- any code following this expression is unreachable
6 | let _ = x;
| ^ unreachable expression
|
= note: `#[warn(unreachable_code)]` on by default
note: this expression has type `Foo`, which is uninhabited
--> t1.rs:5:13
|
5 | let x = f();
| ^^^
warning: unused variable: `x`
--> t1.rs:5:9
|
5 | let x = f();
| ^ help: if this is intentional, prefix it with an underscore: `_x`
|
= note: `#[warn(unused_variables)]` on by default
warning: 2 warnings emitted
```
My implementation is slightly inelegant because unreachable code warnings can now be issued in two different places (during type checking and during liveness analysis), but I think it is the solution with the least amount of unnecessary code duplication, given that the new warning integrates nicely with liveness analysis, where unreachable code is already implicitly detected for the purpose of finding unused variables.
Get piece unchecked in `write`
We already use specialized `zip`, but it seems like we can do a little better by not checking `pieces` length at all.
`Arguments` constructors are now unsafe. So the `format_args!` expansion now includes an `unsafe` block.
<details>
<summary>Local Bench Diff</summary>
```text
name before ns/iter after ns/iter diff ns/iter diff % speedup
fmt::write_str_macro1 22,967 19,718 -3,249 -14.15% x 1.16
fmt::write_str_macro2 35,527 32,654 -2,873 -8.09% x 1.09
fmt::write_str_macro_debug 571,953 575,973 4,020 0.70% x 0.99
fmt::write_str_ref 9,579 9,459 -120 -1.25% x 1.01
fmt::write_str_value 9,573 9,572 -1 -0.01% x 1.00
fmt::write_u128_max 176 173 -3 -1.70% x 1.02
fmt::write_u128_min 138 134 -4 -2.90% x 1.03
fmt::write_u64_max 139 136 -3 -2.16% x 1.02
fmt::write_u64_min 129 135 6 4.65% x 0.96
fmt::write_vec_macro1 24,401 22,273 -2,128 -8.72% x 1.10
fmt::write_vec_macro2 37,096 35,602 -1,494 -4.03% x 1.04
fmt::write_vec_macro_debug 588,291 589,575 1,284 0.22% x 1.00
fmt::write_vec_ref 9,568 9,732 164 1.71% x 0.98
fmt::write_vec_value 9,516 9,625 109 1.15% x 0.99
```
</details>
Rollup of 6 pull requests
Successful merges:
- #87976 (Account for tabs when highlighting multiline code suggestions)
- #88174 (Clarify some wording in Rust 2021 lint docs)
- #88188 (Greatly improve limitation handling on parallel rustdoc GUI test run)
- #88230 (Fix typos “a”→“an”)
- #88232 (Add notes to macro-not-found diagnostics to point out how things with the same name were not a match.)
- #88259 (Do not mark `-Z thir-unsafeck` as unsound anymore)
Failed merges:
r? `@ghost`
`@rustbot` modify labels: rollup
Otherwise, we can get into a situation where you have
a subtype obligation `#1 <: #2` pending, #1 is constrained
by `check_casts`, but #2` is unaffected.
Co-authored-by: Niko Matsakis <niko@alum.mit.edu>
Do not mark `-Z thir-unsafeck` as unsound anymore
The initial implementation of the THIR unsafety checker is now complete (rust-lang/project-thir-unsafeck#7).
r? `@oli-obk`
Add notes to macro-not-found diagnostics to point out how things with the same name were not a match.
This adds notes like:
```
error: cannot find derive macro `Serialize` in this scope
--> $DIR/issue-88206.rs:22:10
|
LL | #[derive(Serialize)]
| ^^^^^^^^^
|
note: `Serialize` is imported here, but it is not a derive macro
--> $DIR/issue-88206.rs:17:11
|
LL | use hey::{Serialize, Deserialize};
| ^^^^^^^^^
```
Fixes https://github.com/rust-lang/rust/issues/88206
Includes https://github.com/rust-lang/rust/pull/88229
r? `@estebank`
2229: Handle MutBorrow/UniqueImmBorrow better
We only want to use UniqueImmBorrow when the capture place is truncated and we
drop Deref of a MutRef.
r? `@nikomatsakis`
Fixes: https://github.com/rust-lang/project-rfc-2229/issues/56
Stop emitting the `dso_local` LLVM attribute for external symbols under the static relocation model on macOS.
This matches Clang's behavior:
973cb2c326/clang/lib/CodeGen/CodeGenModule.cpp (L1038-L1040)
Even if `dso_local` were properly supported in this way on macOS, it seems
incorrect to add this annotation as liberally as we did. The `dso_local`
annotation is for symbols that ultimately end up in the same linkage unit, but
we were adding this annotation even for `static` values inside `extern` blocks
marked with `#[link(type="framework")]`, which should be considered dynamically
linked. Note that Clang likewise avoids emitting `dso_local` for `dllimport`
symbols:
973cb2c326/clang/lib/CodeGen/CodeGenModule.cpp (L1005-L1007)
This issue caused breakage in the `ring` crate, which links to a symbol defined
in `Security.framework` that ultimately resolves to address `0x0`:
b94d61e044/src/rand.rs (L390)
For this symbol, the use of `dso_local` causes LLVM to emit a relocation of
type `X86_64_RELOC_SIGNED`, which is a 32-bit signed PC-relative offset. If the
binary is large enough, `0x0` might be out of range, and the link will fail.
Avoiding `dso_local` causes LLVM to use the GOT instead, emitting a relocation
of type `X86_64_RELOC_GOT_LOAD`, which will properly handle the large offset
and cause the link to succeed.
As a side note, the static relocation model is effectively deprecated for
security reasons on macOS, as it prohibits PIE. It's also completely
unsupported on Apple Silicon, so I don't think it's worth going to the effort
of properly supporting this model on that platform.
Rollup of 7 pull requests
Successful merges:
- #86747 (Improve wording of the `drop_bounds` lint)
- #87166 (Show discriminant before overflow in diagnostic for duplicate values.)
- #88077 (Generate an iOS LLVM target with a specific version)
- #88164 (PassWrapper: adapt for LLVM 14 changes)
- #88211 (cleanup: `Span::new` -> `Span::with_lo`)
- #88229 (Suggest importing the right kind of macro.)
- #88238 (Stop tracking namespace in used_imports.)
Failed merges:
r? `@ghost`
`@rustbot` modify labels: rollup
Stop tracking namespace in used_imports.
This changes `used_imports` from a `FxHashSet<(NodeId, Namespace)>` to a `FxHashSet<NodeId>`, as the Namespace information isn't used.
The only point that uses it did three lookups, `|=`'ing them together.
r? `@estebank`
PassWrapper: adapt for LLVM 14 changes
These API changes appear to have all taken place in
https://reviews.llvm.org/D105007, which moved HWAddressSanitizerPass and
AddressSanitizerPass to only accept their options type as a ctor
argument instead of the sequence of bools etc. This required a couple of
parameter additions, which I made match the default prior to the
mentioned upstream LLVM change.
This patch restores rustc to building (though not quite passing all
tests, I've mailed other patches for those issues) against LLVM HEAD.
Generate an iOS LLVM target with a specific version
This commit adds the `LC_VERSION_MIN_IPHONEOS` load command to the Mach-O header generated for `aarch64-apple-ios` binaries. The operating system will look for this load command to determine the minimum supported operating system version and will not allow the binary to run if it's absent. This logic already exists for the simulator toolchain.
I've been using `otool` from a [cctools](https://github.com/tpoechtrager/cctools-port) toolchain to parse the header and validate that this change adds the required load command.
This change appears to be enough to build Rust binaries that can run on a jailbroken iPhone.
Improve wording of the `drop_bounds` lint
This PR addresses #86653. The issue is sort of a false positive of the `drop_bounds` lint, but I would argue that the best solution for #86653 is simply a rewording of the warning message and lint description, because even if the lint is _technically_ wrong, it still forces the programmer to think about what they are doing, and they can always use `#[allow(drop_bounds)]` if they think that they really need the `Drop` bound.
There are two issues with the current warning message and lint description:
- First, it says that `Drop` bounds are "useless", which is technically incorrect because they actually do have the effect of allowing you e.g. to call methods that also have a `Drop` bound on their generic arguments for some reason. I have changed the wording to emphasize not that the bound is "useless", but that it is most likely not what was intended.
- Second, it claims that `std::mem::needs_drop` detects whether a type has a destructor. But I think this is also technically wrong: The `Drop` bound says whether the type has a destructor or not, whereas `std::mem::needs_drop` also takes nested types with destructors into account, even if the top-level type does not itself have one (although I'm not 100% sure about the exact terminology here, i.e. whether the "drop glue" of the top-level type counts as a destructor or not).
cc `@jonhoo,` does this solve the issue for you?
r? `@GuillaumeGomez`
try_execute_query is now able to centralize the path for query
get/ensure/force.
try_execute_query now takes the dep_node as a parameter, so it can
accommodate `force`. This dep_node is an Option to avoid computing it in
the `get` fast path.
try_execute_query now returns both the result and the dep_node_index to
allow the caller to handle the dep graph.
The caller is responsible for marking the dependency.
`with_taks_impl` is only called from `with_eval_always_task` and
`with_task` . The former is only used in query invocation, while the
latter is also used to start the `tcx` and to trigger codegen.
This move should not change significantly the number of calls to this
assertion.
canonicalize consts before calling try_unify_abstract_consts query
Fixes#88022Fixes#86953Fixes#77708Fixes#82034Fixes#85031
these ICEs were all caused by calling the `try_unify_abstract_consts` query with inference vars in substs
r? `@lcnr`
marker_traits: require `EvaluatedToOk` during winnowing
closes#84955, while it doesn't really fix it in a way that makes me happy it should prevent the issue for now and this
test can't be reproduced anyways, so it doesn't make much sense to keep it open.
fixes#84917 as only one of the impls depends on regions, so we now drop the ambiguous one instead of the correct one.
cc https://rust-lang.zulipchat.com/#narrow/stream/144729-wg-traits/topic/winnowing.20soundly/near/247899832
r? `@nikomatsakis`
Instead, avoid registering the problematic well-formed obligation
to begin with. This removes global untracked mutable state,
and avoids potential issues with incremental compilation.
Trait upcasting coercion (part 3)
By using separate candidates for each possible choice, this fixes type-checking issues in previous commits.
r? `@nikomatsakis`
Instead of updating global state to mark attributes as used,
we now explicitly emit a warning when an attribute is used in
an unsupported position. As a side effect, we are to emit more
detailed warning messages (instead of just a generic "unused" message).
`Session.check_name` is removed, since its only purpose was to mark
the attribute as used. All of the callers are modified to use
`Attribute.has_name`
Additionally, `AttributeType::AssumedUsed` is removed - an 'assumed
used' attribute is implemented by simply not performing any checks
in `CheckAttrVisitor` for a particular attribute.
We no longer emit unused attribute warnings for the `#[rustc_dummy]`
attribute - it's an internal attribute used for tests, so it doesn't
mark sense to treat it as 'unused'.
With this commit, a large source of global untracked state is removed.
Force warn improvements
As part of stablization of the `--force-warn` option (#86516) I've made the following changes:
* Error when the `warnings` lint group is based to the `--force-warn` option
* Tests have been updated to make it easier to understand the semantics of `--force-warn`
r? `@nikomatsakis`
Upgrade to LLVM 13
Work in progress update to LLVM 13. Main changes:
* InlineAsm diagnostics reported using SrcMgr diagnostic kind are now handled. Previously these used a separate diag handler.
* Codegen tests are updated for additional attributes.
* Some data layouts have changed.
* Switch `#[used]` attribute from `llvm.used` to `llvm.compiler.used` to avoid SHF_GNU_RETAIN flag introduced in https://reviews.llvm.org/D97448, which appears to trigger a bug in older versions of gold.
* Set `LLVM_INCLUDE_TESTS=OFF` to avoid Python 3.6 requirement.
Upstream issues:
* ~~https://bugs.llvm.org/show_bug.cgi?id=51210 (InlineAsm diagnostic reporting for module asm)~~ Fixed by 1558bb80c0.
* ~~https://bugs.llvm.org/show_bug.cgi?id=51476 (Miscompile on AArch64 due to incorrect comparison elimination)~~ Fixed by 81b106584f.
* https://bugs.llvm.org/show_bug.cgi?id=51207 (Can't set custom section flags anymore). Problematic change reverted in our fork, https://reviews.llvm.org/D107216 posted for upstream revert.
* https://bugs.llvm.org/show_bug.cgi?id=51211 (Regression in codegen for #83623). This is an optimization regression that we may likely have to eat for this release. The fix for #83623 was based on an incorrect premise, and this needs to be properly addressed in the MergeICmps pass.
The [compile-time impact](https://perf.rust-lang.org/compare.html?start=ef9549b6c0efb7525c9b012148689c8d070f9bc0&end=0983094463497eec22d550dad25576a894687002) is mixed, but quite positive as LLVM upgrades go.
The LLVM 13 final release is scheduled for Sep 21st. The current nightly is scheduled for stable release on Oct 21st.
r? `@ghost`
Refactor fallback code to prepare for never type
This PR contains cherry-picks of some of `@nikomatsakis's` work from #79366, and shouldn't (AFAICT) represent any change in behavior. However, the refactoring is good regardless of the never type work being landed, and will reduce the size of those eventual PR(s) (and rebase pain).
I am not personally an expert on this code, and the commits are essentially 100% `@nikomatsakis's,` but they do seem reasonable to me by my understanding. Happy to edit with review, of course. Commits are best reviewed in sequence rather than all together.
r? `@jackh726` perhaps?
This matches Clang's behavior:
973cb2c326/clang/lib/CodeGen/CodeGenModule.cpp (L1038-L1040)
Even if `dso_local` were properly supported in this way on macOS, it seems
incorrect to add this annotation as liberally as we did. The `dso_local`
annotation is for symbols that ultimately end up in the same linkage unit, but
we were adding this annotation even for `static` values inside `extern` blocks
marked with `#[link(type="framework")]`, which should be considered dynamically
linked. Note that Clang likewise avoids emitting `dso_local` for `dllimport`
symbols:
973cb2c326/clang/lib/CodeGen/CodeGenModule.cpp (L1005-L1007)
This issue caused breakage in the `ring` crate, which links to a symbol defined
in `Security.framework` that ultimately resolves to address `0x0`:
b94d61e044/src/rand.rs (L390)
For this symbol, the use of `dso_local` causes LLVM to emit a relocation of
type `X86_64_RELOC_SIGNED`, which is a 32-bit signed PC-relative offset. If the
binary is large enough, `0x0` might be out of range, and the link will fail.
Avoiding `dso_local` causes LLVM to use the GOT instead, emitting a relocation
of type `X86_64_RELOC_GOT_LOAD`, which will properly handle the large offset
and cause the link to succeed.
As a side note, the static relocation model is effectively deprecated for
security reasons on macOS, as it prohibits PIE. It's also completely
unsupported on Apple Silicon, so I don't think it's worth going to the effort
of properly supporting this model on that platform.
Motivation: in upcoming commits, we are going to create a graph of the
coercion relationships between variables. We want to
distinguish *coercion* specifically from other sorts of subtyping, as
it indicates values flowing from one place to another via assignment.
I didn't like the sub-unify code executing when a predicate was
ENQUEUED, that felt fragile. I would have preferred to move the
sub-unify code so that it only occurred during generalization, but
that impacted diagnostics, so having it also occur when we process
subtype predicates felt pretty reasonable. (I guess we only need one
or the other, but I kind of prefer both, since the generalizer
ultimately feels like the *right* place to guarantee the properties we
want.)
RFC2229 Only compute place if upvars can be resolved
Closes https://github.com/rust-lang/rust/issues/87987
This PR fixes an ICE when trying to unwrap an Err. This error appears when trying to convert a PlaceBuilder into Place when upvars can't yet be resolved. We should only try to convert a PlaceBuilder into Place if upvars can be resolved.
r? `@nikomatsakis`
RFC2229 Add missing edge case
Closes https://github.com/rust-lang/rust/issues/87988
This PR fixes an ICE where a match discriminant is not being read when expected. This ICE was the result of a missing edge case which assumed that if a pattern is of type `PatKind::TupleStruct(..) | PatKind::Path(..) | PatKind::Struct(..) | PatKind::Tuple(..)` then a place could only be a multi variant if the place is of type kind Adt.
We used to avoid doing this because we didn't want to make coercion depend on
the state of inference. For better or worse, we have moved away from this
position over time. Therefore, I am going to go ahead and resolve the `b`
target type early on so that it is done uniformly.
(The older technique for managing this was always something of a hack
regardless; if we really wanted to avoid integrating coercion and inference we
needed to be more disciplined about it.)
Remove `HashStable` impls for `FileName` and `RealFileName`
These impls were unused, and incorrectly hashed the local
(non-remapped) path for `RealFileName::Remapped` (which would
break reproducible builds if these impls were used).
Preserve more spans in internal `rustc_queries!` macro
We now preserve the span of the various query modifiers, and
use the span of the query's name for the commas that we
generate to separate the modifiers. This makes debugging issues with the
internal query macro infrastructure much nicer - previously, we
would get errors messages pointing at the entire call site
(the `rustc_queries!` invocation), which isn't very useful.
This should have no effect when compilation succeeds.
A concrete example of an error message produced after this changed:
```
error: local ambiguity: multiple parsing options: built-in NTs tt ('modifiers') or 1 other option.
--> /home/aaron/repos/rust/compiler/rustc_middle/src/query/mod.rs:23:11
|
12 | / rustc_queries! {
13 | | query trigger_delay_span_bug(key: DefId) -> () {
14 | | desc { "trigger a delay span bug" }
15 | | }
... |
23 | | query hir_crate(key: ()) -> &'tcx Crate<'tcx> {
| | ^^^^^^^^^
... |
1715 | | }
1716 | | }
| |_- in this expansion of `rustc_query_append!`
|
::: compiler/rustc_query_impl/src/lib.rs:51:1
|
51 | rustc_query_append! { [define_queries!][<'tcx>] }
| ------------------------------------------------- in this macro invocation
```
The particular bug shown in this error message will be fixed
in a separate PR.
Update the backtrace crate in libstd
This commit updates the backtrace crate in libstd now that dependencies
have been updated to use `memchr` from the standard library as well.
This is mostly just making sure deps are up-to-date and have all the
latest-and-greatest fixes and such.
Closesrust-lang/backtrace-rs#432
These API changes appear to have all taken place in
https://reviews.llvm.org/D105007, which moved HWAddressSanitizerPass and
AddressSanitizerPass to only accept their options type as a ctor
argument instead of the sequence of bools etc. This required a couple of
parameter additions, which I made match the default prior to the
mentioned upstream LLVM change.
This patch restores rustc to building (though not quite passing all
tests, I've mailed other patches for those issues) against LLVM HEAD.
This commit updates the backtrace crate in libstd now that dependencies
have been updated to use `memchr` from the standard library as well.
This is mostly just making sure deps are up-to-date and have all the
latest-and-greatest fixes and such.
Closesrust-lang/backtrace-rs#432
Prevent double panic when handling incremental fingerprint mismatch
When an incremental fingerprint mismatch occurs, we debug-print
our `DepNode` and query result. Unfortunately, the debug printing
process may cause us to run additional queries, which can result
in a re-entrant fingerprint mismatch error.
To avoid a double panic, this commit adds a thread-local variable
to detect re-entrant calls.
Rollup of 10 pull requests
Successful merges:
- #87818 (Fix anchors display in rustdoc)
- #87983 (Use more accurate spans when proposing adding lifetime to item)
- #88012 (Change WASI's `RawFd` from `u32` to `c_int` (`i32`).)
- #88031 (Make `BuildHasher` object safe)
- #88036 (Fix dead code warning when inline const is used in pattern)
- #88082 (Take into account jobs number for rustdoc GUI tests)
- #88109 (Fix environment variable getter docs)
- #88111 (Add background-color on clickable definitions in source code)
- #88129 (Fix dataflow graphviz bug, make dataflow graphviz modules public)
- #88136 (Move private_unused.rs test to impl-trait)
Failed merges:
r? `@ghost`
`@rustbot` modify labels: rollup
Fix dataflow graphviz bug, make dataflow graphviz modules public
I'm working on a rustc plugin that uses the dataflow framework for MIR analysis. I've found the graphviz utilities extremely helpful for debugging. However, I had to fork the compiler to expose them since they're currently private. I would appreciate if they could be made public so I can build against a nightly instead of a custom fork. Specifically, this PR:
* Makes public the `rustc_mir::dataflow::framework::graphviz` module.
* Makes public the `rustc_mir::util::pretty::write_mir_fn` function.
Here's a concrete example of how I'm using the graphviz module: 97b843b8b0/src/slicing/mod.rs (L186-L203)
Additionally, this PR fixes a small bug in the diff code that incorrectly shows the updated object as the old object.
r? `@ecstatic-morse`
Matthew's work on improving NLL's "higher-ranked subtype error"s
This PR rebases `@matthewjasper's` [branch](https://github.com/matthewjasper/rust/tree/nll-hrtb-errors) which has great work to fix the obscure higher-ranked subtype errors that are tracked in #57374.
These are a blocker to turning full NLLs on, and doing some internal cleanups to remove some of the old region code.
The goal is so `@nikomatsakis` can take a look at this early, and I'll then do my best to help do the changes and followup work to land this work, and move closer to turning off the migration mode.
I've only updated the branch and made it compile, removed a warning or two.
r? `@nikomatsakis`
(Here's the [zulip topic to discuss this](https://rust-lang.zulipchat.com/#narrow/stream/122657-t-compiler.2Fwg-nll/topic/.2357374.3A.20improving.20higher-ranked.20subtype.20errors.20via.20.2386700) that Niko wanted)
Remove box syntax from compiler and tools
Removes box syntax from the compiler and tools. In #49733, the future of box syntax is uncertain and the use in the compiler was listed as one of the reasons to keep it. Removal of box syntax [might affect the code generated](https://github.com/rust-lang/rust/pull/49646#issuecomment-379219615) and slow down the compiler so I'd recommend doing a perf run on this.
Update `polonius-engine` to 0.13.0
This PR updates the use of `polonius-engine` to the recently released 0.13.0:
- this version renamed a lot of relations to match the current terminology
- "illegal subset relationships errors" (AKA "subset errors" or "universal region errors" in rustc parlance) have been implemented in all variants, and therefore the `Hybrid` variant can be the rustc default once again
- some of the blessed expectations were updated: new tests have been added since the last time I updated the tests, diagnostics have changed, etc.
In particular:
- a few tests had trivial expectations changes such as basic diagnostics changes for the migrate-mode and full NLLs
- others were recursion and lengths limits which emits a file, and under the polonius compare-mode, the folder has a different name
- a few tests were ignored in the NLL compare-mode for reasons that obviously also apply to Polonius
- some diagnostics were unified so that older expectations no longer made sense: the NLL and Polonius outputs were identical.
- in a few cases Polonius gets a chance to emit more errors than NLLs
A few tests in the compare-mode still are super slow and trigger the 60s warning, or OOM rustc during fact generation, and I've detailed these [on Zulip](https://rust-lang.zulipchat.com/#narrow/stream/186049-t-compiler.2Fwg-polonius/topic/Challenges.20for.20move.2Finit.2C.20liveness.2C.20and.20.60Location.3A.3AAll.60):
- `src/test/ui/numbers-arithmetic/saturating-float-casts.rs` -> OOM during rustc fact generation
- `src/test/ui/numbers-arithmetic/num-wrapping.rs`
- `src/test/ui/issues/issue-72933-match-stack-overflow.rs`
- `src/test/ui/issues/issue-74564-if-expr-stack-overflow.rs`
- `src/test/ui/repr/repr-no-niche.rs`
In addition, 2 tests don't currently pass and I didn't want to bless them now: they deal with HRTBs and miss errors that NLLs emit. We're currently trying to see if we need chalk to deal with HRTB errors (as we thought we would have to) but during the recent sprint, we discovered that we may be able to detect some of these errors in a way that resembles subset errors:
- `ui/hrtb/hrtb-just-for-static.rs` -> 3 errors in NLL, 2 in polonius: a missing error about HRTB + needing to outlive 'static
- `ui/issues/issue-26217.rs` -> missing HRTB that makes the test compile instead of emitting an error
We'll keep talking about this at the next sprint as well.
cc `@rust-lang/wg-polonius` r? `@nikomatsakis`
Enable compiler consumers to obtain mir::Body with Polonius facts.
This PR adds a function (``get_body_with_borrowck_facts``) that can be used by compiler consumers to obtain ``mir::Body`` with accompanying borrow checker information.
The most important borrow checker information that [our verifier called Prusti](https://github.com/viperproject/prusti-dev) needs is lifetime constraints. I have not found a reasonable way to compute the lifetime constraints on the Prusti side. In the compiler, the constraints are computed during the borrow checking phase and then dropped. This PR adds an additional parameter to the `do_mir_borrowck` function that tells it to return the computed information instead of dropping it.
The additionally returned information by `do_mir_borrowck` contains a ``mir::Body`` with non-erased lifetime regions and Polonius facts. I have decided to reuse the Polonius facts because this way I needed fewer changes to the compiler and Polonius facts contains other useful information that we otherwise would need to recompute.
Just FYI: up to now, Prusti was obtaining this information by [parsing the compiler logs](b58ced8dfd/prusti-interface/src/environment/borrowck/regions.rs (L25-L39)). This is not only a hacky approach, but we also reached its limits.
r? `@nikomatsakis`
Improve non_fmt_panics suggestion based on trait impls.
This improves the non_fmt_panics lint suggestions by checking first which trait (Display or Debug) are actually implemented on the type.
Fixes https://github.com/rust-lang/rust/issues/87313
Fixes https://github.com/rust-lang/rust/issues/87999
Before:
```
help: add a "{}" format string to Display the message
|
2 | panic!("{}", Some(1));
| +++++
help: or use std::panic::panic_any instead
|
2 | std::panic::panic_any(Some(1));
| ~~~~~~~~~~~~~~~~~~~~~
```
After:
```
help: add a "{:?}" format string to use the Debug implementation of `Option<i32>`
|
2 | panic!("{:?}", Some(1));
| +++++++
help: or use std::panic::panic_any instead
|
2 | std::panic::panic_any(Some(1));
| ~~~~~~~~~~~~~~~~~~~~~
```
r? `@estebank`
Revert "Auto merge of #83417 - erikdesjardins:enableremovezsts, r=oli-obk"
This reverts commit 8007b506ac, reversing changes made to e55c13e109.
Fixes#88043
r? `@oli-obk`
Use note for pointing at bound introducing requirement
Modify output for pointing where a trait bound obligation is introduced in an E0277 from using a span label to using a note in order to always preserve order of the output:
Before:
```
error[E0277]: `<<Self as Case1>::A as Iterator>::Item` doesn't implement `Debug`
--> $DIR/bounds-on-assoc-in-trait.rs:18:28
|
LL | type A: Iterator<Item: Debug>;
| ^^^^^ `<<Self as Case1>::A as Iterator>::Item` cannot be formatted using `{:?}` because it doesn't implement `Debug`
|
::: $SRC_DIR/core/src/fmt/mod.rs:LL:COL
|
LL | pub trait Debug {
| --------------- required by this bound in `Debug`
|
= help: the trait `Debug` is not implemented for `<<Self as Case1>::A as Iterator>::Item`
```
After:
```
error[E0277]: `<<Self as Case1>::A as Iterator>::Item` doesn't implement `Debug`
--> $DIR/bounds-on-assoc-in-trait.rs:18:28
|
LL | type A: Iterator<Item: Debug>;
| ^^^^^ `<<Self as Case1>::A as Iterator>::Item` cannot be formatted using `{:?}` because it doesn't implement `Debug`
|
= help: the trait `Debug` is not implemented for `<<Self as Case1>::A as Iterator>::Item`
note: required by a bound in `Debug`
--> $SRC_DIR/core/src/fmt/mod.rs:LL:COL
|
LL | pub trait Debug {
| ^^^^^^^^^^^^^^^ required by this bound in `Debug`
```
Include (potentially remapped) working dir in crate hash
Fixes#85019
A `SourceFile` created during compilation may have a relative
path (e.g. if rustc itself is invoked with a relative path).
When we write out crate metadata, we convert all relative paths
to absolute paths using the current working directory.
However, the working directory is not included in the crate hash.
This means that the crate metadata can change while the crate
hash remains the same. Among other problems, this can cause a
fingerprint mismatch ICE, since incremental compilation uses
the crate metadata hash to determine if a foreign query is green.
This commit moves the field holding the working directory from
`Session` to `Options`, including it as part of the crate hash.
cc `@ohsayan`
Skip assert ICE with default_method_body_is_const
functions marked with #[default_method_body_is_const] would
ICE when being const checked due to it not being a const function:
`tcx.is_const_fn_raw(did)` returns false. We should skip this assert
when it is marked with that attribute.
r? `@oli-obk`
Assign FIXMEs to me and remove obsolete ones
Also fixed capitalization of documentation
We also don't need to transform predicates to be non-const since we basically ignore const predicates in non-const contexts.
r? `````@oli-obk`````
Detect fake spans in non_fmt_panic lint.
This addresses https://github.com/rust-lang/rust/issues/87621
Some proc_macros claim that the user wrote all of the tokens it outputs, by applying a span from the input to all of the produced tokens. That can result in confusing suggestions, as in #87621. This is a simple patch that avoids suggesting anything for `panic!("{}")` if the span of `"{}"` and `panic!(..)` are identical, which is normally not possible.
Closure migration multispan suggestions
This changes the `rust_2021_incompatible_closure_captures` lint to only suggest inserting the parts that need to be inserted, instead of suggesting to replace the entire closure by an almost identical closure with one statement added.
Before:
```
[...]
help: add a dummy let to cause `a` to be fully captured
|
5 ~ let _ = || {
6 + let _ = &a;
7 + dbg!(a.0);
8 + println!("1");
9 + println!("2");
10 + println!("3");
...
|
[...]
help: add a dummy let to cause `b` to be fully captured
|
14 | let _ = || { let _ = &b; dbg!(b.0); };
| ~~~~~~~~~~~~~~~~~~~~~~~~~~
[...]
help: add a dummy let to cause `c` to be fully captured
|
16 | let _ = || { let _ = &c; dbg!(c.0) };
| ~~~~~~~~~~~~~~~~~~~~~~~~~
```
After:
```
[...]
help: add a dummy let to cause `a` to be fully captured
|
5 ~ let _ = || {
6 + let _ = &a;
|
[...]
help: add a dummy let to cause `b` to be fully captured
|
14 | let _ = || { let _ = &b; dbg!(b.0); };
| +++++++++++
[...]
help: add a dummy let to cause `c` to be fully captured
|
16 | let _ = || { let _ = &c; dbg!(c.0) };
| +++++++++++++ +
```
The TargetMachine may be referencing data in the context. In
particular, at least the GlobalISel instruction selector stored
in the TM may reference a TrackedMDNode DebugLoc that destruction
of the TargetMachine will try to untrack.
The #[used] attribute explicitly only requires symbols to be
retained in object files, but allows the linker to drop them
if dead. This corresponds to llvm.compiler.used semantics.
The motivation to change this *now* is that https://reviews.llvm.org/D97448
starts emitting #[used] symbols into unique sections with
SHF_GNU_RETAIN flag. This triggers a bug in some version of gold,
resulting in the ARGV_INIT_ARRAY symbol part of the .init_array
section to be incorrectly placed.
Add future-incompat lint for `doc(primitive)`
## What is `doc(primitive)`?
`doc(primitive)` is an attribute recognized by rustdoc which adds documentation for the built-in primitive types, such as `usize` and `()`. It has been stable since Rust 1.0.
## Why change anything?
`doc(primitive)` is useless for anyone outside the standard library. Since rustdoc provides no way to combine the documentation on two different primitive items, you can only replace the docs, and since the standard library already provides extensive documentation there is no reason to do so.
While fixing rustdoc's handling of primitive items (https://github.com/rust-lang/rust/pull/87073) I discovered that even rustdoc's existing handling of primitive items was broken if you had more than two crates using it (it would pick randomly between them). That meant both:
- Keeping rustdoc's existing treatment was nigh-impossible, because it was random.
- doc(primitive) was even more useless than it would otherwise be.
The only use-case for this outside the standard library is for no-std libraries which want to link to primitives (https://github.com/rust-lang/rust/issues/73423) which is being fixed in https://github.com/rust-lang/rust/pull/87073.
https://github.com/rust-lang/rust/pull/87073 makes various breaking changes to `doc(primitive)` (breaking in the sense that they change the semantics, not in that they cause code to fail to compile). It's not possible to avoid these and still fix rustdoc's issues.
## What can we do about it?
As shown by the crater run (https://github.com/rust-lang/rust/pull/87050#issuecomment-886166706), no one is actually using doc(primitive), there wasn't a single true regression in the whole run. We can either:
1. Feature gate it completely, breaking anyone who crater missed. They can easily fix the breakage just by removing the attribute.
2. add it to the `INVALID_DOC_ATTRIBUTES` future-incompat lint, and at the same time make it a no-op unless you add a feature gate. That would mean rustdoc has to look at the features of dependent crates, because it needs to know where primitives are defined in order to link to them.
3. add it to `INVALID_DOC_ATTRIBUTES`, but still use it to determine where primitives come from
4. do nothing; the behavior will silently change in https://github.com/rust-lang/rust/pull/87073.
My preference is for 2, but I would also be happy with 1 or 3. I don't think we should silently change the behavior.
This PR currently implements 3.
functions marked with #[default_method_body_is_const] would
ICE when being const checked due to it not being a const function:
`tcx.is_const_fn_raw(did)` returns false. We should skip this assert
when it is marked with that attribute.
Without the specific version, the mach-o header will be missing the
minimum supported operating system version. This is mandatory for
running Rust binaries on iOS devices.
Uplift the invalid_atomic_ordering lint from clippy to rustc
This is mostly just a rebase of https://github.com/rust-lang/rust/pull/79654; I've copy/pasted the text from that PR below.
r? `@lcnr` since you reviewed the last one, but feel free to reassign.
---
This is an implementation of https://github.com/rust-lang/compiler-team/issues/390.
As mentioned, in general this turns an unconditional runtime panic into a (compile time) lint failure. It has no false positives, and the only false negatives I'm aware of are if `Ordering` isn't specified directly and is comes from an argument/constant/whatever.
As a result of it having no false positives, and the alternative always being strictly wrong, it's on as deny by default. This seems right.
In the [zulip stream](https://rust-lang.zulipchat.com/#narrow/stream/233931-t-compiler.2Fmajor-changes/topic/Uplift.20the.20.60invalid_atomic_ordering.60.20lint.20from.20clippy/near/218483957) `@joshtriplett` suggested that lang team should FCP this before landing it. Perhaps libs team cares too?
---
Some notes on the code for reviewers / others below
## Changes from clippy
The code is changed from [the implementation in clippy](68cf94f6a6/clippy_lints/src/atomic_ordering.rs) in the following ways:
1. Uses `Symbols` and `rustc_diagnostic_item`s instead of string literals.
- It's possible I should have just invoked Symbol::intern for some of these instead? Seems better to use symbol, but it did require adding several.
2. The functions are moved to static methods inside the lint struct, as a way to namespace them.
- There's a lot of other code in that file — which I picked as the location for this lint because `@jyn514` told me that seemed reasonable.
3. Supports unstable AtomicU128/AtomicI128.
- I did this because it was almost easier to support them than not — not supporting them would have (ideally) required finding a way not to give them a `rustc_diagnostic_item`, which would have complicated an already big macro.
- These don't have tests since I wasn't sure if/how I should make tests conditional on whether or not the target has the atomic... This is to a certain extent an issue of 64bit atomics too, but 128-bit atomics are much less common. Regardless, the existing tests should be *more* than thorough enough here.
4. Minor changes like:
- grammar tweaks ("loads cannot have `Release` **and** `AcqRel` ordering" => "loads cannot have `Release` **or** `AcqRel` ordering")
- function renames (`match_ordering_def_path` => `matches_ordering_def_path`),
- avoiding clippy-specific helper methods that don't exist in rustc_lint and didn't seem worth adding for this case (for example `cx.struct_span_lint` vs clippy's `span_lint_and_help` helper).
## Potential issues
(This is just about the code in this PR, not conceptual issues with the lint or anything)
1. I'm not sure if I should have used a diagnostic item for `Ordering` and its variants (I couldn't figure out how really, so if I should do this some pointers would be appreciated).
- It seems possible that failing to do this might possibly mean there are more cases this lint would miss, but I don't really know how `match_def_path` works and if it has any pitfalls like that, so maybe not.
2. I *think* I deprecated the lint in clippy (CC `@flip1995` who asked to be notified about clippy changes in the future in [this comment](https://github.com/rust-lang/rust/pull/75671#issuecomment-718731659)) but I'm not sure if I need to do anything else there.
- I'm kind of hoping CI will catch if I missed anything, since `x.py test src/tools/clippy` fails with a lot of errors with and without my changes (and is probably a nonsense command regardless). Running `cargo test` from src/tools/clippy also fails with unrelated errors that seem like refactorings that didnt update clippy? So, honestly no clue.
3. I wasn't sure if the description/example I gave good. Hopefully it is. The example is less thorough than the one from clippy here: https://rust-lang.github.io/rust-clippy/master/index.html#invalid_atomic_ordering. Let me know if/how I should change it if it needs changing.
4. It pulls in the `if_chain` crate. This crate was already used in clippy, and seems like it's used elsewhere in rustc, but I'm willing to rewrite it to not use this if needed (I'd prefer not to, all things being equal).
- Deprecate clippy::invalid_atomic_ordering
- Use rustc_diagnostic_item for the orderings in the invalid_atomic_ordering lint
- Reduce code duplication
- Give up on making enum variants diagnostic items and just look for
`Ordering` instead
I ran into tons of trouble with this because apparently the change to
store HIR attrs in a side table also gave the DefIds of the
constructor instead of the variant itself. So I had to change
`matches_ordering` to also check the grandparent of the defid as well.
- Rename `atomic_ordering_x` symbols to just the name of the variant
- Fix typos in checks - there were a few places that said "may not be
Release" in the diagnostic but actually checked for SeqCst in the lint.
- Make constant items const
- Use fewer diagnostic items
- Only look at arguments after making sure the method matches
This prevents an ICE when there aren't enough arguments.
- Ignore trait methods
- Only check Ctors instead of going through `qpath_res`
The functions take values, so this couldn't ever be anything else.
- Add if_chain to allowed dependencies
- Fix grammar
- Remove unnecessary allow
Fixes#85019
A `SourceFile` created during compilation may have a relative
path (e.g. if rustc itself is invoked with a relative path).
When we write out crate metadata, we convert all relative paths
to absolute paths using the current working direction.
However, the working directory is not included in the crate hash.
This means that the crate metadata can change while the crate
hash remains the same. Among other problems, this can cause a
fingerprint mismatch ICE, since incremental compilation uses
the crate metadata hash to determine if a foreign query is green.
This commit moves the field holding the working directory from
`Session` to `Options`, including it as part of the crate hash.
These impls were unused, and incorrectly hashed the local
(non-remapped) path for `RealFileName::Remapped` (which would
break reproducible builds if these impls were used).
Add support for clobber_abi to asm!
This PR adds the `clobber_abi` feature that was proposed in #81092.
Fixes#81092
cc `@rust-lang/wg-inline-asm`
r? `@nagisa`
