Replace raw #ifdef _MSC_VER with BOOST_CAPY_WORKAROUND macro#232
Merged
mvandeberg merged 1 commit intocppalliance:developfrom Mar 12, 2026
Merged
Replace raw #ifdef _MSC_VER with BOOST_CAPY_WORKAROUND macro#232mvandeberg merged 1 commit intocppalliance:developfrom
mvandeberg merged 1 commit intocppalliance:developfrom
Conversation
📝 WalkthroughWalkthroughThis PR centralizes compiler workarounds by introducing BOOST_CAPY_WORKAROUND and MSVC warning control macros, and replaces scattered MSVC-specific Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~22 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
📝 Coding Plan
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
An automated preview of the documentation is available at https://232.capy.prtest3.cppalliance.org/index.html If more commits are pushed to the pull request, the docs will rebuild at the same URL. 2026-03-12 20:01:51 UTC |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
include/boost/capy/ex/execution_context.hpp (1)
526-531: Minor: Comment placement appears after the macro.The explanatory comment on line 528 is placed between
BOOST_CAPY_MSVC_WARNING_DISABLE(4251)and the affected members. This differs from lines 165-170 and 504-506 where the comment precedes thePUSHmacro. Consider moving the comment beforeBOOST_CAPY_MSVC_WARNING_PUSHfor consistency with the other regions in this file.Suggested fix for comment placement consistency
+// warning C4251: std::mutex, std::shared_ptr need dll-interface BOOST_CAPY_MSVC_WARNING_PUSH BOOST_CAPY_MSVC_WARNING_DISABLE(4251) -// warning C4251: std::mutex, std::shared_ptr need dll-interface mutable std::mutex mutex_; std::shared_ptr<void> owned_; BOOST_CAPY_MSVC_WARNING_POP🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@include/boost/capy/ex/execution_context.hpp` around lines 526 - 531, Move the explanatory comment about warning C4251 so it precedes BOOST_CAPY_MSVC_WARNING_PUSH for consistency with other regions; specifically, relocate the comment that currently sits between BOOST_CAPY_MSVC_WARNING_DISABLE(4251) and the member declarations so it appears before BOOST_CAPY_MSVC_WARNING_PUSH that wraps the declarations of mutable std::mutex mutex_ and std::shared_ptr<void> owned_.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@include/boost/capy/detail/config.hpp`:
- Around line 24-56: Remove the three _WORKAROUND_GUARD helper macros
(_MSC_VER_WORKAROUND_GUARD, __GNUC___WORKAROUND_GUARD,
__clang_major___WORKAROUND_GUARD) and simplify BOOST_CAPY_WORKAROUND to test the
compiler symbol directly (use the symbol in the boolean and modulus expression)
since undefined preprocessor identifiers evaluate as 0 in `#if` contexts; update
the macro BOOST_CAPY_WORKAROUND(symbol, test) to use (symbol) directly (e.g.
check (symbol != 0) and the 1 % (((symbol) test) + 1) expression) and remove any
references to the removed guard macros (search for BOOST_CAPY_WORKAROUND and the
three guard macro names to locate all occurrences).
---
Nitpick comments:
In `@include/boost/capy/ex/execution_context.hpp`:
- Around line 526-531: Move the explanatory comment about warning C4251 so it
precedes BOOST_CAPY_MSVC_WARNING_PUSH for consistency with other regions;
specifically, relocate the comment that currently sits between
BOOST_CAPY_MSVC_WARNING_DISABLE(4251) and the member declarations so it appears
before BOOST_CAPY_MSVC_WARNING_PUSH that wraps the declarations of mutable
std::mutex mutex_ and std::shared_ptr<void> owned_.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 56af68d2-c436-40ce-ab39-81ba100b6511
⛔ Files ignored due to path filters (1)
test/unit/test_helpers.hppis excluded by!**/test/**
📒 Files selected for processing (12)
include/boost/capy/buffers/make_buffer.hppinclude/boost/capy/delay.hppinclude/boost/capy/detail/await_suspend_helper.hppinclude/boost/capy/detail/config.hppinclude/boost/capy/ex/async_event.hppinclude/boost/capy/ex/async_mutex.hppinclude/boost/capy/ex/detail/timer_service.hppinclude/boost/capy/ex/execution_context.hppinclude/boost/capy/ex/recycling_memory_resource.hppinclude/boost/capy/io_result.hppsrc/cond.cppsrc/error.cpp
|
GCOVR code coverage report https://232.capy.prtest3.cppalliance.org/gcovr/index.html Build time: 2026-03-12 20:16:35 UTC |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## develop #232 +/- ##
===========================================
+ Coverage 92.39% 92.48% +0.08%
===========================================
Files 162 162
Lines 8866 8958 +92
===========================================
+ Hits 8192 8285 +93
+ Misses 674 673 -1
Flags with carried forward coverage won't be shown. Click here to find out more.
... and 11 files with indirect coverage changes Continue to review full report in Codecov by Sentry.
🚀 New features to boost your workflow:
|
…ance#144) Add a standalone BOOST_CAPY_WORKAROUND(symbol, test) macro to detail/config.hpp, modeled on Boost.Config's BOOST_WORKAROUND. The guard mechanism evaluates to 0 when the compiler symbol is undefined, making workaround sites safe and self-documenting across compilers. Also add BOOST_CAPY_MSVC_WARNING_PUSH/DISABLE/POP helpers that use __pragma() on MSVC and expand to nothing elsewhere, replacing the repetitive 4-line #ifdef/#pragma/#endif sandwich pattern. Feature-detection sites (_MSC_VER for TLS keyword, __forceinline, RTTI, __FUNCSIG__) are intentionally left as raw checks since they are not bug workarounds.
mvandeberg
force-pushed
the
pr/144-workaround
branch
from
50fbbed to
ed8bb80
Compare
There was a problem hiding this comment.
🧹 Nitpick comments (2)
include/boost/capy/detail/await_suspend_helper.hpp (1)
54-54: Includedetail/config.hppdirectly in this header.This condition now depends on
BOOST_CAPY_WORKAROUND, but the header only includesboost/capy/ex/io_env.hpp. Relying on a transitive include makes the header brittle and can turn a future include cleanup into a preprocessing error.♻️ Proposed fix
`#include` <coroutine> +#include <boost/capy/detail/config.hpp> `#include` <boost/capy/ex/io_env.hpp>🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@include/boost/capy/detail/await_suspend_helper.hpp` at line 54, This header uses the BOOST_CAPY_WORKAROUND macro but only includes boost/capy/ex/io_env.hpp (relying on a transitive include); update await_suspend_helper.hpp to directly include detail/config.hpp so BOOST_CAPY_WORKAROUND is available; locate the top of the file near the existing include of boost/capy/ex/io_env.hpp and add a direct include for detail/config.hpp (ensuring the include uses the same relative path pattern as other headers in this directory).include/boost/capy/detail/config.hpp (1)
18-22: Drop the divider banner here.The new dashed block adds noise in a short header and conflicts with the repo rule to keep ASCII art to a minimum.
As per coding guidelines, "Keep the ascii-art to a minimum, only add dividers if the class declaration is very long."
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@include/boost/capy/detail/config.hpp` around lines 18 - 22, Remove the ASCII-art divider lines around the "Compiler bug workarounds" header: replace the two "//------------------------------------------------" lines and the surrounding blank lines with a single simple comment like "// Compiler bug workarounds" so only a minimal, non-decorative header remains; update the block containing the existing "//------------------------------------------------" markers and the "Compiler bug workarounds" text in config.hpp (i.e., the commented divider around that section) accordingly.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@include/boost/capy/detail/await_suspend_helper.hpp`:
- Line 54: This header uses the BOOST_CAPY_WORKAROUND macro but only includes
boost/capy/ex/io_env.hpp (relying on a transitive include); update
await_suspend_helper.hpp to directly include detail/config.hpp so
BOOST_CAPY_WORKAROUND is available; locate the top of the file near the existing
include of boost/capy/ex/io_env.hpp and add a direct include for
detail/config.hpp (ensuring the include uses the same relative path pattern as
other headers in this directory).
In `@include/boost/capy/detail/config.hpp`:
- Around line 18-22: Remove the ASCII-art divider lines around the "Compiler bug
workarounds" header: replace the two
"//------------------------------------------------" lines and the surrounding
blank lines with a single simple comment like "// Compiler bug workarounds" so
only a minimal, non-decorative header remains; update the block containing the
existing "//------------------------------------------------" markers and the
"Compiler bug workarounds" text in config.hpp (i.e., the commented divider
around that section) accordingly.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 4c2c5c75-bfc2-4696-8208-4fe652214cb2
⛔ Files ignored due to path filters (1)
test/unit/test_helpers.hppis excluded by!**/test/**
📒 Files selected for processing (12)
include/boost/capy/buffers/make_buffer.hppinclude/boost/capy/delay.hppinclude/boost/capy/detail/await_suspend_helper.hppinclude/boost/capy/detail/config.hppinclude/boost/capy/ex/async_event.hppinclude/boost/capy/ex/async_mutex.hppinclude/boost/capy/ex/detail/timer_service.hppinclude/boost/capy/ex/execution_context.hppinclude/boost/capy/ex/recycling_memory_resource.hppinclude/boost/capy/io_result.hppsrc/cond.cppsrc/error.cpp
🚧 Files skipped from review as they are similar to previous changes (2)
- include/boost/capy/buffers/make_buffer.hpp
- include/boost/capy/delay.hpp
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #144
Add a standalone BOOST_CAPY_WORKAROUND(symbol, test) macro to detail/config.hpp, modeled on Boost.Config's BOOST_WORKAROUND. The guard mechanism evaluates to 0 when the compiler symbol is undefined, making workaround sites safe and self-documenting across compilers.
Also add BOOST_CAPY_MSVC_WARNING_PUSH/DISABLE/POP helpers that use __pragma() on MSVC and expand to nothing elsewhere, replacing the repetitive 4-line #ifdef/#pragma/#endif sandwich pattern.
Feature-detection sites (_MSC_VER for TLS keyword, __forceinline, RTTI, FUNCSIG) are intentionally left as raw checks since they are not bug workarounds.
Summary by CodeRabbit