Skip to content

Correctly set __cplusplus for chosen C++ standard - #264

Merged
jefftrull merged 1 commit into
boostorg:developfrom
martis42:correctly_set_cplusplus
Oct 13, 2026
Merged

jefftrull merged 1 commit into
boostorg:developfrom
martis42:correctly_set_cplusplus

Conversation

@martis42

Copy link
Copy Markdown
Contributor

Before this patch some language standards would set __cplusplus properly and some not. Now all currently available C++ language standards set __cplusplus correctly.

Related to #263
This patch is doing the obvious changes to make the existing behavior consistent and correct.
Adding a C++14 language standard and maybe even C++26 will be dedicated contributions due to them requiring a larger diff.

@martis42
martis42 force-pushed the correctly_set_cplusplus branch from fb7034f to a2f550c Compare September 27, 2026 16:52
Comment thread include/boost/wave/util/cpp_macromap_predef.hpp
@martis42

martis42 commented Oct 1, 2026 •

Copy link
Copy Markdown
Contributor Author

@jefftrull Is there an open point for this PR preventing its merge? If you need more time for review, all find. Just want to clarify if there are problems with it, since I would not start with C++14, before this first step is accepted.

@jefftrull

Copy link
Copy Markdown
Collaborator

I'm traveling (returning Monday) and don't have time to review this one thoroughly. Thanks for your patience!

Comment thread include/boost/wave/util/cpp_macromap.hpp Outdated
Comment thread include/boost/wave/util/cpp_macromap.hpp Outdated
@martis42

martis42 commented Oct 12, 2026 •

Copy link
Copy Markdown
Contributor Author

@jefftrull I rebased the PR on the latest develop HEAD.
Also I found a bug in the meantime and fixed it. See the new diff in include/boost/wave/cpplexer/re2clex/cpp_re2c_lexer.hpp. I found this missing usage of need_cpp1z() by looking again for usages of all the other need_cppXX() functions and realizing I missed some places. However, it seems the tests don't care either way. Now I am wondering if some tests are missing 🤔 Did not have time to look for the lexer tests and to analyze them, though.
I did not introduce scanner.act_in_cpp1z_mode. To my understanding, if it was not needed until now, my change should not touch that. I only needed to touch include/boost/wave/cpplexer/re2clex/cpp_re2c_lexer.hpp because I had to introduce need_cpp1z() and wanted it to be used consistently in all the places where all the other need_cppXX() functions are being used as well

@codecov

codecov Bot commented Oct 12, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 64.00000% with 9 lines in your changes missing coverage. Please review.
⚠️ Please upload report for BASE (develop@7ed288a). Learn more about missing BASE report.

Files with missing lines Patch % Lines
include/boost/wave/util/cpp_macromap.hpp 60.00% 0 Missing and 4 partials ⚠️
include/boost/wave/util/cpp_macromap_predef.hpp 66.66% 0 Missing and 2 partials ⚠️
samples/cpp_tokens/slex/cpp_slex_lexer.hpp 0.00% 0 Missing and 1 partial ⚠️
samples/list_includes/lexertl/lexertl_lexer.hpp 0.00% 0 Missing and 1 partial ⚠️
samples/token_statistics/xlex/xlex_lexer.hpp 0.00% 0 Missing and 1 partial ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             develop     #264   +/-   ##
==========================================
  Coverage           ?   44.85%           
==========================================
  Files              ?       76           
  Lines              ?     8010           
  Branches           ?     4080           
==========================================
  Hits               ?     3593           
  Misses             ?     1542           
  Partials           ?     2875           

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

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

if (boost::wave::need_cpp1z(ctx.get_language())) {
// define C++17 specifics
for (int i = 0; 0 != predef.static_data_cpp1z(i).name; ++i) {
predefined_macros::static_macros const& m = predef.static_data_cpp1z(i);

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is it really formatted like this or is this some kind of artifact...

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My bad. The #if and #endif were indented instead of being at the beginning of the lines. Now the code is formatted exactly like the code around it.

Comment thread test/testwave/testfiles/t_3_008.cpp Outdated
Comment thread test/testwave/testfiles/test.cfg Outdated
@martis42
martis42 force-pushed the correctly_set_cplusplus branch from 9a56fe5 to d736482 Compare October 13, 2026 08:41
Before this patch some language standards would set `__cplusplus`
properly and some not. Now all currently available C++ language standards
set `__cplusplus` correctly.
@martis42
martis42 force-pushed the correctly_set_cplusplus branch from d736482 to 6f981d2 Compare October 13, 2026 08:57
@martis42

martis42 commented Oct 13, 2026 •

Copy link
Copy Markdown
Contributor Author

@jefftrull I implemented your requests.

By now I also found the tests for the lexer. By editing and executing https://github.com/boostorg/wave/blob/develop/test/testlexers/test_re2c_lexer.cpp I managed to reproduce that my first version of this contribution broke something, which I fixed with my changes to include/boost/wave/cpplexer/re2clex/cpp_re2c_lexer.hpp. But I did not manage to find a nice way for contributing to the lexer tests. The problem is, the lexer tests execute with boost::wave::support_cpp2b as only C++ standard mode. Thus, one does not see bugs appearing only when setting other C++ standards. Simply testing multiple C++ standards is not straight forward due to the test data being a single C-style array and the lexer test also serving as benchmark target. It is for sure possible to restructure things so one can also test older C++ standards. But I think it is out of scope of this PR to perform this refactoring.

On an unrelated note. I see a lot of #if BOOST_WAVE_SUPPORT_CPP.. != 0 inside wave. At the same time we have interfaces for setting and querying the active C++ standard. Is this a pattern for performance optimization? Meaning one can compile a more efficient version of wave by omitting modern C++ support? I am just asking because to me it seem in some places things are more complicated than required due to this pattern and the benefit is not always clear to me as outsider.

@jefftrull

Copy link
Copy Markdown
Collaborator

Thanks I believe that the effort required to properly test all of the BOOST_WAVE_SUPPORT_CPPXX macros is not worth it compared to a lot of other things Wave needs. Thanks for trying to be universal with your testing, though.

As for the support macros themselves, I have always assumed they date back to the early days of Wave when compilers were less powerful and struggled with the extensive C++03 style metaprogramming. @hkaiser can you weigh in on their purpose? Would it be OK to let this be a runtime-only choice for future standards?

@jefftrull jefftrull left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for the contribution @martis42 and for being flexible

@jefftrull
jefftrull merged commit d30def1 into boostorg:develop Oct 13, 2026
50 of 58 checks passed
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