Skip to content

Trying to use export import name and writing up the module expereince#212

Open
dietmarkuehl wants to merge 9 commits intomainfrom
module-test
Open

Trying to use export import name and writing up the module expereince#212
dietmarkuehl wants to merge 9 commits intomainfrom
module-test

Conversation

@dietmarkuehl
Copy link
Member

No description provided.

@coveralls
Copy link

coveralls commented Feb 5, 2026

Coverage Status

coverage: 93.745% (+1.4%) from 92.3%
when pulling f4df4d9 on module-test
into 75663a1 on main.

@dietmarkuehl dietmarkuehl marked this pull request as ready for review February 5, 2026 23:25
Copilot AI review requested due to automatic review settings February 5, 2026 23:25
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This pull request adds module support to the beman.execution library by removing the BEMAN_EXECUTION_EXPORT macro infrastructure and refactoring code to be module-compatible. The PR enables C++20 modules support while maintaining backward compatibility with traditional header-based builds.

Changes:

  • Removed BEMAN_EXECUTION_EXPORT macro definitions and usages throughout the codebase
  • Refactored lambda functions in impls_for specializations to named structs for module compatibility
  • Added comprehensive documentation describing the module implementation journey and challenges
  • Reorganized header includes to place all standard library headers before module imports
  • Added example demonstrating module usage with headers

Reviewed changes

Copilot reviewed 100 out of 101 changed files in this pull request and generated 6 comments.

Show a summary per file
File Description
docs/modules.md New documentation describing module support implementation experience
.codespellignore Added "Claus" to the ignore list
include/beman/execution/detail/common.hpp Removed BEMAN_EXECUTION_EXPORT macro definition
include/beman/execution/detail/*.hpp (60+ files) Removed BEMAN_EXECUTION_EXPORT from declarations and type aliases
include/beman/execution/detail/when_all.hpp, then.hpp, etc. Converted lambda functions to structs in impls_for specializations
include/beman/execution/detail/basic_sender.hpp Commented out friend declarations and private specifier (workaround)
include/beman/execution/detail/product_type.hpp Added missing #include <tuple>
tests/beman/execution/*.test.cpp (multiple files) Removed redundant includes, reordered headers for module support
tests/beman/execution/include/test/stop_token.hpp Moved test/execution.hpp include and added stoppable_source.hpp
tests/beman/execution/include/test/inline_scheduler.hpp Moved test/execution.hpp before module import
tests/beman/execution/exec-affine-on.test.cpp Added #include <tuple> for structured bindings
examples/modules-and-header.cpp New example demonstrating module usage with headers
examples/CMakeLists.txt Added modules-and-header example

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines 31 to 46
@@ -43,7 +43,7 @@ struct basic_sender : ::beman::execution::detail::product_type<Tag, Data, Child.
requires(!::beman::execution::receiver<Receiver>)
auto connect(Receiver receiver) = BEMAN_EXECUTION_DELETE("the passed receiver doesn't model receiver");

private:
//-dk:TODO private:
Copy link

Copilot AI Feb 5, 2026

Choose a reason for hiding this comment

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

The friend declarations and private access specifier are commented out with TODO comments. This appears to be a workaround for module support. However, this effectively makes the connect member functions public, which may not be the intended API design. If these functions need to remain accessible, consider using a more permanent solution than commented-out code with TODO markers, or document why this change is necessary for module support.

Copilot uses AI. Check for mistakes.
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.

2 participants