We want to evaluate the rhs of a type_const.
Also added an early return/guard in eval_in_interpreter which is used in functions like `eval_to_allocation_raw_provider`
Lastly add a debug assert to `thir_body()` if we have gotten there with a type_const something as gone wrong.
Get rid of a call to is_type_const() and instead use a match arm.
Change this is_type_const() check to a debug_assert!()
Change to use an if else statment instead.
Update type_const-pub.rs
Fix formatting.
Noticed that this is the same check as is_type_const() centralize it.
Add test case for pub type_const.
Closing brackets were missing on AdtDef, the field_types list in FruInfo, and InlineAsmExpr, breaking folding in some editors;
Fields were incorrectly (?) indexed in the list for functional update syntax, showing the (implicit, irrelevant) iteration index instead of the field index;
also spurious colon after Pat.
mir_build: Simplify length-determination and indexing for array/slice patterns
The existing length-determination code in `prefix_slice_suffix` has ended up overly complicated, partly because it doesn't know in advance whether the pattern is supposed to be an array pattern or a slice pattern.
Pulling most of that step out into the `PatKind::Array` arm makes the whole thing a bit nicer overall.
There should (hopefully) be no change to compiler output. The biggest “functional” change is that we now discard the subpatterns of an array pattern of unknowable length, instead of treating it as a slice pattern. I'm not aware of any way for this to make an observable difference, and it can only occur when compilation is already doomed to fail.
include `HirId`s directly in the THIR, not wrapped in `LintLevel`s
Occurrences of `LintLevel` in the THIR were always `LintLevel::Explicit`, containing a `HirId`, so we don't need to make it possible to put `LintLevel::Inherited` there. Removing the unused case where `HirId`s aren't present in the THIR slightly simplifies diagnostics/lints/tools that want to map from the THIR back to the HIR, e.g. rust-lang/rust#145569.
Since `LintLevel` is no longer present in the THIR, I've moved it in the second commit to live in `rustc_mir_build`; that's where it's actually used. I'm not sure exactly where exactly it should live there, but I put it in the `builder::scope` module since it's used by `Builder::in_scope` for determining when to introduce source scopes.
r? lcnr as the reviewer of rust-lang/rust#145569, since this was discussed there
It's described as a "backwards compatibility hack to keep the diff
small". Removing it requires only a modest amount of churn, and the
resulting code is clearer without the invisible derefs.
Use a hook to decouple `rustc_mir_transform` from `rustc_mir_build`
I noticed that the only point of direct contact between the `rustc_mir_transform` and `rustc_mir_build` crates is a single `build_mir` function, which could easily be changed to a hook function instead.
By making that function a hook, we can make `rustc_mir_transform` no longer have a dependency on `rustc_mir_build`, allowing them to be built/rebuilt independently. That should hopefully allow slightly more parallelism in clean builds and incremental rebuilds of the compiler.
Fix ICE: can't type-check body of DefId for issue #148729
This commit fixes https://github.com/rust-lang/rust/issues/148729 for min_const_generic_args https://github.com/rust-lang/rust/issues/132980.
It's pretty small PR. The first commit makes sure that the `type_const`s are made into normal consts in const expressions.
The next one just handles the case https://github.com/rust-lang/rust/issues/148729 of where the type of the const was omitted at which point it was trying to treat a `type_const` again as a regular const. That obviously will fail since a type_const does not have a body.
@rustbot label +F-associated_const_equality +F-min_generic_const_args +I-ICE
Handling for inherent associated consts is missing elsewhere, remove so it can be handled later in that handling.
Diagnostic not always be emitted on associated constant
Add a test case and Fix for a different ICE I encountered.
I noticed when trying various permuations of the test case code to see if I could find anymore ICEs. I did, but not one that I expected. So in the instances of the a named const not having any args, insantiate it directly. Since it is likely an inherent assocaiated const.
Added tests.
Centralize the is_type_const() logic.
I also noticed basically the exact same check in other part the code.
Const blocks can't be a type_const, therefore this check is uneeded.
Fix comment spelling error.
get_all_attrs is not valid to call for all DefIds it seems.
Make sure that if the type is omitted for a type_const that we don't ICE.
Co-Authored-By: Boxy <rust@boxyuwu.dev>
THIR pattern building: Move all `thir::Pat` creation into `rustc_mir_build::thir::pattern`
The code in `thir::cx::block` was the only code outside of `thir::pattern` that created `thir::Pat` nodes.
Moving it makes the signature of `pat_from_hir` a bit messier, but ensures that all `thir::Pat` creation occurs in one module. That will be useful when trying to change `thir::Pat`, e.g. by adding fields to remove the need for wrapper nodes.
There should be no change to compiler output.
mir_build: Add a `SliceLenOp` enum for use by slice-length cases/tests
The main change in this PR introduces a `SliceLenOp` enum, to be used by the enum variants for slice-length tests in match lowering.
Slice-length tests must distinguish between `==` tests, and `>=` tests. The existing code represents this distinction with either a boolean (in `TestableCase`) or a `mir::BinOp` (in `TestKind`). Using a dedicated enum makes it easier to see what the two possible values represent.
This PR also includes a few relevant cleanups that would otherwise conflict.
r? Nadrieril
Fix ICE by rejecting const blocks in patterns during AST lowering (closes#148138)
This PR fixes the ICE reported in rust-lang/rust#148138.
The root cause is that `const` blocks aren’t allowed in pattern position, but the AST lowering logic still attempted to create `PatExprKind::ConstBlock`, allowing invalid HIR to reach type checking and trigger a `span_bug!`.
Following the discussion in the issue, this patch removes the `ConstBlock` lowering path from `lower_expr_within_pat`. Any `ExprKind::ConstBlock` inside a pattern is now handled consistently with other invalid pattern expressions.
A new UI test is included to ensure the compiler reports a proper error and to prevent regressions.
Closesrust-lang/rust#148138.
This fixes the ICE reported by rejecting `const` blocks in
pattern position during AST lowering.
Previously, `ExprKind::ConstBlock` could reach HIR as `PatExprKind::ConstBlock`,
allowing invalid patterns to be type-checked and triggering an ICE.
This patch removes the lowering path for const blocks in patterns
and emits a proper diagnostic instead.
A new UI test is added to ensure the compiler reports a regular error
and to prevent regressions.
mir_build: Classify `TestableCase::Constant` into multiple sub-kinds
In match lowering, when choosing a test for a `TestableCase::Constant`, there is some ad-hoc logic for inspecting the pattern type and deciding what kind of test is suitable. There is also some very similar logic later, when partitioning cases into buckets based on the chosen test.
Instead of having that ad-hoc logic in multiple places, I think it's better to perform an up-front classification when lowering `thir::PatKind::Constant` to `TestableCase::Constant`, and then have the later steps simply match on an enum variant.
There should be no change to the resulting built MIR.
(I will note that the logic/invariants involved are a bit unclear, so there is a risk of accidental minor differences.)
Replace Rvalue::NullaryOp by a variant in mir::Operand.
Based on https://github.com/rust-lang/rust/pull/148151
This PR fully removes the MIR `Rvalue::NullaryOp`. After rust-lang/rust#148151, it was only useful for runtime checks like `ub_checks`, `contract_checks` and `overflow_checks`.
These are "runtime" checks, boolean constants that may only be `true` in codegen. It depends on a rustc flag passed to codegen, so we need to represent those flags cross-crate.
This PR replaces those runtime checks by special variants in MIR `ConstValue`. This allows code that expects constants to manipulate those as such, even if we may not always be able to evaluate them to actual scalars.
Perf regression fix
The only thing changed from the previous PR is that I removed `output_is_inhabited` from hot path, and hide it behind condition, so now it will compute it less frequent
follow up on https://github.com/rust-lang/rust/pull/149664
mir_build: Don't use a mixture of THIR pattern kinds for pin-patterns
While looking for improvements to match-lowering, I had been trying to figure out why pin-patterns sometimes use `PatKind::Deref` and sometimes use `PatKind::DerefPattern`, which seemed confusing to me. In particular, both pattern kinds are being used with an inner type of `&` or `&mut`, which is very surprising.
The conclusion I came to was that `DerefPattern` (normally associated with calls to Deref/DerefMut) was only being used for *implicit* pin-patterns produced by match-ergonomics adjustment, whereas `Deref` was being used for explicit pin-patterns.
The inconsistency seems like a mistake to me, so this PR removes all uses of `DerefPattern` for pin-patterns, and consistently uses `PatKind::Deref` for pin-patterns instead. I'm not entirely happy with that outcome, because I think pin-patterns should probably have their own `thir::PatKind` variant, but this change will at least make that easier to achieve in a later PR.
r? Nadrieril
Make closure capturing have consistent and correct behaviour around patterns
Reference PR:
- https://github.com/rust-lang/reference/pull/1837
This PR has two goals:
- firstly, it fixesrust-lang/rust#137467. In order to do so, it needs to introduce a small breaking change surrounding the interaction of closure captures with matching against enums with uninhabited variants. Yes – to fix an ICE!
- this also fixesrust-lang/rust#138973, a slightly different case with the same root cause.
- likewise, fixesrust-lang/rust#140011.
- secondly, it fixesrust-lang/rust#137553, making the closure capturing rules consistent between `let` patterns and `match` patterns. This is new insta-stable behavior.
## Background
This change concerns how precise closure captures interact with patterns. As a little known feature, patterns that require inspecting only part of a value will only cause that part of the value to get captured:
```rust
fn main() {
let mut a = (21, 37);
// only captures a.0, writing to a.1 does not invalidate the closure
let mut f = || {
let (ref mut x, _) = a;
*x = 42;
};
a.1 = 69;
f();
}
```
I was not able to find any discussion of this behavior being introduced, or discussion of its edge-cases, but it is [documented in the Rust reference](https://doc.rust-lang.org/reference/types/closure.html#r-type.closure.capture.precision.wildcard).
The currently stable behavior is as follows:
- if any pattern contains a binding, the place it binds gets captured (implemented in current `walk_pat`)
- patterns in refutable positions (`match`, `if let`, `let ... else`, but not destructuring `let` or destructuring function parameters) get processed as follows (`maybe_read_scrutinee`):
- if matching against the pattern will at any point require inspecting a discriminant, or it includes a variable binding not followed by an ``@`-pattern,` capture *the entire scrutinee* by reference
You will note that this behavior is quite weird and it's hard to imagine a sensible rationale for at least some of its aspects. It has the following issues:
- firstly, it assumes that matching against an irrefutable pattern cannot possibly require inspecting any discriminants. With or-patterns, this isn't true, and it is the cause of the rust-lang/rust#137467 ICE.
- secondly, the presence of an ``@`-pattern` doesn't really have any semantics by itself. This is the weird behavior tracked as rust-lang/rust#137553.
- thirdly, the behavior is different between pattern-matching done through `let` and pattern-matching done through `match` – which is a superficial syntactic difference
This PR aims to address all of the above issues. The new behavior is as follows:
- like before, if a pattern contains a binding, the place it binds gets captured as required by the binding mode
- if matching against the pattern requires inspecting a disciminant, the place whose discriminant needs to be inspected gets captured by reference
"requires inspecting a discriminant" is also used here to mean "compare something with a constant" and other such decisions. For types other than ADTs, the details are not interesting and aren't changing.
## The breaking change
During closure capture analysis, matching an `enum` against a constructor is considered to require inspecting a discriminant if the `enum` has more than one variant. Notably, this is the case even if all the other variants happen to be uninhabited. This is motivated by implementation difficulties involved in querying whether types are inhabited before we're done with type inference – without moving mountains to make it happen, you hit this assert: 43f0014ef0/compiler/rustc_middle/src/ty/inhabitedness/mod.rs (L121)
Now, because the previous implementation did not concern itself with capturing the discriminants for irrefutable patterns at all, this is a breaking change – the following example, adapted from the testsuite, compiles on current stable, but will not compile with this PR:
```rust
#[derive(Clone, Copy, PartialEq, Eq, Debug)]
enum Void {}
pub fn main() {
let mut r = Result::<Void, (u32, u32)>::Err((0, 0));
let mut f = || {
let Err((ref mut a, _)) = r;
*a = 1;
};
let mut g = || {
//~^ ERROR: cannot borrow `r` as mutable more than once at a time
let Err((_, ref mut b)) = r;
*b = 2;
};
f();
g();
assert_eq!(r, Err((1, 2)));
}
```
## Is the breaking change necessary?
One other option would be to double down, and introduce a set of syntactic rules for determining whether a sub-pattern is in an irrefutable position, instead of querying the types and checking how many variants there are.
**This would not eliminate the breaking change,** but it would limit it to more contrived examples, such as
```rust
let ((true, Err((ref mut a, _, _))) | (false, Err((_, ref mut a, _)))) = x;
```
In this example, the `Err`s would not be considered in an irrefutable position, because they are part of an or-pattern. However, current stable would treat this just like a tuple `(bool, (T, U, _))`.
While introducing such a distinction would limit the impact, I would say that the added complexity would not be commensurate with the benefit it introduces.
## The new insta-stable behavior
If a pattern in a `match` expression or similar has parts it will never read, this part will not be captured anymore:
```rust
fn main() {
let mut a = (21, 37);
// now only captures a.0, instead of the whole a
let mut f = || {
match a {
(ref mut x, _) => *x = 42,
}
};
a.1 = 69;
f();
}
```
Note that this behavior was pretty much already present, but only accessible with this One Weird Trick™:
```rust
fn main() {
let mut a = (21, 37);
// both stable and this PR only capture a.0, because of the no-op `@-pattern`
let mut f = || {
match a {
(ref mut x @ _, _) => *x = 42,
}
};
a.1 = 69;
f();
}
```
## The second, more practically-relevant breaking change
After running crater, we have discovered that the aforementioned insta-stable behavior, where sometimes closures will now capture less, can also manifest as a breaking change. This is because it is possible that previously a closure would capture an entire struct by-move, and now it'll start capturing only part of it – some by move, and some by reference. This then causes the closure to have a more restrictive lifetime than it did previously.
See:
- https://github.com/rust-lang/rust/pull/138961#issuecomment-2761888557
- https://github.com/EC-labs/cec-assignment/pull/1
- https://github.com/tryandromeda/andromeda/pull/43
## Implementation notes
The PR has two main commits:
- "ExprUseVisitor: properly report discriminant reads" makes `walk_pat` perform all necessary capturing. This is the part that fixesrust-lang/rust#137467.
- "ExprUseVisitor: remove maybe_read_scrutinee" removes the unnecessary "capture the entire scrutinee" behavior, fixing rust-lang/rust#137553.
The new logic stops making the distinction between one particular example that used to work, and another ICE, tracked as rust-lang/rust#119786. As this requires an unstable feature, I am leaving this as future work.
