Skip to content

add f16 and f128 support #2941

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

Open
wants to merge 6 commits into
base: master
Choose a base branch
from
Open

add f16 and f128 support #2941

wants to merge 6 commits into from

Conversation

Vrtgs
Copy link

@Vrtgs Vrtgs commented Jun 19, 2025

add initial support for f16 and f128, next steps are to add this support in serde_test if this lands

helps with adding ecosystem support par in rust-lang/rust#116909

@Vrtgs Vrtgs requested a review from Mingun June 21, 2025 12:20
@@ -1439,6 +1463,17 @@ pub trait Visitor<'de>: Sized {
))
}

/// The input contains a `f16`.
///
/// The default implementation forwards to visit_f64.
Copy link
Contributor

Choose a reason for hiding this comment

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

Like the others:

Suggested change
/// The default implementation forwards to visit_f64.
/// The default implementation forwards to [`visit_f64`].

Copy link
Author

Choose a reason for hiding this comment

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

this break documentation build sadly

Choose a reason for hiding this comment

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

You'll need to add something like:

[`visit_f64`]: #method.visit_f64

at the bottom of the doc comment.

Copy link
Contributor

Choose a reason for hiding this comment

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

Very strange, that documentation is broken, it should be broken in other places as well. The correct link should be

[`visit_f64`](Self::visit_f64)

Choose a reason for hiding this comment

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

I think either way should work. You can see that it does link correctly on docs.rs for the other links for this trait. For example: https://docs.rs/serde/latest/serde/de/trait.Visitor.html#method.visit_u64

Copy link
Contributor

Choose a reason for hiding this comment

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

Of course, just intralinks can be checked by the compiler

Copy link
Contributor

@Mingun Mingun left a comment

Choose a reason for hiding this comment

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

You need to roll back the removal of integer128, the maintainers will not approve it.

My reviews means nothing, I do not have any rights in this repository. So probably request my reviews is pointless...

Vrtgs and others added 4 commits June 22, 2025 08:34
Co-authored-by: Mingun <Alexander_Sergey@mail.ru>
Co-authored-by: Mingun <Alexander_Sergey@mail.ru>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

3 participants