Skip to content
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

Implement inline generics #212

Merged
merged 3 commits into from
Jan 29, 2024
Merged

Implement inline generics #212

merged 3 commits into from
Jan 29, 2024

Conversation

escritorio-gustavo
Copy link
Contributor

Implementation of inlined generics. Not 100% sure it works, but it did pass the #[ignore]d test

Fixes #56

@escritorio-gustavo escritorio-gustavo added bug Something isn't working P-HIGH This will be treated with high priority labels Jan 26, 2024
@escritorio-gustavo
Copy link
Contributor Author

This may have a flaw for the case where generics have both a trait bound and a default type, in which case it will keep the previous behavior. Not sure how to avoid this though

Copy link
Collaborator

@NyxCode NyxCode left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Besides maybe adding a small comment why there's this "null" literal (to make exporting generic types with T::<()>::decl() work, right?), this looks great!
Otherwise, this looks great, thanks!

@escritorio-gustavo
Copy link
Contributor Author

escritorio-gustavo commented Jan 29, 2024

to make exporting generic types with T::<()>::decl() work, right?

Yes, when exporting the struct itself, () is used as the default type, which turns into "null". When you give it a type argument in a struct field, it uses the type's .name()

@escritorio-gustavo
Copy link
Contributor Author

escritorio-gustavo commented Jan 29, 2024

This may have a flaw for the case where generics have both a trait bound and a default type

The reason for this is a test that has something along the lines of Struct<T: ToString>. In this case, the default type is not (), it is &'static str. I don't know how the default type is decided in this case, so I wasn't able to implement that, and even if I did, if someone were to use the default type (or any type where .name() has the same result) this implementation would just give them T (or whatever the name of the type param is)

@NyxCode
Copy link
Collaborator

NyxCode commented Jan 29, 2024

This may have a flaw for the case where generics have both a trait bound and a default type

The reason for this is a test that has something along the lines of Struct<T: ToString>. In this case, the default type is not (), it is &'static str. I don't know how the default type is decided in this case, so I wasn't able to implement that, and even if I did, if someone were to use the default type (or any type where .name() has the same result) this implementation would just give them T (or whatever the name of the type param is)

I think that's fine for now, this PR is still an improvement on the status quo. Maybe we should create an issue for this, together with an ignored test case, so that we dont forget about this?

@escritorio-gustavo
Copy link
Contributor Author

That might be the best way to go. Btw, I tried an aproach using std::any::Any and std::any::TypeId. That seems like the best way to check if two Rust types are equal, but I ran into lifetime problems (T is required to be 'static) so I went for this approach. Maybe that will be useful in the future?

@escritorio-gustavo
Copy link
Contributor Author

escritorio-gustavo commented Jan 29, 2024

This may have a flaw for the case where generics have both a trait bound and a default type, in which case it will keep the previous behavior. Not sure how to avoid this though

This actually happens in case either a default type or a trait bound exist, it's not necessary to have both at the same time

@escritorio-gustavo
Copy link
Contributor Author

The best way to solve this permanently would be a way to identify whether we are expanding a flatten/inline or an export.
flatten and inline should always emit T::name()
export should always emit T

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working P-HIGH This will be treated with high priority
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Support inline and flatten for generic types
2 participants