-
Notifications
You must be signed in to change notification settings - Fork 13.3k
Fix regression introduced with #99383 #99714
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
Merged
Merged
Changes from all commits
Commits
Show all changes
2 commits
Select commit
Hold shift + click to select a range
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,8 @@ | ||
// check-pass | ||
|
||
#![feature(type_alias_impl_trait)] | ||
type Opq = impl Sized; | ||
fn test() -> impl Iterator<Item = Opq> { | ||
Box::new(0..) as Box<dyn Iterator<Item = _>> | ||
} | ||
fn main(){} |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,7 @@ | ||
// check-pass | ||
|
||
fn test() -> impl Iterator<Item = impl Sized> { | ||
Box::new(0..) as Box<dyn Iterator<Item = _>> | ||
} | ||
|
||
fn main() {} |
4 changes: 3 additions & 1 deletion
4
src/test/ui/type-alias-impl-trait/issue-53398-cyclic-types.stderr
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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 one seems odd. The only change from adding this that I can envision is moving from an error to an ICE (via
delay_span_bug
due totake_opaque_types
not getting called.)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.
If I remove that we get this:
Backtrace
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.
uuuh... huh. that's super weird. I don't see how that relates to opaque types at all. Or how it relates to this function
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.
Should I inquire further or leave this as it is
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.
Yea, please investigate. During which test is this? The new one?
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.
Ah sorry, the backtrace I posted above is completely wrong... here is the new one (it's the same error as the original one). Code I am testing is this :
Backtrace
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 one:
just gives
error: unconstrained opaque type
error when I removewith_opaque_type_inference
.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.
Which query is that happening in? Use -Ztreat-err-as-bug to get a useful backtrace as well as a query backtrace.
If we're using the bubble scheme, I'd expect not using take_opaque_type to also ICE. If that doesn't happen, something is either broken, or... oh probably
probe
is involved, just like withevaluate_obligation
. If that is the case, usingwith_..inference
is ok, but both cases should have a comment and the non-evaluate_obligations
case needs to be set up to fail less extremely than with a type mismatch error (idk if that's possible easily, so we may have to leave that to a follow up)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.
Here is the backtrace:
Backtrace
I found this to be weird in
mir_borrowck
we infer withwith_opaque_type_inference(DefiningAnchor::Bind(hir_owner))
it still fails...So to make this pass we just need this change to stay.
But we don't use
take_opaque_type
here ? I checked probe it doesn't affect this.I don't understand this 😅
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... some borrowck code calls
type_op_prove_predicate
, but the only things it can return are success orNoSolution
. Without addingwith_opaque_type_inference(DefiningAnchor::Bubble)
, this query will fail.probe does affect this by removing all hidden types added within the probe ^^
I was trying to propose that we make
type_op_prove_predicate
work likeevaluate_obligation
and return something likeSucceededModuloOpaqueTypes
, but I now realize that's not actually helpful. What we could do instead is eitherCanonical<'tcx, QueryResponse
to contain all the data to equate what ever types came out of it with the opaque type and haveborrowck
thus figure out the hidden typesprobe
to lose them. Do we even need theprobe
?