-
-
Notifications
You must be signed in to change notification settings - Fork 14.7k
FindParamInClause handle edge-cases
#153614
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
base: main
Are you sure you want to change the base?
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 |
|---|---|---|
|
|
@@ -9,7 +9,6 @@ use derive_where::derive_where; | |
| use rustc_type_ir::inherent::*; | ||
| use rustc_type_ir::lang_items::SolverTraitLangItem; | ||
| use rustc_type_ir::search_graph::CandidateHeadUsages; | ||
| use rustc_type_ir::solve::Certainty::Maybe; | ||
| use rustc_type_ir::solve::{AliasBoundKind, SizedTraitKind}; | ||
| use rustc_type_ir::{ | ||
| self as ty, Interner, TypeFlags, TypeFoldable, TypeFolder, TypeSuperFoldable, | ||
|
|
@@ -1286,28 +1285,34 @@ where | |
| return ControlFlow::Break(Err(NoSolution)); | ||
| }; | ||
|
|
||
| if let ty::Placeholder(p) = ty.kind() { | ||
| if p.universe() == ty::UniverseIndex::ROOT { | ||
| ControlFlow::Break(Ok(Certainty::Yes)) | ||
| } else { | ||
| ControlFlow::Continue(()) | ||
| match ty.kind() { | ||
| ty::Placeholder(p) => { | ||
| if p.universe() == ty::UniverseIndex::ROOT { | ||
| ControlFlow::Break(Ok(Certainty::Yes)) | ||
| } else { | ||
| ControlFlow::Continue(()) | ||
| } | ||
| } | ||
| } else if ty.has_type_flags(TypeFlags::HAS_PLACEHOLDER | TypeFlags::HAS_RE_INFER) { | ||
| self.recursion_depth += 1; | ||
| if self.recursion_depth > self.ecx.cx().recursion_limit() { | ||
| return ControlFlow::Break(Ok(Maybe { | ||
| cause: MaybeCause::Overflow { | ||
| suggest_increasing_limit: true, | ||
| keep_constraints: false, | ||
| }, | ||
| opaque_types_jank: OpaqueTypesJank::AllGood, | ||
| })); | ||
| ty::Infer(_) => ControlFlow::Break(Ok(Certainty::AMBIGUOUS)), | ||
| _ if ty.has_type_flags( | ||
| TypeFlags::HAS_PLACEHOLDER | TypeFlags::HAS_INFER | TypeFlags::HAS_ALIAS, | ||
| ) => | ||
| { | ||
| self.recursion_depth += 1; | ||
| if self.recursion_depth > self.ecx.cx().recursion_limit() { | ||
| return ControlFlow::Break(Ok(Certainty::Maybe { | ||
| cause: MaybeCause::Overflow { | ||
| suggest_increasing_limit: true, | ||
| keep_constraints: false, | ||
| }, | ||
| opaque_types_jank: OpaqueTypesJank::AllGood, | ||
| })); | ||
| } | ||
| let result = ty.super_visit_with(self); | ||
| self.recursion_depth -= 1; | ||
| result | ||
| } | ||
| let result = ty.super_visit_with(self); | ||
| self.recursion_depth -= 1; | ||
| result | ||
| } else { | ||
| ControlFlow::Continue(()) | ||
| _ => ControlFlow::Continue(()), | ||
| } | ||
| } | ||
|
|
||
|
|
@@ -1317,16 +1322,22 @@ where | |
| return ControlFlow::Break(Err(NoSolution)); | ||
| }; | ||
|
|
||
| if let ty::ConstKind::Placeholder(p) = ct.kind() { | ||
| if p.universe() == ty::UniverseIndex::ROOT { | ||
| ControlFlow::Break(Ok(Certainty::Yes)) | ||
| } else { | ||
| ControlFlow::Continue(()) | ||
| match ct.kind() { | ||
| ty::ConstKind::Placeholder(p) => { | ||
| if p.universe() == ty::UniverseIndex::ROOT { | ||
| ControlFlow::Break(Ok(Certainty::Yes)) | ||
| } else { | ||
| ControlFlow::Continue(()) | ||
| } | ||
| } | ||
| } else if ct.has_type_flags(TypeFlags::HAS_PLACEHOLDER | TypeFlags::HAS_RE_INFER) { | ||
| ct.super_visit_with(self) | ||
| } else { | ||
| ControlFlow::Continue(()) | ||
| ty::ConstKind::Infer(_) => ControlFlow::Break(Ok(Certainty::AMBIGUOUS)), | ||
| _ if ct.has_type_flags( | ||
| TypeFlags::HAS_PLACEHOLDER | TypeFlags::HAS_INFER | TypeFlags::HAS_ALIAS, | ||
| ) => | ||
| { | ||
| ct.super_visit_with(self) | ||
|
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. why the not doing recursion depth stuff here 🤔
Contributor
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. existing xd :3 because the only way constants recurse is through types atm
Contributor
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. I guess this has to change with mgca
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. up to u if u want to handle it in this PR, it'd be a fairly reasonable |
||
| } | ||
| _ => ControlFlow::Continue(()), | ||
| } | ||
| } | ||
|
|
||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,19 @@ | ||
| error[E0308]: mismatched types | ||
| --> $DIR/is-global-norm-concrete-alias-to-generic.rs:26:5 | ||
| | | ||
| LL | fn foo<T>(x: <(*const T,) as Id>::This) -> (*const T,) | ||
| | ----------- expected `(*const T,)` because of return type | ||
| ... | ||
| LL | x | ||
| | ^ expected `(*const T,)`, found associated type | ||
| | | ||
| = note: expected tuple `(*const T,)` | ||
| found associated type `<(*const T,) as Id>::This` | ||
| = help: consider constraining the associated type `<(*const T,) as Id>::This` to `(*const T,)` | ||
| = note: for more information, visit https://doc.rust-lang.org/book/ch19-03-advanced-traits.html | ||
| = note: the associated type `<(*const T,) as Id>::This` is defined as `(*const T,)` in the implementation, but the where-bound `(*const T,)` shadows this definition | ||
| see issue #152409 <https://github.com/rust-lang/rust/issues/152409> for more information | ||
|
|
||
| error: aborting due to 1 previous error | ||
|
|
||
| For more information about this error, try `rustc --explain E0308`. |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,19 @@ | ||
| error[E0308]: mismatched types | ||
| --> $DIR/is-global-norm-concrete-alias-to-generic.rs:26:5 | ||
| | | ||
| LL | fn foo<T>(x: <(*const T,) as Id>::This) -> (*const T,) | ||
| | ----------- expected `(*const T,)` because of return type | ||
| ... | ||
| LL | x | ||
| | ^ types differ | ||
| | | ||
| = note: expected tuple `(*const T,)` | ||
| found associated type `<(*const T,) as Id>::This` | ||
| = help: consider constraining the associated type `<(*const T,) as Id>::This` to `(*const T,)` | ||
| = note: for more information, visit https://doc.rust-lang.org/book/ch19-03-advanced-traits.html | ||
| = note: the associated type `<(*const T,) as Id>::This` is defined as `(*const T,)` in the implementation, but the where-bound `(*const T,)` shadows this definition | ||
| see issue #152409 <https://github.com/rust-lang/rust/issues/152409> for more information | ||
|
|
||
| error: aborting due to 1 previous error | ||
|
|
||
| For more information about this error, try `rustc --explain E0308`. |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,29 @@ | ||
| //@ revisions: current next | ||
| //@ ignore-compare-mode-next-solver (explicit revisions) | ||
| //@[next] compile-flags: -Znext-solver | ||
|
|
||
| // A regression test making sure that where-bounds with concrete aliases | ||
| // which normalize to something mentioning a generic parameters are | ||
| // considered non-global. | ||
| // | ||
| // When checking this, we previously didn't recur into types if they didn't | ||
| // mention any generic parameters, causing us to consider the `(<() as Id>::This,): Id` | ||
| // where-bound as global, even though it normalizes to `(T,): Id`. | ||
|
|
||
| trait Id { | ||
| type This; | ||
| } | ||
|
|
||
| impl<T> Id for T { | ||
| type This = T; | ||
| } | ||
|
|
||
| fn foo<T>(x: <(*const T,) as Id>::This) -> (*const T,) | ||
| where | ||
| (): Id<This = *const T>, | ||
| (<() as Id>::This,): Id, | ||
| { | ||
| x //~ ERROR mismatched types | ||
| } | ||
|
|
||
| fn main() {} |
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.
so there are two meaningful changes here:
recursing if there are aliases makes sense (that's certainly the point of this PR) but I don't fully understand why we care about infer vars or not.
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.
to treat ambiguous normalization as
Global. You could imagine normalizingT::AssoctoVec<T::AmbiguousAlias>which ends up asVec<?infer>I guess we could always force
GLobalif there are infer vars instead of recursingThere 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.
do we have a test with ambiguous aliases? I guess you could only really get ambiguity here via overflow (are opaque types relevant?)?