Skip to content

Make Float work with no_std#50

Closed
vkahl wants to merge 3 commits into
rust-num:masterfrom
vkahl:master
Closed

Make Float work with no_std#50
vkahl wants to merge 3 commits into
rust-num:masterfrom
vkahl:master

Conversation

@vkahl

@vkahl vkahl commented Mar 22, 2022

Copy link
Copy Markdown
Contributor

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

cuviper commented Jun 23, 2022

Copy link
Copy Markdown
Member

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

cuviper commented Jul 5, 2023

Copy link
Copy Markdown
Member

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