Change a commit_if_ok call to probe

[JulianKnodt]

Removes an over-eager commit_if_ok which makes inference worse.

I'm not entirely sure whether it's ok to remove the check that types are the same, because casting seems to cause equality checks with incorrect types?

Fixes#105037

r? @BoxyUwU

[rustbot]

Some changes occurred in const_evaluatable.rs

cc @lcnr

[bors]

☔ The latest upstream changes (presumably #105365) made this pull request unmergeable. Please resolve the merge conflicts.

[BoxyUwU]

replacing this with a probe seems right, we dont want to constrain the infer vars in self.ct unless we're sure there are no other things that we could have unified with to consider it evaluatable. It's still not quite right though, we do have to actually constrain the infer vars that eq constraints at some point in order to ensure that the const is actually evaluatable.

What's needed is to have the visitor visit all the consts regardless of it succeeds in unifying with something, and if it succeed in unifying with something (in the probe) track that in Visitor. Once the visitor is finished you want to be able to see of all the consts that we were able to unify with, and if there was only one, redo the eq and select_all_or_error() outside of a probe.

[JulianKnodt]

I updated to check for only a single match and commit the change if there is a single one, but somehow this results in an error for the original test case. Is this an issue with the way I implemented it?

[BoxyUwU]

the current code only works for a single ConstEvaluatable predicate in the paramenv, i.e. if you had where [(); N +1 + 1]: and were trying to prove ConstEvalutable(N + _) it'd avoid unifying with 1 + 1 or 1 because either would work.
The test case you're working with has two separate things in param env: ConstEvaluatable(Table1<D>) and ConstEvaluatable(Table2<D>) so you'll also have to update the for loop to make you a single Option<Result<Const<'tcx>>, ()> and do the ocx.eq stuff after the for loop instead of inside

[BoxyUwU]

I think all the CI failures are caused by cases where we have duplicate candidates, i.e. two N + 1 in the param env, you probably want to avoid setting single_match to Err instead of Ok if the candidate that matched is == to the existing Ok

[BoxyUwU]

Can you add the following as tests:

---

#![feature(generic_const_exprs)]#![allow(incomplete_features)]fnfoo<constN:usize>()where[();N + 1]:,[();N + 1]:,{bar::<N>();}fnbar<constN:usize>()where[();N + 1]:,{}fnmain(){}

---

#![feature(generic_const_exprs)]#![allow(incomplete_features)]constfnboth(_:usize,b:usize) -> usize{
b
}fnfoo<constN:usize>()where[();both(N + 1,N + 1)]:,{bar::<N>();}fnbar<constN:usize>()where[();N + 1]:,{}fnmain(){}

---

#![feature(generic_const_exprs)]#![allow(incomplete_features)]constfnboth(_:usize,b:usize) -> usize{
b
}fnfoo<constN:usize,constM:usize>() -> [();N + 2]where[();both(N + 1,M + 1)]:,{bar()}fnbar<constN:usize>() -> [();N]where[();N + 1]:,{[();N]}fnmain(){}

---

[BoxyUwU]

will r+ this once its rebased

[BoxyUwU]

@bors r+ rollup

[bors]

📌 Commit 21c5ffe has been approved by BoxyUwU

It is now in the queue for this repository.