-
Notifications
You must be signed in to change notification settings - Fork 6
Description
Description
The parse_float method returns F: Float, which enables the user to parse into either an f32 or f64. This requires the Float trait to be crate-public, since there is no way to abstract over f32 and f64 otherwise. Additionally:
Floathas various associated constants and methods that (I presume) are present as "helpers", implementing details specific to the target float type. These are documented as "non-public" (which I presume means they are intended for crate-private use only).Floatis nameable (asminimal_lexical::Float), meaning that crate users canimpl Float for TheirCustomFloat.
This means that someone downstream could implement Float, and then have their code broken by a point release to minimal-lexical. Additionally, they could quite easily implement or rely on the trait incorrectly: some of the Float methods are unsafe and have undocumented safety requirements (e.g. #9).
Is there any value in having Float be implementable? If there are no known use cases for it, and it is not part of the intended usage of the crate, then it would be safer to make the Float trait unnameable by removing this line:
Line 67 in e997c46
| pub use self::num::Float; |
This would make Float a sealed trait: part of the public API, but unimplementable by anyone outside of the crate. parse_float would then only be able to return either f32 or f64.
Additional Context
I'm in the process of migrating Fuchsia to use nom 7, which depends on minimal-lexical 0.1, and am opening issues in response to review comments (visible here: https://fuchsia-review.googlesource.com/c/fuchsia/+/589324).