-
Notifications
You must be signed in to change notification settings - Fork 191
gccrs: fix ICE in TyVar constructor #4206
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
Conversation
gcc/rust/typecheck/rust-tyty-util.cc
Outdated
{ | ||
return; | ||
} |
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.
Usually for single statement, we skip the braces...
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.
Thanks for the review, @dkm
I've updated the code style to remove the unnecessary braces!
gcc/rust/typecheck/rust-tyty-util.cc
Outdated
if (!ok || lookup == nullptr) | ||
{ | ||
return nullptr; |
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.
ditto
gcc/rust/typecheck/rust-tyty-util.cc
Outdated
{ | ||
return TyVar::get_implicit_infer_var (UNKNOWN_LOCATION); | ||
} |
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.
ditto
gcc/rust/typecheck/rust-tyty-util.cc
Outdated
{ | ||
return TyVar::get_implicit_infer_var (UNKNOWN_LOCATION); | ||
} |
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.
ditto
28f220a
to
841af29
Compare
gcc/rust/ChangeLog: * typecheck/rust-tyty-util.cc (TyVar::TyVar): Add null check to avoid ICE. (TyVar::get_tyty): Return nullptr when lookup fails. (TyVar::clone): Handle null base type safely. (TyVar::monomorphized_clone): Add fallback for error types. gcc/testsuite/ChangeLog: * rust/compile/issue-3556.rs: New test. Signed-off-by: lishin <[email protected]>
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.
Actually really good idea on failure to return an implicit infer var... nice one
fix #3556
The changes ensure that type lookup failures no longer trigger assertions.
If further support for this pattern is needed, I'm happy to discuss possible approaches further!
gcc/rust/ChangeLog:
gcc/testsuite/ChangeLog: