Skip to content

Conversation

@ClausKlein
Copy link
Collaborator

@ClausKlein ClausKlein commented Dec 7, 2025

@ClausKlein ClausKlein marked this pull request as draft December 7, 2025 23:57
@coveralls
Copy link

coveralls commented Dec 8, 2025

Coverage Status

coverage: 92.047% (+0.2%) from 91.811%
when pulling c70e096 on ClausKlein:feature/prepare-cxx_module-tests
into 6a0bc94 on bemanproject:main.

Copy link
Collaborator Author

@ClausKlein ClausKlein left a comment

Choose a reason for hiding this comment

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

it seems the including inside module code is tricky and not clear to me?

@ClausKlein ClausKlein self-assigned this Dec 8, 2025
Copy link
Member

@dietmarkuehl dietmarkuehl left a comment

Choose a reason for hiding this comment

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

I don't have experience with modules, yet. My original hope/plan was to effectively include all the headers implementing beman::execution into a module unit and export the various visible names. The various exposition-only names would, hopefully, not get exported (although I'm currently using some for beman::task - I may need to move that to beman::execution).

Since there are constraints on how the standard library is made available, I may need to conditionally import std; instead of including the various headers. The approach is probably migrating things individually in dependency order or so: get some feedback on what works/doesn't work before finding out it doesn't work once everything is migrated.

Comment on lines +48 to +53
add_library(${TARGET_NAME} STATIC)

# CMake requires the language standard to be specified as compile feature
# when a target provides C++23 modules and the target will be installed
target_compile_features(${TARGET_NAME} PUBLIC cxx_std_23)

Copy link
Member

Choose a reason for hiding this comment

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

Should/could this section be conditional on whether modules are available/enabled? It looks as if this is unconditionally set up. I believe beman::execution is currently build with multiple C++ standards.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I need a target to set target_compile_features()

# export LDFLAGS=-L$(LLVM_DIR)/lib/c++ -lc++abi -lc++ # -lc++experimental
# export GCOV="llvm-cov gcov"
export CMAKE_CXX_STDLIB_MODULES_JSON=${LLVM_DIR}/lib/c++/libc++.modules.json
export CXX=clang++
Copy link
Member

Choose a reason for hiding this comment

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

I think I would prefer if the compiler were not defaulted like this in the Makefile (although I actually don't even know what export does in Makefiles; it seems something similar to bash's export).

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yes

Copy link
Collaborator Author

@ClausKlein ClausKlein Dec 9, 2025

Choose a reason for hiding this comment

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

I think I would prefer if the compiler were not defaulted like this in the Makefile

as long the cmake feature is experimental and the compiler have problems to find there own json files for stdc++ lib it is needed in that way at leas on OSX!

you my try it in cmake code like here: https://github.com/ClausKlein/cmake-init-modules/blob/develop/cmake/prelude.cmake

export CXXFLAGS:=-stdlib=libstdc++
export GCOV="gcov"
# export CMAKE_CXX_STDLIB_MODULES_JSON=${GCC_DIR}/lib/gcc/current/libstdc++.modules.json
# export CXX:=g++-15
Copy link
Member

Choose a reason for hiding this comment

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

At least, I prefer clang++ over g++-15: I'm mostly using Mac's with Apple silicon and there is no released gcc for these (as far as I know; there is patch from Ian Sandoe I managed to build but even so the result would be named g++-15 for me).

Copy link
Collaborator Author

@ClausKlein ClausKlein Dec 9, 2025

Choose a reason for hiding this comment

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

This is an option you may change like you want, also the version may be changed a you need.

Comment on lines +81 to +85
```cpp
// Copyright (c) 2016-2025 Antony Polukhin
//
// Distributed under the Boost Software License, Version 1.0. (See accompanying
// file LICENSE_1_0.txt or copy at http://www.boost.org/LICENSE_1_0.txt)
Copy link
Member

Choose a reason for hiding this comment

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

I think this implies that I'd prefer a link to the location of where this "cheat sheet" lives rather than including it over here! I'm not opposed to the Boost license but I'd prefer avoiding mixing licenses in a respository.

Copy link
Collaborator Author

@ClausKlein ClausKlein Dec 9, 2025

Choose a reason for hiding this comment

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

this must be at the beman standard an is only an example for you and me.

You will find a simple and cleaned asio module but this is only usable as CXX_MODULES and with or without import std;

@ClausKlein
Copy link
Collaborator Author

I don't have experience with (coding) modules, yet.

me too, but im interesting to know ! see https://github.com/ClausKlein/fmt-module/tree/develop?tab=readme-ov-file#readme

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