hardening: close header-only safety gaps (#2)

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Author: LessUp

examples/03-modern-cpp/include/buffer.hpp

@@ -86,6 +86,7 @@ class Buffer {
     }
 
     size_t size() const { return size_; }
+    bool empty() const { return size_ == 0 || data_ == nullptr; }
     char* data() { return data_; }
     const char* data() const { return data_; }
 
@@ -104,6 +105,15 @@ class Buffer {
     }
 };
 
+inline void observe_buffer(const Buffer& buf) {
+    if (buf.empty()) {
+        return;
+    }
+
+    volatile char c = buf.data()[0];
+    (void)c;
+}
+
 //------------------------------------------------------------------------------
 // Functions demonstrating copy vs move
 //------------------------------------------------------------------------------
@@ -112,18 +122,14 @@ class Buffer {
  * @brief Process buffer by copy (expensive)
  */
 inline void process_by_copy(Buffer buf) {
-    // Do something with buf
-    volatile char c = buf.data()[0];
-    (void)c;
+    observe_buffer(buf);
 }
 
 /**
  * @brief Process buffer by const reference (no copy)
  */
 inline void process_by_ref(const Buffer& buf) {
-    // Do something with buf
-    volatile char c = buf.data()[0];
-    (void)c;
+    observe_buffer(buf);
 }
 
 }  // namespace hpc::move_semantics

examples/03-modern-cpp/include/compile_time.hpp

