Adds C++20 module support #70
Conversation
📝 WalkthroughWalkthroughThis PR adds C++20 module support to boost_corosio by introducing a CMake configuration option, conditional module import paths across header files, and a new module interface file that re-exports the library's public API under the boost::corosio namespace. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related issues
Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 2✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
Comment |
|
An automated preview of the documentation is available at https://70.corosio.prtest3.cppalliance.org/index.html If more commits are pushed to the pull request, the docs will rebuild at the same URL. 2026-01-23 15:38:08 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: 3
🤖 Fix all issues with AI agents
In `@CMakeLists.txt`:
- Around line 189-192: The CMake block using FILE_SET CXX_MODULES
(target_sources boost_corosio PUBLIC FILE_SET CXX_MODULES ...) requires CMake
3.28+, but cmake_minimum_required allows older versions; update the CMake logic
so that when BOOST_COROSIO_USE_MODULES is ON you either enforce
cmake_minimum_required(VERSION 3.28) or add an explicit runtime check (if
(CMAKE_VERSION VERSION_LESS "3.28") message(FATAL_ERROR ...) ) immediately
before the target_sources block to prevent using FILE_SET CXX_MODULES on
unsupported CMake versions.
In `@example/client/http_client.cpp`:
- Around line 38-39: The call to corosio::write currently discards its result,
hiding errors and partial writes; change the co_await corosio::write(stream,
capy::const_buffer(request.data(), request.size())) call to capture and unwrap
the result using the exception-based pattern (i.e., call .value() on the awaited
result) so failures throw instead of being ignored; locate the write call in the
example client where stream and request are used and replace the discarded await
with an awaited result that invokes .value() to restore exception-based error
handling.
- Line 62: The call to co_await s.connect(corosio::endpoint(addr, port))
currently ignores the returned io_result so connection failures are silently
dropped; update the coroutine to capture the result (e.g., auto res = co_await
s.connect(...)) and then check res for errors—either call res.value() inside a
try/catch to propagate exceptions or test res.has_error()/res.error() and
handle/log/return on failure; reference s.connect and the returned
io_result/.value() when making the change.
🧹 Nitpick comments (6)
include/boost/corosio/tcp_server.hpp (1)
41-48: Missing class documentation.Per coding guidelines, docstrings are required for all classes in public headers. The
tcp_serverclass and its public nested classes (worker_base,workers,launcher) lack documentation. Consider adding docstrings explaining their purpose, thread safety guarantees, and usage patterns.modules/boost_corosio.cppm (3)
1-6: Missing high-level overview comment.Per coding guidelines, files should have a
/* */section after includes giving a high-level overview of how the implementation works. Consider adding documentation explaining the module's purpose, what symbols it exports, and any design decisions (e.g., why TLS types are exported at both top-level and in thetlsnamespace).
30-33: Address the TODO comment before merging.The comment suggests uncertainty about whether
tls_stream,openssl_stream, andwolfssl_streamshould be in thetlsnamespace. This API design decision should be resolved to ensure a consistent and intentional public module interface. If they should be intls, move them; otherwise, remove the comment.Do you want me to open an issue to track this design decision?
45-48: Minor: trailing blank lines.There are extra blank lines at the end of the file. Consider removing them for consistency.
include/boost/corosio/socket.hpp (1)
341-342: Add end-of-conditional comments for the nested#endifs.Two back-to-back
#endifs are easy to misread. A short trailing comment improves maintainability.♻️ Proposed tweak
-#endif -#endif +#endif // !defined(BOOST_COROSIO_SOURCE) && defined(BOOST_COROSIO_USE_MODULES) +#endif // BOOST_COROSIO_SOCKET_HPPinclude/boost/corosio/io_buffer_param.hpp (1)
389-390: Add end-of-conditional comments for the nested#endifs.Two back-to-back
#endifs are easy to misread. A short trailing comment improves maintainability.♻️ Proposed tweak
-#endif -#endif +#endif // !defined(BOOST_COROSIO_SOURCE) && defined(BOOST_COROSIO_USE_MODULES) +#endif // BOOST_COROSIO_IO_BUFFER_PARAM_HPP
| if (BOOST_COROSIO_USE_MODULES) | ||
| target_sources(boost_corosio PUBLIC FILE_SET CXX_MODULES BASE_DIRS modules FILES modules/boost_corosio.cppm) | ||
| target_compile_definitions(boost_corosio PUBLIC BOOST_COROSIO_USE_MODULES) | ||
| endif() |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
cat -n CMakeLists.txt | head -50Repository: cppalliance/corosio
Length of output: 2331
🌐 Web query:
CMake FILE_SET CXX_MODULES minimum version requirement
💡 Result:
FILE_SET support in target_sources() starts in CMake 3.23, but the specific file-set type TYPE CXX_MODULES was added in CMake 3.28—so the practical minimum version for FILE_SET ... TYPE CXX_MODULES is 3.28. [1]
(Older CMake versions may mention CXX_MODULES only under experimental modules support, but it’s not the non-experimental FILE_SET CXX_MODULES API as documented for 3.28+.) [1]
Recommended:
cmake_minimum_required(VERSION 3.28)Sources: [1] https://cmake.org/cmake/help/v3.31/command/target_sources.html
🏁 Script executed:
sed -n '185,195p' CMakeLists.txtRepository: cppalliance/corosio
Length of output: 519
Add CMake version check for C++20 modules feature.
The FILE_SET CXX_MODULES feature requires CMake 3.28+, but the current cmake_minimum_required(VERSION 3.16...3.28) permits older versions. When BOOST_COROSIO_USE_MODULES=ON is used with CMake 3.16–3.27, the build will fail. Either add a version check before line 189 or bump cmake_minimum_required to enforce CMake 3.28+ when this feature is enabled.
🤖 Prompt for AI Agents
In `@CMakeLists.txt` around lines 189 - 192, The CMake block using FILE_SET
CXX_MODULES (target_sources boost_corosio PUBLIC FILE_SET CXX_MODULES ...)
requires CMake 3.28+, but cmake_minimum_required allows older versions; update
the CMake logic so that when BOOST_COROSIO_USE_MODULES is ON you either enforce
cmake_minimum_required(VERSION 3.28) or add an explicit runtime check (if
(CMAKE_VERSION VERSION_LESS "3.28") message(FATAL_ERROR ...) ) immediately
before the target_sources block to prevent using FILE_SET CXX_MODULES on
unsupported CMake versions.
| co_await corosio::write( | ||
| stream, capy::const_buffer(request.data(), request.size())); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, let's look at the file structure and locate the example file
git ls-files | grep -E "(http_client|example)" | head -20Repository: cppalliance/corosio
Length of output: 526
🏁 Script executed:
# Read the relevant section of the http_client.cpp file
cat -n example/client/http_client.cpp | head -80Repository: cppalliance/corosio
Length of output: 2777
🏁 Script executed:
# Search for corosio::write and corosio::read usage patterns
rg "corosio::(write|read)" --context 3 -t cppRepository: cppalliance/corosio
Length of output: 5381
🏁 Script executed:
# Look for how io_result is handled elsewhere in the codebase
rg "io_result" --context 2 -t cppRepository: cppalliance/corosio
Length of output: 14490
🏁 Script executed:
# Check if there's any documentation about corosio error handling
fd -e hpp -e cpp | xargs grep -l "corosio" | head -10Repository: cppalliance/corosio
Length of output: 438
Restore exception-based error handling for corosio::write.
The result of co_await corosio::write() is discarded, silently ignoring write failures and partial writes. The file's stated purpose is to "demonstrate the concise exception-based pattern using .value()", so apply that pattern here.
Fix
- co_await corosio::write(
- stream, capy::const_buffer(request.data(), request.size()));
+ (co_await corosio::write(
+ stream, capy::const_buffer(request.data(), request.size()))).value();📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| co_await corosio::write( | |
| stream, capy::const_buffer(request.data(), request.size())); | |
| (co_await corosio::write( | |
| stream, capy::const_buffer(request.data(), request.size()))).value(); |
🤖 Prompt for AI Agents
In `@example/client/http_client.cpp` around lines 38 - 39, The call to
corosio::write currently discards its result, hiding errors and partial writes;
change the co_await corosio::write(stream, capy::const_buffer(request.data(),
request.size())) call to capture and unwrap the result using the exception-based
pattern (i.e., call .value() on the awaited result) so failures throw instead of
being ignored; locate the write call in the example client where stream and
request are used and replace the discarded await with an awaited result that
invokes .value() to restore exception-based error handling.
|
|
||
| // Connect to the server (throws on error) | ||
| (co_await s.connect(corosio::endpoint(addr, port))).value(); | ||
| co_await s.connect(corosio::endpoint(addr, port)); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
find . -name "http_client.cpp" -type fRepository: cppalliance/corosio
Length of output: 95
🏁 Script executed:
head -100 example/client/http_client.cpp | cat -nRepository: cppalliance/corosio
Length of output: 3464
🏁 Script executed:
sed -n '50,80p' example/client/http_client.cpp | cat -nRepository: cppalliance/corosio
Length of output: 990
🏁 Script executed:
# Check the connect method's return type and what it does
rg -A 5 -B 5 "co_await s\.connect" example/client/http_client.cppRepository: cppalliance/corosio
Length of output: 263
🏁 Script executed:
# Search for "throws on error" comment mentioned in the review
rg "throws on error" example/client/http_client.cppRepository: cppalliance/corosio
Length of output: 109
🏁 Script executed:
# Check what io_result is and how it's used elsewhere
rg "io_result" --type cpp | head -20Repository: cppalliance/corosio
Length of output: 1642
🏁 Script executed:
rg -A 10 "class socket" include/boost/corosio/ --type cppRepository: cppalliance/corosio
Length of output: 45
🏁 Script executed:
# Find the connect method in socket class
rg -A 15 "connect\(" include/boost/corosio/ --type cpp | grep -A 15 "co_await\|io_result"Repository: cppalliance/corosio
Length of output: 5319
🏁 Script executed:
# Search for socket connect implementation
fd -e hpp include/boost/corosio | xargs grep -l "socket" | head -5Repository: cppalliance/corosio
Length of output: 452
🏁 Script executed:
# Look at the main socket header
cat -n include/boost/corosio/socket.hpp | head -150Repository: cppalliance/corosio
Length of output: 5305
🏁 Script executed:
# Find io_result definition and check if value() throws
rg -A 20 "io_result.*value" --type cpp | head -40Repository: cppalliance/corosio
Length of output: 45
🏁 Script executed:
# Look for io_result in capy headers
find . -name "*.hpp" -o -name "*.cpp" | xargs grep -l "class io_result\|struct io_result" | head -3Repository: cppalliance/corosio
Length of output: 45
🏁 Script executed:
# Search for uses of .value() on io_result to understand error handling
rg "\.value\(\)" --type cpp example/ | head -10Repository: cppalliance/corosio
Length of output: 975
🏁 Script executed:
# Check how io_result is used elsewhere in the example code
rg "io_result\|co_await.*connect" --type cpp example/Repository: cppalliance/corosio
Length of output: 45
🏁 Script executed:
# Look at the write operation to see the error handling pattern
rg -B 5 -A 5 "co_await corosio::write" example/client/http_client.cppRepository: cppalliance/corosio
Length of output: 429
🏁 Script executed:
# Search for documentation about io_result and how discarding it behaves
rg -B 3 -A 10 "struct connect_awaitable" include/boost/corosio/socket.hppRepository: cppalliance/corosio
Length of output: 416
🏁 Script executed:
# Check the write operation to see if it has similar issues
rg -B 5 -A 5 "co_await corosio::write" include/boost/corosio/Repository: cppalliance/corosio
Length of output: 1347
Handle connect errors—the code ignores the io_result without error checking.
co_await s.connect(...) discards the result without handling it, allowing the coroutine to continue silently even if the connection fails. The comment claims it "throws on error," but that only occurs when explicitly calling .value() on the result.
🐛 Proposed fix (enable exception-based error handling)
- co_await s.connect(corosio::endpoint(addr, port));
+ (co_await s.connect(corosio::endpoint(addr, port))).value();📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| co_await s.connect(corosio::endpoint(addr, port)); | |
| (co_await s.connect(corosio::endpoint(addr, port))).value(); |
🤖 Prompt for AI Agents
In `@example/client/http_client.cpp` at line 62, The call to co_await
s.connect(corosio::endpoint(addr, port)) currently ignores the returned
io_result so connection failures are silently dropped; update the coroutine to
capture the result (e.g., auto res = co_await s.connect(...)) and then check res
for errors—either call res.value() inside a try/catch to propagate exceptions or
test res.has_error()/res.error() and handle/log/return on failure; reference
s.connect and the returned io_result/.value() when making the change.
2 participants
Summary by CodeRabbit
Release Notes
New Features
Improvements
✏️ Tip: You can customize this high-level summary in your review settings.