Skip to content

Add cxx14_constexpr requirement #1276

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

Merged
merged 1 commit into from
Jun 16, 2025
Merged

Add cxx14_constexpr requirement #1276

merged 1 commit into from
Jun 16, 2025

Conversation

mborland
Copy link
Member

Partial for: #1275

Copy link

codecov bot commented Jun 16, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 95.07%. Comparing base (39c2fda) to head (eb2930d).
Report is 2 commits behind head on develop.

Additional details and impacted files

Impacted file tree graph

@@           Coverage Diff            @@
##           develop    #1276   +/-   ##
========================================
  Coverage    95.07%   95.07%           
========================================
  Files          792      792           
  Lines        66882    66882           
========================================
  Hits         63589    63589           
  Misses        3293     3293           

Continue to review full report in Codecov by Sentry.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 39c2fda...eb2930d. Read the comment docs.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@mborland mborland merged commit e1158bb into develop Jun 16, 2025
64 checks passed
@jzmaddock
Copy link
Collaborator

While completely harmless, I'm not at all convinced that this fixes the original issue (or that there is an easy fix), the point being that only a vanishingly small number of dependents will be using the Math binaries as opposed to the headers, and if you're just using the headers, then there would appear to be no appropriate target to annotate with our requirements. Unless I'm missing something - @Lastique.

@mborland mborland deleted the build_reqs branch June 16, 2025 17:35
@mborland
Copy link
Member Author

That's true. Theoretically I could port: #1277 to config and then we could static_assert in the boost.math config header if it's not set. That would be a slightly nicer error message than what he had in his issue.

@Lastique
Copy link
Member

Given the Boost.Build modularization efforts, header-only libraries now also have targets in b2. Those targets can also have the C++ requirements, as well as dependencies, attached. Those requirements and dependencies propagate to downstream users.

Specifically, if Boost.Math ever becomes header-only, it will still have a b2 target with all C++ requirements attached. Downstream users will still "link" with that target and thus inherit the C++ requirements.

Now, to this PR. I'm not sure cxx14_constexpr alone will be enough to fix the problem. The compiler error messages in Boost.Parameter tests suggest that it is the <type_traits> lacking C++14 stuff that is the problem. And for that there currently is no Boost.Config check. Either Boost.Math will have to add its own check, or (preferably) one needs to be added to Boost.Config.

@mborland
Copy link
Member Author

Given the Boost.Build modularization efforts, header-only libraries now also have targets in b2. Those targets can also have the C++ requirements, as well as dependencies, attached. Those requirements and dependencies propagate to downstream users.

Specifically, if Boost.Math ever becomes header-only, it will still have a b2 target with all C++ requirements attached. Downstream users will still "link" with that target and thus inherit the C++ requirements.

Now, to this PR. I'm not sure cxx14_constexpr alone will be enough to fix the problem. The compiler error messages in Boost.Parameter tests suggest that it is the <type_traits> lacking C++14 stuff that is the problem. And for that there currently is no Boost.Config check. Either Boost.Math will have to add its own check, or (preferably) one needs to be added to Boost.Config.

#1277 Should cover the type traits issue. For that one I had to ask Rene how to globally apply a library specific configuration since requires in the Jamfile only applies to Boost.Config.

@jzmaddock
Copy link
Collaborator

Specifically, if Boost.Math ever becomes header-only, it will still have a b2 target with all C++ requirements attached. Downstream users will still "link" with that target and thus inherit the C++ requirements.

In 99.999% of cases it is header only, if users are linking to the compiled library just to express a dependency then that's flat out wrong.

@Lastique
Copy link
Member

Well, the dependency is necessary at least to get include paths to Boost.Math headers, as the monolithic include directory may no longer exist.

If you want to make linking with Boost.Math binary optional, you need to introduce a new target that corresponds to the header-only Boost.Math. The existing target should still link, since otherwise you would break that 0.001% of users.

@Lastique
Copy link
Member

BTW, the same is true with CMake, only it was the case from the start. In CMake, you also have targets to express dependencies, whether those targets are built or not.

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.

3 participants