From 5e6794683ab17eb01b8e52b1655e7a1844991ff1 Mon Sep 17 00:00:00 2001 From: Petri Savolainen Date: Fri, 3 Jul 2026 12:40:25 +0300 Subject: [PATCH] =?UTF-8?q?Phase=202.1:=20modules=20canary=20=E2=80=94=20s?= =?UTF-8?q?treamr-eventemitter=20+=20streamr-json=20fa=C3=A7ades=20(iosbui?= =?UTF-8?q?ld)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit First C++ named modules in the codebase, in the façade shape from MODERNIZATION.md Part 2: one .cppm partition per public header (header included in the global module fragment, names re-exported via export using), a primary interface unit export-importing the partitions, INTERFACE -> STATIC via streamr_add_module_library(), and the packages' own tests + example flipped from #include to import. - export(TARGETS ... CXX_MODULES_DIRECTORY ...) smoke test passed: the standalone-package export path carries module info. - Downstream find_package + #include consumers build unchanged (GMF facade is ODR-safe as designed). - Root tree: 307/307 tests green; 73 of them now execute through import. - clangd canary better than planned: clangd 22 lints import-using files via the CMake module maps; no lint exclusions needed. .cppm files get clang-format (package lint.sh extended); one clangd-modules quirk (constrained string(string_view) ctor) documented and sidestepped. - Flipped TUs now include what they use (import does not leak transitive std includes). Co-Authored-By: Claude Fable 5 --- MODERNIZATION.md | 37 ++++++++++++++++++- packages/streamr-eventemitter/CMakeLists.txt | 27 +++++++++++--- packages/streamr-eventemitter/lint.sh | 9 +++++ .../streamr.eventemitter-EventEmitter.cppm | 23 ++++++++++++ .../modules/streamr.eventemitter.cppm | 7 ++++ .../test/unit/EventEmitterTest.cpp | 4 +- packages/streamr-json/CMakeLists.txt | 34 ++++++++++++++--- packages/streamr-json/lint.sh | 9 +++++ .../modules/streamr.json-jsonConcepts.cppm | 23 ++++++++++++ .../modules/streamr.json-toJson.cppm | 22 +++++++++++ .../modules/streamr.json-toString.cppm | 13 +++++++ .../streamr-json/modules/streamr.json.cppm | 9 +++++ .../streamr-json/src/examples/JsonExample.cpp | 22 ++++++++--- .../test/unit/TestJsonConcepts.cpp | 3 +- .../streamr-json/test/unit/toJsonTest.cpp | 2 +- .../streamr-json/test/unit/toStringTest.cpp | 4 +- 16 files changed, 225 insertions(+), 23 deletions(-) create mode 100644 packages/streamr-eventemitter/modules/streamr.eventemitter-EventEmitter.cppm create mode 100644 packages/streamr-eventemitter/modules/streamr.eventemitter.cppm create mode 100644 packages/streamr-json/modules/streamr.json-jsonConcepts.cppm create mode 100644 packages/streamr-json/modules/streamr.json-toJson.cppm create mode 100644 packages/streamr-json/modules/streamr.json-toString.cppm create mode 100644 packages/streamr-json/modules/streamr.json.cppm diff --git a/MODERNIZATION.md b/MODERNIZATION.md index 64d05af6..ff9ac6f9 100644 --- a/MODERNIZATION.md +++ b/MODERNIZATION.md @@ -409,7 +409,7 @@ document/replace in 1.4. | 2.5 | streamr-trackerless-network, streamr-libstreamrproxyclient | tn `:protos` over NetworkRpc; proxyclient imports only, C header untouched; full iOS XCFramework + Android smoke. **Final metrics** | | 2.6 | Consolidation (MANDATORY, interleaved) | Per package, once its last dependent is module-based: move declarations into module purview, delete the package's `include/` tree (grep-enforced). End state: no internal headers anywhere; `#include` only for third-party, generated proto, and the C API header. Finalize lint posture; docs | -## Phase 2.0 — Scaffolding + baselines (PR pending) +## Phase 2.0 — Scaffolding + baselines ✅ (PR #28, merged) - `cmake/StreamrModules.cmake` (canonical, synced to all packages): Ninja + non-AppleClang guards, CMP0155 NEW, `streamr_add_module_library()` (STATIC + `FILE_SET CXX_MODULES` rooted at `modules/`), @@ -456,6 +456,41 @@ header parsing, and the top of the expensive list is exactly the primary metric). Capture the Linux numbers on the self-hosted arm64 box and an iOS build-only timing before the Phase 2.4 checkpoint. +## Phase 2.1 — Canary: streamr-eventemitter + streamr-json (PR pending) +First real modules. Both packages carry the designed façade shape: one +`.cppm` partition per public header (header `#include`d in the global +module fragment, public names re-exported with `export using`), a primary +interface unit `export import`ing the partitions, INTERFACE→STATIC via +`streamr_add_module_library()`, package tests + example flipped to +`import streamr.;`. +- **All gates passed on macOS:** + - Standalone package builds produce BMIs + archives; `export(TARGETS … + CXX_MODULES_DIRECTORY …)` generates the module-consumption info + (smoke-test gate — no fallback needed). + - Downstream `find_package` + `#include` consumers (streamr-utils + standalone) build unchanged — the GMF façade is ODR-safe as designed. + - Root tree: full build + **307/307 tests** (16 eventemitter + 57 json of + them now run through `import`). + - **clangd canary came out BETTER than planned**: clangd 22 lints + import-using `.cpp` files through the CMake-generated module maps — + no experimental flag, no lint exclusions. The planned fallback + (excluding import-using files) was not needed. +- **Findings for the next phases:** + - `import` does not leak transitive std includes the way textual + inclusion did — flipped TUs must include what they use (test needed + ``/``; the example needed ``). Expect a small + include-adding pass with every package flip. + - One genuine clangd-modules quirk: the *constrained* + `std::string(std::string_view)` constructor template is not resolved + in import-using files (plain constructors are fine; the compiler + accepts either). Worked around in the example; watch for recurrence. + - clangd's diagnostics in import-using files can carry module-expanded + line numbers in secondary notes — primary locations are correct. +- Lint posture per plan: `.cppm` files get clang-format only (package + lint.sh extended); headers remain the fully-linted source of truth. +- iOS gate via the PR's `iosbuild` keyword (module lib builds; tests are + host-only). Android modules validation lands with its phase-2.5 gate. + ## Lint/IDE survival - During the façade stage, headers remain the fully-linted source of truth (`lint.sh` globs only `*.hpp/*.cpp`); `.cppm` added to clang-format only. diff --git a/packages/streamr-eventemitter/CMakeLists.txt b/packages/streamr-eventemitter/CMakeLists.txt index de6613b5..76a01577 100644 --- a/packages/streamr-eventemitter/CMakeLists.txt +++ b/packages/streamr-eventemitter/CMakeLists.txt @@ -23,12 +23,22 @@ set(CMAKE_TOOLCHAIN_FILE "$ENV{VCPKG_ROOT}/scripts/buildsystems/vcpkg.cmake") project(streamr-eventemitter CXX) message(STATUS "CMAKE_PREFIX_PATH: ${CMAKE_PREFIX_PATH}") - -add_library(streamr-eventemitter INTERFACE) + +# C++ modules support (guards, policies, helpers) — must come after +# project() because it inspects the compiler id. +include(${CMAKE_CURRENT_SOURCE_DIR}/StreamrModules.cmake) + +# Module façade (MODERNIZATION.md Part 2): the package is now a STATIC +# library whose module interface units re-export the public headers. +# #include consumers are unaffected; import consumers get the BMIs. +streamr_add_module_library(streamr-eventemitter + FILES + modules/streamr.eventemitter.cppm + modules/streamr.eventemitter-EventEmitter.cppm) add_library(streamr::streamr-eventemitter ALIAS streamr-eventemitter) target_include_directories( - streamr-eventemitter INTERFACE $ + streamr-eventemitter PUBLIC $ $) # TODO: Is there a better way to do this? @@ -36,9 +46,13 @@ target_include_directories( # - At least parse the deps from the vcpkg.eventemitter file and use them to find_package() in this file and to add the # find_package() calls to the wrapper +# CXX_MODULES_DIRECTORY: the export now also carries the C++ module +# interface information so that a standalone downstream build can compile +# BMIs from this build tree (Phase 2.1 smoke-test gate). export(TARGETS streamr-eventemitter NAMESPACE streamr:: - FILE streamr-eventemitter-config-in.cmake) + FILE streamr-eventemitter-config-in.cmake + CXX_MODULES_DIRECTORY streamr-eventemitter-modules) file(WRITE "${CMAKE_BINARY_DIR}/streamr-eventemitter-config.cmake" "list(APPEND CMAKE_PREFIX_PATH ${CMAKE_PREFIX_PATH})\n" @@ -48,7 +62,10 @@ if(NOT IOS) find_package(GTest CONFIG REQUIRED) add_executable(streamr-eventemitter-test-unit test/unit/EventEmitterTest.cpp) - target_link_libraries(streamr-eventemitter-test-unit + # The test imports streamr.eventemitter, so its sources need module + # dependency scanning. + streamr_enable_imports(streamr-eventemitter-test-unit) + target_link_libraries(streamr-eventemitter-test-unit PUBLIC streamr-eventemitter PUBLIC GTest::gtest PUBLIC GTest::gtest_main diff --git a/packages/streamr-eventemitter/lint.sh b/packages/streamr-eventemitter/lint.sh index f334448a..547d26c0 100755 --- a/packages/streamr-eventemitter/lint.sh +++ b/packages/streamr-eventemitter/lint.sh @@ -9,3 +9,12 @@ clangd-tidy -p ./build $FILES echo "Running clang-format --dry-run on $FILES" ../../run-clang-format.py $FILES + +# Module interface units: format check only. clangd-tidy is not run on +# .cppm files (headers remain the fully linted source of truth during the +# façade migration; clangd modules support is still experimental). +MODULE_FILES=$(find ./modules -type f -name "*.cppm" 2>/dev/null | xargs echo) +if [ -n "$MODULE_FILES" ]; then + echo "Running clang-format --dry-run on $MODULE_FILES" + ../../run-clang-format.py $MODULE_FILES +fi diff --git a/packages/streamr-eventemitter/modules/streamr.eventemitter-EventEmitter.cppm b/packages/streamr-eventemitter/modules/streamr.eventemitter-EventEmitter.cppm new file mode 100644 index 00000000..0bee001a --- /dev/null +++ b/packages/streamr-eventemitter/modules/streamr.eventemitter-EventEmitter.cppm @@ -0,0 +1,23 @@ +// Façade partition over streamr-eventemitter/EventEmitter.hpp. The header +// is included in the global module fragment — its entities stay attached to +// the global module, so `import` and `#include` consumers can be mixed +// ODR-safely during the migration — and the public names are re-exported. +module; + +#include "streamr-eventemitter/EventEmitter.hpp" + +export module streamr.eventemitter:EventEmitter; + +export namespace streamr::eventemitter { + +using streamr::eventemitter::BoundEvent; +using streamr::eventemitter::Event; +using streamr::eventemitter::EventEmitter; +using streamr::eventemitter::EventEmitterImpl; +using streamr::eventemitter::HandlerToken; +using streamr::eventemitter::MatchingCallbackType; +using streamr::eventemitter::MatchingEventType; +using streamr::eventemitter::ReplayEventEmitter; +using streamr::eventemitter::StoredEvent; + +} // namespace streamr::eventemitter diff --git a/packages/streamr-eventemitter/modules/streamr.eventemitter.cppm b/packages/streamr-eventemitter/modules/streamr.eventemitter.cppm new file mode 100644 index 00000000..00c98e1b --- /dev/null +++ b/packages/streamr-eventemitter/modules/streamr.eventemitter.cppm @@ -0,0 +1,7 @@ +// Primary module interface unit of streamr.eventemitter. Consumers write +// `import streamr.eventemitter;` and get every partition re-exported. +// (Façade migration, MODERNIZATION.md Part 2: one partition per public +// header; headers remain the source of truth until consolidation.) +export module streamr.eventemitter; + +export import :EventEmitter; diff --git a/packages/streamr-eventemitter/test/unit/EventEmitterTest.cpp b/packages/streamr-eventemitter/test/unit/EventEmitterTest.cpp index b7912ce7..877dfc15 100644 --- a/packages/streamr-eventemitter/test/unit/EventEmitterTest.cpp +++ b/packages/streamr-eventemitter/test/unit/EventEmitterTest.cpp @@ -2,11 +2,13 @@ #include #include #include +#include #include #include +#include #include -#include "streamr-eventemitter/EventEmitter.hpp" +import streamr.eventemitter; using streamr::eventemitter::Event; using streamr::eventemitter::EventEmitter; diff --git a/packages/streamr-json/CMakeLists.txt b/packages/streamr-json/CMakeLists.txt index 56c6ef5e..7f550d6a 100644 --- a/packages/streamr-json/CMakeLists.txt +++ b/packages/streamr-json/CMakeLists.txt @@ -44,21 +44,38 @@ if(NOT TARGET Boost::pfr) set_target_properties(Boost::pfr PROPERTIES INTERFACE_INCLUDE_DIRECTORIES ${Boost_INCLUDE_DIRS}) endif() -add_library(streamr-json INTERFACE) +# C++ modules support (guards, policies, helpers) — must come after +# project() because it inspects the compiler id. +include(${CMAKE_CURRENT_SOURCE_DIR}/StreamrModules.cmake) + +# Module façade (MODERNIZATION.md Part 2): the package is now a STATIC +# library whose module interface units re-export the public headers. +# #include consumers are unaffected; import consumers get the BMIs. +streamr_add_module_library(streamr-json + FILES + modules/streamr.json.cppm + modules/streamr.json-jsonConcepts.cppm + modules/streamr.json-toJson.cppm + modules/streamr.json-toString.cppm) add_library(streamr::streamr-json ALIAS streamr-json) target_include_directories( - streamr-json INTERFACE $ + streamr-json PUBLIC $ $) -target_link_libraries(streamr-json INTERFACE Boost::pfr) -target_link_libraries(streamr-json INTERFACE nlohmann_json::nlohmann_json) +# PUBLIC (was INTERFACE): the module interface units themselves compile +# against boost-pfr/nlohmann in their global module fragments. +target_link_libraries(streamr-json PUBLIC Boost::pfr) +target_link_libraries(streamr-json PUBLIC nlohmann_json::nlohmann_json) if(NOT IOS) enable_testing() add_executable(streamr-json-test-unit test/unit/TestJsonConcepts.cpp test/unit/toStringTest.cpp test/unit/toJsonTest.cpp) - target_link_libraries(streamr-json-test-unit + # The tests import streamr.json, so their sources need module dependency + # scanning. + streamr_enable_imports(streamr-json-test-unit) + target_link_libraries(streamr-json-test-unit PRIVATE streamr-json PRIVATE GTest::gtest PRIVATE GTest::gtest_main @@ -71,6 +88,7 @@ if(NOT IOS) endif() add_executable(streamr-json-example src/examples/JsonExample.cpp) + streamr_enable_imports(streamr-json-example) target_link_libraries(streamr-json-example PRIVATE streamr-json) endif() @@ -80,9 +98,13 @@ endif() # - At least parse the deps from the vcpkg.json file and use them to find_package() in this file and to add the # find_package() calls to the wrapper +# CXX_MODULES_DIRECTORY: the export now also carries the C++ module +# interface information so that a standalone downstream build can compile +# BMIs from this build tree (Phase 2.1 smoke-test gate). export(TARGETS streamr-json NAMESPACE streamr:: - FILE streamr-json-config-in.cmake) + FILE streamr-json-config-in.cmake + CXX_MODULES_DIRECTORY streamr-json-modules) file(WRITE "${CMAKE_BINARY_DIR}/streamr-json-config.cmake" "list(APPEND CMAKE_FIND_ROOT_PATH \"${CMAKE_FIND_ROOT_PATH}\")\n" diff --git a/packages/streamr-json/lint.sh b/packages/streamr-json/lint.sh index f334448a..547d26c0 100755 --- a/packages/streamr-json/lint.sh +++ b/packages/streamr-json/lint.sh @@ -9,3 +9,12 @@ clangd-tidy -p ./build $FILES echo "Running clang-format --dry-run on $FILES" ../../run-clang-format.py $FILES + +# Module interface units: format check only. clangd-tidy is not run on +# .cppm files (headers remain the fully linted source of truth during the +# façade migration; clangd modules support is still experimental). +MODULE_FILES=$(find ./modules -type f -name "*.cppm" 2>/dev/null | xargs echo) +if [ -n "$MODULE_FILES" ]; then + echo "Running clang-format --dry-run on $MODULE_FILES" + ../../run-clang-format.py $MODULE_FILES +fi diff --git a/packages/streamr-json/modules/streamr.json-jsonConcepts.cppm b/packages/streamr-json/modules/streamr.json-jsonConcepts.cppm new file mode 100644 index 00000000..2a6fcb24 --- /dev/null +++ b/packages/streamr-json/modules/streamr.json-jsonConcepts.cppm @@ -0,0 +1,23 @@ +// Façade partition over streamr-json/jsonConcepts.hpp (see the +// streamr.eventemitter partitions for the pattern rationale). nlohmann and +// boost-pfr arrive in the global module fragment and are NOT re-exported — +// third-party libraries stay #include on the consumer side. +module; + +#include "streamr-json/jsonConcepts.hpp" + +export module streamr.json:jsonConcepts; + +export namespace streamr::json { + +using streamr::json::AssignableToNlohmannJson; +using streamr::json::AssociativeType; +using streamr::json::InitializerList; +using streamr::json::IterableType; +using streamr::json::NotAssignableToNlohmannJson; +using streamr::json::PointerLike; +using streamr::json::PointerType; +using streamr::json::ReflectableType; +using streamr::json::TypeWithToJson; + +} // namespace streamr::json diff --git a/packages/streamr-json/modules/streamr.json-toJson.cppm b/packages/streamr-json/modules/streamr.json-toJson.cppm new file mode 100644 index 00000000..a23de1c2 --- /dev/null +++ b/packages/streamr-json/modules/streamr.json-toJson.cppm @@ -0,0 +1,22 @@ +// Façade partition over streamr-json/toJson.hpp. `using +// streamr::json::toJson;` re-exports the whole overload set (all concept +//-constrained specializations). +module; + +#include "streamr-json/toJson.hpp" + +export module streamr.json:toJson; + +export namespace streamr::json { + +using streamr::json::addStructElementsToJson; +using streamr::json::addStructElementToJson; +using streamr::json::AssignableToJsonBuilder; +using streamr::json::json; +using streamr::json::JsonBuilder; +using streamr::json::JsonInitializerList; +using streamr::json::pointerToJson; +using streamr::json::StreamrJsonInitializerList; +using streamr::json::toJson; + +} // namespace streamr::json diff --git a/packages/streamr-json/modules/streamr.json-toString.cppm b/packages/streamr-json/modules/streamr.json-toString.cppm new file mode 100644 index 00000000..7e29de99 --- /dev/null +++ b/packages/streamr-json/modules/streamr.json-toString.cppm @@ -0,0 +1,13 @@ +// Façade partition over streamr-json/toString.hpp. +module; + +#include "streamr-json/toString.hpp" + +export module streamr.json:toString; + +export namespace streamr::json { + +using streamr::json::toString; +using streamr::json::TypeWithToString; + +} // namespace streamr::json diff --git a/packages/streamr-json/modules/streamr.json.cppm b/packages/streamr-json/modules/streamr.json.cppm new file mode 100644 index 00000000..82973eb1 --- /dev/null +++ b/packages/streamr-json/modules/streamr.json.cppm @@ -0,0 +1,9 @@ +// Primary module interface unit of streamr.json. Consumers write +// `import streamr.json;` and get every partition re-exported. +// (Façade migration, MODERNIZATION.md Part 2: one partition per public +// header; headers remain the source of truth until consolidation.) +export module streamr.json; + +export import :jsonConcepts; +export import :toJson; +export import :toString; diff --git a/packages/streamr-json/src/examples/JsonExample.cpp b/packages/streamr-json/src/examples/JsonExample.cpp index 571dd2d7..5f247e46 100644 --- a/packages/streamr-json/src/examples/JsonExample.cpp +++ b/packages/streamr-json/src/examples/JsonExample.cpp @@ -1,7 +1,10 @@ +#include #include -#include -#include "streamr-json/toJson.hpp" -#include "streamr-json/toString.hpp" +#include +#include +#include + +import streamr.json; using streamr::json::StreamrJsonInitializerList; using streamr::json::toJson; @@ -34,8 +37,12 @@ class ClassWithPrivateSection { std::string name; public: - ClassWithPrivateSection(int data, std::string_view name) - : data(data), name(name) {} + // NB: takes std::string by value instead of std::string_view: clangd's + // (still experimental) modules support cannot resolve the constrained + // std::string(string_view) constructor template in import-using files, + // and this is an example — not worth a lint exclusion. + ClassWithPrivateSection(int data, std::string name) + : data(data), name(std::move(name)) {} // We need to provide a toJson()/toString() methods to be able // to serialize the class because it has private sections @@ -46,7 +53,7 @@ class ClassWithPrivateSection { [[nodiscard]] std::string toString() const { return (toJson()).dump(); } }; -int main() { +int main() try { MyStruct s{.x = 1, .y = "hello"}; // Converting a struct to a json object @@ -85,4 +92,7 @@ int main() { std::cout << str2 << '\n'; return 0; +} catch (const std::exception& e) { + std::cerr << "Example failed: " << e.what() << '\n'; + return 1; } \ No newline at end of file diff --git a/packages/streamr-json/test/unit/TestJsonConcepts.cpp b/packages/streamr-json/test/unit/TestJsonConcepts.cpp index e537cb14..d7abc40a 100644 --- a/packages/streamr-json/test/unit/TestJsonConcepts.cpp +++ b/packages/streamr-json/test/unit/TestJsonConcepts.cpp @@ -1,5 +1,6 @@ #include -#include "streamr-json/toJson.hpp" + +import streamr.json; using streamr::json::toJson; // NOLINT diff --git a/packages/streamr-json/test/unit/toJsonTest.cpp b/packages/streamr-json/test/unit/toJsonTest.cpp index 9fa5b63d..f8e49bf0 100644 --- a/packages/streamr-json/test/unit/toJsonTest.cpp +++ b/packages/streamr-json/test/unit/toJsonTest.cpp @@ -10,7 +10,7 @@ #include "TestClass.hpp" #include "WeatherData.hpp" -#include "streamr-json/toJson.hpp" +import streamr.json; using streamr::json::toJson; diff --git a/packages/streamr-json/test/unit/toStringTest.cpp b/packages/streamr-json/test/unit/toStringTest.cpp index 5833fc4a..94756839 100644 --- a/packages/streamr-json/test/unit/toStringTest.cpp +++ b/packages/streamr-json/test/unit/toStringTest.cpp @@ -7,11 +7,11 @@ #include #include -#include "streamr-json/toString.hpp" - #include "TestClass.hpp" #include "WeatherData.hpp" +import streamr.json; + using streamr::json::toString; // NOLINTBEGIN(readability-magic-numbers)