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

Always implement Default for Vec #63

Merged
merged 3 commits into from
Mar 18, 2024

Conversation

schneiderfelipe
Copy link
Contributor

I propose to always implement Default for Vec.

This makes it behave more like std::vec::Vec<T>, which implements Default regardless of T.

@@ -116,6 +116,7 @@ impl Input {
match meta.path.get_ident() {
Some(ident) => {
assert!(ident != "Copy", "can not derive Copy for SoA vectors");
assert!(ident != "Default", "Default is already derived for SoA vectors");
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This makes this PR a breaking change.

Copy link
Member

@Luthaf Luthaf left a comment

Choose a reason for hiding this comment

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

Could you also add default for the slice and slice_mut types? And then, we can ignore Default in soa_derive without error!

@schneiderfelipe
Copy link
Contributor Author

@Luthaf nice, I'll take a look at this and #64 somewhere this week.

This will silently ignore Default derive requests as well.
Copy link
Member

@Luthaf Luthaf left a comment

Choose a reason for hiding this comment

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

Thanks!

@Luthaf Luthaf merged commit c6ef5a6 into lumol-org:master Mar 18, 2024
5 checks passed
@schneiderfelipe schneiderfelipe deleted the impl-default branch March 18, 2024 11:06
@Luthaf Luthaf mentioned this pull request May 22, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants