Skip to content

fix: replace C++20 designated initializers with C++17 compatible assignments #448

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
Ryan-Millard merged 3 commits into from
Jun 21, 2026
Merged

fix: replace C++20 designated initializers with C++17 compatible assignments #448

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
51f887e
fix: replace C++20 designated initializers with C++17 compatible assi…
Ryan-Millard Jun 20, 2026
a1173c0
refactor(bindings/c): strictly conform to C++17 standard
Ryan-Millard Jun 20, 2026
651438d
chore(.coderabbit.yml): update to review every PR
Ryan-Millard Jun 21, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions .coderabbit.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,8 @@ reviews:
enabled: true
auto_incremental_review: true
drafts: false
base_branches:
- ".*"
ignore_title_keywords:
- "WIP"
- "wip"
Expand Down
11 changes: 10 additions & 1 deletion CMakeLists.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,10 +2,19 @@
# Img2Num Top-Level (Manages all libs and apps)
# =================================================================
cmake_minimum_required(VERSION 3.16)
project(Img2Num LANGUAGES CXX)
project(Img2NumRootManager LANGUAGES CXX)

set(CMAKE_CXX_STANDARD 17)
set(CMAKE_CXX_STANDARD_REQUIRED ON)
Comment on lines 7 to 8

@coderabbitai coderabbitai Bot Jun 21, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

# Read the CMakeLists.txt file to see lines 7-8 in context
head -20 CMakeLists.txt | cat -n

Repository: Ryan-Millard/Img2Num

Length of output: 1101

🏁 Script executed:

# Search for C++ standard specifications in all configuration files
rg "CMAKE_CXX_STANDARD|Standard:" --type cmake --type conf

Repository: Ryan-Millard/Img2Num

Length of output: 93

🏁 Script executed:

# Check if .clang-format file exists and verify C++20 requirement
if [ -f .clang-format ]; then
  echo "=== .clang-format content (relevant lines) ===" 
  cat .clang-format | grep -A 2 -B 2 "Standard"
fi

Repository: Ryan-Millard/Img2Num

Length of output: 174

Resolve the C++17 vs C++20 standard mismatch.

CMakeLists.txt line 7 enforces C++17 (set(CMAKE_CXX_STANDARD 17)), but the repository coding guidelines (.clang-format) require C++20 (Standard: c++20). This creates a configuration conflict that must be resolved—either update CMakeLists.txt to enforce C++20, or explicitly update the project standards documentation and all configuration files to C++17.

Note: Line 9 contains a comment explicitly stating C++17 intent, and line 15 includes -Werror=c++20-extensions, suggesting this may be a deliberate choice. However, it contradicts the stated coding standards and requires alignment.

