-
-
Notifications
You must be signed in to change notification settings - Fork 15.5k
fix supposedly unreachable bug! being reachable
#162173
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -504,7 +504,16 @@ impl<'a, 'tcx> WfPredicates<'a, 'tcx> { | |
| // (*) The predicates of an inherent associated type include the | ||
| // predicates of the impl that it's contained in. | ||
|
|
||
| if !data.self_ty().has_escaping_bound_vars() { | ||
| // In an ideal world, there are no escaping bound vars here. However, WF is jank, and | ||
| // sometimes there are. We can only `compute_inherent_assoc_term_args` if the Self ty in the | ||
| // args has no escaping bound vars. If we already have impl format args, though, | ||
| // `compute_inherent_assoc_term_args` is a no-op (and we have no Self type), so no need to | ||
| // check for escaping bound vars. | ||
| let can_compute_impl_args = | ||
| matches!(data.kind, ty::AliasTermKind::InherentConstImpl { .. }) | ||
| || !data.self_ty().has_escaping_bound_vars(); | ||
|
|
||
| if can_compute_impl_args { | ||
| // FIXME(inherent_associated_types): Should this happen inside of a snapshot? | ||
| // FIXME(inherent_associated_types): This is incompatible with the new solver and lazy norm! | ||
| let args = traits::project::compute_inherent_assoc_term_args( | ||
|
|
@@ -1099,10 +1108,12 @@ impl<'a, 'tcx> TypeVisitor<TyCtxt<'tcx>> for WfPredicates<'a, 'tcx> { | |
| self.add_wf_preds_for_inherent_projection(alias_const.into()); | ||
| return; // Subtree is handled by above function | ||
| } | ||
| // 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." | ||
| ), | ||
| // FIXME: This should be unreachable but isn't because we normalize in item | ||
| // wfck before computing wf requirements | ||
|
Comment on lines
+1111
to
+1112
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. isn't this more generally true? as in "FIXME(#100041): This should be unreachable, except that we sometimes compute the WF requirements for normalized types" This is not just a thing in WF checking I think. It may also affect e.g. #100041 (comment) 🤔 For MIR typeck we fetch WF obligations for both normalized and unnormalized types
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. probably, that makes sense to me at least! boop @BoxyUwU who wrote this (I just copied it into my PR here)
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. ah right there are other type system bugs |
||
| ty::AliasConstKind::InherentImpl { .. } => { | ||
| self.add_wf_preds_for_inherent_projection(alias_const.into()); | ||
| return; | ||
| } | ||
| ty::AliasConstKind::Projection { def_id } | ||
| | ty::AliasConstKind::Free { def_id } | ||
| | ty::AliasConstKind::Anon { def_id } => { | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,10 @@ | ||
| error: `generic_const_args` requires -Znext-solver=globally to be enabled | ||
| --> $DIR/wf-inherentimpl.rs:7:12 | ||
| | | ||
| LL | #![feature(generic_const_args, min_generic_const_args)] | ||
| | ^^^^^^^^^^^^^^^^^^ | ||
| | | ||
| = help: enable all of these features | ||
|
|
||
| error: aborting due to 1 previous error | ||
|
|
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,16 @@ | ||
| //@[next] check-pass | ||
| //@ revisions: next old | ||
| //@[next] compile-flags: -Znext-solver | ||
| //@ ignore-compare-mode-next-solver (explicit revisions) | ||
| #![feature(inherent_associated_types)] | ||
| #![feature(macroless_generic_const_args)] | ||
| #![feature(generic_const_args, min_generic_const_args)] | ||
| //[old]~^ ERROR `generic_const_args` requires -Znext-solver=globally to be enabled | ||
| struct Foo<const A: usize>; | ||
| impl<const A: usize> Foo<A> { | ||
| const SIZE: usize = { todo!() }; | ||
| fn to_bytes() -> [u8; Self::SIZE] { | ||
| todo!() | ||
| } | ||
| } | ||
| fn main() {} |
Uh oh!
There was an error while loading. Please reload this page.
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.
comment on why ignoring escaping bound vars is fine with inherent impl form
View changes since the review