C++20 module support #255
C++20 module support #255
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## develop #255 +/- ##
========================================
Coverage 94.85% 94.85%
========================================
Files 69 69
Lines 9077 9077
========================================
Hits 8610 8610
Misses 467 467
Continue to review full report in Codecov by Sentry.
|
| target_sources(boost_charconv PUBLIC FILE_SET CXX_MODULES BASE_DIRS modules FILES modules/boost_charconv.cppm) | ||
|
|
||
| # Enable and propagate C++23, import std, and the modules macro | ||
| target_compile_features(boost_charconv PUBLIC cxx_std_23) |
There was a problem hiding this comment.
Have you tried this with C++20 instead of 23? I know officially import std and import std.compat are C++23, but all of the compiler implementors have said they'll allow it at 20.
There was a problem hiding this comment.
Do you think there's a use to it? My understanding was that using C++20 over C++23 is about being able to use older compilers that might not support C++23. Such compilers won't have proper module implementations, likely, since modules are quite behind everything else.
There was a problem hiding this comment.
Standards like MISRA are pinned to language standards not toolchains. Theoretically someone could have a modern compiler but be stuck at C++20. Probably best to have someone in the real world complain before changing it.
|
|
||
| namespace boost { namespace charconv { namespace detail { | ||
|
|
||
| static constexpr unsigned char uchar_values[] = | ||
| BOOST_INLINE_CONSTEXPR unsigned char uchar_values[] = |
There was a problem hiding this comment.
It is my understanding that the macros from other modules are not visible. Do we need to just copy and paste the logic we need from config into charconv/detail/config.hpp for this to work properly?
There was a problem hiding this comment.
You can access macros via #include. This is the purpose of detail/global_module_fragment.hpp: all macros inside that file are made available to all headers in the library when building in module mode.
I'll apply the suggestions that Peter made on the ML that might change how this works and will PR the changes if something better comes out.
| #include <cerrno> | ||
| #include <boost/config.hpp> | ||
| #include <boost/assert.hpp> | ||
|
|
There was a problem hiding this comment.
Would it be better instead of adding #ifdef BOOST_USE_MODULES in every file to move every include into this file and only include this file and other charconv files in the charconv files?
eg
#include <boost/charconv/detail/global_module_fragement.hpp>
#include <boost/charconv/detail/emulated128.hpp>
ETC
In the top of a file? Then you could write some kind of code scanner that will error in your CI if a new contributor adds a STL header in a regular file which would break your module. Boost.Math has a CI run with Circle that does this to check for some of the boost guidelines (min/max, non-ascii characters, copyright, etc)
|
This is cool progress. Thanks for continuing to plug away on it. |
Adds support for
import boost.charconv.This pull request is intended to get feedback and support discussion in the ML. It depends on changes in Boost.CMake, Boost.Core, Boost.ThrowException and Boost.Assert that aren't in develop.
Any feedback is welcome.