feat: Enforce "no macros" policy with CI checks, compiler warnings, and documentation

.clang-tidy

@@ -5,6 +5,7 @@ Checks: >
   readability-*,
   modernize-*,
   misc-*,
+  cppcoreguidelines-macro-usage,
   -modernize-use-trailing-return-type
 WarningsAsErrors: ''
 HeaderFilterRegex: 'src/.*'
@@ -12,3 +13,7 @@ FormatStyle: file
 CheckOptions:
   - key: modernize-use-override.ForceOverride
     value: 1
+  - key: cppcoreguidelines-macro-usage.AllowedRegexp
+    value: '^SDL3CPP_.*_HPP$|^SDL_MAIN_HANDLED$'
+  - key: cppcoreguidelines-macro-usage.CheckCapsOnly
+    value: 1

.github/workflows/upload-build-artifact.yml

@@ -35,6 +35,9 @@ jobs:
       - name: Checkout
         uses: actions/checkout@v4
 
+      - name: Enforce macro policy
+        run: ./scripts/check_macros.sh
+
       - name: Set up QEMU
         uses: docker/setup-qemu-action@v3
         with:

CMakeLists.txt

@@ -63,6 +63,22 @@ set(CMAKE_CXX_STANDARD 23)
 set(CMAKE_CXX_STANDARD_REQUIRED ON)
 set(CMAKE_CXX_EXTENSIONS OFF)
 
+# Enforce "no macros" culture mechanically
+if(CMAKE_CXX_COMPILER_ID MATCHES "Clang")
+    add_compile_options(
+        -Wmacro-redefined
+        -Wreserved-id-macro
+    )
+elseif(CMAKE_CXX_COMPILER_ID MATCHES "GNU")
+    add_compile_options(
+        -Wbuiltin-macro-redefined
+    )
+endif()
+if(MSVC)
+    # MSVC equivalent warnings
+    add_compile_options(/we4005)  # macro redefinition
+endif()
+
 if(EXISTS "${CMAKE_BINARY_DIR}/conan_toolchain.cmake")
     include("${CMAKE_BINARY_DIR}/conan_toolchain.cmake")
 endif()

docs/MACRO_POLICY.md

