Skip to content

Commit 09ea107

Browse files
author
SidneyCogdill
committed
resolve problems mentioned in code review ...
1. Reverted `extract_example_code_from_docs.py`. 2. Added "Module definition:" and "Client:" to cpp20_modules_support.md, to exclude it from the doc test generator. 3. Minor formatting issues (trailing EOL, empty lines ...) are fixed.
1 parent 3a0157d commit 09ea107

14 files changed

+50
-326
lines changed

.github/workflows/bvt-appleclang.yml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,7 @@ jobs:
2020
2121
- name: build and run test with AppleClang
2222
run: |
23-
cmake -B build -GNinja -DCMAKE_CXX_STANDARD=23 -DCMAKE_BUILD_TYPE=Release -DPROXY_BUILDING_WITH_MODULE=FALSE
23+
cmake -B build -GNinja -DCMAKE_CXX_STANDARD=23 -DCMAKE_BUILD_TYPE=Release -DPROXY_BUILD_MODULES=FALSE
2424
cmake --build build -j
2525
ctest --test-dir build -j
2626
mkdir build/drop

.github/workflows/bvt-clang.yml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,7 @@ jobs:
2626
2727
- name: build and run test with clang 20
2828
run: |
29-
cmake -B build -GNinja -DCMAKE_C_COMPILER=clang-20 -DCMAKE_CXX_COMPILER=clang++-20 -DCMAKE_CXX_FLAGS="-stdlib=libc++" -DCMAKE_CXX_STANDARD=23 -DCMAKE_BUILD_TYPE=Release
29+
cmake -B build -GNinja -DCMAKE_C_COMPILER=clang-20 -DCMAKE_CXX_COMPILER=clang++-20 -DCMAKE_CXX_FLAGS="-stdlib=libc++" -DCMAKE_CXX_STANDARD=23 -DCMAKE_BUILD_TYPE=Release -DPROXY_BUILD_MODULES=TRUE
3030
cmake --build build -j
3131
ctest --test-dir build -j
3232
mkdir build/drop

