Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 12 additions & 6 deletions examples/03-modern-cpp/include/buffer.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -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_; }

Expand All @@ -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
//------------------------------------------------------------------------------
Expand All @@ -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
2 changes: 1 addition & 1 deletion examples/03-modern-cpp/include/compile_time.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
22 changes: 11 additions & 11 deletions examples/03-modern-cpp/include/ranges_utils.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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; });
}
Expand All @@ -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) {
Expand All @@ -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),
Expand All @@ -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; });
}

Expand All @@ -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) {
Expand All @@ -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; });
}
Expand All @@ -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;
Expand All @@ -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});
}

Expand All @@ -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;
Expand Down
63 changes: 39 additions & 24 deletions examples/05-concurrency/include/lock_free_queue.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@
#include <atomic>
#include <optional>
#include <thread>
#include <utility>

#include "concurrency_utils.hpp"

Expand Down Expand Up @@ -150,7 +151,7 @@ class MPMCQueue {

struct Cell {
std::atomic<size_t> sequence;
T data;
std::optional<T> data;
};

public:
Expand All @@ -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];
Expand Down
27 changes: 27 additions & 0 deletions openspec/changes/header-only-hardening/design.md
Original file line number Diff line number Diff line change
@@ -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.
26 changes: 26 additions & 0 deletions openspec/changes/header-only-hardening/proposal.md
Original file line number Diff line number Diff line change
@@ -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.
Original file line number Diff line number Diff line change
@@ -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
Original file line number Diff line number Diff line change
@@ -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
22 changes: 22 additions & 0 deletions openspec/changes/header-only-hardening/tasks.md
Original file line number Diff line number Diff line change
@@ -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`.
Loading
Loading