Prevent generic pattern types from being used in libstd #136584
Conversation
|
changes to the core type system |
rustbot
added
S-waiting-on-review
This comment has been minimized.
This comment has been minimized.
oli-obk
force-pushed
the
pattern-types-generic
branch
from
b025089 to
f12b963
Compare
| self.out.push(traits::Obligation::with_depth( | ||
| tcx, | ||
| cause.clone(), | ||
| self.recursion_depth, | ||
| self.param_env, | ||
| ty::Binder::dummy(ty::PredicateKind::Clause( | ||
| ty::ClauseKind::ConstArgHasType(c, subty), | ||
| )), | ||
| )); |
There was a problem hiding this comment.
For now this check is redundant, but it may not be anymore in the future, so I put it here just in case
There was a problem hiding this comment.
This feels somewhat structurally wrong to me 🤔 I think I would expect us to want a more general way to require a pattern to be of a specific type in order for a pattern type to be wf. So pretty much PatArgHasType similar to how we have ConstArgHasType. Kind of thinking about this as if we had pattern generics and is was some struct Is<T, pat P: T>(/* builtin */) and so we need to be checking the range pattern is correctly typed.
I think this is in practice correct though- we're just skipping having a PatArgHasType. I think for wf(E..E2) to hold we'd require PatArgHasType(E..E2, T) which itself would require ConstArgHasType(E, T) ConstArgHasType(E2, T) which is just what you're doing here 🤔 And if we dont have a general pattern generics system, only these TyKind::Pat we might aswell just not add a new predicate and inline the logic into wf predicates for TyKind::Pat 👍
lgtm
| let c = self.tcx().normalize_erasing_regions( | ||
| self.infcx.typing_env(self.param_env), | ||
| c, | ||
| ); | ||
| if c.has_aliases() || c.has_param() { |
There was a problem hiding this comment.
This is a bit fishy, but I needed more than ClauseKind::ConstEvaluatable. ConstEvaluatable only checks that the constant itself will not fail to evaluate, which a generic parameter will also not (due to how we're enforcing things with const generics elsewhere).
So I took the knowledge that these constants always have concrete types and will normalize to error constants if they have errors themselves, to just evaluate here and check if there are still params or aliases around after normalization.
Oh and I need to normalize, can't just check for aliases and params, since we treat any unevaluated const as an alias, even if it refers to a free const. Which I guess makes sense as we want to cause normalization to actually get to the const and evaluate it.
There was a problem hiding this comment.
I think you can/should just use c.has_param() and forget about even normalizing- min const generics doesn't permit Foo<{ <T as Trait>::USIZE }> where the assoc const can be normalized to usize regardless of T so I think design wise it's fine to not be "clever" here and just say that referring to generic parameters syntactically is forbidden.
There was a problem hiding this comment.
I'm not sure why you need to check for aliases 🤔
|
r? @BoxyUwU unless you specifically want a review from lcnr? though my understanding was that they are not particularly involved with pattern types |
BoxyUwU
added
S-waiting-on-author
Pattern types should follow the same rules that patterns follow. So a pattern type range must not wrap and not be empty. While we reject such invalid ranges at layout computation time, that only happens during monomorphization in the case of const generics. This is the exact same issue as other const generic math has, and since there's no solution there yet, I put these pattern types behind a separate incomplete feature.
These are not necessary for the pattern types MVP (replacing the layout range attributes in libcore and rustc).
cc #136574 (new tracking issue for the
generic_pattern_typesfeature gate)r? @lcnr
cc @scottmcm @joshtriplett