Skip to content

Fix generation of NAN, INFINITY, and NEG_INFINITY for f32 #3300

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 2 commits into from
Oct 2, 2025
Merged

Fix generation of NAN, INFINITY, and NEG_INFINITY for f32 #3300

merged 2 commits into from
Oct 2, 2025

Conversation

nicoburns
Copy link
Contributor

@nicoburns nicoburns commented Sep 28, 2025 •
edited
Loading

This fixes #2854 to work with f32 as well as f64.

Without this patch we are getting the following error when trying to compile bindgen-generated bindings for Yoga in the yoga-rs project

error[E0308]: mismatched types
 --> /home/runner/work/yoga-rs/yoga-rs/target/debug/build/yoga-77b71d5cb8c24830/out/bindings.rs:3:21698
  |
3 | ...GEdge) -> f32 ; } pub const YGUndefined : f32 = f64 :: NAN ; # [doc = " Structure used to represent a dimension in a style."] # [repr ...
  |                                                    ^^^^^^^^^^ expected `f32`, found `f64`

For more information about this error, try `rustc --explain E0308`.`

The C/C++ input causing this failure is:

#ifdef __cplusplus
#include <limits>
constexpr float YGUndefined = std::numeric_limits<float>::quiet_NaN();
#else
#include <math.h>
#define YGUndefined NAN
#endif

(and I think we're hitting the c++ branch here as we're running bindgen with .clang_args(&["-x", "c++", "-std=c++20", "-Isrc/yoga"])

@nicoburns nicoburns mentioned this pull request Sep 28, 2025
Open
@nicoburns nicoburns force-pushed the fix-float-literals-for-f32 branch from 144e2c4 to 78e4135 Compare September 28, 2025 16:04
emilio
emilio requested changes Sep 29, 2025
View reviewed changes
Copy link
Contributor

@emilio emilio left a comment

Choose a reason for hiding this comment

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

Looks fine but can you add a .hpp file that triggers the issue (static const float kFoo = 1/0 or so?)

It'd be even better to just use the right type for the constant tho, it can probably be provided by the caller of float_expr? But if it's too much of a hassle this is probably ok too.

@nicoburns nicoburns force-pushed the fix-float-literals-for-f32 branch from 29afa28 to 6c71374 Compare October 2, 2025 00:07
@nicoburns nicoburns force-pushed the fix-float-literals-for-f32 branch from 6c71374 to bd1da85 Compare October 2, 2025 00:17
@nicoburns
Copy link
Contributor Author

Looks fine but can you add a .hpp file that triggers the issue (static const float kFoo = 1/0 or so?)

Ah, yes static const seems to work. I was trying to use constexpr which doesn't let you use 1/0. I've added them in to the existing test for non-numerical float literals.

The type-correct fix looks doable but quite a bit more involved as it's going to need to handle f16 (as __BindgenFloat16) and f128 (as u128). I'd prefer to do that as a follow-up if possible.

emilio
emilio approved these changes Oct 2, 2025
View reviewed changes
Copy link
Contributor

@emilio emilio left a comment

Choose a reason for hiding this comment

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

Fair enough :)

@emilio emilio added this pull request to the merge queue Oct 2, 2025
Merged via the queue into rust-lang:main with commit e604f6b Oct 2, 2025
51 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@emilio emilio emilio approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@nicoburns @emilio