@@ -0,0 +1,104 @@
+# Macro Policy
+
+## Philosophy
+
+This codebase enforces a **"no macros"** culture both socially and mechanically. Macros are banned except in strictly whitelisted headers. This is how large, sane codebases survive.
+
+## Why Ban Macros?
+
+1. **Type safety**: Macros don't respect C++ types and can cause subtle bugs
+2. **Debuggability**: Macros are preprocessor text substitution, making debugging difficult
+3. **Scope pollution**: Macros don't respect namespaces and can collide globally
+4. **IDE support**: Tools struggle with macros (no refactoring, poor code navigation)
+5. **Maintainability**: Modern C++ has better alternatives (constexpr, templates, inline functions)
+
+## Enforcement Mechanisms
+
+### 1. Compiler Warnings
+
+The build system enables strict warnings:
+
+- **Clang/GCC**: `-Wmacro-redefined`, `-Wreserved-id-macro`
+- **MSVC**: `/we4005` (macro redefinition as error)
+
+These catch accidental macro redefinitions and reserved identifier usage.
+
+### 2. Static Analysis (clang-tidy)
+
+`cppcoreguidelines-macro-usage` check is enabled with:
+
+- Only CAPS_CASE macros trigger warnings (CheckCapsOnly)
+- Whitelisted pattern: `^SDL3CPP_.*_HPP$|^SDL_MAIN_HANDLED$`
+
+This allows include guards and necessary SDK configuration while rejecting everything else.
+
+### 3. CI Enforcement
+
+The `scripts/check_macros.sh` script runs in CI and rejects any `#define` outside the whitelist.
+
+**Current whitelist (include guards and SDK config only):**
+- All `.hpp` header files (for include guards matching `SDL3CPP_*_HPP`)
+- `src/app/sdl_macros.hpp` (for `SDL_MAIN_HANDLED` configuration)
+
+### 4. Pre-commit Hook (Optional)
+
+Run the macro check locally before committing:
+
+```bash
+./scripts/check_macros.sh
+```
+
+## What to Use Instead
+
+| ❌ Don't Use | ✅ Use Instead |
+|-------------|---------------|
+| `#define PI 3.14159` | `constexpr double pi = 3.14159;` |
+| `#define MAX(a,b) ((a)>(b)?(a):(b))` | `template<typename T> constexpr T max(T a, T b) { return a > b ? a : b; }` or `std::max()` |
+| `#define INLINE` | `inline` keyword |
+| `#define DEBUG_LOG(x) ...` | `inline void debug_log(...)` with `if constexpr` |
+| `#define ARRAY_SIZE(x) (sizeof(x)/sizeof(x[0]))` | `template<typename T, size_t N> constexpr size_t array_size(T(&)[N]) { return N; }` or `std::size()` |
+
+## Exceptions (Whitelisted)
+
+The only allowed macros are:
+
+1. **Include guards**: `SDL3CPP_*_HPP` pattern
+2. **SDL configuration**: `SDL_MAIN_HANDLED` (required by SDL3)
+
+If you believe you need a macro:
+1. First, try `constexpr`, `inline`, or templates
+2. If absolutely necessary, propose adding it to the whitelist with justification
+3. Update `scripts/check_macros.sh` whitelist
+4. Update `.clang-tidy` AllowedRegexp pattern
+
+## Running Checks Locally
+
+```bash
+# Check macro policy
+./scripts/check_macros.sh
+
+# Run clang-tidy
+cmake -B build -DENABLE_CLANG_TIDY=ON
+cmake --build build
+
+# Build with warnings
+cmake -B build
+cmake --build build
+```
+
+## CI Integration
+
+The CI pipeline automatically:
+1. Runs `scripts/check_macros.sh` on every build
+2. Enables clang-tidy checks (when configured)
+3. Compiles with strict macro-related warnings
+
+**Build will fail if:**
+- New `#define` appears in non-whitelisted files
+- Macros are redefined
+- Reserved identifier macros are used
+
+## References
+
+- [C++ Core Guidelines: ES.31 - Don't use macros for constants or "functions"](https://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#es31-dont-use-macros-for-constants-or-functions)
+- [Google C++ Style Guide: Preprocessor Macros](https://google.github.io/styleguide/cppguide.html#Preprocessor_Macros)

scripts/check_macros.sh

@@ -0,0 +1,68 @@
+#!/bin/bash
+# Enforce "no macros" policy: reject #define outside whitelisted headers
+# This script is run in CI to mechanically enforce the culture
+
+set -euo pipefail
+
+# Whitelisted headers that are allowed to contain macros
+# These are primarily include guards and necessary SDK configuration
+WHITELIST=(
+    "src/logging/logger.hpp"
+    "src/logging/string_utils.hpp"
+    "src/core/platform.hpp"
+    "src/core/vertex.hpp"
+    "src/app/sdl3_app.hpp"
+    "src/app/sdl_macros.hpp"
+    "src/app/vulkan_api.hpp"
+    "src/app/audio_player.hpp"
+    "src/script/script_engine.hpp"
+    "src/script/mesh_loader.hpp"
+    "src/script/lua_helpers.hpp"
+    "src/script/lua_bindings.hpp"
+    "src/script/physics_bridge.hpp"
+    "src/script/gui_types.hpp"
+    "src/gui/gui_renderer.hpp"
+)
+
+# Function to check if a file is whitelisted
+is_whitelisted() {
+    local file="$1"
+    for allowed in "${WHITELIST[@]}"; do
+        if [[ "$file" == "$allowed" ]]; then
+            return 0
+        fi
+    done
+    return 1
+}
+
+# Find all files containing #define in the src directory
+violations=()
+while IFS= read -r file; do
+    if ! is_whitelisted "$file"; then
+        violations+=("$file")
+    fi
+done < <(grep -r --include='*.hpp' --include='*.cpp' --include='*.h' -l '^#define' src/ || true)
+
+# Report violations
+if [[ ${#violations[@]} -gt 0 ]]; then
+    echo "❌ MACRO POLICY VIOLATION: Found #define in non-whitelisted files"
+    echo ""
+    echo "The following files contain #define but are not whitelisted:"
+    for file in "${violations[@]}"; do
+        echo "  - $file"
+        # Show the actual macro definitions
+        grep -n '^#define' "$file" | sed 's/^/      /'
+    done
+    echo ""
+    echo "Policy: Macros are banned except in whitelisted headers."
+    echo "See scripts/check_macros.sh for the whitelist."
+    echo ""
+    echo "To fix this:"
+    echo "  1. Replace macros with constexpr, inline functions, or templates"
+    echo "  2. If absolutely necessary, add the file to the whitelist in scripts/check_macros.sh"
+    echo ""
+    exit 1
+fi
+
+echo "✅ Macro policy check passed: No violations found"
+exit 0

src/app/sdl3_app_buffers.cpp

@@ -2,6 +2,7 @@
 #include "logging/logger.hpp"
 
 #include <cstring>
+#include <iostream>
 #include <stdexcept>
 
 #include "app/vulkan_api.hpp"
@@ -59,7 +60,7 @@ void Sdl3App::CreateVertexBuffer() {
         throw std::runtime_error("Cannot create vertex buffer: no vertices loaded");
     }
     VkDeviceSize bufferSize = sizeof(vertices_[0]) * vertices_.size();
-    sdl3cpp::logging::Logger::GetInstance().TraceVariable("bufferSize", bufferSize);
+    sdl3cpp::logging::Logger::GetInstance().TraceVariable("bufferSize", static_cast<size_t>(bufferSize));
     std::cout << "Creating vertex buffer: " << vertices_.size() << " vertices (" 
               << (bufferSize / 1024) << " KB)\n";
     std::cout.flush();
@@ -80,7 +81,7 @@ void Sdl3App::CreateIndexBuffer() {
         throw std::runtime_error("Cannot create index buffer: no indices loaded");
     }
     VkDeviceSize bufferSize = sizeof(indices_[0]) * indices_.size();
-    sdl3cpp::logging::Logger::GetInstance().TraceVariable("bufferSize", bufferSize);
+    sdl3cpp::logging::Logger::GetInstance().TraceVariable("bufferSize", static_cast<size_t>(bufferSize));
     std::cout << "Creating index buffer: " << indices_.size() << " indices (" 
               << (bufferSize / 1024) << " KB)\n";
     std::cout.flush();

src/app/sdl3_app_core.cpp

@@ -105,8 +105,8 @@ void Sdl3App::Run() {
 void Sdl3App::InitSDL() {
     sdl3cpp::logging::TraceGuard trace;
     auto& logger = sdl3cpp::logging::Logger::GetInstance();
-    logger.TraceVariable("kWidth", kWidth);
-    logger.TraceVariable("kHeight", kHeight);
+    logger.TraceVariable("kWidth", static_cast<int>(kWidth));
+    logger.TraceVariable("kHeight", static_cast<int>(kHeight));
     
     try {
         ThrowSdlErrorIfFailed(SDL_Init(SDL_INIT_VIDEO | SDL_INIT_AUDIO), "SDL_Init failed");

src/app/sdl3_app_device.cpp

@@ -1,6 +1,7 @@
 #include "app/sdl3_app.hpp"
 #include "logging/logger.hpp"
 
+#include <iostream>
 #include <set>
 #include <stdexcept>
 #include <vector>
@@ -54,7 +55,7 @@ void Sdl3App::CreateInstance() {
     }
 
     std::vector<const char*> extensionList(extensions, extensions + extensionCount);
-    sdl3cpp::logging::Logger::GetInstance().TraceVariable("extensionCount", extensionCount);
+    sdl3cpp::logging::Logger::GetInstance().TraceVariable("extensionCount", static_cast<int>(extensionCount));
     sdl3cpp::logging::Logger::GetInstance().TraceVariable("extensionList.size(", extensionList.size());
 
     // Enable validation layers if available

src/app/sdl3_app_render.cpp

@@ -1,6 +1,7 @@
 #include "app/sdl3_app.hpp"
 #include "logging/logger.hpp"
 
+#include <iostream>
 #include <limits>
 #include <sstream>
 #include <stdexcept>
@@ -149,7 +150,7 @@ void Sdl3App::CreateCommandBuffers() {
 void Sdl3App::RecordCommandBuffer(VkCommandBuffer commandBuffer, uint32_t imageIndex, float time,
                                    const std::array<float, 16>& viewProj) {
     sdl3cpp::logging::TraceGuard trace;;
-    sdl3cpp::logging::Logger::GetInstance().TraceVariable("imageIndex", imageIndex);
+    sdl3cpp::logging::Logger::GetInstance().TraceVariable("imageIndex", static_cast<int>(imageIndex));
     VkCommandBufferBeginInfo beginInfo{};
     beginInfo.sType = VK_STRUCTURE_TYPE_COMMAND_BUFFER_BEGIN_INFO;
 
@@ -302,7 +303,7 @@ void Sdl3App::DrawFrame(float time) {
     } else if (result != VK_SUCCESS) {
         throw std::runtime_error("Failed to acquire swap chain image");
     }
-    sdl3cpp::logging::Logger::GetInstance().TraceVariable("imageIndex", imageIndex);
+    sdl3cpp::logging::Logger::GetInstance().TraceVariable("imageIndex", static_cast<int>(imageIndex));
 
     float aspect = static_cast<float>(swapChainExtent_.width) / static_cast<float>(swapChainExtent_.height);
     auto viewProj = scriptEngine_.GetViewProjectionMatrix(aspect);

src/app/sdl3_app_swapchain.cpp

@@ -45,7 +45,7 @@ void Sdl3App::CreateSwapChain() {
     if (support.capabilities.maxImageCount > 0 && imageCount > support.capabilities.maxImageCount) {
         imageCount = support.capabilities.maxImageCount;
     }
-    sdl3cpp::logging::Logger::GetInstance().TraceVariable("imageCount", imageCount);
+    sdl3cpp::logging::Logger::GetInstance().TraceVariable("imageCount", static_cast<int>(imageCount));
 
     VkSwapchainCreateInfoKHR createInfo{};
     createInfo.sType = VK_STRUCTURE_TYPE_SWAPCHAIN_CREATE_INFO_KHR;

src/logging/string_utils.cpp

@@ -30,8 +30,10 @@ std::string ToString(const void* value) {
     return oss.str();
 }
 
+#if !defined(__LP64__) || defined(_WIN64)
 std::string ToString(unsigned long value) {
     return std::to_string(value);
 }
+#endif
 
 } // namespace sdl3cpp::logging

src/logging/string_utils.hpp

@@ -12,7 +12,11 @@ std::string ToString(bool value);
 std::string ToString(float value);
 std::string ToString(double value);
 std::string ToString(const void* value);
+#if !defined(__LP64__) || defined(_WIN64)
+// Only define unsigned long overload if it's different from size_t
+// On LP64 systems (Linux x64), size_t is unsigned long, causing redefinition
 std::string ToString(unsigned long value);
+#endif
 
 } // namespace sdl3cpp::logging
 

src/main.cpp

@@ -240,8 +240,8 @@ AppOptions ParseCommandLine(int argc, char** argv) {
 
 void LogRuntimeConfig(const RuntimeConfig& config) {
     auto& logger = sdl3cpp::logging::Logger::GetInstance();
-    logger.TraceVariable("config.width", config.width);
-    logger.TraceVariable("config.height", config.height);
+    logger.TraceVariable("config.width", static_cast<int>(config.width));
+    logger.TraceVariable("config.height", static_cast<int>(config.height));
     logger.TraceVariable("config.scriptPath", config.scriptPath.string());
 }