Skip to content

Adding '#define _USE_MATH_DEFINES' to make M_PI declared in Intel and MSVC compilers#3238

Merged
paulromano merged 4 commits into
openmc-dev:developfrom
MAS-OUD:develop
Jan 7, 2025
Merged

Adding '#define _USE_MATH_DEFINES' to make M_PI declared in Intel and MSVC compilers#3238
paulromano merged 4 commits into
openmc-dev:developfrom
MAS-OUD:develop

Conversation

@MAS-OUD

@MAS-OUD MAS-OUD commented Dec 25, 2024

Copy link
Copy Markdown
Contributor

the M_PI used in mesh.cpp, plot.cpp, and quartic_solver.cpp is undeclared in Intel and MSVC compilers. To make it declared in these compilers #define _USE_MATH_DEFINES must be added before #include <cmath>.

M_PI is undeclared in Intel and MSVC compilers. To make it declared in these compilers '#define _USE_MATH_DEFINES' must be added before '#include <cmath>'.
M_PI is undeclared in Intel and MSVC compilers. To make it declared in these compilers '#define _USE_MATH_DEFINES' must be added before '#include <cmath>'.
M_PI is undeclared in Intel and MSVC compilers. To make it declared in these compilers '#define _USE_MATH_DEFINES' must be added before '#include <cmath>'.
@MAS-OUD
MAS-OUD requested a review from pshriwise as a code owner December 25, 2024 08:36
@MAS-OUD MAS-OUD changed the title Adding '#define _USE_MATH_DEFINES' to some files to make M_PI declared in Intel and MSVC compilers Adding '#define _USE_MATH_DEFINES' to make M_PI declared in Intel and MSVC compilers Dec 25, 2024

@paulromano paulromano left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Thanks @MAS-OUD!

@paulromano
paulromano enabled auto-merge (squash) January 6, 2025 12:46
@MAS-OUD

MAS-OUD commented Jan 6, 2025

Copy link
Copy Markdown
Contributor Author

Thanks @MAS-OUD!

You're welcome @paulromano.

@paulromano
paulromano merged commit 3933348 into openmc-dev:develop Jan 7, 2025
apingegno pushed a commit to apingegno/openmc that referenced this pull request May 7, 2026
… MSVC compilers (openmc-dev#3238)

Co-authored-by: Paul Romano <paul.k.romano@gmail.com>
lzpel added a commit to lzpel/openmc that referenced this pull request Jul 20, 2026
In mesh.cpp and plot.cpp the definition sits below the first include, which
transitively pulls in <cmath>:

  mesh.cpp:1 -> openmc/mesh.h -> openmc/position.h:4 -> <cmath>
  plot.cpp:1 -> openmc/plot.h:4                      -> <cmath>

<math.h> is #pragma once, so defining the macro afterwards cannot take effect
and M_PI stays undeclared. Moving the definition above the first include
restores the intent of openmc-dev#3238.

In mesh.cpp the definition was also splitting the include block in two, which
let the two halves stay individually sorted; moving it merges them and
clang-format sorts the result.

external/quartic_solver.cpp is unaffected: only <algorithm> precedes the
definition there.
lzpel added a commit to lzpel/openmc that referenced this pull request Jul 20, 2026
In mesh.cpp and plot.cpp the definition sits below the first include, which
transitively pulls in <cmath>:

  mesh.cpp:1 -> openmc/mesh.h -> openmc/position.h:4 -> <cmath>
  plot.cpp:1 -> openmc/plot.h:4                      -> <cmath>

Once math.h has been processed its include guard is set, so defining the macro
afterwards cannot bring M_PI into scope.

Measured with MinGW-w64 GCC 14.2.0 on Windows 11, compiling each file with the
project's flags and no external -D_USE_MATH_DEFINES:

  before: mesh.cpp 2 errors, plot.cpp 1 error (M_PI was not declared)
  after:  both compile

There the macro matters because -std=c++17 defines __STRICT_ANSI__, which
hides M_PI in the MinGW headers unless _USE_MATH_DEFINES is set.

openmc-dev#3238 added these definitions for Intel and MSVC. I have no Intel or MSVC
toolchain, so whether they are affected is untested here and worth confirming.

In mesh.cpp the definition was also splitting the include block in two, which
let the two halves stay individually sorted; moving it merges them and
clang-format sorts the result.

external/quartic_solver.cpp is unaffected: only <algorithm> precedes the
definition there, and it compiles as-is.
lzpel added a commit to lzpel/openmc that referenced this pull request Jul 20, 2026
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.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants