Make the RAND_MAX to float conversions explicit - #72
Open
frewsxcv wants to merge 1 commit into
Open
Conversation
RAND_MAX is 32767 on Windows and converts to float exactly, so this is
invisible on our primary target. Everywhere else it is INT_MAX, and
2147483647 has no exact float representation, so clang raises
-Wimplicit-const-int-float-conversion and the build fails under
-Werror. This is what stops trinity compiling anywhere but Windows.
32 conversions across 11 files, all the same idiom:
before: ( (float)rand() / RAND_MAX )
after: ( (float)rand() / float( RAND_MAX ) )
The value is unchanged on every platform. The conversion was already
happening; it is now written out rather than left implicit, which is
what silences the diagnostic.
Two sites in ProcessLifetime.cpp have RAND_MAX as the numerator rather
than the divisor. They divide by a float, so it is the same conversion
and they are fixed the same way.
This takes an arm64-osx-debug build of trinity_metal from 30 errors to
1. The remaining error is an unused variable in EveChildMesh.cpp, an
unrelated diagnostic that is deliberately left alone here.
No behaviour change on Windows, and no new build configuration is
needed to review it: the diff is one token per line, no logic touched.
Claude-Session: https://claude.ai/code/session_01NSpJ5h8fGynb2qyHa4NE7Z
There was a problem hiding this comment.
Pull request overview
This PR fixes non-Windows (notably macOS/Clang) build failures caused by -Wimplicit-const-int-float-conversion when using RAND_MAX in floating-point expressions, by making the conversion to float explicit at each affected call site.
Changes:
- Replaced implicit
RAND_MAX-to-floatconversions withfloat( RAND_MAX )in random-normalization expressions across the codebase. - Updated both divisor-form (
rand() / RAND_MAX) and numerator-form occurrences inProcessLifetime.cppto make the conversion explicit while preserving behavior. - Kept logic unchanged (mechanical cast insertion) to silence Clang warnings under
-Werror.
Reviewed changes
Copilot reviewed 11 out of 11 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| trinity/Eve/Volume/EveSphereVolume.cpp | Makes RAND_MAX float conversion explicit in point generation randomization. |
| trinity/Eve/Volume/EveEllipsoidVolume.cpp | Makes RAND_MAX float conversion explicit in ellipsoid point distribution math. |
| trinity/Eve/SpaceObject/Utils/EveDistributionMethods/DistributionSpawners/EveDistributionSpawnerTriggerSnake.cpp | Makes RAND_MAX float conversion explicit in trigger timing interpolation. |
| trinity/Eve/SpaceObject/Children/SmartLightSets/attributeModifiers/EveSmartLightAttributeModifierExpressionBucket.cpp | Makes RAND_MAX float conversion explicit in expression-bucket random helpers/constant. |
| trinity/Eve/SpaceObject/Children/EveChildParticleSphere.cpp | Makes RAND_MAX float conversion explicit in particle random helper lambdas. |
| trinity/Eve/SpaceObject/Children/Behaviors/ProcessLifetime.cpp | Makes RAND_MAX float conversion explicit in lifetime behavior random offset/spawn computations. |
| trinity/Eve/Renderable/Stretch/EveStretch2.cpp | Makes RAND_MAX float conversion explicit in effect random seed initialization/use. |
| trinity/Curves/Tr2ScalarExprKeyCurve.cpp | Makes RAND_MAX float conversion explicit in scalar expression random function. |
| trinity/Curves/Tr2CurveVector3Expression.cpp | Makes RAND_MAX float conversion explicit in vector3 expression random helpers/constants. |
| trinity/Curves/Tr2CurveScalarExpression.cpp | Makes RAND_MAX float conversion explicit in scalar expression random helpers/constants. |
| trinity/Curves/Tr2CurveEulerRotationExpression.cpp | Makes RAND_MAX float conversion explicit in euler rotation expression random helpers/constants. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
RAND_MAXis32767on Windows and converts tofloatexactly, but it isINT_MAXeverywhere else, where2147483647has no exactfloatrepresentation. Clang raises
-Wimplicit-const-int-float-conversionand thebuild fails under
-Werror, which is what currently stopstrinitycompiling on anything but Windows. This makes the conversion explicit at all
32 sites.
AI assistance disclosure
Substantial. Claude (via Claude Code) found the failures by building
trinity_metalon macOS, applied the edits, and wrote the commit message andthis description. Every changed line was reviewed and the 30 -> 1 error count
is from real builds, not an estimate. Not verified: no Windows build, and no
runtime testing (there is no behaviour change to test).
Type of change
Both apply: it fixes a build failure, and the emitted code is unchanged.
Linked issue (optional)
None.
What changed
RAND_MAX's conversion tofloatexplicit at 32 sites across 11files, all the same idiom:
( (float)rand() / RAND_MAX )->( (float)rand() / float( RAND_MAX ) )ProcessLifetime.cpp(lines 217 and 405) haveRAND_MAXasthe numerator rather than the divisor. They divide by a
float, so it isthe same conversion, fixed the same way.
now written out, which is what silences the diagnostic. Values are
bit-identical on every platform, Windows included.
Testing
trinity_metalonarm64-osx-debug: 30 errors before, 1 after. Theremaining error is
variable 'mesh' set but not usedinEveChildMesh.cpp:886, an unrelated diagnostic left alone here. With thatone also fixed locally, the target links cleanly.
No tests added:
trinityhas no test target or test sources, so there isnothing to extend. Per CONTRIBUTING I would have run the component's tests
locally, but none exist for this component.
This is a compile-time diagnostic fix with no runtime behaviour change, so
there is nothing to exercise at runtime beyond the build itself.
Platforms tested
Not built on Windows. That is the platform most likely to regress from a
careless edit here, so it is worth a maintainer confirming. The change should
be a no-op there:
RAND_MAXis32767, the conversion was already implicit,and making it explicit does not alter the value.
Screenshots / captures
Not applicable.
Checklist
On the unticked boxes: no tests were added because
trinityhas no testinfrastructure, and no docs were updated because there is no behaviour change
to document. Neither is being skipped silently. The CLA box is left for the
submitter to confirm.
Commit checked against the seven rules: 47-character subject, imperative
mood, no trailing period, body wrapped at 72.