Implement MVP for opaque generic const arguments#150823
Implement MVP for opaque generic const arguments#150823camelid wants to merge 5 commits intorust-lang:mainfrom
Conversation
|
TODO:
|
This comment has been minimized.
This comment has been minimized.
|
When we avoid normalizing type consts to opaque anon consts we need to make sure that the normalization is ambiguous in coherence instead of #![feature(min_generic_const_args, opaque_generic_const_args)]
#[type_const]
const FOO<const N: usize>: usize = const { N + 1 };
#[type_const]
const BAR<const N: usize>: usize = const { N + 1 };
trait Trait {}
impl Trait for [(); FOO::<1>] {}
impl Trait for [(); BAR::<1>] {}ie we don't want this code to pass due to the two opaque anon consts being unequal. |
|
Ooh good point 👍 |
This comment has been minimized.
This comment has been minimized.
|
HIR ty lowering was modified cc @fmease |
This comment has been minimized.
This comment has been minimized.
|
also need to add the coherence test :3 |
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This is meant to be the interim successor to generic const expressions. Essentially, const item RHS's will be allowed to do arbitrary const operations using generics. The limitation is that these const items will be treated opaquely, like ADTs in nominal typing, such that uses of them will only be equal if the same const item is referenced. In other words, two const items with the exact same RHS will not be considered equal. I also added some logic to check feature gates that depend on others being enabled (like oGCA depending on mGCA).
This comment has been minimized.
This comment has been minimized.
During coherence, OGCA consts should be normalized ambiguously because they are opaque but eventually resolved to a real value. We don't want two OGCAs that have the same value to be treated as distinct for coherence purposes. (Just like opaque types.)
|
This PR was rebased onto a different main commit. Here's a range-diff highlighting what actually changed. Rebasing is a normal part of keeping PRs up to date, so no action is needed—this note is just to help reviewers. |
|
The job Click to see the possible cause of the failure (guessed by this bot) |
| cx.type_of(free_alias.def_id).instantiate(cx, free_alias.args).into() | ||
| } else { | ||
| cx.const_of_item(free_alias.def_id).instantiate(cx, free_alias.args).into() | ||
| let ct = cx.const_of_item(free_alias.def_id).instantiate(cx, free_alias.args); |
There was a problem hiding this comment.
I think we basically don't want this change 🤔 The soundness critical thing for coherence is that normalizing oGCA anon consts is ambiguous. We handle this (incidentally) in normalize_anon_const which will also handle this when normalizing associated constants, not just free consts like we do here.
I would drop this change. I expect this will make diagnostics worse in coherence but I think that's fine for now. Or at least I think we should improve diagnostics of errors involving oGCA anon consts in a different way than complicating soundness critical normalization logic 🤔
There was a problem hiding this comment.
I realise I previously stated:
(less urgent:) don't normalize opaque const items (those with OGCA RHS)
but I disagree with myself now 😆 complicated normalization logic by doing this for diagnostics reasons is pretty meh
| &mut self, | ||
| goal: Goal<I, ty::NormalizesTo<I>>, | ||
| ) -> QueryResult<I> { | ||
| // FIXME(ogca): should we also stop ogca normalization during coherence here or just in normalize_free_alias? |
There was a problem hiding this comment.
The existing logic here implicitly gives ambiguity during coherence but also outside of coherence which I think is undesirable. In the failure case we should check if goal.predicate.alias.def_id is an oGCA anon const, and if so return NoSolution if we're outside of coherence, otherwise return AMBIGUOUS
If we don't do this I think we'll get type inference errors whenever the new solver tries to normalize oGCA anon consts 🤔 e.g.
//@ compile-flags: -Znext-solver
#![feature(generic_const_items, min_generic_const_args, opaque_generic_const_args)]
#[type_const]
const FOO<const N: usize>: usize = const { N + 1 };
fn foo() {
let a: [(); FOO::<1>];
}probably fails under your PR right now?
There was a problem hiding this comment.
i was wrong ^ the evaluate_const here is not try_evalaute_const from in rustc_trait_selection and implicitly turns the HasInfersOrParams cases into Ok. We need to check for oGCA consts here and return ambiguity explicitly in coherence
|
@rustbot author |
| // TODO: create tracking issue | ||
| (unstable, opaque_generic_const_args, "CURRENT_RUSTC_VERSION", None), |
There was a problem hiding this comment.
| // TODO: create tracking issue | |
| (unstable, opaque_generic_const_args, "CURRENT_RUSTC_VERSION", None), | |
| (unstable, opaque_generic_const_args, "CURRENT_RUSTC_VERSION", Some(151972)), |
|
☔ The latest upstream changes (presumably #152163) made this pull request unmergeable. Please resolve the merge conflicts. |
This is meant to be the interim successor to generic const expressions.
Essentially, const item RHS's will be allowed to do arbitrary const
operations using generics. The limitation is that these const items will
be treated opaquely, like ADTs in nominal typing, such that uses of them
will only be equal if the same const item is referenced. In other words,
two const items with the exact same RHS will not be considered equal.
I also added some logic to check feature gates that depend on others
being enabled (like oGCA depending on mGCA).
r? @BoxyUwU