-
Notifications
You must be signed in to change notification settings - Fork 182
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
Add FnDef
to TypeName
#449
Conversation
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 looks really good! The main thing I think we might want is to try and cleanup the program clause generation code, but maybe that's better left for another PR.
(I'm also pondering the implied bounds generation logic, but I think it's probably what we want, even though it can't actually be observed by users at the moment.)
7651ead
to
7ca82d1
Compare
Thanks for the helpful feedback! I've added the implied bounds test and cleaned up the program clause generation code quite a bit, so it should be ready for re-review. |
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.
Requested a few nits! Looking very close to me.
} | ||
|
||
#[derive(Clone, Debug, PartialEq, Eq, Hash, Fold, HasInterner)] | ||
pub struct FnDefDatumBound<I: Interner> { |
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 feel like we are going to need .. well, eventually we'll have to add ABI and other details, but maybe we can add that together with adding the same details to the function pointer types
73a456e
to
826055d
Compare
r=me but this needs to be rebased. |
And we should probably file a followup issue regarding ABI details (which apply equally to function pointers) |
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.
r=me but requires rebase and we should break out a follow-up issue regarding handling ABIs, both in these types and function pointers.
Simplifies creation of `FnDefDatum` and `StructDatum` program clauses
826055d
to
4b23e07
Compare
Done and done! |
Addresses the
FnDef
part of #368.Lowering and program clause generation are probably the areas that need the most review. I'm also a bit unsure about if/how the builtin traits apply to
FnDef
, so some guidance on that would be helpful. For parsing I've chosen to only handle function declarations for simplicity's sake (plus I'm not sure why we would need to inspect the function bodies).In its current state the PR could use some refactoring/polish, but I'll defer that until the more substantive aspects of the PR are in good shape.