Fix M_PI being unavailable where _USE_MATH_DEFINES comes too late#4023
Merged
Conversation
openmc-dev#3238 added _USE_MATH_DEFINES to mesh.cpp, plot.cpp and quartic_solver.cpp so that M_PI would be declared on Intel and MSVC. In all three the definition sits below an include that has already pulled in <cmath>, and once math.h has been processed its include guard is set, so the definition never takes effect. mesh.cpp:1 -> openmc/mesh.h -> openmc/position.h:4 -> <cmath> plot.cpp:1 -> openmc/plot.h:4 -> <cmath> quartic_solver.cpp:1 -> <algorithm> -> <cmath> (MSVC) Measured with MSVC 19.51.36248 (VS 2026) and MinGW-w64 GCC 14.2.0, compiling each file with no external -D_USE_MATH_DEFINES: MSVC before MSVC after MinGW before MinGW after mesh.cpp 2 errors ok 2 errors ok plot.cpp 1 error ok 1 error ok quartic_solver.cpp 3 errors ok ok ok quartic_solver.cpp is where the two toolchains differ: MSVC's <algorithm> reaches math.h transitively, libstdc++'s does not. Which header first reaches math.h is implementation-defined, so the only position that is safe everywhere is above every include. mesh.cpp and plot.cpp therefore drop the macro entirely and use the PI constant already in constants.h, which both files already include and which is bit-identical to M_PI. quartic_solver.cpp is vendored third-party code in namespace oqs, so it keeps M_PI and just moves the definition to the top. In mesh.cpp the definition was also splitting the include block in two, which let the two halves stay individually sorted; removing it merges them and clang-format sorts the result.
paulromano
approved these changes
Jul 21, 2026
paulromano
left a comment
paulromano
left a comment
Contributor
There was a problem hiding this comment.
Thanks for the fix @lzpel!
paulromano
enabled auto-merge (squash)
July 21, 2026 14:54
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
2 participants
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.
Description
This PR fixes
M_PIbeing undeclared in three files ( Resolve #3175 ). #3238 added#define _USE_MATH_DEFINESto them, but in all three it sits below an include that hasalready pulled in
<cmath>, somath.his guarded by then and the definition never takeseffect.
quartic_solver.cppdiffers between the two because MSVC's<algorithm>reachesmath.htransitively and MinGW's libstdc++ does not. Since which header gets there first isimplementation-defined, the only position that is safe everywhere is above every include —
so
mesh.cppandplot.cppsidestep the macro entirely. No test is added:PIisbit-identical to
M_PI, so there is no behaviour change to assert.src/mesh.cppPIfrominclude/openmc/constants.hsrc/plot.cppPIfrominclude/openmc/constants.hsrc/external/quartic_solver.cppnamespace oqs: keepM_PI, move#defineabove the includesAll three compile successfully with no external
-D_USE_MATH_DEFINESafter this fix.Checklist
I have followed the style guidelines for Python source files (if applicable)I have made corresponding changes to the documentation (if applicable)I have added tests that prove my fix is effective or that my feature works (if applicable)