.github/workflows/bvt-compatibility.yml

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -28,37 +28,37 @@ jobs:
2828
2929
- name: build and run test with gcc 14
3030
run: |
31-
cmake -B build-gcc-14 -GNinja -DCMAKE_C_COMPILER=gcc-14 -DCMAKE_CXX_COMPILER=g++-14 -DCMAKE_BUILD_TYPE=Release
31+
cmake -B build-gcc-14 -GNinja -DCMAKE_C_COMPILER=gcc-14 -DCMAKE_CXX_COMPILER=g++-14 -DCMAKE_BUILD_TYPE=Release -DPROXY_BUILD_MODULES=TRUE
3232
cmake --build build-gcc-14 -j
3333
ctest --test-dir build-gcc-14 -j
3434
3535
- name: build and run test with gcc 13
3636
run: |
37-
cmake -B build-gcc-13 -DCMAKE_C_COMPILER=gcc-13 -DCMAKE_CXX_COMPILER=g++-13 -DCMAKE_BUILD_TYPE=Release -DPROXY_BUILDING_WITH_MODULE=FALSE
37+
cmake -B build-gcc-13 -DCMAKE_C_COMPILER=gcc-13 -DCMAKE_CXX_COMPILER=g++-13 -DCMAKE_BUILD_TYPE=Release -DPROXY_BUILD_MODULES=FALSE
3838
cmake --build build-gcc-13 -j
3939
ctest --test-dir build-gcc-13 -j
4040
4141
- name: build and run test with clang 19
4242
run: |
43-
cmake -B build-clang-19 -DCMAKE_C_COMPILER=clang-19 -DCMAKE_CXX_COMPILER=clang++-19 -DCMAKE_CXX_FLAGS="-stdlib=libc++" -DCMAKE_BUILD_TYPE=Release -DPROXY_BUILDING_WITH_MODULE=FALSE
43+
cmake -B build-clang-19 -DCMAKE_C_COMPILER=clang-19 -DCMAKE_CXX_COMPILER=clang++-19 -DCMAKE_CXX_FLAGS="-stdlib=libc++" -DCMAKE_BUILD_TYPE=Release -DPROXY_BUILD_MODULES=FALSE
4444
cmake --build build-clang-19 -j
4545
ctest --test-dir build-clang-19 -j
4646
4747
- name: build and run test with clang 18
4848
run: |
49-
cmake -B build-clang-18 -DCMAKE_C_COMPILER=clang-18 -DCMAKE_CXX_COMPILER=clang++-18 -DCMAKE_CXX_FLAGS="-stdlib=libc++" -DCMAKE_BUILD_TYPE=Release -DPROXY_BUILDING_WITH_MODULE=FALSE
49+
cmake -B build-clang-18 -DCMAKE_C_COMPILER=clang-18 -DCMAKE_CXX_COMPILER=clang++-18 -DCMAKE_CXX_FLAGS="-stdlib=libc++" -DCMAKE_BUILD_TYPE=Release -DPROXY_BUILD_MODULES=FALSE
5050
cmake --build build-clang-18 -j
5151
ctest --test-dir build-clang-18 -j
5252
5353
- name: build and run test with clang 17
5454
run: |
55-
cmake -B build-clang-17 -DCMAKE_C_COMPILER=clang-17 -DCMAKE_CXX_COMPILER=clang++-17 -DCMAKE_CXX_FLAGS="-stdlib=libc++" -DCMAKE_BUILD_TYPE=Release -DPROXY_BUILDING_WITH_MODULE=FALSE
55+
cmake -B build-clang-17 -DCMAKE_C_COMPILER=clang-17 -DCMAKE_CXX_COMPILER=clang++-17 -DCMAKE_CXX_FLAGS="-stdlib=libc++" -DCMAKE_BUILD_TYPE=Release -DPROXY_BUILD_MODULES=FALSE
5656
cmake --build build-clang-17 -j
5757
ctest --test-dir build-clang-17 -j
5858
5959
# Note that libc++ in Clang 19 is not compatible with Clang 16. Therefore, we fallback to libstdc++.
6060
- name: build and run test with clang 16
6161
run: |
62-
cmake -B build-clang-16 -DCMAKE_C_COMPILER=clang-16 -DCMAKE_CXX_COMPILER=clang++-16 -DCMAKE_BUILD_TYPE=Release -DPROXY_BUILDING_WITH_MODULE=FALSE
62+
cmake -B build-clang-16 -DCMAKE_C_COMPILER=clang-16 -DCMAKE_CXX_COMPILER=clang++-16 -DCMAKE_BUILD_TYPE=Release -DPROXY_BUILD_MODULES=FALSE
6363
cmake --build build-clang-16 -j
6464
ctest --test-dir build-clang-16 -j

.github/workflows/bvt-gcc.yml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,7 @@ jobs:
2323
2424
- name: build and run test with gcc 14
2525
run: |
26-
cmake -B build -GNinja -DCMAKE_C_COMPILER=gcc-14 -DCMAKE_CXX_COMPILER=g++-14 -DCMAKE_CXX_STANDARD=23 -DCMAKE_BUILD_TYPE=Release
26+
cmake -B build -GNinja -DCMAKE_C_COMPILER=gcc-14 -DCMAKE_CXX_COMPILER=g++-14 -DCMAKE_CXX_STANDARD=23 -DCMAKE_BUILD_TYPE=Release -DPROXY_BUILD_MODULES=TRUE
2727
cmake --build build -j
2828
ctest --test-dir build -j
2929
mkdir build/drop

.github/workflows/bvt-msvc.yml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,7 @@ jobs:
1818

1919
- name: build and run test with MSVC
2020
run: |
21-
cmake -B build -DCMAKE_CXX_STANDARD=23
21+
cmake -B build -DCMAKE_CXX_STANDARD=23 -DPROXY_BUILD_MODULES=TRUE
2222
cmake --build build --config Release -j
2323
ctest --test-dir build -j
2424
mkdir build\drop > $null