@@ -26,7 +26,7 @@ namespace hpc::compile_time {
 /**
  * @brief Runtime factorial (for comparison)
  */
-int64_t factorial_runtime(int n) {
+inline int64_t factorial_runtime(int n) {
     int64_t result = 1;
     for (int i = 2; i <= n; ++i) {
         result *= i;

examples/03-modern-cpp/include/ranges_utils.hpp

@@ -29,7 +29,7 @@ namespace hpc::ranges {
 /**
  * @brief Transform using raw loop
  */
-void transform_raw_loop(const std::vector<int>& input, std::vector<int>& output) {
+inline void transform_raw_loop(const std::vector<int>& input, std::vector<int>& output) {
     output.resize(input.size());
     for (size_t i = 0; i < input.size(); ++i) {
         output[i] = input[i] * 2 + 1;
@@ -39,15 +39,15 @@ void transform_raw_loop(const std::vector<int>& input, std::vector<int>& output)
 /**
  * @brief Transform using std::transform
  */
-void transform_algorithm(const std::vector<int>& input, std::vector<int>& output) {
+inline void transform_algorithm(const std::vector<int>& input, std::vector<int>& output) {
     output.resize(input.size());
     std::transform(input.begin(), input.end(), output.begin(), [](int x) { return x * 2 + 1; });
 }
 
 /**
  * @brief Transform using ranges
  */
-void transform_ranges(const std::vector<int>& input, std::vector<int>& output) {
+inline void transform_ranges(const std::vector<int>& input, std::vector<int>& output) {
     output.resize(input.size());
     std::ranges::transform(input, output.begin(), [](int x) { return x * 2 + 1; });
 }
@@ -59,7 +59,7 @@ void transform_ranges(const std::vector<int>& input, std::vector<int>& output) {
 /**
  * @brief Filter using raw loop
  */
-std::vector<int> filter_raw_loop(const std::vector<int>& input) {
+inline std::vector<int> filter_raw_loop(const std::vector<int>& input) {
     std::vector<int> output;
     output.reserve(input.size() / 2);  // Estimate
     for (int x : input) {
@@ -73,7 +73,7 @@ std::vector<int> filter_raw_loop(const std::vector<int>& input) {
 /**
  * @brief Filter using std::copy_if
  */
-std::vector<int> filter_algorithm(const std::vector<int>& input) {
+inline std::vector<int> filter_algorithm(const std::vector<int>& input) {
     std::vector<int> output;
     output.reserve(input.size() / 2);
     std::copy_if(input.begin(), input.end(), std::back_inserter(output),
@@ -84,7 +84,7 @@ std::vector<int> filter_algorithm(const std::vector<int>& input) {
 /**
  * @brief Filter using ranges view (lazy)
  */
-auto filter_ranges_view(const std::vector<int>& input) {
+inline auto filter_ranges_view(const std::vector<int>& input) {
     return input | std::views::filter([](int x) { return x % 2 == 0; });
 }
 
@@ -95,7 +95,7 @@ auto filter_ranges_view(const std::vector<int>& input) {
 /**
  * @brief Filter then transform using raw loops
  */
-std::vector<int> chain_raw_loop(const std::vector<int>& input) {
+inline std::vector<int> chain_raw_loop(const std::vector<int>& input) {
     std::vector<int> output;
     output.reserve(input.size() / 2);
     for (int x : input) {
@@ -109,7 +109,7 @@ std::vector<int> chain_raw_loop(const std::vector<int>& input) {
 /**
  * @brief Filter then transform using ranges (lazy, single pass)
  */
-auto chain_ranges_view(const std::vector<int>& input) {
+inline auto chain_ranges_view(const std::vector<int>& input) {
     return input | std::views::filter([](int x) { return x % 2 == 0; }) |
            std::views::transform([](int x) { return x * 2 + 1; });
 }
@@ -133,7 +133,7 @@ std::vector<std::ranges::range_value_t<R>> to_vector(R&& range) {
 /**
  * @brief Sum using raw loop
  */
-int64_t sum_raw_loop(const std::vector<int>& input) {
+inline int64_t sum_raw_loop(const std::vector<int>& input) {
     int64_t sum = 0;
     for (int x : input) {
         sum += x;
@@ -144,7 +144,7 @@ int64_t sum_raw_loop(const std::vector<int>& input) {
 /**
  * @brief Sum using std::accumulate
  */
-int64_t sum_algorithm(const std::vector<int>& input) {
+inline int64_t sum_algorithm(const std::vector<int>& input) {
     return std::accumulate(input.begin(), input.end(), int64_t{0});
 }
 
@@ -154,7 +154,7 @@ int64_t sum_algorithm(const std::vector<int>& input) {
  * C++23 introduces std::ranges::fold_left for this purpose.
  * In C++20 we iterate over a ranges::subrange to stay within the ranges API.
  */
-int64_t sum_ranges(const std::vector<int>& input) {
+inline int64_t sum_ranges(const std::vector<int>& input) {
     int64_t sum = 0;
     for (int x : std::ranges::subrange(input)) {
         sum += x;

examples/05-concurrency/include/lock_free_queue.hpp

@@ -19,6 +19,7 @@
 #include <atomic>
 #include <optional>
 #include <thread>
+#include <utility>
 
 #include "concurrency_utils.hpp"
 
@@ -150,7 +151,7 @@ class MPMCQueue {
 
     struct Cell {
         std::atomic<size_t> sequence;
-        T data;
+        std::optional<T> data;
     };
 
 public:
@@ -166,70 +167,84 @@ class MPMCQueue {
      * @return true if successful, false if queue is full
      */
     bool push(const T& value) {
+        return push_impl(value);
+    }
+
+    /**
+     * @brief Push an element with move semantics (thread-safe)
+     * @param value Element to push
+     * @return true if successful, false if queue is full
+     */
+    bool push(T&& value) {
+        return push_impl(std::move(value));
+    }
+
+    /**
+     * @brief Pop an element from the queue (thread-safe)
+     * @return optional containing the value, or empty if queue is empty
+     */
+    std::optional<T> pop() {
         Cell* cell;
-        size_t pos = enqueue_pos_.load(std::memory_order_relaxed);
+        size_t pos = dequeue_pos_.load(std::memory_order_relaxed);
         int backoff = 1;
 
         for (;;) {
             cell = &cells_[pos & MASK];
             size_t seq = cell->sequence.load(std::memory_order_acquire);
-            intptr_t diff = static_cast<intptr_t>(seq) - static_cast<intptr_t>(pos);
+            intptr_t diff = static_cast<intptr_t>(seq) - static_cast<intptr_t>(pos + 1);
 
             if (diff == 0) {
-                if (enqueue_pos_.compare_exchange_weak(pos, pos + 1, std::memory_order_relaxed)) {
+                if (dequeue_pos_.compare_exchange_weak(pos, pos + 1, std::memory_order_relaxed)) {
                     break;
                 }
             } else if (diff < 0) {
-                return false;
+                return std::nullopt;
             } else {
                 for (int i = 0; i < backoff; ++i) {
                     std::this_thread::yield();
                 }
                 backoff = std::min(backoff * 2, 64);
-                pos = enqueue_pos_.load(std::memory_order_relaxed);
+                pos = dequeue_pos_.load(std::memory_order_relaxed);
             }
         }
 
-        cell->data = value;
-        cell->sequence.store(pos + 1, std::memory_order_release);
-        return true;
+        T value = std::move(*cell->data);
+        cell->data.reset();
+        cell->sequence.store(pos + Capacity, std::memory_order_release);
+        return value;
     }
 
-    /**
-     * @brief Pop an element from the queue (thread-safe)
-     * @return optional containing the value, or empty if queue is empty
-     */
-    std::optional<T> pop() {
+private:
+    template <typename U>
+    bool push_impl(U&& value) {
         Cell* cell;
-        size_t pos = dequeue_pos_.load(std::memory_order_relaxed);
+        size_t pos = enqueue_pos_.load(std::memory_order_relaxed);
         int backoff = 1;
 
         for (;;) {
             cell = &cells_[pos & MASK];
             size_t seq = cell->sequence.load(std::memory_order_acquire);
-            intptr_t diff = static_cast<intptr_t>(seq) - static_cast<intptr_t>(pos + 1);
+            intptr_t diff = static_cast<intptr_t>(seq) - static_cast<intptr_t>(pos);
 
             if (diff == 0) {
-                if (dequeue_pos_.compare_exchange_weak(pos, pos + 1, std::memory_order_relaxed)) {
+                if (enqueue_pos_.compare_exchange_weak(pos, pos + 1, std::memory_order_relaxed)) {
                     break;
                 }
             } else if (diff < 0) {
-                return std::nullopt;
+                return false;
             } else {
                 for (int i = 0; i < backoff; ++i) {
                     std::this_thread::yield();
                 }
                 backoff = std::min(backoff * 2, 64);
-                pos = dequeue_pos_.load(std::memory_order_relaxed);
+                pos = enqueue_pos_.load(std::memory_order_relaxed);
             }
         }
 
-        T value = std::move(cell->data);
-        cell->sequence.store(pos + Capacity, std::memory_order_release);
-        return value;
+        cell->data.emplace(std::forward<U>(value));
+        cell->sequence.store(pos + 1, std::memory_order_release);
+        return true;
     }
-
-private:
     static constexpr size_t MASK = Capacity - 1;
 
     alignas(hpc::core::CACHE_LINE_SIZE) Cell cells_[Capacity];

openspec/changes/header-only-hardening/design.md

@@ -0,0 +1,27 @@
+# Design: Header-Only Hardening
+
+## Overview
+
+This change deepens three fragile modules by moving hidden invariants behind their own interfaces instead of forcing callers and future maintainers to remember them.
+
+## Design Decisions
+
+### 1. `MPMCQueue` owns payload lifetime explicitly
+
+The queue cell will store `std::optional<T>` instead of a permanently live `T`. This removes the accidental default-constructible requirement and gives the module a real interface for generic payloads. The queue will also expose an rvalue enqueue path so move-only and expensive-to-copy types can cross the seam safely.
+
+### 2. `Buffer` processing absorbs the empty-buffer invariant
+
+The move-semantics example currently requires callers to know that `buf.data()[0]` is only valid when `size() > 0`. That is a shallow interface. The hardening change moves the empty-buffer check into the processing helpers so the module remains safe for default-constructed and zero-sized buffers.
+
+### 3. Modern C++ example headers become explicitly link-safe
+
+The repository treats these example headers as header-only modules. Non-template free functions defined in headers will be marked `inline`, and a dedicated multi-translation-unit smoke target will prove the contract during normal builds.
+
+## Testing Strategy
+
+1. Add a failing queue test that uses non-default-constructible and move-only payloads.
+2. Add a failing move-semantics test that exercises empty buffers.
+3. Add a failing multi-translation-unit smoke target for the modern C++ headers.
+4. Apply the minimal production changes to make each loop pass.
+5. Re-run the full preset-driven debug validation path.

openspec/changes/header-only-hardening/proposal.md

@@ -0,0 +1,26 @@
+# Proposal: Header-Only Hardening
+
+## Summary
+
+Harden the header-only teaching modules where the current interface promises more than the implementation can safely deliver, focusing on queue payload support, empty-buffer safety, and multi-translation-unit link safety.
+
+## Motivation
+
+The repository is in closure and hardening mode. Several teaching modules already pass today's tests, but still carry design drift that weakens long-term maintainability:
+
+1. `MPMCQueue` documents generic payload support, yet its implementation requires default-constructible element types and does not expose a move-oriented enqueue path.
+2. The move-semantics example leaks a hidden caller invariant: `process_by_copy` and `process_by_ref` dereference element zero even when the buffer is empty.
+3. Header-defined non-template functions in the modern C++ examples are not explicitly `inline`, which makes the header-only contract fragile across multiple translation units.
+
+## Scope
+
+- Harden the concurrency queue interface so the implementation matches its documented payload story.
+- Harden the move-semantics example against empty-buffer use.
+- Enforce link-safe header-only behavior in modern C++ example headers.
+- Add targeted regression coverage for each fix.
+
+## Out of Scope
+
+- New performance claims or benchmark rewrites.
+- Broad API redesign across unrelated modules.
+- Reworking the docs site or unrelated repository governance surfaces.

openspec/changes/header-only-hardening/specs/concurrency-multithreading/spec.md

@@ -0,0 +1,17 @@
+# Concurrency and Multithreading
+
+## ADDED Requirements
+
+### Requirement: Queue Payload Lifetime Safety
+
+THE Example_Module SHALL allow the lock-free queue interface to store payload types without requiring a default constructor.
+
+#### Scenario: MPMC queue accepts non-default-constructible payloads
+
+- **WHEN** a maintainer instantiates `MPMCQueue<T, N>` with a payload type that is move-constructible but has no default constructor
+- **THEN** the queue builds successfully and preserves payload values through push/pop operations
+
+#### Scenario: MPMC queue preserves move-only payloads
+
+- **WHEN** a maintainer enqueues move-only payloads into `MPMCQueue<T, N>`
+- **THEN** the queue transfers ownership correctly and pops each payload exactly once

openspec/changes/header-only-hardening/specs/modern-cpp-features/spec.md

@@ -0,0 +1,17 @@
+# Modern C++ Features
+
+## ADDED Requirements
+
+### Requirement: Header-Only Example Safety
+
+THE Example_Module SHALL keep header-defined utilities safe to include from multiple translation units.
+
+#### Scenario: Header-only examples link from multiple translation units
+
+- **WHEN** the modern C++ example headers are included from more than one translation unit in the same target
+- **THEN** the target links successfully without duplicate symbol errors
+
+#### Scenario: Empty buffers are safe to observe
+
+- **WHEN** the move-semantics helper functions process a default-constructed or zero-sized `Buffer`
+- **THEN** they do not dereference invalid memory and complete without crashing

openspec/changes/header-only-hardening/tasks.md

@@ -0,0 +1,22 @@
+# Tasks: Header-Only Hardening
+
+## 1. Queue payload hardening
+
+- [x] Add regression coverage showing `MPMCQueue` handles non-default-constructible and move-only payloads.
+- [x] Refactor queue cell storage so element lifetime matches the queue seam.
+- [x] Add move enqueue support without weakening existing behavior.
+
+## 2. Empty-buffer safety
+
+- [x] Add regression coverage for processing default-constructed and zero-sized buffers.
+- [x] Move the empty-buffer invariant behind the move-semantics helper interface.
+
+## 3. Header-only link safety
+
+- [x] Add a multi-translation-unit smoke target that includes the modern C++ headers from more than one source file.
+- [x] Mark header-defined non-template free functions `inline` where needed.
+
+## 4. Validation
+
+- [x] Run the targeted red/green loops for the concurrency and modern C++ test surfaces.
+- [x] Run `cmake --preset=debug && cmake --build build/debug && ctest --preset=debug`.