🤖 Prompt for AI Agents 
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@CMakeLists.txt` around lines 7 - 8, Update the CMAKE_CXX_STANDARD value in
CMakeLists.txt from 17 to 20 to align with the C++20 standard specified in the
.clang-format configuration file. Additionally, review and update the comment on
line 9 that explicitly states C++17 intent to reflect the new standard, and
verify that the -Werror=c++20-extensions flag on line 15 is still appropriate
for C++20 enforcement or remove it if no longer needed.

Source: Coding guidelines

# Ensure we strictly conform to the C++17 standard
# This is important for inter-OS reproducibility
set(CMAKE_CXX_EXTENSIONS OFF)

# Reusable flags to ensure good quality code
set(IMG2NUM_STRICT_CXX_FLAGS
$<$<AND:$<COMPILE_LANGUAGE:CXX>,$<OR:$<CXX_COMPILER_ID:GNU>,$<CXX_COMPILER_ID:Clang>,$<CXX_COMPILER_ID:AppleClang>>>:-Wpedantic -Werror=pedantic -Werror=c++20-extensions>
$<$<AND:$<COMPILE_LANGUAGE:CXX>,$<CXX_COMPILER_ID:MSVC>>:/permissive->
Comment on lines +14 to +16

@coderabbitai coderabbitai Bot Jun 21, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🔴 Critical

Fix indentation, line length, and generator expression syntax for strict compiler flags.

Line 15 exceeds the 120-character maximum (174 chars) and both lines 15–16 use 4-space indentation instead of the required 2-space. The space-separated compiler flags in the GNU/Clang generator expression must be quoted with the SHELL: prefix to prevent CMake from misinterpreting them during expansion.

Suggested fix 
 set(IMG2NUM_STRICT_CXX_FLAGS
-    $<$<AND:$<COMPILE_LANGUAGE:CXX>,$<OR:$<CXX_COMPILER_ID:GNU>,$<CXX_COMPILER_ID:Clang>,$<CXX_COMPILER_ID:AppleClang>>>:-Wpedantic -Werror=pedantic -Werror=c++20-extensions>
-    $<$<AND:$<COMPILE_LANGUAGE:CXX>,$<CXX_COMPILER_ID:MSVC>>:/permissive->
+  "$<$<AND:$<COMPILE_LANGUAGE:CXX>,$<OR:$<CXX_COMPILER_ID:GNU>,$<CXX_COMPILER_ID:Clang>,$<CXX_COMPILER_ID:AppleClang>>>:SHELL:-Wpedantic -Werror=pedantic -Werror=c++20-extensions>"
+  "$<$<AND:$<COMPILE_LANGUAGE:CXX>,$<CXX_COMPILER_ID:MSVC>>:/permissive->"
 )

Additionally, line 7 sets CMAKE_CXX_STANDARD 17 but the repository requires C++20 per coding guidelines.

📝 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.

Suggested change
set(IMG2NUM_STRICT_CXX_FLAGS
$<$<AND:$<COMPILE_LANGUAGE:CXX>,$<OR:$<CXX_COMPILER_ID:GNU>,$<CXX_COMPILER_ID:Clang>,$<CXX_COMPILER_ID:AppleClang>>>:-Wpedantic -Werror=pedantic -Werror=c++20-extensions>
$<$<AND:$<COMPILE_LANGUAGE:CXX>,$<CXX_COMPILER_ID:MSVC>>:/permissive->
set(IMG2NUM_STRICT_CXX_FLAGS
"$<$<AND:$<COMPILE_LANGUAGE:CXX>,$<OR:$<CXX_COMPILER_ID:GNU>,$<CXX_COMPILER_ID:Clang>,$<CXX_COMPILER_ID:AppleClang>>>:SHELL:-Wpedantic -Werror=pedantic -Werror=c++20-extensions>"
"$<$<AND:$<COMPILE_LANGUAGE:CXX>,$<CXX_COMPILER_ID:MSVC>>:/permissive->"
)
🤖 Prompt for AI Agents 
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@CMakeLists.txt` around lines 14 - 16, Fix the indentation for the
IMG2NUM_STRICT_CXX_FLAGS set command by changing from 4-space to 2-space
indentation on lines 15 and 16. Add the SHELL: prefix before the space-separated
compiler flags in the GNU/Clang generator expression (the flags like -Wpedantic,
-Werror=pedantic, -Werror=c++20-extensions) to ensure CMake properly interprets
them during expansion. Break line 15 into multiple lines to keep each line under
the 120-character maximum. Additionally, locate CMAKE_CXX_STANDARD on line 7 and
change its value from 17 to 20 to match the repository's C++20 requirement.

)

option(BUILD_PYTHON "Build Python bindings" OFF)
option(IMG2NUM_BUILD_EXAMPLES "Build example applications" ON)
Expand Down
5 changes: 2 additions & 3 deletions bindings/c/CMakeLists.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,9 +1,6 @@
# =================================================================
# Img2Num C Bindings
# =================================================================
set(CMAKE_CXX_STANDARD 17)
set(CMAKE_CXX_STANDARD_REQUIRED ON)

