-
Notifications
You must be signed in to change notification settings - Fork 12.9k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Split out lit_to_const_for_patterns
#116251
Conversation
Some changes occurred in cc @BoxyUwU Some changes might have occurred in exhaustiveness checking cc @Nadrieril |
query lit_to_const( | ||
key: LitToConstInput<'tcx> | ||
) -> Result<ty::Const<'tcx>, LitToConstError> { | ||
) -> Option<ty::Const<'tcx>> { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this query could alternatively just return an (option of) valtree to make it clear what it's doing
@@ -8,6 +8,66 @@ use crate::build::parse_float_into_scalar; | |||
pub(crate) fn lit_to_const<'tcx>( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
naming: we could call this lit_to_const_eager
or something, to help make it clear that it's trying to eagerly convert lits when possible, but will hold off from doing so if not possible (i.e. we don't have the right type info).
let LitToConstInput { lit, ty, neg } = lit_input; | ||
|
||
let valtree = match (lit, &ty.kind()) { | ||
(ast::LitKind::Str(s, _), ty::Ref(_, inner_ty, _)) if inner_ty.is_str() => { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this function could ignore the types in the litkinds that don't actually matter?
☔ The latest upstream changes (presumably #116260) made this pull request unmergeable. Please resolve the merge conflicts. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Though generally i'm not confident having two queries here is fun lol
// This is currently not possible to use projections as const generics. | ||
// More information about this available here: | ||
// https://github.com/rust-lang/rust/pull/104443#discussion_r1029375633 | ||
// build-pass |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
imo this should be a revision, we want to keep asserting that aliases dont work as the type of const params without adt_const_param i think
@@ -117,7 +117,11 @@ fn recurse_build<'tcx>( | |||
} | |||
&ExprKind::Literal { lit, neg } => { | |||
let sp = node.span; | |||
match tcx.at(sp).lit_to_const(LitToConstInput { lit: &lit.node, ty: node.ty, neg }) { | |||
match tcx.at(sp).lit_to_const_for_patterns(LitToConstInput { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this is definitely not a pattern xd we'd end up here for things like the 1
in foo::<{ N + 1 }>();
don't have the time to bring this back into cache. my understanding is that we should not be so type-dependent in lit_to_const, because we may receive unnormalized typed. |
This allows us to delay normalizing const types in adt_const_params, fixing
tests/ui/const-generics/projection-as-arg-const.rs
r? @BoxyUwU
Let me know if anything needs more comments, more validation or tests.