.github/workflows/bvt-nvhpc.yml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,7 @@ jobs:
2323
- name: build and run test with NVHPC 24.9
2424
run: |
2525
PATH=/opt/nvidia/hpc_sdk/Linux_x86_64/24.9/compilers/bin:$PATH; export PATH
26-
cmake -B build -GNinja -DCMAKE_C_COMPILER=nvc -DCMAKE_CXX_COMPILER=nvc++ -DCMAKE_BUILD_TYPE=Release -DPROXY_BUILDING_WITH_MODULE=FALSE
26+
cmake -B build -GNinja -DCMAKE_C_COMPILER=nvc -DCMAKE_CXX_COMPILER=nvc++ -DCMAKE_BUILD_TYPE=Release -DPROXY_BUILD_MODULES=FALSE
2727
cmake --build build -j
2828
ctest --test-dir build -j
2929
mkdir build/drop

CMakeLists.txt

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -13,9 +13,9 @@ endif()
1313

1414
if(PROJECT_IS_TOP_LEVEL)
1515
option(
16-
PROXY_BUILDING_WITH_MODULE
16+
PROXY_BUILD_MODULES
1717
"When this project is top level, build the docs and tests with C++ module support."
18-
ON
18+
OFF
1919
)
2020
endif()
2121

@@ -38,7 +38,7 @@ target_include_directories(msft_proxy INTERFACE $<BUILD_INTERFACE:${CMAKE_CURREN
3838
# Do not set the module target if proxy is consumed as a subdirectory.
3939
if(PROJECT_IS_TOP_LEVEL)
4040
set(proxy_INCLUDE_DIR include)
41-
if(PROXY_BUILDING_WITH_MODULE)
41+
if(PROXY_BUILD_MODULES)
4242
include(cmake/proxyModuleTargets.cmake)
4343
endif()
4444
else()

docs/cpp20_modules_support.cmake.in

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
if(PROXY_BUILDING_WITH_MODULE)
1+
if(PROXY_BUILD_MODULES)
22
add_executable($NAME$ code_2.cpp)
33

44
target_link_libraries($NAME$ msft_proxy::module)

docs/cpp20_modules_support.md

Lines changed: 16 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -1,14 +1,14 @@
11
# C++20 Modules support
22

3-
proxy ships with `.ixx` files starting with version **4.0.0**. Compared to traditional headers, modules offer faster compilation speed and isolation against preprocessor macro definitions.
3+
The "Proxy" library ships with `.ixx` files starting with version **4.0.0**. Compared to traditional headers, modules offer faster compilation speed and isolation against preprocessor macro definitions.
44

55
As of 2025-05-11, CMake lacks support for forward compatibility when consuming C++ modules, which causes consumers with newer C++ standard to be unable to use modules with older standard. Until this is implemented by CMake, a CMake target containing the module can be manually declared using the following CMake script:
66

77
```cmake
88
find_package(proxy REQUIRED)
99
1010
if(NOT DEFINED proxy_INCLUDE_DIR) # [1]
11-
message(FATAL_ERROR "proxy_INCLUDE_DIR must be defined to use this script.")
11+
message(FATAL_ERROR "proxy_INCLUDE_DIR must be defined to use this script.")
1212
endif()
1313
1414
message(STATUS "Declaring `msft_proxy::module` target for include path ${proxy_INCLUDE_DIR}")
@@ -32,8 +32,8 @@ target_compile_features(msft_proxy_module PUBLIC cxx_std_20) # [2]
3232
target_link_libraries(msft_proxy_module PUBLIC msft_proxy)
3333
```
3434

35-
- [1] `proxy_INCLUDE_DIR` is automatically declared after `find_package(proxy)`
36-
- [2] The C++ standard version for `msft_proxy_module` target should be the same or higher than the consumer CMake target. For example if your project is using C++23 mode, this line should be changed to `cxx_std_23` or `cxx_std_26` / newer standards.
35+
- (1) `proxy_INCLUDE_DIR` is automatically declared after `find_package(proxy)`
36+
- (2) The C++ standard version for `msft_proxy_module` target should be the same or higher than the consumer CMake target. For example if your project is using C++23 mode, this line should be changed to `cxx_std_23` or `cxx_std_26` / newer standards.
3737

3838
It can then be consumed like this:
3939

@@ -43,19 +43,21 @@ target_link_libraries(main PRIVATE msft_proxy::module)
4343

4444
## Example
4545

46+
Module definition:
47+
4648
```cpp
4749
// dictionary.cpp
4850
module;
4951

50-
#include <string> // [1]
52+
#include <string> // (1)
5153

52-
#include <proxy/proxy_macros.h> // [2]
54+
#include <proxy/proxy_macros.h> // (2)
5355

5456
export module dictionary;
5557

56-
import proxy; // [3]
58+
import proxy; // (3)
5759

58-
extern "C++" { // [4]
60+
extern "C++" { // (4)
5961
PRO_DEF_MEM_DISPATCH(MemAt, at);
6062
}
6163

@@ -65,6 +67,8 @@ export struct Dictionary : pro::facade_builder
6567

6668
```
6769
70+
Client:
71+
6872
```cpp
6973
// main.cpp
7074
#include <vector>
@@ -81,8 +85,8 @@ int main() {
8185
8286
```
8387

84-
- [1] This is a traditional header rather than a module. It should be declared in global fragment (after `module` and before `export module`).
85-
- [2] This makes all `PRO_DEF_` macros available. This header file contains only some macros and are therefore very fast to compile.
86-
- [3] `import proxy;` makes all public interfaces from `pro` namespace available in the current translation unit.
87-
- [4] As of 2025-05-11, clangd requires the accessor struct to be either `export`-ed, or be declared within an `extern "C++"` block, in order to have auto completion working.
88+
- (1) This is a traditional header rather than a module. It should be declared in global fragment (after `module` and before `export module`).
89+
- (2) This makes all `PRO_DEF_` macros available. This header file contains only some macros and are therefore very fast to compile.
90+
- (3) `import proxy;` makes all public interfaces from `pro` namespace available in the current translation unit.
91+
- (4) As of 2025-05-11, clangd requires the accessor struct to be either `export`-ed, or be declared within an `extern "C++"` block, in order to have auto completion working.
8892

include/proxy/proxy.ixx

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -89,4 +89,4 @@ using std::formatter;
8989
export namespace pro::details {
9090
using details::adl_accessor_arg_t;
9191
using details::non_proxy_arg;
92-
} // namespace pro::details
92+
} // namespace pro::details

include/proxy/proxy_macros.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -180,4 +180,4 @@ ___PRO_DEBUG( \
180180
#define PRO_DEF_FREE_AS_MEM_DISPATCH(__NAME, ...) \
181181
___PRO_EXPAND_MACRO(___PRO_DEF_FREE_AS_MEM_DISPATCH, __NAME, __VA_ARGS__)
182182

183-
#endif // _MSFT_PROXY_MACROS_
183+
#endif // _MSFT_PROXY_MACROS_

tests/cpp20_modules/CMakeLists.txt

Lines changed: 2 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1,23 +1,18 @@
11

2-
if(PROXY_BUILDING_WITH_MODULE)
3-
2+
if(PROXY_BUILD_MODULES)
43
add_executable(msft_proxy_tests_cpp20_modules
54
main.cpp
65
)
7-
86
target_sources(msft_proxy_tests_cpp20_modules
97
PRIVATE
108
FILE_SET CXX_MODULES FILES
119
foo.cpp
1210
impl.cpp
1311
)
14-
1512
target_compile_features(
1613
msft_proxy_tests_cpp20_modules
1714
PRIVATE
1815
cxx_std_20
1916
)
20-
2117
target_link_libraries(msft_proxy_tests_cpp20_modules PRIVATE msft_proxy msft_proxy::module)
22-
23-
endif()
18+
endif()

tests/cpp20_modules/foo.cpp

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -16,10 +16,10 @@ PRO_DEF_FREE_AS_MEM_DISPATCH(FreeGetBaz, GetBaz);
1616
}
1717

1818
export struct Foo
19-
: pro::facade_builder ::add_convention<MemGetFoo, int() const>::build {};
19+
: pro::facade_builder::add_convention<MemGetFoo, int() const>::build {};
2020

2121
export struct Bar
22-
: pro::facade_builder ::add_convention<FreeGetBar, int() const>::build {};
22+
: pro::facade_builder::add_convention<FreeGetBar, int() const>::build {};
2323

2424
export struct Baz
25-
: pro::facade_builder ::add_convention<FreeGetBaz, int() const>::build {};
25+
: pro::facade_builder::add_convention<FreeGetBaz, int() const>::build {};

0 commit comments

Comments
 (0)