-
Notifications
You must be signed in to change notification settings - Fork 23
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
Better support for FnPtr
...
#854
Conversation
@ranjitjhala -- Awesome, thanks. This pr fixes two of the issues related to FnPtr, and makes the rest into #829 errors. don't think we need any more FnPtr support from what I can see. |
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.
Ok, so this is unsound if you coerce a function definition to a function pointer. We can merge it but please create an issue so we can track the unsoundness
crates/flux-refineck/src/checker.rs
Outdated
} | ||
mir::CallKind::FnPtr { operand, .. } => { | ||
let ty = self.check_operand(infcx, env, terminator_span, operand)?; | ||
if let Some(BaseTy::FnPtr(fn_sig)) = ty.as_bty_skipping_existentials() { |
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.
Let's try not to skip existentials if we can avoid it
if let Some(BaseTy::FnPtr(fn_sig)) = ty.as_bty_skipping_existentials() { | |
if let TyKind::Indexed(BaseTy::FnPtr(fn_sig), _) = ty.kind() { |
If ty
is not directly a TyKind::Indexed
we should unpack it first.
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.
I'm assuming that if its already an Indexed
then unpack
will not do anything?
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.
Yes, if it's already Indexed no need to do unpack (it would be a noop)
fixes #807