Const-to-pattern-to-MIR cleanup

[RalfJung]

Now that all uses of constants without structural equality are hard errors, there's a bunch of cleanup we can do in the code that handles patterns: we can always funnel patterns through valtrees first (rather than having a fallback path for when valtree construction fails), and we can make sure that if we emit a PartialEq call it is not calling anything user-defined.

To keep the error messages the same, I made valtree construction failures return the information of which type it is that cannot be valtree'd. search_for_structural_match_violation is now not needed any more at all, so I removed it.

r? @oli-obk

[rustbot]

Some changes occurred in exhaustiveness checking

cc @Nadrieril

These commits modify the Cargo.lock file. Unintentional changes to Cargo.lock can be introduced when switching branches and rebasing PRs.

If this was unintentional then you should revert the changes before this PR is merged.
Otherwise, you can ignore this comment.

Some changes occurred to the core trait solver

cc @rust-lang/initiative-trait-system-refactor

Some changes occurred to the CTFE / Miri engine

cc @rust-lang/miri

Some changes occurred in match lowering

cc @Nadrieril

Some changes occurred in match checking

cc @Nadrieril

[RalfJung]

This code looks a bit odd... we create an instance only to immediately take it apart again. Can this be simplified further? We have def_id and args already before the let instance = ..., can we just use those directly? But there are some specific error cases up there that I don't know how to handle then, in particular AssocConstInPattern.

[RalfJung]

If I remove the Instance::try_resolve, then the only change in the tests is that "associated consts cannot be referenced in patterns" becomes "constant pattern depends on a generic parameter". Which seems okay? It does depend on a generic parameter:

pub fn test<A: Foo, B: Foo>(arg: EFoo, A::X: EFoo) {
    //~^ ERROR associated consts cannot be referenced in patterns
    let A::X = arg;
    //~^ ERROR associated consts cannot be referenced in patterns
}

If the associated const can be resolved, I am fairly sure we already accept that code, so the general claim "associated consts cannot be referenced in patterns" is already not true I think.

Any objections to changing the error message for that case?

EDIT: I have now done this in the 4th commit.

[lcnr]

I think that change is fine 👍

[rustbot]

Some changes occurred in src/tools/clippy

cc @rust-lang/clippy

[RalfJung]

I was not able to reproduce any ICEs here with the new version that directly constructs an unevaluated const from the args. So I removed the normalization step. If that can cause ICEs I am sure matthiaskrgr's fuzzer will find them quickly, and then we'll have a test. ;)

[lcnr]

use the mir::interpret::EvalToValTreeResult type alias here 🤔

[RalfJung]

Is that a good idea? This seems like a very dangerous trait to implement, it just transmutes some lifetimes around, so maybe it'd be dangerous to just change the type alias without realizing that this also changes this here.

Unfortunately the EraseType trait has no comment at all, and to my great surprise it is a safe trait, so I really don't understand what happens here.

[lcnr]

can this check inlined_const_as_pat.references_error() instead? I think we only set saw_const_match_error when also replacing that pattern with Pat::Error

[RalfJung]

I had no idea that function existed.^^ Yes, that's a great way to get rid of this annoying Cell.

[RalfJung]

Hm, it doesn't seem to work though. The error is shown now even if we already previously showed other errors, e.g.

error: to use a constant of type `MyType` in a pattern, `MyType` must be annotated with `#[derive(PartialEq)]`
  --> $DIR/const-partial_eq-fallback-ice.rs:14:12
   |
LL |     if let CONSTANT = &&MyType {
   |            ^^^^^^^^
   |
   = note: the traits must be derived, manual `impl`s are not sufficient
   = note: see https://doc.rust-lang.org/stable/std/marker/trait.StructuralPartialEq.html for details

error: to use a constant of type `&&MyType` in a pattern, the type must implement `PartialEq`
  --> $DIR/const-partial_eq-fallback-ice.rs:14:12
   |
LL |     if let CONSTANT = &&MyType {
   |            ^^^^^^^^

error: aborting due to 2 previous errors

[lcnr]

that is very confusing to me and I can't see how that would happen 🤔 all the cases which previously set saw_const_match_error returns as PatKind::Error and afaict we never discard subpattern 🤔 could you revert the commit and test with assert_eq!(!inlined_const_as_pat.references_error(), saw_const_match_error.get().is_none(), "pat={inlined_const_as_pat:?}");

[oli-obk]

The problem is that TypeVisitable does not have a dedicated method for ErrorGuaranteed, so only errors in types, constants and lifetimes are actually found.

[lcnr]

ah 🤔 should we add that method? it feels useful and is surprising otherwise 😊

[oli-obk]

#127808 should fix this

[lcnr]

really happy with these changes 😊 I haven't interacted with this area recently so I don't feel fully confident in signing this off myself, would like oli to do a final review

[oli-obk]

r=me,lcnr after #127808

[RalfJung]

#127808 landed and tests now pass locally, so all looking good. :)

@bors r=oli-obk,lcnr

[bors]

📌 Commit 303a2db has been approved by oli-obk,lcnr

It is now in the queue for this repository.