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

std::is_integral has compiler/platform-dependent behavior for __int128_t #669

Closed
j2kun opened this issue Feb 12, 2024 · 1 comment · Fixed by #671
Closed

std::is_integral has compiler/platform-dependent behavior for __int128_t #669

j2kun opened this issue Feb 12, 2024 · 1 comment · Fixed by #671
Assignees
Milestone

Comments

@j2kun
Copy link

j2kun commented Feb 12, 2024

When compiling the example from #667 with clang-17 with -std=c++17, I see

external/openfhe/src/core/lib/math/hal/bigintfxd/ubintfxd.cpp:121:13: error: no matching function for call to 'GetMSB'
  121 |     m_MSB = lbcrypto::GetMSB(val);
      |             ^~~~~~~~~~~~~~~~
external/openfhe/src/core/include/math/nbtheory.h:167:24: note: candidate template ignored: requirement 'std::is_integral_v<unsigned __int128>' was not satisfied [with T = U128BITS]
  167 | inline constexpr usint GetMSB(T x) {
      |

After reading https://quuxplusone.github.io/blog/2019/02/28/is-int128-integral/ it seems this is intended behavior, but I wanted to surface it because the readme in the repository states

OpenFHE supports any GNU C++ compiler version 9 or above and clang C++ compiler version 10 or above.

It would be nice to make the build succeed for both -std=c++XX and -std=gnu++XX, though I don't know enough to say what would be required to do that. I was able to get everything to compile with -std=gnu++XX for any XX

@j2kun j2kun changed the title std::is_integral has platform-dependent behavior for __int128_t std::is_integral has compiler/platform-dependent behavior for __int128_t Feb 12, 2024
@yspolyakov yspolyakov added this to the Release 1.1.3 milestone Feb 12, 2024
@pascoec pascoec self-assigned this Feb 12, 2024
@yspolyakov
Copy link
Contributor

@j2kun The fix has been merged to the dev branch, It will be merged to master as part of releasing v1.1.3.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants