build: ask the build too whether a cross target compiles without floats - #7238
Conversation
A cross target borrows `mrbc` from a build that answers `MRB_NO_FLOAT` the way
it does, because `src/load.c` refuses a whole irep over a pool entry the
target cannot represent. The question was asked of the compiler alone, and a
build config can write the define on the build:
```ruby
MRuby::CrossBuild.new('target') do |conf|
conf.defines << 'MRB_NO_FLOAT'
end
```
`Command::Compiler#all_flags` puts `build.defines` on every command line, so
the target compiles without floats while `cc.has_define?` says it does not. It
borrowed a `host` that answers otherwise, and the `mrbc` of that host wrote a
float pool entry the target refused to load:
```console
$ rake -m test
.........(unknown):0: irep load error (ScriptError)
```
Read both lists, the way `Build#has_define?` reads them. `Build#has_define?`
itself cannot be asked here: it refuses until the gems are set up, and a cross
build binds its `mrbc` as it is declared.
The question also moves to `MRBC_DEFINES`, which the comparison and the
defines the generated build carries now both read, so a second define that
decides what a pool entry may hold is one entry rather than three places.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
Included review availability: Your plan includes up to 8 reviews per rolling hour; 6 remain after this review. 📝 WalkthroughWalkthrough
ChangesCross-build mrbc compatibility
Estimated code review effort: 3 (Moderate) | ~15–30 minutes Merge Risk: 🔵 Low · up to The change aligns the generated mrbc with build-level target defines, preventing incompatible float bytecode from reaching no-float targets. The bounded risk is compiler/target define misalignment in less-common cross-build configurations, so the change is mergeable with owner awareness. Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
2 participants
Since #7230 a cross target borrows
mrbcfrom a build that answersMRB_NO_FLOATthe way it does, becausesrc/load.crefuses a whole irep overa single pool entry the target cannot represent. The question is asked of the
compiler alone, and a build config can write the define on the build instead:
Command::Compiler#all_flagsputsbuild.defineson every command line(
lib/mruby/build/command.rb:96on master):so the target compiles without floats while
cc.has_define?('MRB_NO_FLOAT')says it does not. It borrows the
host, whosemrbchas floats, and thatmrbcwrites a pool entry the target refuses to load.rake -m testwith the config abovemrbcit compiles the tests withhostbuild's, floats enabledMRB_NO_FLOATirep load error, at loadtest/t/array.rb:65, at the literalOn master the target builds, and refuses its own test suite the moment a float
literal reaches it:
With this patch the target gets a
mrbcthat answers as it does, whichrefuses the float literal where it is written, the same place
build_config/no-float.rbstops:What this does
Read both lists a build config writes, the way
Build#has_define?reads them.Build#has_define?itself cannot be asked here: it refuses until the gems areset up, and a cross build binds its
mrbcas it is declared, which is why thecompiler was read directly in the first place.
The question moves into
MRBC_DEFINESas well, which the comparison and thedefines a generated build carries now both read, so a second define that
decides what a pool entry may hold is one entry rather than three places.
src/load.crefusingIREP_TT_INT64on anMRB_INT32target is one candidateand is left alone here: it is not a regression of this kind, and the
mrbcacross target borrows follows the machine doing the building on integer width
today.
Verified
nofloat-build-define/mrbc, and stops at the literalhostthat agreehostbuild_config/no-float.rbbuild_config/default.rbrake -m test, 2123 assertions, 0 KOEnvironment
No C source changes, so
.textis unaffected in every build.🤖 Generated with Claude Code
https://claude.ai/code/session_01JJf1R2cytz2gAnJ2CgpGp1
Summary by CodeRabbit