In the AST, currently we use `BinOpKind` within `ExprKind::AssignOp` and
`AssocOp::AssignOp`, even though this allows some nonsensical
combinations. E.g. there is no `&&=` operator. Likewise for HIR and
THIR.
This commit introduces `AssignOpKind` which only includes the ten
assignable operators, and uses it in `ExprKind::AssignOp` and
`AssocOp::AssignOp`. (And does similar things for `hir::ExprKind` and
`thir::ExprKind`.) This avoids the possibility of nonsensical
combinations, as seen by the removal of the `bug!` case in
`lang_item_for_binop`.
The commit is mostly plumbing, including:
- Adds an `impl From<AssignOpKind> for BinOpKind` (AST) and `impl
From<AssignOp> for BinOp` (MIR/THIR).
- `BinOpCategory` can now be created from both `BinOpKind` and
`AssignOpKind`.
- Replaces the `IsAssign` type with `Op`, which has more information and
a few methods.
- `suggest_swapping_lhs_and_rhs`: moves the condition to the call site,
it's easier that way.
- `check_expr_inner`: had to factor out some code into a separate
method.
I'm on the fence about whether avoiding the nonsensical combinations is
worth the extra code.
Rollup of 6 pull requests
Successful merges:
- #138992 (literal pattern lowering: use the pattern's type instead of the literal's in `const_to_pat`)
- #139211 (interpret: add a version of run_for_validation for &self)
- #139235 (`AstValidator` tweaks)
- #139237 (Add a dep kind for use of the anon node with zero dependencies)
- #139260 (Add dianqk to codegen reviewers)
- #139264 (Fix two incorrect turbofish suggestions)
r? `@ghost`
`@rustbot` modify labels: rollup
Add a dep kind for use of the anon node with zero dependencies
This adds a dep kind for use of the anon node with zero dependencies instead of making use of the null node. I don't think this matters, but it is nicer than random null nodes in the dep graph.
literal pattern lowering: use the pattern's type instead of the literal's in `const_to_pat`
This has two purposes:
- First, it enables removing the `treat_byte_string_as_slice` fields from `TypeckResults` and `ConstToPat`. A byte string pattern's type will be `&[u8]` when matching on a slice reference, so `const_to_pat` will lower it to a slice ref pattern. I believe this is tested by `tests/ui/match/pattern-deref-miscompile.rs`.
- Second, it will simplify the implementation of byte string literals in deref patterns. If byte string patterns can be given the type `[u8; N]` or `[u8]` during HIR typeck, then nothing needs to be changed in `const_to_pat` in order to lower the patterns `deref!(b"..."): Vec<u8>` and `deref!(b"..."): Box<[u8; 3]>`.
Implementation-wise, this uses `lit_to_const` to make a const with the pattern's type and the literal's valtree; that feels to me like the best way to make sure that the valtree representations of the pattern type and literal are the same. Though it may necessitate later changes to `lit_to_const` to accommodate giving byte string literal patterns non-reference types—would that be reasonable?
This unfortunately doesn't work for the `string_deref_patterns` feature (since that gives string literal patterns the `String` type), so I added a workaround for that. However, once `deref_patterns` supports string literals, it may be able to replace `string_deref_patterns`; the special case for `String` can removed at that point.
r? ``@oli-obk``
Move methods from `Map` to `TyCtxt`, part 5.
This eliminates all methods on `Map`. Actually removing `Map` will occur in a follow-up PR.
A follow-up to #137504.
r? `@Zalathar`
Various local trait item iteration cleanups
Adding a trait impl for `Foo` unconditionally affected all queries that are interested in a completely independent trait `Bar`. Perf has no effect on this. We probably don't have a good perf test for this tho.
r? `@compiler-errors`
I am unsure about https://github.com/rust-lang/rust/pull/139018/commits/9d05efb66f7b599eeacb5d2456f844fe4768e865 as it doesn't improve anything wrt incremental, because we still do all the checks for valid `Drop` impls, which subsequently will still invoke many queries and basically keep the depgraph the same.
I want to do
9549077a47/compiler/rustc_middle/src/ty/trait_def.rs (L141)
but would leave that to a follow-up PR, this one changes enough things as it is
Encode synthetic by-move coroutine body with a different `DefPathData`
See the included test. In the first revision rpass1, we have an async closure `{closure#0}` which has a coroutine as a child `{closure#0}::{closure#0}`. We synthesize a by-move coroutine body, which is `{closure#0}::{closure#1}` which depends on the mir_built query, which depends on the typeck query.
In the second revision rpass2, we've replaced the coroutine-closure by a closure with two children closure. Notably, the def path of the second child closure is the same as the synthetic def id from the last revision: `{closure#0}::{closure#1}`. When type-checking this closure, we end up trying to compute its def_span, which tries to fetch it from the incremental cache; this will try to force the dependencies from the last run, which ends up forcing the mir_built query, which ends up forcing the typeck query, which ends up with a query cycle.
The problem here is that we really should never have used the same `DefPathData` for the synthetic by-move coroutine body, since it's not a closure. Changing the `DefPathData` will mean that we can see that the def ids are distinct, which means we won't try to look up the closure's def span from the incremental cache, which will properly skip replaying the node's dependencies and avoid a query cycle.
Fixes#139142
Emit `unused_attributes` for `#[inline]` on exported functions
I saw someone post a code sample that contained these two attributes, which immediately made me suspicious.
My suspicions were confirmed when I did a small test and checked the compiler source code to confirm that in these cases, `#[inline]` is indeed ignored (because you can't exactly `LocalCopy`an unmangled symbol since that would lead to duplicate symbols, and doing a mix of an unmangled `GloballyShared` and mangled `LocalCopy` instantiation is too complicated for our current instatiation mode logic, which I don't want to change right now).
So instead, emit the usual unused attribute lint with a message saying that the attribute is ignored in this position.
I think this is not 100% true, since I expect LLVM `inlinehint` to still be applied to such a function, but that's not why people use this attribute, they use it for the `LocalCopy` instantiation mode, where it doesn't work.
r? saethlin as the instantiation guy
Procedurally, I think this should be fine to merge without any lang involvement, as this only does a very minor extension to an existing lint.
Prefer built-in sized impls (and only sized impls) for rigid types always
This PR changes the confirmation of `Sized` obligations to unconditionally prefer the built-in impl, even if it has nested obligations. This also changes all other built-in impls (namely, `Copy`/`Clone`/`DiscriminantKind`/`Pointee`) to *not* prefer built-in impls over param-env impls. This aligns the old solver with the behavior of the new solver.
---
In the old solver, we register many builtin candidates with the `BuiltinCandidate { has_nested: bool }` candidate kind. The precedence this candidate takes over other candidates is based on the `has_nested` field. We only prefer builtin impls over param-env candidates if `has_nested` is `false`
2b4694a698/compiler/rustc_trait_selection/src/traits/select/mod.rs (L1804-L1866)
Preferring param-env candidates when the builtin candidate has nested obligations *still* ends up leading to detrimental inference guidance, like:
```rust
fn hello<T>() where (T,): Sized {
let x: (_,) = Default::default();
// ^^ The `Sized` obligation on the variable infers `_ = T`.
let x: (i32,) = x;
// We error here, both a type mismatch and also b/c `T: Default` doesn't hold.
}
```
Therefore this PR adjusts the candidate precedence of `Sized` obligations by making them a distinct candidate kind and unconditionally preferring them over all other candidate kinds.
Special-casing `Sized` this way is necessary as there are a lot of traits with a `Sized` super-trait bound, so a `&'a str: From<T>` where-bound results in an elaborated `&'a str: Sized` bound. People tend to not add explicit where-clauses which overlap with builtin impls, so this tends to not be an issue for other traits.
We don't know of any tests/crates which need preference for other builtin traits. As this causes builtin impls to diverge from user-written impls we would like to minimize the affected traits. Otherwise e.g. moving impls for tuples to std by using variadic generics would be a breaking change. For other builtin impls it's also easier for the preference of builtin impls over where-bounds to result in issues.
---
There are two ways preferring builtin impls over where-bounds can be incorrect and undesirable:
- applying the builtin impl results in undesirable region constraints. E.g. if only `MyType<'static>` implements `Copy` then a goal like `(MyType<'a>,): Copy` would require `'a == 'static` so we must not prefer it over a `(MyType<'a>,): Copy` where-bound
- this is mostly not an issue for `Sized` as all `Sized` impls are builtin and don't add any region constraints not already required for the type to be well-formed
- however, even with `Sized` this is still an issue if a nested goal also gets proven via a where-bound: [playground](https://play.rust-lang.org/?version=stable&mode=debug&edition=2024&gist=30377da5b8a88f654884ab4ebc72f52b)
- if the builtin impl has associated types, we should not prefer it over where-bounds when normalizing that associated type. This can result in normalization adding more region constraints than just proving trait bounds. https://github.com/rust-lang/rust/issues/133044
- not an issue for `Sized` as it doesn't have associated types.
r? lcnr
Remove attribute `#[rustc_error]`
It was an ancient way to write `check-pass` tests, but now it's no longer necessary (except for the `delayed_bug_from_inside_query` flavor, which is retained).
It's an old (2017 or earlier) type that describes a `self` receiver.
It's only used in `rustc_hir_analysis` for two error messages, and much
of the complexity isn't used. I suspect it used to be used for more
things.
This commit removes it, and moves a greatly simplified version of the
`determine` method into `rustc_hir_analysis`, renamed as
`get_self_string`. The big comment on the method is removed because it
no longer seems relevant.
"Missing" patterns are possible in bare fn types (`fn f(u32)`) and
similar places. Currently these are represented in the AST with
`ast::PatKind::Ident` with no `by_ref`, no `mut`, an empty ident, and no
sub-pattern. This flows through to `{hir,thir}::PatKind::Binding` for
HIR and THIR.
This is a bit nasty. It's very non-obvious, and easy to forget to check
for the exceptional empty identifier case.
This commit adds a new variant, `PatKind::Missing`, to do it properly.
The process I followed:
- Add a `Missing` variant to `{ast,hir,thir}::PatKind`.
- Chang `parse_param_general` to produce `ast::PatKind::Missing`
instead of `ast::PatKind::Missing`.
- Look through `kw::Empty` occurrences to find functions where an
existing empty ident check needs replacing with a `PatKind::Missing`
check: `print_param`, `check_trait_item`, `is_named_param`.
- Add a `PatKind::Missing => unreachable!(),` arm to every exhaustive
match identified by the compiler.
- Find which arms are actually reachable by running the test suite,
changing them to something appropriate, usually by looking at what
would happen to a `PatKind::Ident`/`PatKind::Binding` with no ref, no
`mut`, an empty ident, and no subpattern.
Quite a few of the `unreachable!()` arms were never reached. This makes
sense because `PatKind::Missing` can't happen in every pattern, only
in places like bare fn tys and trait fn decls.
I also tried an alternative approach: modifying `ast::Param::pat` to
hold an `Option<P<Pat>>` instead of a `P<Pat>`, but that quickly turned
into a very large and painful change. Adding `PatKind::Missing` is much
easier.
Clean up a few things in rustc_hir_analysis::check::region
Each commit is independent. They are all small clean-ups in rustc_hir_analysis::check::region.
Remove `kw::Empty` uses from `rustc_middle`.
There are several places in `rustc_middle` that check for an empty lifetime name. These checks appear to be totally unnecessary, because empty lifetime names aren't produced here. (Empty lifetime names *are* possible in `hir::Lifetime`. Perhaps there was some confusion between it and the `rustc_middle` types?)
This commit removes the `kw::Empty` checks.
r? `@lcnr`
Avoiding calling queries when collecting active queries
This PR changes active query collection to no longer call queries. Instead the fields needing queries have their computation delayed to when an cycle error is emitted or when printing the query backtrace in a panic.
This is done by splitting the fields in `QueryStackFrame` needing queries into a new `QueryStackFrameExtra` type. When collecting queries `QueryStackFrame` will contain a closure that can create `QueryStackFrameExtra`, which does make use of queries. Calling `lift` on a `QueryStackFrame` or `CycleError` will convert it to a variant containing `QueryStackFrameExtra` using those closures.
This also only calls queries needed to collect information on a cycle errors, instead of information on all active queries.
Calling queries when collecting active queries is a bit odd. Calling queries should not be done in the deadlock handler at all.
This avoids the out of memory scenario in https://github.com/rust-lang/rust/issues/124901.
Add environment variable query
Generally, `rustc` prefers command-line arguments, but in some cases, an environment variable really is the most sensible option. We should make sure that this works properly with the compiler's change-tracking mechanisms, such that changing the relevant environment variable causes a rebuild.
This PR is a first step forwards in doing that.
Part of the work needed to do https://github.com/rust-lang/rust/issues/118204, see https://github.com/rust-lang/rust/pull/129342 for some discussion.
r? ``@petrochenkov``
This allows us to remove the field `treat_byte_string_as_slice` from
`TypeckResults`, since the pattern's type contains everything necessary
to get the correct lowering for byte string literal patterns.
This leaves the implementation of `string_deref_patterns` broken, to be
fixed in the next commit.
Along with `TyCtx::env_var` helper. These can be used to track
environment variable accesses in the query system.
Since `TyCtx::env_var_os` uses `OsStr`, this commit also adds the
necessary trait implementations for that to work.
