-
Notifications
You must be signed in to change notification settings - Fork 228
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
Generic types #425
Generic types #425
Conversation
This looks like the right approach to me. Is something missing? How does it interact with lifetimes? |
I don't know whether this feature is now already sufficiently tested, or whether I should write an extra test somewhere, and if yes what it should contain.
So far only on the syntactical level. I didn't implement lifetime polymorphism here because I don't understand yet how they interact with the lifetimes of the |
Other derive macros are missing, e.g. for structs and enums. |
3da1e00
to
599cf04
Compare
Ok, this should be good to go now. I added type parameters to all cases where it made sense (unit enums didn't make sense), and added appropriate tests I think. |
Ping |
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 like where this is going. If we could also lift the restriction on a single lifetime, that would be great.
) -> GenericUntaggedEnum<GenericMap<i32, i32>, &str> { | ||
generic_untagged_enum | ||
} | ||
|
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.
A test with explicit lifetimes would be nice. Something along these lines maybe? Testing with lifetimes could be tricky, but a 'static
string might suffice.
#[derive(NifStruct)]
struct Slice<'a> {
my_str: &'a str
}
#[rustler::nif]
pub fn from_a_string_slice() -> Slice {
Slice { my_str: "hello world" }
}
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.
Do you mean an explicit lifetime together with a type parameter? I (hope I) haven't changed anything about the behaviour with lifetimes here. I'll send a short PR ahead that adds a lifetime test (since it seems there isn't one yet in test_codegen.rs
) and rebase onto that, then we know that I didn't break them.
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, seems like lifetimes are broken anyways, #428. This looks like an orthogonal issue to me.
Ping @evnu ;) |
Doesn't look like it. It's a too complicated issue, and orthogonal in content to this here. (But there will be merge conflicts) |
Clippy fails, but I didn't touch the failing code 😕 . The same with the formatter. |
@turion can you rebase on master? |
@evnu The lifetimes MR has changed so much that this PR here can basically be thrown away. I'm not even sure, maybe generic types are already handled now? |
|
Fixes #424 . I'm trying to implement support for generic/polymorphic types. Can you give me some feedback whether I'm on the right track? If yes, I'll proceed with the other macros.