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

Improve ccmath support detection. #1046

Merged
merged 4 commits into from
Nov 7, 2023
Merged

Conversation

jzmaddock
Copy link
Collaborator

See discussion in #1045.

@ckormanyos
Copy link
Member

ckormanyos commented Nov 5, 2023

This post confirms that this fixes the client code which first detected the issue discussed toward to bottom of #1045.

Thank you so much John (@jzmaddock) for such a quick response to even this trivial issue!

@mborland
Copy link
Member

mborland commented Nov 6, 2023

Should we apply this logic to all of the headers? Isfinite has the old logic, and some have none at all.

@ckormanyos
Copy link
Member

ckormanyos commented Nov 6, 2023

Should we apply this logic to all of the headers?

This is, indeed, a slim area, ... Clients using MSVC language standard 14 or lower. Anyway, I think that seems sensible both to repair it and to repair it consistently.

For more information, by sheer random chance, I was revitalizing a very old legacy project recently and just so happened to have MSVC 141 C++ standard 14 running on CI (otherwise I would not have seen this at all...).

Anyway, the error message(s) I got were here.

There are errors on isnan, isinf, abs and ldexp. But these might be the ones that were simply picked up by MSVC in my particular instantiations.

@ckormanyos
Copy link
Member

ckormanyos commented Nov 6, 2023

Hi Matt (@mborland) I was thinking about this issue and noticed C++17 is not so far away from C++14 (the language standard supported by all of Math/Multiprecision).

I suppose you won't really like this question but, ...

  • How far is ccmath let's say, away from C++14?
  • Could/should we backport it?
  • I could help backport it for 1.85 if this makes things easier or more clear...

@mborland
Copy link
Member

mborland commented Nov 6, 2023

Hi Matt (@mborland) I was thinking about this issue and noticed C++17 is not so far away from C++14 (the language standard supported by all of Math/Multiprecision).

I suppose you won't really like this question but, ...

  • How far is ccmath let's say, away from C++14?
  • Could/should we backport it?
  • I could help backport it for 1.85 if this makes things easier or more clear...

It's not particularly far away from C++14, but that is not the pressing issue. For it to work we require proper __builtin_is_constant_evaluated which is GCC >= 9, clang >= 9, or MSVC > 1925. All those compilers support C++17 just fine.

@ckormanyos
Copy link
Member

It's not particularly far away from C++14, but that is not the pressing issue. For it to work we require proper __builtin_is_constant_evaluated

Understood. Challenging, getting through the maze of compilers...

@ckormanyos
Copy link
Member

ckormanyos commented Nov 6, 2023

2023-11-06T09:50:49.0791974Z ../tools/process_perf_results.cpp:12:10: fatal error: boost/format.hpp: No such file or directory
2023-11-06T09:50:49.0793363Z    12 | #include <boost/format.hpp>
2023-11-06T09:50:49.0794313Z       |          ^~~~~~~~~~~~~~~~~~
2023-11-06T09:50:49.0795175Z compilation terminated.

This is now creeping into CI on some standalone. Maybe more but that's all I saw at a quick glance.

@jzmaddock
Copy link
Collaborator Author

WTF? Nothing has changed, either here, or in boostdep or Boost.Format... rerunning just to see what happens!

@ckormanyos
Copy link
Member

ckormanyos commented Nov 6, 2023

WTF? Nothing has changed, either here, or in boostdep or Boost.Format... rerunning just to see what happens!

If it runs, you might have hit something that I've always wondered about, non-atomic checkins in the modular-boost. I wonder if something like that might have happened?

Let's see.

I don't even know if commits to the master project or develop are atomic. That would surprise me...

@ckormanyos
Copy link
Member

The thing is still looking for <boost/format.hpp> for several exemplary CPP-Files.

This file is present in 1.83.

I wonder if this header has been either knowingly or mistakenly removed from Boost on Develop in preparation for 1.84?

@pdimov
Copy link
Member

pdimov commented Nov 6, 2023

In short, add -I example -I tools to the depinst invocation to tell it to scan these directories for dependencies (in addition to the default include, src, test).

boostorg/boostdep#20

@jzmaddock jzmaddock merged commit 8c72a85 into develop Nov 7, 2023
61 checks passed
@ckormanyos
Copy link
Member

Awesome! Thanks John and Matt

@ckormanyos
Copy link
Member

It is running perfectly in the field on my unrelated project that actually uses it on MSVC std=c++14. I just check on the runners for my unrelated project. Great!

@NAThompson NAThompson deleted the ccmath_support_detection branch January 24, 2024 21:33
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.

4 participants