From 9ecb6642685b6812c06766bdc6566fd1265ec97e Mon Sep 17 00:00:00 2001 From: Davide Faconti Date: Sat, 18 Jul 2026 14:55:50 +0200 Subject: [PATCH] fix(dialog): make PJ_DIALOG_PLUGIN arg-count dispatch MSVC-legacy-preprocessor-safe MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The two-arg PJ_DIALOG_PLUGIN(Class, kManifest) form mis-expanded under MSVC's traditional preprocessor (the default without /Zc:preprocessor): __VA_ARGS__ forwarded into PJ_DIALOG_PLUGIN_SELECT arrives as a single glued argument, so the dispatch picks the one-arg LEGACY branch with "Class, kManifest" as ClassName (C2064/C2912 at the call site). Found via pj-official-plugins#230's Windows CI. - Add the canonical PJ_DIALOG_PLUGIN_EXPAND rescan around the selected call, making the macro expand identically under both MSVC preprocessors and GCC/Clang. Header-only; no ABI or call-site change. - Drop the INTERFACE /Zc:preprocessor injection from pj_dialog_sdk: it is no longer needed, imposed a preprocessor mode on consumers, and never reached Conan consumers anyway (CMakeDeps regenerates targets from package_info() and drops upstream INTERFACE_COMPILE_OPTIONS — which is exactly how this shipped unnoticed). - Add mock_dialog_legacy_pp_plugin, a compile-only regression target built with /Zc:preprocessor- on MSVC, so Windows CI now exercises the legacy-preprocessor path the conformant-only SDK build never covered. - CHANGELOG: recorded under the in-flight 0.18.0 (no version bump — folds into the upcoming release). Validated: ./build.sh --debug && ./test.sh — 49/49 tests pass. Co-Authored-By: Claude Opus 4.8 --- CHANGELOG.md | 24 ++++++++++++++++++- CMakeLists.txt | 8 ++++--- pj_plugins/dialog_protocol/CMakeLists.txt | 24 +++++++++++++++---- .../pj_plugins/sdk/dialog_plugin_base.hpp | 10 +++++++- 4 files changed, 57 insertions(+), 9 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 73731008..4ab1a12e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -24,7 +24,7 @@ JSON addition; no C ABI change, `PJ_DIALOG_PROTOCOL_VERSION` unchanged): omit-unchanged-fields pattern (with measured costs) and the delta ops. - SDK-side only: hosts apply `table_delta` from the companion PlotJuggler change onward; older hosts ignore the key (harmless no-op). - + ### Feature: QDateTimeEdit event surface (MINOR) The dialog protocol's QDateTimeEdit setters (`setDateTime` / `setDateTimeRange`, @@ -42,6 +42,28 @@ input, not just a display (backward-compatible JSON addition; no C ABI change, ignored — the widget keeps its current value; `QDateEdit`/`QTimeEdit` subclasses share the binding). +### Fix: PJ_DIALOG_PLUGIN two-arg form broke under the MSVC legacy preprocessor (PATCH-level) + +The overload-by-arg-count dispatch behind `PJ_DIALOG_PLUGIN(Class, kManifest)` +mis-expanded under MSVC's traditional preprocessor (the default without +`/Zc:preprocessor`): `__VA_ARGS__` was forwarded into the selector as a single +glued argument, so the two-arg call silently picked the one-arg legacy branch +with `"Class, kManifest"` as the class name (C2064/C2912 at the call site — +found via pj-official-plugins#230's Windows CI). + +- `PJ_DIALOG_PLUGIN_EXPAND` rescan added to the dispatch — the macro now + expands identically under both MSVC preprocessors and GCC/Clang. No ABI or + call-site change; purely a header fix. +- `pj_dialog_sdk` no longer injects `INTERFACE /Zc:preprocessor` into + consumers — the flag is unnecessary now, and it never reached Conan + consumers anyway (CMakeDeps regenerates targets from `package_info()` and + drops upstream `INTERFACE_COMPILE_OPTIONS`, which is how the breakage + shipped unnoticed). Consumers that want the conformant preprocessor set it + themselves. +- New compile-only regression target `mock_dialog_legacy_pp_plugin` builds the + mock dialog with `/Zc:preprocessor-` on MSVC, so Windows CI now exercises + the legacy-preprocessor path the SDK's conformant-only build never covered. + ## [0.17.0] ### Feature: dialog-protocol additions for deletable lists and chart/list placeholders (MINOR) diff --git a/CMakeLists.txt b/CMakeLists.txt index afa90a5a..6d7d5d9b 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -30,9 +30,11 @@ option(PJ_WARNINGS_AS_ERRORS "Treat compiler warnings as errors (-Werror / /WX)" # --------------------------------------------------------------------------- if(MSVC) - # /Zc:preprocessor: conformant preprocessor — required so __VA_ARGS__ inside - # nested macro calls (e.g. PJ_DIALOG_PLUGIN's overload-by-arg-count idiom) - # splits on commas instead of being passed as a single token. + # /Zc:preprocessor: conformant preprocessor. A strictness choice for the + # SDK's own build — no installed header requires it anymore + # (PJ_DIALOG_PLUGIN's overload-by-arg-count idiom is legacy-PP-safe via + # PJ_DIALOG_PLUGIN_EXPAND; a dedicated /Zc:preprocessor- target in + # dialog_protocol guards that). set(PJ_WARNING_FLAGS /W4 /permissive- /Zc:preprocessor) if(PJ_WARNINGS_AS_ERRORS) list(APPEND PJ_WARNING_FLAGS /WX) diff --git a/pj_plugins/dialog_protocol/CMakeLists.txt b/pj_plugins/dialog_protocol/CMakeLists.txt index 9e92c039..c1f04abe 100644 --- a/pj_plugins/dialog_protocol/CMakeLists.txt +++ b/pj_plugins/dialog_protocol/CMakeLists.txt @@ -16,10 +16,12 @@ find_package(nlohmann_json REQUIRED) add_library(pj_dialog_sdk INTERFACE) target_compile_features(pj_dialog_sdk INTERFACE cxx_std_17) -# PJ_DIALOG_PLUGIN's variadic overload needs conformant __VA_ARGS__ expansion on MSVC. -target_compile_options(pj_dialog_sdk INTERFACE - $<$:/Zc:preprocessor> -) +# NOTE: no INTERFACE /Zc:preprocessor here. PJ_DIALOG_PLUGIN's arg-count +# dispatch is legacy-preprocessor-safe via PJ_DIALOG_PLUGIN_EXPAND (see +# dialog_plugin_base.hpp), so the SDK no longer imposes a preprocessor mode +# on consumers. (The old INTERFACE flag never reached Conan consumers anyway: +# CMakeDeps regenerates targets from package_info(), which drops upstream +# INTERFACE_COMPILE_OPTIONS.) target_link_libraries(pj_dialog_sdk INTERFACE pj_dialog_protocol nlohmann_json::nlohmann_json) target_include_directories(pj_dialog_sdk INTERFACE $ @@ -71,6 +73,20 @@ add_library(mock_dialog_plugin SHARED examples/mock_dialog.cpp) target_compile_options(mock_dialog_plugin PRIVATE ${PJ_WARNING_FLAGS}) target_link_libraries(mock_dialog_plugin PRIVATE pj_dialog_sdk) +# Regression guard for the PJ_DIALOG_PLUGIN arg-count dispatch: compile the +# same example under MSVC's TRADITIONAL preprocessor (/Zc:preprocessor- undoes +# the conformant-mode flag PJ_WARNING_FLAGS adds). The SDK's own CI otherwise +# builds everything conformant-only, which is exactly how the legacy-PP +# breakage of the two-arg macro form shipped unnoticed (pj-official-plugins +# PR #230). On non-MSVC compilers the flag is a no-op and this is just a +# second compile of the example. +add_library(mock_dialog_legacy_pp_plugin SHARED examples/mock_dialog.cpp) +target_compile_options(mock_dialog_legacy_pp_plugin PRIVATE + ${PJ_WARNING_FLAGS} + $<$:/Zc:preprocessor-> +) +target_link_libraries(mock_dialog_legacy_pp_plugin PRIVATE pj_dialog_sdk) + add_library(missing_dialog_abi_plugin SHARED tests/missing_dialog_abi_plugin.cpp) target_compile_options(missing_dialog_abi_plugin PRIVATE ${PJ_WARNING_FLAGS}) target_link_libraries(missing_dialog_abi_plugin PRIVATE pj_dialog_protocol) diff --git a/pj_plugins/dialog_protocol/include/pj_plugins/sdk/dialog_plugin_base.hpp b/pj_plugins/dialog_protocol/include/pj_plugins/sdk/dialog_plugin_base.hpp index 3bd4285f..72ff4eaf 100644 --- a/pj_plugins/dialog_protocol/include/pj_plugins/sdk/dialog_plugin_base.hpp +++ b/pj_plugins/dialog_protocol/include/pj_plugins/sdk/dialog_plugin_base.hpp @@ -277,8 +277,16 @@ PJ_borrowed_dialog_t borrowDialog(DialogT& dialog) noexcept { /// the vtable pointer type-safely via `PJ::borrowDialog(member)` — /// no `extern "C"` forward declaration required in the plugin source. #define PJ_DIALOG_PLUGIN_SELECT(_1, _2, NAME, ...) NAME +// The extra PJ_DIALOG_PLUGIN_EXPAND pass makes the overload-by-arg-count +// dispatch correct under BOTH MSVC preprocessors. The traditional MSVC +// preprocessor (the default without /Zc:preprocessor) forwards __VA_ARGS__ +// into a nested macro call as a single argument, so without this rescan a +// two-argument PJ_DIALOG_PLUGIN(Class, kManifest) selects the one-arg LEGACY +// branch with the glued pair as ClassName (C2064/C2912 at the call site). +#define PJ_DIALOG_PLUGIN_EXPAND(x) x #define PJ_DIALOG_PLUGIN(...) \ - PJ_DIALOG_PLUGIN_SELECT(__VA_ARGS__, PJ_DIALOG_PLUGIN_WITH_MANIFEST, PJ_DIALOG_PLUGIN_LEGACY)(__VA_ARGS__) + PJ_DIALOG_PLUGIN_EXPAND( \ + PJ_DIALOG_PLUGIN_SELECT(__VA_ARGS__, PJ_DIALOG_PLUGIN_WITH_MANIFEST, PJ_DIALOG_PLUGIN_LEGACY)(__VA_ARGS__)) #define PJ_DIALOG_PLUGIN_LEGACY(ClassName) PJ_DIALOG_PLUGIN_WITH_MANIFEST(ClassName, nullptr) #define PJ_DIALOG_PLUGIN_WITH_MANIFEST(ClassName, ManifestJson) \ PJ_EXPORT_PLUGIN_ABI_VERSION(PJ_DIALOG_EXPORT) \