From 4e88115157c42128886d2ca82f7e8eea880ab0a4 Mon Sep 17 00:00:00 2001 From: Revar Desmera Date: Sat, 25 Jul 2026 18:52:49 -0700 Subject: [PATCH 1/6] Add Windows to CI matrix CMakeLists.txt: winflexbison3 (the Chocolatey package CI installs) provides win_bison.exe/win_flex.exe, not binaries literally named bison/flex -- a new WIN32 block points BISON_EXECUTABLE/FLEX_EXECUTABLE at those explicitly, mirroring the existing macOS Homebrew override right above it rather than relying on FindBISON/FindFLEX's own search to already know the Windows-specific name. ci.yml: adds windows-latest to the matrix, a winflexbison3 install step, and --config/-C Release on the build/test steps (a no-op on the single-config generators Linux/macOS use, but required on Windows's default multi-config Visual Studio generator, which ignores CMAKE_BUILD_TYPE entirely). Also bumps actions/checkout v4 -> v7 while here (v4 still targets node20, which GitHub Actions runners now flag for deprecation). This is the first time this codebase has ever been built with MSVC -- expect follow-up fixes once CI actually runs. Co-Authored-By: Claude Sonnet 5 --- .github/workflows/ci.yml | 20 ++++++++++++++++---- CMakeLists.txt | 17 +++++++++++++++++ 2 files changed, 33 insertions(+), 4 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 6e1e095..9ec765a 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -9,10 +9,10 @@ jobs: test: strategy: matrix: - os: [ubuntu-latest, macos-latest] + os: [ubuntu-latest, macos-latest, windows-latest] runs-on: ${{ matrix.os }} steps: - - uses: actions/checkout@v4 + - uses: actions/checkout@v7 - name: Install bison/flex (Linux) if: runner.os == 'Linux' @@ -22,11 +22,23 @@ jobs: if: runner.os == 'macOS' run: brew install bison flex + - name: Install bison/flex (Windows) + if: runner.os == 'Windows' + # winflexbison3 installs win_bison.exe/win_flex.exe (not + # literally named bison/flex) -- CMakeLists.txt's own WIN32 block + # points BISON_EXECUTABLE/FLEX_EXECUTABLE at those explicitly. + run: choco install winflexbison3 -y + - name: Configure run: cmake -S . -B build -DCMAKE_BUILD_TYPE=Release + # --config/-C Release: a no-op on the single-config generators + # Linux/macOS use (CMAKE_BUILD_TYPE already picked the config), but + # required on Windows's default multi-config Visual Studio + # generator, which ignores CMAKE_BUILD_TYPE entirely -- passing it + # unconditionally on all three platforms is simpler than branching. - name: Build - run: cmake --build build -j + run: cmake --build build --config Release -j - name: Test - run: ctest --test-dir build --output-on-failure + run: ctest --test-dir build --output-on-failure -C Release diff --git a/CMakeLists.txt b/CMakeLists.txt index 065d748..5263197 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -16,6 +16,23 @@ elseif(APPLE AND EXISTS "/usr/local/opt/bison/bin/bison") set(BISON_EXECUTABLE "/usr/local/opt/bison/bin/bison" CACHE FILEPATH "Bison executable" FORCE) endif() +# ponytail: Windows has no bison/flex at all by default; the winflexbison +# Chocolatey package (`choco install winflexbison3`) provides win_bison.exe/ +# win_flex.exe on PATH, not binaries literally named bison/flex -- point +# CMake at those explicitly (mirrors the macOS Homebrew override above) +# rather than relying on FindBISON/FindFLEX's own search to happen to know +# the Windows-specific name. +if(WIN32) + find_program(_win_bison_exe NAMES win_bison) + if(_win_bison_exe) + set(BISON_EXECUTABLE "${_win_bison_exe}" CACHE FILEPATH "Bison executable" FORCE) + endif() + find_program(_win_flex_exe NAMES win_flex) + if(_win_flex_exe) + set(FLEX_EXECUTABLE "${_win_flex_exe}" CACHE FILEPATH "Flex executable" FORCE) + endif() +endif() + find_package(BISON 3.5 REQUIRED) find_package(FLEX REQUIRED) From acb6610e0e061df73286c21b1073c2099328a1e5 Mon Sep 17 00:00:00 2001 From: Revar Desmera Date: Sat, 25 Jul 2026 19:01:56 -0700 Subject: [PATCH 2/6] Windows CI: source bison/flex from MSYS2 instead of winflexbison3 winflexbison3's Chocolatey package is stuck on its 2021 release (bison 3.7.4), which fails parser.y's own `%require "3.8"` check ("require bison 3.8, but have 3.7.4", confirmed by the actual CI run). windows-latest ships MSYS2 pre-installed at C:\msys64 with a current bison (3.8.2); point CMake at those binaries directly, falling back to win_bison/win_flex for a dev machine that has winflexbison instead. Also bumps find_package(BISON ...)'s own minimum from 3.5 to 3.8 to match the grammar's real requirement, so a too-old bison anywhere fails at configure time with a clear message rather than a confusing one from bison itself mid-build. --- .github/workflows/ci.yml | 11 +++++++---- CMakeLists.txt | 36 +++++++++++++++++++++++------------- 2 files changed, 30 insertions(+), 17 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 9ec765a..5a612de 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -24,10 +24,13 @@ jobs: - name: Install bison/flex (Windows) if: runner.os == 'Windows' - # winflexbison3 installs win_bison.exe/win_flex.exe (not - # literally named bison/flex) -- CMakeLists.txt's own WIN32 block - # points BISON_EXECUTABLE/FLEX_EXECUTABLE at those explicitly. - run: choco install winflexbison3 -y + # windows-latest ships MSYS2 pre-installed at C:\msys64 (not on + # PATH). Its bison (3.8.2) satisfies parser.y's %require "3.8"; + # winflexbison3's Chocolatey package is stuck on bison 3.7.4 (2021 + # release) and fails that check. CMakeLists.txt's own WIN32 block + # points BISON_EXECUTABLE/FLEX_EXECUTABLE straight at the MSYS2 + # binaries. + run: C:\msys64\usr\bin\bash.exe -lc "pacman -Sy --noconfirm bison flex" - name: Configure run: cmake -S . -B build -DCMAKE_BUILD_TYPE=Release diff --git a/CMakeLists.txt b/CMakeLists.txt index 5263197..5652344 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -16,24 +16,34 @@ elseif(APPLE AND EXISTS "/usr/local/opt/bison/bin/bison") set(BISON_EXECUTABLE "/usr/local/opt/bison/bin/bison" CACHE FILEPATH "Bison executable" FORCE) endif() -# ponytail: Windows has no bison/flex at all by default; the winflexbison -# Chocolatey package (`choco install winflexbison3`) provides win_bison.exe/ -# win_flex.exe on PATH, not binaries literally named bison/flex -- point -# CMake at those explicitly (mirrors the macOS Homebrew override above) -# rather than relying on FindBISON/FindFLEX's own search to happen to know -# the Windows-specific name. +# ponytail: Windows has no bison/flex at all by default. Prefer MSYS2's +# (kept current -- our grammar needs 3.8+, see parser.y's %require) over the +# winflexbison3 Chocolatey package, whose last release (2021) bundles bison +# 3.7.4 -- confirmed too old by an actual CI failure ("require bison 3.8, +# but have 3.7.4"), not assumed. MSYS2 ships pre-installed on GitHub's +# windows-latest runner image at C:/msys64 (just not on PATH); falls back to +# win_bison/win_flex (e.g. a local dev machine with winflexbison instead) if +# that path doesn't exist. if(WIN32) - find_program(_win_bison_exe NAMES win_bison) - if(_win_bison_exe) - set(BISON_EXECUTABLE "${_win_bison_exe}" CACHE FILEPATH "Bison executable" FORCE) + if(EXISTS "C:/msys64/usr/bin/bison.exe") + set(BISON_EXECUTABLE "C:/msys64/usr/bin/bison.exe" CACHE FILEPATH "Bison executable" FORCE) + else() + find_program(_win_bison_exe NAMES win_bison) + if(_win_bison_exe) + set(BISON_EXECUTABLE "${_win_bison_exe}" CACHE FILEPATH "Bison executable" FORCE) + endif() endif() - find_program(_win_flex_exe NAMES win_flex) - if(_win_flex_exe) - set(FLEX_EXECUTABLE "${_win_flex_exe}" CACHE FILEPATH "Flex executable" FORCE) + if(EXISTS "C:/msys64/usr/bin/flex.exe") + set(FLEX_EXECUTABLE "C:/msys64/usr/bin/flex.exe" CACHE FILEPATH "Flex executable" FORCE) + else() + find_program(_win_flex_exe NAMES win_flex) + if(_win_flex_exe) + set(FLEX_EXECUTABLE "${_win_flex_exe}" CACHE FILEPATH "Flex executable" FORCE) + endif() endif() endif() -find_package(BISON 3.5 REQUIRED) +find_package(BISON 3.8 REQUIRED) find_package(FLEX REQUIRED) include(FetchContent) From 093638cc42720cd5ea2ede782aaf1d0cc04eee11 Mon Sep 17 00:00:00 2001 From: Revar Desmera Date: Sat, 25 Jul 2026 19:04:05 -0700 Subject: [PATCH 3/6] Guard -Wall/-Wextra behind non-MSVC check for Windows CI MSVC's cl.exe doesn't understand GCC/Clang-style -Wall/-Wextra flags ("invalid numeric argument '/Wextra'") -- gate them with a CXX_COMPILER_ID generator expression so Linux/macOS keep the same warnings unchanged and Windows just skips them (MSVC has its own default warning level already). --- src/CMakeLists.txt | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/CMakeLists.txt b/src/CMakeLists.txt index 6ed85be..8a5e156 100644 --- a/src/CMakeLists.txt +++ b/src/CMakeLists.txt @@ -39,4 +39,6 @@ target_include_directories(openscad_cpp_parser PRIVATE ${CMAKE_CURRENT_BINARY_DIR} ) target_link_libraries(openscad_cpp_parser PUBLIC nlohmann_json::nlohmann_json) -target_compile_options(openscad_cpp_parser PRIVATE -Wall -Wextra) +target_compile_options(openscad_cpp_parser PRIVATE + $<$>:-Wall;-Wextra> +) From 2bec715b67190275ff1e404b31cab72669c5c60c Mon Sep 17 00:00:00 2001 From: Revar Desmera Date: Sat, 25 Jul 2026 19:09:23 -0700 Subject: [PATCH 4/6] Fix Bison/Flex grammar for MSVC: automove + nounistd Two genuine first-ever-MSVC compile errors, both from this being the parser's first Windows build: - parser.y: NodePtr (unique_ptr) is move-only. Every action already moves it explicitly, which GCC/Clang tolerate, but MSVC's more eager instantiation of value_type::copy (referenced by emplace whether or not it's ever actually called) fails to compile for a non-copyable T. `%define api.value.automove` is Bison's own documented fix for move-only semantic value types. - lexer.l: flex's generated scanner includes by default, which doesn't exist on Windows/MSVC. `%option nounistd` disables it; we never use the isatty()/POSIX-I/O codepath it guards anyway (never-interactive already opts out of interactive-terminal handling). Verified locally on macOS/Clang first: clean rebuild from scratch, full 626-test suite passes unchanged -- confirms automove didn't alter parse behavior on the platforms this was already working on. --- src/grammar/lexer.l | 5 ++++- src/grammar/parser.y | 7 +++++++ 2 files changed, 11 insertions(+), 1 deletion(-) diff --git a/src/grammar/lexer.l b/src/grammar/lexer.l index c99f377..764c560 100644 --- a/src/grammar/lexer.l +++ b/src/grammar/lexer.l @@ -59,7 +59,10 @@ const std::unordered_map& keywordTable() { #define YY_USER_ACTION updateLocation(driver, yytext, yyleng); %} -%option noyywrap nounput noinput never-interactive +/* nounistd: MSVC/Windows has no ; flex only includes it for + * isatty()/POSIX I/O we never use (never-interactive already disables the + * interactive-terminal codepath), so this is a no-op everywhere else. */ +%option noyywrap nounput noinput never-interactive nounistd %x STR %x INCFILE diff --git a/src/grammar/parser.y b/src/grammar/parser.y index 52edae3..9f4cbcb 100644 --- a/src/grammar/parser.y +++ b/src/grammar/parser.y @@ -4,6 +4,13 @@ %define api.token.constructor %define api.value.type variant +// NodePtr (unique_ptr) is move-only. Every action already moves it +// explicitly (std::move($1) etc.), which is enough for GCC/Clang, but +// MSVC's more eager template instantiation of value_type::copy (used +// internally by emplace, referenced whether or not it's ever actually +// called at runtime) fails to compile for a non-copyable T without this -- +// the Bison manual's own documented fix for move-only semantic types. +%define api.value.automove %define parse.error verbose %locations From b19872d956ce3e8a792d12cb981068a81f016e66 Mon Sep 17 00:00:00 2001 From: Revar Desmera Date: Sat, 25 Jul 2026 19:15:02 -0700 Subject: [PATCH 5/6] Add /Zc:__cplusplus for MSVC: root cause of the C2280 unique_ptr error Root-caused the "attempting to reference a deleted function" copy-ctor error that survived the api.value.automove fix: Bison's own lalr1.cc-skeleton stack-symbol move/copy constructor picks move vs. copy via a YY_MOVE_OR_COPY macro gated purely on `__cplusplus >= 201103L` (parser.tab.hpp) -- nothing to do with automove, which only affects user action code, not this internal stack-management path. MSVC famously always reports __cplusplus as 199711L for legacy-compatibility reasons regardless of the real /std: level unless /Zc:__cplusplus is explicitly passed, so Bison's generated code silently took the pre-C++11 copy-based branch and failed to compile for our move-only NodePtr semantic type. Confirmed by inspecting the actual generated parser.tab.cpp/.hpp locally (YY_MOVE_OR_COPY expands to `copy` under the pre-C++11 branch, `move` otherwise) rather than guessing from the error message alone. --- CMakeLists.txt | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/CMakeLists.txt b/CMakeLists.txt index 5652344..5559131 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -7,6 +7,18 @@ set(CMAKE_CXX_STANDARD 20) set(CMAKE_CXX_STANDARD_REQUIRED ON) set(CMAKE_EXPORT_COMPILE_COMMANDS ON) +# ponytail: MSVC always reports __cplusplus as 199711L regardless of the +# actual /std: level, for legacy compatibility -- a long-standing MSVC-only +# quirk, unless this flag opts into reporting the real value. Bison's own +# generated variant.hh (parser.tab.hpp) branches its whole emplace/copy/move +# API on `__cplusplus >= 201103L`; without this flag MSVC silently takes the +# pre-C++11 copy-based branch, which then fails to compile for any move-only +# %type (this grammar's NodePtr = unique_ptr) with a "deleted copy +# constructor" error that has nothing to do with our own code. +if(MSVC) + add_compile_options(/Zc:__cplusplus) +endif() + # ponytail: macOS ships an ancient (GPLv2, 2.3) bison; point CMake at a # homebrew bison (3.5+) if one is present, since our grammar needs # %skeleton "lalr1.cc" + api.value.type variant support. From b5ab0e7bce3d2126ccf65308eb09c225693cd40a Mon Sep 17 00:00:00 2001 From: Revar Desmera Date: Sat, 25 Jul 2026 19:20:08 -0700 Subject: [PATCH 6/6] Fix Windows test failure: writeFile test helper wrote text-mode SourceMap.ProcessIncludesSplicesFileContentInPlace failed only on windows-latest: the local writeFile() test helper (duplicated in test_source_map.cpp and test_comments_and_files.cpp) opened its ofstream in text mode, so on Windows every '\n' in a fixture's content silently became '\r\n' on disk. The production reader (source_map.cpp's processIncludes) opens included files in binary mode, so it never strips that '\r' back out -- a real, if latent, mismatch that Linux/macOS masked completely (text and binary mode are identical there) until this was the first-ever Windows test run. Fixed by writing the fixture in binary mode too, so what's on disk matches the literal string passed in on every platform. --- tests/test_comments_and_files.cpp | 5 ++++- tests/test_source_map.cpp | 6 +++++- 2 files changed, 9 insertions(+), 2 deletions(-) diff --git a/tests/test_comments_and_files.cpp b/tests/test_comments_and_files.cpp index 8667729..43ff93d 100644 --- a/tests/test_comments_and_files.cpp +++ b/tests/test_comments_and_files.cpp @@ -57,7 +57,10 @@ class TempDir { }; void writeFile(const fs::path& p, const std::string& content) { - std::ofstream out(p); + // Binary mode: see the identical comment in test_source_map.cpp's own + // writeFile -- avoids Windows text-mode \r\n injection that the + // production reader (binary mode) would then faithfully preserve. + std::ofstream out(p, std::ios::binary); out << content; } diff --git a/tests/test_source_map.cpp b/tests/test_source_map.cpp index 524d296..2ecf1bd 100644 --- a/tests/test_source_map.cpp +++ b/tests/test_source_map.cpp @@ -52,7 +52,11 @@ class TempDir { }; void writeFile(const fs::path& p, const std::string& content) { - std::ofstream out(p); + // Binary mode: the production code (source_map.cpp) reads included + // files back in binary mode too, so a text-mode write here would + // silently inject \r\n on Windows that the reader (correctly) never + // strips, corrupting every fixture's expected content on that platform. + std::ofstream out(p, std::ios::binary); out << content; }