From 8c38f9f825de0e1f49005710dcd281fcfefdcc50 Mon Sep 17 00:00:00 2001 From: Bhagirath Mehta Date: Tue, 7 Jul 2026 18:38:43 -0500 Subject: [PATCH 1/2] Make public headers safe for consumers building with -Werror Consumers that embed the SDK and compile with -Wall -Wextra -Werror (e.g. ONNX Runtime / Foundry Local via add_subdirectory) were broken by warnings emitted from inside the SDK's public headers. Two complementary changes fix this: Primary (covers every consumer + every warning flag): mark mat's exported public include directory as SYSTEM. find_package consumers already treat an imported target's includes as system; SYSTEM extends that to add_subdirectory/FetchContent consumers, so their -Werror no longer promotes SDK-header warnings to errors. Verified: an -isystem consumer compiles clean under -Wall -Wextra -Wpedantic -Wshadow -Wconversion -Werror on gcc and clang. Defense in depth (also helps non-CMake consumers and NO_SYSTEM_FROM_IMPORTED): - UNREFERENCED_PARAMETER(...) expanded to nothing on gcc/clang, leaving the parameter unused; it now casts to void, eliminating 5 -Wunused-parameter warnings across NullObjects.hpp and LogManagerProvider.hpp. - assert(!"msg") triggered -Wstring-conversion on clang; switched to the canonical assert(false && "msg") in ISemanticContext.hpp (2 sites). Result: including the main public headers under -Wall -Wextra now yields 0 warnings (was 5). SDK's own top-level build verified unaffected (75 UNREFERENCED_PARAMETER call sites compile clean). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- lib/CMakeLists.txt | 8 +++++++- lib/include/public/ISemanticContext.hpp | 4 ++-- lib/include/public/ctmacros.hpp | 8 ++++++-- 3 files changed, 15 insertions(+), 5 deletions(-) diff --git a/lib/CMakeLists.txt b/lib/CMakeLists.txt index 0d4161811..ba142d53e 100644 --- a/lib/CMakeLists.txt +++ b/lib/CMakeLists.txt @@ -352,7 +352,13 @@ endif() # BUILD_INTERFACE is used during the SDK build; INSTALL_INTERFACE is used # by consumers after cmake --install. target_include_directories(mat - PUBLIC + SYSTEM PUBLIC + # SYSTEM marks the public headers as system includes for consumers, so a + # consumer building with -Wall -Wextra -Werror is not broken by warnings + # originating inside the SDK's headers (e.g. -Wpedantic variadic-macro or + # -Wconversion diagnostics). find_package consumers already treat an imported + # target's includes as system; SYSTEM extends the same courtesy to + # add_subdirectory/FetchContent consumers. $ $ PRIVATE diff --git a/lib/include/public/ISemanticContext.hpp b/lib/include/public/ISemanticContext.hpp index 76d9f991b..28d9d97b4 100644 --- a/lib/include/public/ISemanticContext.hpp +++ b/lib/include/public/ISemanticContext.hpp @@ -141,7 +141,7 @@ namespace MAT_NS_BEGIN break; default: - assert(!"Unknown NetworkCost enum value"); + assert(false && "Unknown NetworkCost enum value"); value = ""; break; } @@ -180,7 +180,7 @@ namespace MAT_NS_BEGIN break; default: - assert(!"Unknown NetworkType enum value"); + assert(false && "Unknown NetworkType enum value"); value = ""; break; } diff --git a/lib/include/public/ctmacros.hpp b/lib/include/public/ctmacros.hpp index cabd36f5f..026176a04 100644 --- a/lib/include/public/ctmacros.hpp +++ b/lib/include/public/ctmacros.hpp @@ -67,9 +67,13 @@ # endif #endif -// TODO: [MG] - ideally we'd like to use __attribute__((unused)) with gcc/clang +// Cast the argument(s) to void so the parameter is genuinely referenced. An empty +// expansion left the parameter unused under -Wunused-parameter, which broke +// consumers compiling the SDK headers with -Wextra -Werror. On Windows the Win32 +// SDK provides its own UNREFERENCED_PARAMETER, so this definition only applies +// where that macro is not already defined. #ifndef UNREFERENCED_PARAMETER -#define UNREFERENCED_PARAMETER(...) +#define UNREFERENCED_PARAMETER(...) (void)(__VA_ARGS__) #endif #define OACR_USE_PTR(...) From 98f5338b4f7a59fc6c44fad7e265a00b19c13a3f Mon Sep 17 00:00:00 2001 From: Bhagirath Mehta Date: Tue, 7 Jul 2026 18:50:44 -0500 Subject: [PATCH 2/2] Scope SYSTEM to the public include dir only Address Copilot review comment on lib/CMakeLists.txt:358. The SYSTEM keyword is a per-call flag, so a single target_include_directories(mat SYSTEM PUBLIC ... PRIVATE ...) also marked the SDK-internal PRIVATE include dirs as system for mat's own build, which would suppress warnings in the SDK's own internal headers under -Werror. Split into two calls: SYSTEM PUBLIC for the exported public dir, and a separate non-SYSTEM PRIVATE call for internal dirs. Verified via CMake property query: INTERFACE_SYSTEM_INCLUDE_DIRECTORIES contains only the public dir; the private internal dirs return to their original (non-system) treatment. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- lib/CMakeLists.txt | 17 +++++++++++------ 1 file changed, 11 insertions(+), 6 deletions(-) diff --git a/lib/CMakeLists.txt b/lib/CMakeLists.txt index ba142d53e..9307c05ea 100644 --- a/lib/CMakeLists.txt +++ b/lib/CMakeLists.txt @@ -351,16 +351,21 @@ endif() # PUBLIC propagates to consumers; PRIVATE is SDK-internal only. # BUILD_INTERFACE is used during the SDK build; INSTALL_INTERFACE is used # by consumers after cmake --install. +# +# The public headers are added in a separate SYSTEM call: SYSTEM marks them as +# system includes for consumers, so a consumer building with -Wall -Wextra +# -Werror is not broken by warnings originating inside the SDK's headers (e.g. +# -Wpedantic variadic-macro or -Wconversion diagnostics). find_package consumers +# already treat an imported target's includes as system; SYSTEM extends the same +# courtesy to add_subdirectory/FetchContent consumers. The PRIVATE internal +# include dirs are deliberately kept out of this SYSTEM call so the SDK's own +# -Werror build still diagnoses warnings in its internal headers. target_include_directories(mat SYSTEM PUBLIC - # SYSTEM marks the public headers as system includes for consumers, so a - # consumer building with -Wall -Wextra -Werror is not broken by warnings - # originating inside the SDK's headers (e.g. -Wpedantic variadic-macro or - # -Wconversion diagnostics). find_package consumers already treat an imported - # target's includes as system; SYSTEM extends the same courtesy to - # add_subdirectory/FetchContent consumers. $ $ +) +target_include_directories(mat PRIVATE ${CMAKE_CURRENT_SOURCE_DIR} ${CMAKE_CURRENT_SOURCE_DIR}/include