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

Make Float work with no_std #50

Closed
wants to merge 3 commits into from
Closed

Make Float work with no_std #50

wants to merge 3 commits into from

Conversation

vkahl
Copy link
Contributor

@vkahl vkahl commented Mar 22, 2022

The Float macro is referencing ::std::num::FpCategory and thus doesn't work with no_std yet. Replacing it with ::core::num::FpCategory shouldn't break any existing code but provides no_std compatibility when needed.

After doing this change, I added two more commits to my fork. I think these should be pretty noncontroversial. Otherwise I'd already be happy if only the first one (e1a99e1) got merged.

vkahl and others added 3 commits March 22, 2022 13:41
This is just anecdotal evidence, but in some cases, I did measure
faster runtimes when using #[inline(always)] for delegating methods
in the past. Since most methods (all except 2) implemented by this
crate are solely delegating calls to the wrapped type, I don't see
any reason why not to give the compiler the strongest possible
inlining suggestion for these methods.
@cuviper
Copy link
Member

cuviper commented Jun 23, 2022

The core change and the double-borrowing fix are fine.

I'm less comfortable with the inline(always) change. We don't have enough information to judge that this is always best. For example, a bottom-up inliner might have already pushed a lot of code inline into these delegation functions, but then forcing that to inline even more could be worse for all those additional call sites. Better to let the optimizer figure it out.

bors bot added a commit that referenced this pull request Jul 5, 2023
56: Make Float work with no_std (rebased #50) r=cuviper a=cuviper

- Use ::core::num::FpCategory when deriving Float to work with no_std
- Get rid of unnecessary double-borrowing to fix clippy lint


Co-authored-by: vkahl <42325420+vkahl@users.noreply.github.com>
Co-authored-by: Valentin Kahl <code@valentin-kahl.de>
@cuviper
Copy link
Member

cuviper commented Jul 5, 2023

Merged in #56 without the inline change.

@cuviper cuviper closed this Jul 5, 2023
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