project(CImg2Num
LANGUAGES CXX
# Don't change the inline comment below - release-please needs it
Expand All @@ -21,6 +18,8 @@ set(C_BINDINGS_SRC

add_library(CImg2Num ${C_BINDINGS_SRC})

target_compile_options(CImg2Num PRIVATE ${IMG2NUM_STRICT_CXX_FLAGS})

set_target_properties(CImg2Num PROPERTIES
VERSION ${PROJECT_VERSION}
SOVERSION 1
Expand Down
54 changes: 24 additions & 30 deletions bindings/c/src/cimg2num.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,41 +8,35 @@
extern "C" {

static img2num::ImageToSvgConfig to_cpp(const img2num_ImageToSvgConfig& c) {
// clang-format off
return {
.bilateral_filter {
.sigma_spatial = c.bilateral_filter.sigma_spatial,
.sigma_range = c.bilateral_filter.sigma_range
},
img2num::ImageToSvgConfig cfg {};

.kmeans {
.k = c.kmeans.k,
.max_iter = c.kmeans.max_iter
},
cfg.bilateral_filter.sigma_spatial = c.bilateral_filter.sigma_spatial;
cfg.bilateral_filter.sigma_range = c.bilateral_filter.sigma_range;

cfg.kmeans.k = c.kmeans.k;
cfg.kmeans.max_iter = c.kmeans.max_iter;

.min_cluster_area = c.min_cluster_area,
.min_thickness = c.min_thickness,
.color_space = c.color_space
};
// clang-format on
cfg.min_cluster_area = c.min_cluster_area;
cfg.min_thickness = c.min_thickness;
cfg.color_space = c.color_space;

return cfg;
}

static img2num_ImageToSvgConfig to_c(const img2num::ImageToSvgConfig& cpp) {
// clang-format off
return {
.bilateral_filter {
.sigma_spatial = cpp.bilateral_filter.sigma_spatial,
.sigma_range = cpp.bilateral_filter.sigma_range
},
.kmeans{
.k = cpp.kmeans.k,
.max_iter = cpp.kmeans.max_iter
},
.min_cluster_area = cpp.min_cluster_area,
.min_thickness = cpp.min_thickness,
.color_space = cpp.color_space
};
// clang-format on
img2num_ImageToSvgConfig cfg {};

cfg.bilateral_filter.sigma_spatial = cpp.bilateral_filter.sigma_spatial;
cfg.bilateral_filter.sigma_range = cpp.bilateral_filter.sigma_range;

cfg.kmeans.k = cpp.kmeans.k;
cfg.kmeans.max_iter = cpp.kmeans.max_iter;

cfg.min_cluster_area = cpp.min_cluster_area;
cfg.min_thickness = cpp.min_thickness;
cfg.color_space = cpp.color_space;

return cfg;
}

img2num_ImageToSvgConfig img2num_ImageToSvgConfig_default(void) {
Expand Down
9 changes: 3 additions & 6 deletions bindings/py/CMakeLists.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,9 +1,6 @@
# =================================================================
# Img2Num Python Bindings
# =================================================================
set(CMAKE_CXX_STANDARD 17)
set(CMAKE_CXX_STANDARD_REQUIRED ON)

project(PyImg2Num
LANGUAGES CXX
VERSION 0.0.0
Expand All @@ -12,18 +9,18 @@ project(PyImg2Num
include(CMakePackageConfigHelpers)
include(GNUInstallDirs)

set(CMAKE_CXX_STANDARD 17)
set(CMAKE_CXX_STANDARD_REQUIRED ON)

set(PYBIND11_FINDPYTHON ON)
find_package(Python3 REQUIRED COMPONENTS Interpreter Development NumPy)
find_package(pybind11 REQUIRED)

file(GLOB BINDING_SRC
"${CMAKE_CURRENT_SOURCE_DIR}/src/*.cpp"
)
Comment on lines 16 to 18

@coderabbitai coderabbitai Bot Jun 21, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🧹 Nitpick | 🔵 Trivial | ⚡ Quick win

Make source glob reconfiguration-safe.

Line 16 uses file(GLOB ...) without CONFIGURE_DEPENDS, so newly added binding sources can be skipped until a manual CMake reconfigure.

Suggested fix 
-file(GLOB BINDING_SRC
-    "${CMAKE_CURRENT_SOURCE_DIR}/src/*.cpp"
-)
+file(GLOB BINDING_SRC CONFIGURE_DEPENDS
+  "${CMAKE_CURRENT_SOURCE_DIR}/src/*.cpp"
+)
📝 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.

Suggested change
file(GLOB BINDING_SRC
"${CMAKE_CURRENT_SOURCE_DIR}/src/*.cpp"
)
file(GLOB BINDING_SRC CONFIGURE_DEPENDS
"${CMAKE_CURRENT_SOURCE_DIR}/src/*.cpp"
)
🤖 Prompt for AI Agents 
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@bindings/py/CMakeLists.txt` around lines 16 - 18, The file(GLOB ...) command
used to define BINDING_SRC does not include the CONFIGURE_DEPENDS option, which
means CMake will not automatically reconfigure when new C++ source files are
added to the src directory. Add the CONFIGURE_DEPENDS option to the file(GLOB
BINDING_SRC ...) call to make it reconfiguration-safe and ensure newly added
binding sources are properly detected without requiring manual CMake
reconfiguration.


pybind11_add_module(_img2num MODULE ${BINDING_SRC})

target_compile_options(_img2num PRIVATE ${IMG2NUM_STRICT_CXX_FLAGS})

target_link_libraries(_img2num PRIVATE Img2Num)

target_include_directories(_img2num PRIVATE
Expand Down
5 changes: 2 additions & 3 deletions core/CMakeLists.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,9 +1,6 @@
# =================================================================
# Img2Num Core (C++)
# =================================================================
set(CMAKE_CXX_STANDARD 17)
set(CMAKE_CXX_STANDARD_REQUIRED ON)

project(Img2Num
LANGUAGES CXX
# Don't change the inline comment below - release-please needs it
Expand Down Expand Up @@ -41,6 +38,8 @@ add_custom_target(Img2Num_shaders ALL

add_library(Img2Num ${CORE_SRC})

target_compile_options(Img2Num PRIVATE ${IMG2NUM_STRICT_CXX_FLAGS})

set_target_properties(Img2Num PROPERTIES
VERSION ${PROJECT_VERSION}
SOVERSION 1
Expand Down
13 changes: 7 additions & 6 deletions example-apps/console-c/CMakeLists.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,8 +1,7 @@
# =================================================================
# Example Console App (C)
# =================================================================
cmake_minimum_required(VERSION 3.16)
project(Img2NumExample LANGUAGES C)
project(CImg2NumExample_console_c LANGUAGES C)

set(CMAKE_C_STANDARD 11)
set(CMAKE_C_STANDARD_REQUIRED ON)
Expand All @@ -11,17 +10,19 @@ set(CMAKE_C_STANDARD_REQUIRED ON)
set(EXAMPLE_SRC main.c)

# Build executable
add_executable(console_c_app ${EXAMPLE_SRC})
add_executable(CImg2NumExample_console_c ${EXAMPLE_SRC})

target_compile_options(CImg2NumExample_console_c PRIVATE ${IMG2NUM_STRICT_CXX_FLAGS})

# Include the downloaded headers directly
target_include_directories(console_c_app PRIVATE
target_include_directories(CImg2NumExample_console_c PRIVATE
${CMAKE_SOURCE_DIR}/third_party
)

# Location to store the output images
target_compile_definitions(console_c_app PRIVATE
target_compile_definitions(CImg2NumExample_console_c PRIVATE
OUTPUT_DIR="${CMAKE_BINARY_DIR}/outputs/console-c"
)

# Link against the C bindings
target_link_libraries(console_c_app PRIVATE CImg2Num m)
target_link_libraries(CImg2NumExample_console_c PRIVATE CImg2Num m)

@coderabbitai coderabbitai Bot Jun 21, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

cd example-apps/console-c && cat -n CMakeLists.txt

Repository: Ryan-Millard/Img2Num

Length of output: 1126

🏁 Script executed:

# Check other example apps for patterns of conditional linking
find example-apps -name "CMakeLists.txt" -type f | head -10 | xargs cat -n

Repository: Ryan-Millard/Img2Num

Length of output: 2173

🏁 Script executed:

# Search for existing conditional linking patterns in the main CMakeLists files
rg "if.*MSVC" --type cmake

Repository: Ryan-Millard/Img2Num

Length of output: 46

🏁 Script executed:

# Search for target_link_libraries with math library
rg "target_link_libraries.*\bm\b" --type cmake -A 2 -B 2

Repository: Ryan-Millard/Img2Num

Length of output: 277

🌐 Web query:

MSVC libm math library Windows CMake linking

💡 Result:

When developing with the MSVC compiler on Windows, you should not explicitly link a separate math library (often referred to as 'libm' or 'm') [1][2]. The math functions required by the ISO C standard (such as sin, cos, sqrt, etc.) are provided directly by the Universal C Runtime (UCRT) library [3]. This runtime is automatically linked by MSVC by default [4][2]. Attempting to explicitly link a library named 'm' or 'm.lib' on Windows will typically result in a linker error (e.g., LNK1104: cannot open file 'm.lib'), as no such standalone library exists in the MSVC toolchain [1][5][6]. In CMake projects, you should ensure that 'm' is not added to your link dependencies when targeting MSVC [1][7]. A common, portable approach is to use a conditional check in your CMakeLists.txt to avoid linking 'm' on Windows [1][7][8]: # Example of portable math library linking in CMake if (NOT WIN32) target_link_libraries(your_target PUBLIC m) endif Alternatively, you can use the check_library_exists module to determine if a separate math library is required for your current compiler and platform, which correctly identifies that it is unnecessary on MSVC [8]. If you are working with cross-platform projects or third-party libraries (e.g., via pkg-config), you may occasionally encounter build scripts that incorrectly include '-lm' unconditionally; these should be patched to omit the dependency when the compiler is MSVC [9][6][10].

Citations:

Guard libm linkage for non-MSVC toolchains.

Line 28 links m unconditionally; MSVC does not provide m.lib (math functions are included in UCRT), so this breaks Windows example builds when examples are enabled, resulting in linker error LNK1104.

Suggested fix 
# Link against the C bindings
-target_link_libraries(CImg2NumExample_console_c PRIVATE CImg2Num m)
+target_link_libraries(CImg2NumExample_console_c PRIVATE CImg2Num)
+if(NOT MSVC)
+  target_link_libraries(CImg2NumExample_console_c PRIVATE m)
+endif()
📝 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.

Suggested change
target_link_libraries(CImg2NumExample_console_c PRIVATE CImg2Num m)
target_link_libraries(CImg2NumExample_console_c PRIVATE CImg2Num)
if(NOT MSVC)
target_link_libraries(CImg2NumExample_console_c PRIVATE m)
endif()
🤖 Prompt for AI Agents 
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@example-apps/console-c/CMakeLists.txt` at line 28, The target_link_libraries
call for CImg2NumExample_console_c unconditionally links the math library `m`,
which is not available on Windows with MSVC toolchain and causes linker errors.
Guard the linking of the `m` library by wrapping it in a CMake conditional that
checks if the compiler is not MSVC (using if(NOT MSVC) or similar platform
detection), so that `m` is only linked on non-MSVC toolchains where the math
library is actually available.

16 changes: 7 additions & 9 deletions example-apps/console-cpp/CMakeLists.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,27 +1,25 @@
# =================================================================
# Example Console App (C++)
# =================================================================
cmake_minimum_required(VERSION 3.16)
project(Img2NumExample LANGUAGES CXX)

set(CMAKE_CXX_STANDARD 17)
set(CMAKE_CXX_STANDARD_REQUIRED ON)
project(Img2NumExample_console_cpp LANGUAGES CXX)

# Collect source files
file(GLOB_RECURSE EXAMPLE_SRC "${CMAKE_CURRENT_SOURCE_DIR}/*.cpp")

# Build executable
add_executable(console_cpp_app ${EXAMPLE_SRC})
add_executable(Img2NumExample_console_cpp ${EXAMPLE_SRC})

target_compile_options(Img2NumExample_console_cpp PRIVATE ${IMG2NUM_STRICT_CXX_FLAGS})

# Include the downloaded STB headers
target_include_directories(console_cpp_app PRIVATE
target_include_directories(Img2NumExample_console_cpp PRIVATE
${CMAKE_SOURCE_DIR}/third_party
)

# Location to store the output images
target_compile_definitions(console_cpp_app PRIVATE
target_compile_definitions(Img2NumExample_console_cpp PRIVATE
OUTPUT_DIR="${CMAKE_BINARY_DIR}/outputs/console-cpp"
)

# Link against the core Img2Num library
target_link_libraries(console_cpp_app PRIVATE Img2Num)
target_link_libraries(Img2NumExample_console_cpp PRIVATE Img2Num)
1 change: 0 additions & 1 deletion pyproject.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,6 @@ cmake.source-dir = "."
cmake.build-type = "Release"
cmake.args = [
"-DBUILD_PYTHON=ON",
"-DPython3_FIND_VIRTUALENV=NEVER",
"-DIMG2NUM_BUILD_EXAMPLES=OFF",
]

Expand Down