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

Suggestion: Add #[inline(always)] to most derives #115

Closed
Sympatron opened this issue Dec 20, 2023 · 9 comments · Fixed by #117
Closed

Suggestion: Add #[inline(always)] to most derives #115

Sympatron opened this issue Dec 20, 2023 · 9 comments · Fixed by #117

Comments

@Sympatron
Copy link

Since most derives don't really contain any logic, it would be best to mark then as #[inline] or #[inline(always)] to allow them to be easily optimized away across crates and make them zero cost abstractions.

If you are interested I could work on a PR.

@greyblake
Copy link
Owner

I know that #[inline] is used to instruct compiler to inline functions.
How does it apply to derives? Could you please elaborate on this more?

@Sympatron
Copy link
Author

I mean the implementations of traits nutype generates should have an #[inline] attribute if the are trivial.

@greyblake
Copy link
Owner

I interpret that as "inline functions of traits that nutype generates". (please correct me if that interpretation is wrong).

As far as I know Rust is good at determining what should be inlined automatically.

Could you provide a measuarable example, where inlining of trait methods/functions generated by nutype makes a noticeable difference in performance?

@greyblake
Copy link
Owner

Ok, I checked the implementation generated by Debug and some traits from derive_more. I see what you mean.
If you're fine you can proceed with a PR.
Thanks!

@greyblake
Copy link
Owner

@Sympatron Hi, are you still planning to open a PR for this?

@Sympatron
Copy link
Author

Since I discovered a few other problems with nutype for my use case, I don't really use nutype anymore. So I don't plan on submitting a PR at the moment.

@greyblake
Copy link
Owner

@Sympatron Thanks for the update, no problem at all.
Would you mind describing the problems you have discovered?

@Sympatron
Copy link
Author

It was mostly not very no-std friendly and does not add a const constructor. Apart from that nutype focuses a lot on validation which is probably super useful in some cases, but I just wanted a thin wrapper around a u32 to prevent accidental misuse of my API. So it just wasn't the right fit for me.

@greyblake
Copy link
Owner

@Sympatron Hi, I just published 0.4.1-beta.1 which comes with some support of no_std.
Note:

  • String type is still not supported in no_std environment.
  • Use default-features = false for nutype dependency

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 a pull request may close this issue.

2 participants