Skip to content
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.

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

standalone: c++ usage in preprocessor evaluation #677

Closed
michael-herwig opened this issue Aug 23, 2021 · 6 comments · Fixed by #678
Closed

standalone: c++ usage in preprocessor evaluation #677

michael-herwig opened this issue Aug 23, 2021 · 6 comments · Fixed by #678

Comments

@michael-herwig
Copy link

Compiling for windows on arm64 in standalone mode, I get an error fatal error C1012: unmatched parenthesis: missing ')' at

#if BOOST_MATH_ENDIAN_BIG_BYTE
static constexpr int offset_ = 0;
#elif BOOST_MATH_ENDIAN_LITTLE_BYTE
static constexpr int offset_ = 4;
#else
static_assert(sizeof(double_precision) == 0, "Endian type could not be identified");
#endif
, causing the preprocessor to evaluate a c++ expression from
#elif (__cplusplus > 202000L || _MSVC_LANG > 202000L)
#if __has_include(<bit>)
#include <bit>
#define BOOST_MATH_ENDIAN_BIG_BYTE (std::endian::native == std::endian::big)
#define BOOST_MATH_ENDIAN_LITTLE_BYTE (std::endian::native == std::endian::little)
#endif

I use the arm64 compiler shipped with Visual Studi 2022 Preview 3: Microsoft (R) C/C++ Optimizing Compiler Version 19.30.30423 for ARM64. To reproduce the error one may compile using the STL version including the <bit> header, see github. To reproduce the error it is sufficient to include the corresponding header

#include "boost/math/special_functions/detail/fp_traits.hpp"
int main() { return 0; }

and compile it from the arm64 development environment (ie. cl /nologo -DBOOST_MATH_STANDALONE=1 -Istl\inc -Iboost-math\include /std:c++latest boost-math.cpp).

@NAThompson
Copy link
Collaborator

@mborland : Do you have a Windows machine that you can validate a fix on?

@mborland
Copy link
Member

@mborland : Do you have a Windows machine that you can validate a fix on?

I have a windows machine, but I don't have any kind of development environment setup. I'll grab a copy of the referenced visual studio to try and get it to work.

@NAThompson
Copy link
Collaborator

NAThompson commented Aug 23, 2021

@michael-herwig : My apologies for the rarely-acceptable "submit a PR" trope, but if it's just closing a parenthesis it might get you unblocked faster . . . looks like we might have some time to get your environment set up.

@michael-herwig
Copy link
Author

No worries, I don't think it's a closing parenthesis. As far as I can see, it's the endianness detection stated as a macro used in a preprocessor #if directive. Actually, the offset(s) configured later are the only occurrences I found using standalone endianness detection. Thus one may replace that with a constexpr function or variable that depends on macros or the ::std::endian value based on the environment. I can set up a pull request later if you like or help set up the environment; let me know

@StephanTLavavej
Copy link
Contributor

I'm working on a fix.

NAThompson pushed a commit that referenced this issue Aug 24, 2021
* Fix comment typos: "endinaness" to "endianness"
* Change the C++20 checks (for both `__cplusplus` and `_MSVC_LANG`)
  to be strict: `> 202000L` to `>= 202002L`
* Add an `#error` when C++20 `<bit>` is missing, just in case.
* Fix the `_WIN32` definitions. All Windows platforms are little-endian
  (regardless of x86/x64/ARM/ARM64), `<boost/predef/other/endian.h>`
  correctly reports little-endian, and MSVC's STL simply hardcodes
  `enum class endian { little = 0, big = 1, native = little };`, see
  https://github.com/microsoft/STL/blob/f75c7f596c7b491168fefb5eff5164ab7ae36867/stl/inc/bit#L271 .
* Add a `static_assert` to verify that exactly one of
  `BOOST_MATH_ENDIAN_BIG_BYTE` or `BOOST_MATH_ENDIAN_LITTLE_BYTE` is
  true. This avoids the need for "Endian type could not be identified"
  consistency checks below.
* Fix #677 by not testing `BOOST_MATH_ENDIAN_BIG_BYTE` and
  `BOOST_MATH_ENDIAN_LITTLE_BYTE` with the preprocessor, as
  `std::endian::native` isn't a preprocessor constant. Now, this simply
  expects `BOOST_MATH_ENDIAN_BIG_BYTE` to be usable in a constant
  expression, and assumes (due to the consistency check above)
  that if we aren't big-endian, we must be little-endian.
@NAThompson
Copy link
Collaborator

Release cut with the fix from @StephanTLavavej .

Thanks for your help!

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 a pull request may close this issue.

4 participants