explicitly track inherent const generic args kind - #161929
Conversation
|
Some changes occurred to the CTFE / Miri interpreter cc @rust-lang/miri, @RalfJung, @oli-obk, @lcnr Some changes occurred in compiler/rustc_sanitizers cc @rcvalle changes to the core type system cc @lcnr Some changes occurred in match checking cc @Nadrieril This PR changes rustc_public cc @oli-obk, @celinval, @ouz-a, @makai410 Some changes occurred in cc @BoxyUwU HIR ty lowering was modified cc @fmease changes to the core type system cc @lcnr |
This comment has been minimized.
This comment has been minimized.
|
it is mildly annoying that (will fix later in a batch update with any PR feedback) |
ebbc13d to
7d5a1c7
Compare
| tcx, | ||
| trait_ty.def_id, | ||
| rebased_args, | ||
| ty::AliasConstInherentArgsKind::Impl, |
There was a problem hiding this comment.
Why impl here. does this even encounter inherent associated consts 🤔
There was a problem hiding this comment.
correct, the value here is "dead code", there's a ty::AssocContainer::InherentImpl => bug!() above. ideally, we would not use new_from_def_id and instead construct the variants directly (and so wouldn't have to specify what happens for inherents), but, leaving that as the FIXME note already on new_from_def_id and doing that Later(tm)
anyway, I arbitrarily chose AliasConstInherentArgsKind::Impl because the args being passed in are rebased_args, i.e. impl-format args.
There was a problem hiding this comment.
can you add a comment saying that this is theoretically unused (or better yet also assert that we don't encounter an Inherent assoc const defid here)
There was a problem hiding this comment.
wait what am I even doing, I'm just gonna fix the fixme and do explicit construction in this one spot, it's always a ty here, never a const, so it's just, ty::AliasTerm::new(tcx, ty::AliasTermKind::ProjectionTy { def_id: trait_ty.def_id }, rebased_args).
ty::AliasTerm::new asserts the defid's defkind too (thanks to this very PR) so no need to assert here.
|
|
||
| fn check_args_compatible(self, def_id: DefId, args: ty::GenericArgsRef<'tcx>) -> bool { | ||
| self.check_args_compatible(def_id, args) | ||
| fn check_term_args_compatible( |
There was a problem hiding this comment.
can we call this check_alias_term_args_compatible same w/ the debug assert one. i got confused when reading the call sites of them thinking this was more general than for just aliases :3
| match alias_const.kind { | ||
| ty::AliasConstKind::Inherent { .. } => { | ||
| ty::AliasConstKind::InherentSelf { .. } | ||
| | ty::AliasConstKind::InherentImpl { .. } => { |
There was a problem hiding this comment.
I dont undestand how this can be correct 🤔 add_wf_preds_for_inherent_projection definitely expects Self form args since it converts them to impl form
There was a problem hiding this comment.
add_wf_preds_for_inherent_projection calls compute_inherent_assoc_term_args which returns the args as a no-op if it's already InherentConstImpl, so it could technically function. (compute_inherent_assoc_term_args returning early is kinda bad readability, the return is lexically buried in a match statement, I was considering adding a comment pointing out the early return to make it more visible, or something)
however, replacing this with ty::AliasConstKind::InherentImpl { .. } => panic!("blah") and running tests shows it's unused. I wrote in the PR description:
and finally, I think some of these match statements could theoretically
bug!on one form or the other of InherentSelf/InherentImpl, but for now I'm intentionally being a bit conservative here, we can tighten up later.
to be honest, I'm not 100% sure if this ought to be a panic or if this is theoretically reachable, if you're confident it's unreachable I can replace it with a bug!.
There was a problem hiding this comment.
Making it a bug! and then waiting for a test case for if it's reachable seems reasonable to me
|
@rustbot author |
7d5a1c7 to
892c6bb
Compare
|
@bors r+ |
…-args, r=BoxyUwU explicitly track inherent const generic args kind in doing so, and as an exercise of this new code, implement regular (non-type-const) inherent consts in the type system under `feature(generic_const_args)` see the new big doc comment in `compiler/rustc_type_ir/src/const_kind.rs` if you dunno what the heck I'm on about with "self args" vs "impl args" ✨ on a small note, the FIXME on `alias_term_kind_from_def_id` becomes even more relevant with this PR, `ty::AliasConstInherentArgsKind` is kinda gross tbh. explicitly not refactoring that in this PR though, to keep scope/the diff manageable. also, `check_args_compatible` is very spooky scary in that if you have a `ty::Alias`, you ought to use `check_term_args_compatible`, but nothing's stopping you from calling `check_args_compatible` with the term's DefId. I was unable to think up a clever API that would prevent this misuse. and finally, I think some of these match statements could theoretically `bug!` on one form or the other of InherentSelf/InherentImpl, but for now I'm intentionally being a bit conservative here, we can tighten up later. --- relevant tracking issue: rust-lang/project-const-generics#98 also very related to `feature(inherent_associated_types)`: rust-lang#8995 rust-lang/project-const-generics#71 relevant PR that intentionally failed to implement inherent consts in the type system due to not tracking this: rust-lang#155341 implementing support for a trait with a regular const being refined with an impl that has a directly represented RHS is extremely annoying and difficult without this, the lack of support of which causes rust-lang#161264 (was working on adding support when I got sidetracked with this PR), see also this zulip thread: [#project-const-generics > implementing assoc consts as direct args](https://rust-lang.zulipchat.com/#narrow/channel/260443-project-const-generics/topic/implementing.20assoc.20consts.20as.20direct.20args/with/618953051) (is there an issue for this?) r? @BoxyUwU
…-args, r=BoxyUwU explicitly track inherent const generic args kind in doing so, and as an exercise of this new code, implement regular (non-type-const) inherent consts in the type system under `feature(generic_const_args)` see the new big doc comment in `compiler/rustc_type_ir/src/const_kind.rs` if you dunno what the heck I'm on about with "self args" vs "impl args" ✨ on a small note, the FIXME on `alias_term_kind_from_def_id` becomes even more relevant with this PR, `ty::AliasConstInherentArgsKind` is kinda gross tbh. explicitly not refactoring that in this PR though, to keep scope/the diff manageable. also, `check_args_compatible` is very spooky scary in that if you have a `ty::Alias`, you ought to use `check_term_args_compatible`, but nothing's stopping you from calling `check_args_compatible` with the term's DefId. I was unable to think up a clever API that would prevent this misuse. and finally, I think some of these match statements could theoretically `bug!` on one form or the other of InherentSelf/InherentImpl, but for now I'm intentionally being a bit conservative here, we can tighten up later. --- relevant tracking issue: rust-lang/project-const-generics#98 also very related to `feature(inherent_associated_types)`: rust-lang#8995 rust-lang/project-const-generics#71 relevant PR that intentionally failed to implement inherent consts in the type system due to not tracking this: rust-lang#155341 implementing support for a trait with a regular const being refined with an impl that has a directly represented RHS is extremely annoying and difficult without this, the lack of support of which causes rust-lang#161264 (was working on adding support when I got sidetracked with this PR), see also this zulip thread: [#project-const-generics > implementing assoc consts as direct args](https://rust-lang.zulipchat.com/#narrow/channel/260443-project-const-generics/topic/implementing.20assoc.20consts.20as.20direct.20args/with/618953051) (is there an issue for this?) r? @BoxyUwU
…-args, r=BoxyUwU explicitly track inherent const generic args kind in doing so, and as an exercise of this new code, implement regular (non-type-const) inherent consts in the type system under `feature(generic_const_args)` see the new big doc comment in `compiler/rustc_type_ir/src/const_kind.rs` if you dunno what the heck I'm on about with "self args" vs "impl args" ✨ on a small note, the FIXME on `alias_term_kind_from_def_id` becomes even more relevant with this PR, `ty::AliasConstInherentArgsKind` is kinda gross tbh. explicitly not refactoring that in this PR though, to keep scope/the diff manageable. also, `check_args_compatible` is very spooky scary in that if you have a `ty::Alias`, you ought to use `check_term_args_compatible`, but nothing's stopping you from calling `check_args_compatible` with the term's DefId. I was unable to think up a clever API that would prevent this misuse. and finally, I think some of these match statements could theoretically `bug!` on one form or the other of InherentSelf/InherentImpl, but for now I'm intentionally being a bit conservative here, we can tighten up later. --- relevant tracking issue: rust-lang/project-const-generics#98 also very related to `feature(inherent_associated_types)`: rust-lang#8995 rust-lang/project-const-generics#71 relevant PR that intentionally failed to implement inherent consts in the type system due to not tracking this: rust-lang#155341 implementing support for a trait with a regular const being refined with an impl that has a directly represented RHS is extremely annoying and difficult without this, the lack of support of which causes rust-lang#161264 (was working on adding support when I got sidetracked with this PR), see also this zulip thread: [#project-const-generics > implementing assoc consts as direct args](https://rust-lang.zulipchat.com/#narrow/channel/260443-project-const-generics/topic/implementing.20assoc.20consts.20as.20direct.20args/with/618953051) (is there an issue for this?) r? @BoxyUwU
…-args, r=BoxyUwU explicitly track inherent const generic args kind in doing so, and as an exercise of this new code, implement regular (non-type-const) inherent consts in the type system under `feature(generic_const_args)` see the new big doc comment in `compiler/rustc_type_ir/src/const_kind.rs` if you dunno what the heck I'm on about with "self args" vs "impl args" ✨ on a small note, the FIXME on `alias_term_kind_from_def_id` becomes even more relevant with this PR, `ty::AliasConstInherentArgsKind` is kinda gross tbh. explicitly not refactoring that in this PR though, to keep scope/the diff manageable. also, `check_args_compatible` is very spooky scary in that if you have a `ty::Alias`, you ought to use `check_term_args_compatible`, but nothing's stopping you from calling `check_args_compatible` with the term's DefId. I was unable to think up a clever API that would prevent this misuse. and finally, I think some of these match statements could theoretically `bug!` on one form or the other of InherentSelf/InherentImpl, but for now I'm intentionally being a bit conservative here, we can tighten up later. --- relevant tracking issue: rust-lang/project-const-generics#98 also very related to `feature(inherent_associated_types)`: rust-lang#8995 rust-lang/project-const-generics#71 relevant PR that intentionally failed to implement inherent consts in the type system due to not tracking this: rust-lang#155341 implementing support for a trait with a regular const being refined with an impl that has a directly represented RHS is extremely annoying and difficult without this, the lack of support of which causes rust-lang#161264 (was working on adding support when I got sidetracked with this PR), see also this zulip thread: [#project-const-generics > implementing assoc consts as direct args](https://rust-lang.zulipchat.com/#narrow/channel/260443-project-const-generics/topic/implementing.20assoc.20consts.20as.20direct.20args/with/618953051) (is there an issue for this?) r? @BoxyUwU
…-args, r=BoxyUwU explicitly track inherent const generic args kind in doing so, and as an exercise of this new code, implement regular (non-type-const) inherent consts in the type system under `feature(generic_const_args)` see the new big doc comment in `compiler/rustc_type_ir/src/const_kind.rs` if you dunno what the heck I'm on about with "self args" vs "impl args" ✨ on a small note, the FIXME on `alias_term_kind_from_def_id` becomes even more relevant with this PR, `ty::AliasConstInherentArgsKind` is kinda gross tbh. explicitly not refactoring that in this PR though, to keep scope/the diff manageable. also, `check_args_compatible` is very spooky scary in that if you have a `ty::Alias`, you ought to use `check_term_args_compatible`, but nothing's stopping you from calling `check_args_compatible` with the term's DefId. I was unable to think up a clever API that would prevent this misuse. and finally, I think some of these match statements could theoretically `bug!` on one form or the other of InherentSelf/InherentImpl, but for now I'm intentionally being a bit conservative here, we can tighten up later. --- relevant tracking issue: rust-lang/project-const-generics#98 also very related to `feature(inherent_associated_types)`: rust-lang#8995 rust-lang/project-const-generics#71 relevant PR that intentionally failed to implement inherent consts in the type system due to not tracking this: rust-lang#155341 implementing support for a trait with a regular const being refined with an impl that has a directly represented RHS is extremely annoying and difficult without this, the lack of support of which causes rust-lang#161264 (was working on adding support when I got sidetracked with this PR), see also this zulip thread: [#project-const-generics > implementing assoc consts as direct args](https://rust-lang.zulipchat.com/#narrow/channel/260443-project-const-generics/topic/implementing.20assoc.20consts.20as.20direct.20args/with/618953051) (is there an issue for this?) r? @BoxyUwU
Rollup merge of #161929 - khyperia:explicitly-track-inherent-args, r=BoxyUwU explicitly track inherent const generic args kind in doing so, and as an exercise of this new code, implement regular (non-type-const) inherent consts in the type system under `feature(generic_const_args)` see the new big doc comment in `compiler/rustc_type_ir/src/const_kind.rs` if you dunno what the heck I'm on about with "self args" vs "impl args" ✨ on a small note, the FIXME on `alias_term_kind_from_def_id` becomes even more relevant with this PR, `ty::AliasConstInherentArgsKind` is kinda gross tbh. explicitly not refactoring that in this PR though, to keep scope/the diff manageable. also, `check_args_compatible` is very spooky scary in that if you have a `ty::Alias`, you ought to use `check_term_args_compatible`, but nothing's stopping you from calling `check_args_compatible` with the term's DefId. I was unable to think up a clever API that would prevent this misuse. and finally, I think some of these match statements could theoretically `bug!` on one form or the other of InherentSelf/InherentImpl, but for now I'm intentionally being a bit conservative here, we can tighten up later. --- relevant tracking issue: rust-lang/project-const-generics#98 also very related to `feature(inherent_associated_types)`: #8995 rust-lang/project-const-generics#71 relevant PR that intentionally failed to implement inherent consts in the type system due to not tracking this: #155341 implementing support for a trait with a regular const being refined with an impl that has a directly represented RHS is extremely annoying and difficult without this, the lack of support of which causes #161264 (was working on adding support when I got sidetracked with this PR), see also this zulip thread: [#project-const-generics > implementing assoc consts as direct args](https://rust-lang.zulipchat.com/#narrow/channel/260443-project-const-generics/topic/implementing.20assoc.20consts.20as.20direct.20args/with/618953051) (is there an issue for this?) r? @BoxyUwU
| def_id: Self::DefId, | ||
| inherent_args: ty::AliasConstInherentArgsKind, | ||
| ) -> ty::AliasConstKind<'tcx> { | ||
| match self.def_kind(def_id) { |
There was a problem hiding this comment.
why does this function exist? I guess we pass AliasConstInherentArgsKind in places which could be inherent constants but also other DefKinds?
There was a problem hiding this comment.
yh, i remember having a similar question because it feels like kind of a code smell having the inherent_args param in cases where you might not actually have inherent aliases. but we have places that work with any def-id alias and the codepath needs ot handle both inherents and others iirc
There was a problem hiding this comment.
yeah, there's been a FIXME to yeet it for ages. from the PR description:
on a small note, the FIXME on
alias_term_kind_from_def_idbecomes even more relevant with this PR,ty::AliasConstInherentArgsKindis kinda gross tbh. explicitly not refactoring that in this PR though, to keep scope/the diff manageable.
(the FIXME reads // FIXME: remove in favor of explicit construction)
it's just, vaguely smelly and takes a chunk of refactoring effort to clean up the callsites, and nobody's bothered to do that yet. iirc it's mainly lowering, which, is a tangled mess that's been making my life very difficult whenever I've touched it for IAT stuff and whatnot. tl;dr it passes around Res/DefIds in a bunch of places where it really ought to be passing around AliasTerm or Term or whatever. Because it passes around DefId instead of AliasTerm, this alias_term_kind_from_def_id method is called. This lowering mess is the cause of #160844 among other things.
|
Finished benchmarking commit (f26b04a): comparison URL. Overall result: ✅ improvements - no action neededBenchmarking means the PR may be perf-sensitive. It's automatically marked not fit for rolling up. Overriding is possible but disadvised: it risks changing compiler perf. @bors rollup=never rustc-perf Instruction countOur most reliable metric. Used to determine the overall result above. However, even this metric can be noisy.
Max RSS (memory usage)Results (secondary -2.5%)A less reliable metric. May be of interest, but not used to determine the overall result above.
CyclesResults (secondary -4.5%)A less reliable metric. May be of interest, but not used to determine the overall result above.
Binary sizeThis perf run didn't have relevant results for this metric. Bootstrap: missing data |
| // please ping khyperia and/or BoxyUwU if this `bug!` fires | ||
| ty::AliasConstKind::InherentImpl { .. } => bug!( | ||
| "This ought to be unreachable, the entrypoints of WF should still have InherentSelf-form alias consts." | ||
| ), |
| ty::AliasConstKind::InherentSelf { .. } => { | ||
| panic!( | ||
| "AliasConst::type_of got InherentSelf - args should always be InherentImpl at this point" | ||
| ) | ||
| } | ||
| ty::AliasConstKind::InherentImpl { def_id } => def_id.into(), |
wait what the heck? uuuuh sure I guess, that's unexpected |
It seems like you're in invoking the for projection-caching (opt, incr-full, llvm, x64):
for nalgebra-0.33.0 (check, incr-full, llvm, x64):
|
fix ICE in project_goals/inherent I'm a silly goose. The next solver has the exact same bug as rust-lang#161858 - merely enabling next-solver on the test added in that PR causes an ICE 🙃 ICE was technically introduced by rust-lang#161929 but this bug has always been present, it's just that PR explicitly tracked things better and ICEd on the bug instead of silently continuing. fixes rust-lang#162147 explanation: `push_const_arg_has_type_goal` expects its term to have rebased, `impl`-format args, not `Self` format args. See doc comment on `AliasConstKind::InherentSelf` for what "impl format" and "self format" mean: https://doc.rust-lang.org/nightly/nightly-rustc/rustc_type_ir/enum.AliasConstKind.html#variant.InherentSelf r? @BoxyUwU since we chatted about this yesterday but honestly anyone vaguely t-types and/or const-generics feel free to review as well, should be relatively straightforward!
fix ICE in project_goals/inherent I'm a silly goose. The next solver has the exact same bug as rust-lang#161858 - merely enabling next-solver on the test added in that PR causes an ICE 🙃 ICE was technically introduced by rust-lang#161929 but this bug has always been present, it's just that PR explicitly tracked things better and ICEd on the bug instead of silently continuing. fixes rust-lang#162147 explanation: `push_const_arg_has_type_goal` expects its term to have rebased, `impl`-format args, not `Self` format args. See doc comment on `AliasConstKind::InherentSelf` for what "impl format" and "self format" mean: https://doc.rust-lang.org/nightly/nightly-rustc/rustc_type_ir/enum.AliasConstKind.html#variant.InherentSelf r? @BoxyUwU since we chatted about this yesterday but honestly anyone vaguely t-types and/or const-generics feel free to review as well, should be relatively straightforward!
fix ICE in project_goals/inherent I'm a silly goose. The next solver has the exact same bug as rust-lang#161858 - merely enabling next-solver on the test added in that PR causes an ICE 🙃 ICE was technically introduced by rust-lang#161929 but this bug has always been present, it's just that PR explicitly tracked things better and ICEd on the bug instead of silently continuing. fixes rust-lang#162147 explanation: `push_const_arg_has_type_goal` expects its term to have rebased, `impl`-format args, not `Self` format args. See doc comment on `AliasConstKind::InherentSelf` for what "impl format" and "self format" mean: https://doc.rust-lang.org/nightly/nightly-rustc/rustc_type_ir/enum.AliasConstKind.html#variant.InherentSelf r? @BoxyUwU since we chatted about this yesterday but honestly anyone vaguely t-types and/or const-generics feel free to review as well, should be relatively straightforward!
fix ICE in project_goals/inherent I'm a silly goose. The next solver has the exact same bug as rust-lang#161858 - merely enabling next-solver on the test added in that PR causes an ICE 🙃 ICE was technically introduced by rust-lang#161929 but this bug has always been present, it's just that PR explicitly tracked things better and ICEd on the bug instead of silently continuing. fixes rust-lang#162147 explanation: `push_const_arg_has_type_goal` expects its term to have rebased, `impl`-format args, not `Self` format args. See doc comment on `AliasConstKind::InherentSelf` for what "impl format" and "self format" mean: https://doc.rust-lang.org/nightly/nightly-rustc/rustc_type_ir/enum.AliasConstKind.html#variant.InherentSelf r? @BoxyUwU since we chatted about this yesterday but honestly anyone vaguely t-types and/or const-generics feel free to review as well, should be relatively straightforward!
fix supposedly unreachable `bug!` being reachable `bug!` introduced in rust-lang#161929 fixes rust-lang#162146 wfcheck.rs does a normalize on a type here https://github.com/rust-lang/rust/blob/edc52f87c28f328c61685a02c47887a5cec7d767/compiler/rustc_hir_analysis/src/check/wfcheck.rs#L929 which reduces the contained alias within from a nonrigid InherentSelf to a rigid InherentImpl, because we do so upon encountering a too-generic-to-ctfe alias the very next line, it then `register_wf_obligation` on the resulting normalized type, that contains an InherentImpl inside wf, that eventually hits the `bug!` I added and ICEs https://github.com/rust-lang/rust/blob/a4330234a776684c36428d001721d0320d24dd77/compiler/rustc_trait_selection/src/traits/wf.rs#L1104 r? @BoxyUwU
…uwer Rollup of 16 pull requests Successful merges: - rust-lang/rust#137720 (support `#[target_feature(enable = ...)]` on `#[naked]` functions) - rust-lang/rust#160534 (stabilize smart pointer map functions) - rust-lang/rust#160551 (mir_build: Clearly distinguish or/refutable/irrefutable patterns during match lowering) - rust-lang/rust#160989 (Make sin, cos, exp, exp2, log, log2, log10 generic) - rust-lang/rust#161861 (mir_build: Clarify parts of if-condition lowering) - rust-lang/rust#161929 (explicitly track inherent const generic args kind) - rust-lang/rust#162040 (bootstrap: stage0 to cbae9b4cae2b108f6a3d18cfe6075714bb739463) - rust-lang/rust#162063 (Switch dist-aarch64-linux to EC2 and update dist-x86_64-linux) - rust-lang/rust#161353 (Add test for parallel compiler reproducible build) - rust-lang/rust#161937 (A series of Polonius Alpha refactors) - rust-lang/rust#162051 (`rustc_feature` cleanups) - rust-lang/rust#162055 (remove `_{style}` recovery for diagnostic structs) - rust-lang/rust#162075 (Move track_caller on closures gating to attribute parsing) - rust-lang/rust#162079 (std: implement `File::fsync` for Hermit) - rust-lang/rust#162097 (Deduplicate `InstrumentFnAttr`) - rust-lang/rust#162115 (fix typo in feature documentation)
fix ICE in project_goals/inherent I'm a silly goose. The next solver has the exact same bug as rust-lang#161858 - merely enabling next-solver on the test added in that PR causes an ICE 🙃 ICE was technically introduced by rust-lang#161929 but this bug has always been present, it's just that PR explicitly tracked things better and ICEd on the bug instead of silently continuing. fixes rust-lang#162147 explanation: `push_const_arg_has_type_goal` expects its term to have rebased, `impl`-format args, not `Self` format args. See doc comment on `AliasConstKind::InherentSelf` for what "impl format" and "self format" mean: https://doc.rust-lang.org/nightly/nightly-rustc/rustc_type_ir/enum.AliasConstKind.html#variant.InherentSelf r? @BoxyUwU since we chatted about this yesterday but honestly anyone vaguely t-types and/or const-generics feel free to review as well, should be relatively straightforward!
fix supposedly unreachable `bug!` being reachable `bug!` introduced in rust-lang#161929 fixes rust-lang#162146 wfcheck.rs does a normalize on a type here https://github.com/rust-lang/rust/blob/edc52f87c28f328c61685a02c47887a5cec7d767/compiler/rustc_hir_analysis/src/check/wfcheck.rs#L929 which reduces the contained alias within from a nonrigid InherentSelf to a rigid InherentImpl, because we do so upon encountering a too-generic-to-ctfe alias the very next line, it then `register_wf_obligation` on the resulting normalized type, that contains an InherentImpl inside wf, that eventually hits the `bug!` I added and ICEs https://github.com/rust-lang/rust/blob/a4330234a776684c36428d001721d0320d24dd77/compiler/rustc_trait_selection/src/traits/wf.rs#L1104 r? @BoxyUwU
…uwer Rollup of 16 pull requests Successful merges: - rust-lang/rust#137720 (support `#[target_feature(enable = ...)]` on `#[naked]` functions) - rust-lang/rust#160534 (stabilize smart pointer map functions) - rust-lang/rust#160551 (mir_build: Clearly distinguish or/refutable/irrefutable patterns during match lowering) - rust-lang/rust#160989 (Make sin, cos, exp, exp2, log, log2, log10 generic) - rust-lang/rust#161861 (mir_build: Clarify parts of if-condition lowering) - rust-lang/rust#161929 (explicitly track inherent const generic args kind) - rust-lang/rust#162040 (bootstrap: stage0 to cbae9b4cae2b108f6a3d18cfe6075714bb739463) - rust-lang/rust#162063 (Switch dist-aarch64-linux to EC2 and update dist-x86_64-linux) - rust-lang/rust#161353 (Add test for parallel compiler reproducible build) - rust-lang/rust#161937 (A series of Polonius Alpha refactors) - rust-lang/rust#162051 (`rustc_feature` cleanups) - rust-lang/rust#162055 (remove `_{style}` recovery for diagnostic structs) - rust-lang/rust#162075 (Move track_caller on closures gating to attribute parsing) - rust-lang/rust#162079 (std: implement `File::fsync` for Hermit) - rust-lang/rust#162097 (Deduplicate `InstrumentFnAttr`) - rust-lang/rust#162115 (fix typo in feature documentation)
fix ICE in project_goals/inherent I'm a silly goose. The next solver has the exact same bug as rust-lang#161858 - merely enabling next-solver on the test added in that PR causes an ICE 🙃 ICE was technically introduced by rust-lang#161929 but this bug has always been present, it's just that PR explicitly tracked things better and ICEd on the bug instead of silently continuing. fixes rust-lang#162147 explanation: `push_const_arg_has_type_goal` expects its term to have rebased, `impl`-format args, not `Self` format args. See doc comment on `AliasConstKind::InherentSelf` for what "impl format" and "self format" mean: https://doc.rust-lang.org/nightly/nightly-rustc/rustc_type_ir/enum.AliasConstKind.html#variant.InherentSelf r? @BoxyUwU since we chatted about this yesterday but honestly anyone vaguely t-types and/or const-generics feel free to review as well, should be relatively straightforward!
fix supposedly unreachable `bug!` being reachable `bug!` introduced in rust-lang#161929 fixes rust-lang#162146 wfcheck.rs does a normalize on a type here https://github.com/rust-lang/rust/blob/edc52f87c28f328c61685a02c47887a5cec7d767/compiler/rustc_hir_analysis/src/check/wfcheck.rs#L929 which reduces the contained alias within from a nonrigid InherentSelf to a rigid InherentImpl, because we do so upon encountering a too-generic-to-ctfe alias the very next line, it then `register_wf_obligation` on the resulting normalized type, that contains an InherentImpl inside wf, that eventually hits the `bug!` I added and ICEs https://github.com/rust-lang/rust/blob/a4330234a776684c36428d001721d0320d24dd77/compiler/rustc_trait_selection/src/traits/wf.rs#L1104 r? @BoxyUwU
fix ICE in project_goals/inherent I'm a silly goose. The next solver has the exact same bug as rust-lang#161858 - merely enabling next-solver on the test added in that PR causes an ICE 🙃 ICE was technically introduced by rust-lang#161929 but this bug has always been present, it's just that PR explicitly tracked things better and ICEd on the bug instead of silently continuing. fixes rust-lang#162147 explanation: `push_const_arg_has_type_goal` expects its term to have rebased, `impl`-format args, not `Self` format args. See doc comment on `AliasConstKind::InherentSelf` for what "impl format" and "self format" mean: https://doc.rust-lang.org/nightly/nightly-rustc/rustc_type_ir/enum.AliasConstKind.html#variant.InherentSelf r? @BoxyUwU since we chatted about this yesterday but honestly anyone vaguely t-types and/or const-generics feel free to review as well, should be relatively straightforward!
fix supposedly unreachable `bug!` being reachable `bug!` introduced in rust-lang#161929 fixes rust-lang#162146 wfcheck.rs does a normalize on a type here https://github.com/rust-lang/rust/blob/edc52f87c28f328c61685a02c47887a5cec7d767/compiler/rustc_hir_analysis/src/check/wfcheck.rs#L929 which reduces the contained alias within from a nonrigid InherentSelf to a rigid InherentImpl, because we do so upon encountering a too-generic-to-ctfe alias the very next line, it then `register_wf_obligation` on the resulting normalized type, that contains an InherentImpl inside wf, that eventually hits the `bug!` I added and ICEs https://github.com/rust-lang/rust/blob/a4330234a776684c36428d001721d0320d24dd77/compiler/rustc_trait_selection/src/traits/wf.rs#L1104 r? @BoxyUwU
fix ICE in project_goals/inherent I'm a silly goose. The next solver has the exact same bug as rust-lang#161858 - merely enabling next-solver on the test added in that PR causes an ICE 🙃 ICE was technically introduced by rust-lang#161929 but this bug has always been present, it's just that PR explicitly tracked things better and ICEd on the bug instead of silently continuing. fixes rust-lang#162147 explanation: `push_const_arg_has_type_goal` expects its term to have rebased, `impl`-format args, not `Self` format args. See doc comment on `AliasConstKind::InherentSelf` for what "impl format" and "self format" mean: https://doc.rust-lang.org/nightly/nightly-rustc/rustc_type_ir/enum.AliasConstKind.html#variant.InherentSelf r? @BoxyUwU since we chatted about this yesterday but honestly anyone vaguely t-types and/or const-generics feel free to review as well, should be relatively straightforward!
fix supposedly unreachable `bug!` being reachable `bug!` introduced in rust-lang#161929 fixes rust-lang#162146 wfcheck.rs does a normalize on a type here https://github.com/rust-lang/rust/blob/edc52f87c28f328c61685a02c47887a5cec7d767/compiler/rustc_hir_analysis/src/check/wfcheck.rs#L929 which reduces the contained alias within from a nonrigid InherentSelf to a rigid InherentImpl, because we do so upon encountering a too-generic-to-ctfe alias the very next line, it then `register_wf_obligation` on the resulting normalized type, that contains an InherentImpl inside wf, that eventually hits the `bug!` I added and ICEs https://github.com/rust-lang/rust/blob/a4330234a776684c36428d001721d0320d24dd77/compiler/rustc_trait_selection/src/traits/wf.rs#L1104 r? @BoxyUwU
fix ICE in project_goals/inherent I'm a silly goose. The next solver has the exact same bug as rust-lang#161858 - merely enabling next-solver on the test added in that PR causes an ICE 🙃 ICE was technically introduced by rust-lang#161929 but this bug has always been present, it's just that PR explicitly tracked things better and ICEd on the bug instead of silently continuing. fixes rust-lang#162147 explanation: `push_const_arg_has_type_goal` expects its term to have rebased, `impl`-format args, not `Self` format args. See doc comment on `AliasConstKind::InherentSelf` for what "impl format" and "self format" mean: https://doc.rust-lang.org/nightly/nightly-rustc/rustc_type_ir/enum.AliasConstKind.html#variant.InherentSelf r? @BoxyUwU since we chatted about this yesterday but honestly anyone vaguely t-types and/or const-generics feel free to review as well, should be relatively straightforward!
fix supposedly unreachable `bug!` being reachable `bug!` introduced in rust-lang#161929 fixes rust-lang#162146 wfcheck.rs does a normalize on a type here https://github.com/rust-lang/rust/blob/edc52f87c28f328c61685a02c47887a5cec7d767/compiler/rustc_hir_analysis/src/check/wfcheck.rs#L929 which reduces the contained alias within from a nonrigid InherentSelf to a rigid InherentImpl, because we do so upon encountering a too-generic-to-ctfe alias the very next line, it then `register_wf_obligation` on the resulting normalized type, that contains an InherentImpl inside wf, that eventually hits the `bug!` I added and ICEs https://github.com/rust-lang/rust/blob/a4330234a776684c36428d001721d0320d24dd77/compiler/rustc_trait_selection/src/traits/wf.rs#L1104 r? @BoxyUwU
fix ICE in project_goals/inherent I'm a silly goose. The next solver has the exact same bug as rust-lang#161858 - merely enabling next-solver on the test added in that PR causes an ICE 🙃 ICE was technically introduced by rust-lang#161929 but this bug has always been present, it's just that PR explicitly tracked things better and ICEd on the bug instead of silently continuing. fixes rust-lang#162147 explanation: `push_const_arg_has_type_goal` expects its term to have rebased, `impl`-format args, not `Self` format args. See doc comment on `AliasConstKind::InherentSelf` for what "impl format" and "self format" mean: https://doc.rust-lang.org/nightly/nightly-rustc/rustc_type_ir/enum.AliasConstKind.html#variant.InherentSelf r? @BoxyUwU since we chatted about this yesterday but honestly anyone vaguely t-types and/or const-generics feel free to review as well, should be relatively straightforward!
View all comments
in doing so, and as an exercise of this new code, implement regular (non-type-const) inherent consts in the type system under
feature(generic_const_args)see the new big doc comment in
compiler/rustc_type_ir/src/const_kind.rsif you dunno what the heck I'm on about with "self args" vs "impl args" ✨on a small note, the FIXME on
alias_term_kind_from_def_idbecomes even more relevant with this PR,ty::AliasConstInherentArgsKindis kinda gross tbh. explicitly not refactoring that in this PR though, to keep scope/the diff manageable.also,
check_args_compatibleis very spooky scary in that if you have aty::Alias, you ought to usecheck_term_args_compatible, but nothing's stopping you from callingcheck_args_compatiblewith the term's DefId. I was unable to think up a clever API that would prevent this misuse.and finally, I think some of these match statements could theoretically
bug!on one form or the other of InherentSelf/InherentImpl, but for now I'm intentionally being a bit conservative here, we can tighten up later.relevant tracking issue: rust-lang/project-const-generics#98
also very related to
feature(inherent_associated_types): #8995 rust-lang/project-const-generics#71relevant PR that intentionally failed to implement inherent consts in the type system due to not tracking this: #155341
implementing support for a trait with a regular const being refined with an impl that has a directly represented RHS is extremely annoying and difficult without this, the lack of support of which causes #161264 (was working on adding support when I got sidetracked with this PR), see also this zulip thread: #project-const-generics > implementing assoc consts as direct args (is there an issue for this?)
r? @BoxyUwU