Skip to content

Commit 8485c35

Browse files
perf: make interface destructors protected and non-virtual (#361)
Every toolbox interface declared `virtual ~I() = default`. The library and its embedded users never destroy an object through an interface pointer, but the virtual destructor still puts a deleting destructor in each implementation's vtable. That destructor calls operator delete, which links malloc/free into firmware that never allocates. Under multiple inheritance it also adds one thunk per base. All 24 library interfaces now declare `~I() = default` in a protected section, as EMIL does. SynchronousPidController gets one too, so the derived interface does not reopen a public non-virtual destructor. Deleting through an interface pointer is now a compile error. Compiled at -O2, a translation unit that instantiates Lqr<float, 2, 1, 300> used to emit two deleting destructors and a reference to operator delete. It now emits neither. Code that owned windows through std::unique_ptr<Window> holds them differently now: - TestWindowing holds each window on the stack. - The FFT and PSD simulators pick a static, stateless window by reference. The simulator's own Plant interface keeps its virtual destructor: it is host-only and owned through std::unique_ptr<Plant>. The rule is updated in AGENTS.md, CLAUDE.md, .github/copilot-instructions.md, the reviewer agents and the C++ instructions. Closes #359 Claude-Session: https://claude.ai/code/session_01Q53m7MZDmHwocmKbhhd2o5 Co-authored-by: Claude <noreply@anthropic.com>
1 parent f2e74a4 commit 8485c35

29 files changed

Lines changed: 138 additions & 91 deletions

‎.claude/agents/reviewer.md‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -48,7 +48,7 @@ End with totals + verdict: APPROVE / REQUEST CHANGES.
4848
- [ ] SOLID: one concern per class, constructor injection, depend on abstractions, no duplicated logic.
4949

5050
**Interfaces & errors (WARNING)**
51-
- [ ] `virtual ~I() = default` — never `= 0`. No exceptions — `std::optional`/status enums.
51+
- [ ] Interfaces: `protected:` non-virtual `~I() = default` — no `virtual` or `= 0` destructor, no ownership or deletion through an interface pointer. No exceptions — `std::optional`/status enums.
5252

5353
**Testing (WARNING)**
5454
- [ ] `TEST_F` on `float` — no `TYPED_TEST`, no multi-type, never plain `TEST()`.

‎.github/agents/reviewer.agent.md‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -54,7 +54,7 @@ End with totals + verdict: APPROVE / REQUEST CHANGES.
5454
- [ ] SOLID: one concern per class, constructor injection, depend on abstractions, no duplicated logic.
5555

5656
**Interfaces & errors (WARNING)**
57-
- [ ] `virtual ~I() = default` — never `= 0`. No exceptions — `std::optional`/status enums.
57+
- [ ] Interfaces: `protected:` non-virtual `~I() = default` — no `virtual` or `= 0` destructor, no ownership or deletion through an interface pointer. No exceptions — `std::optional`/status enums.
5858

5959
**Testing (WARNING)**
6060
- [ ] `TEST_F` on `float` — no `TYPED_TEST`, no multi-type, never plain `TEST()`.

‎.github/copilot-instructions.md‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9,5 +9,5 @@ Essentials (full detail in AGENTS.md):
99
- **Embedded** — no `#pragma GCC optimize` / `optimize` attribute (they block inlining); the consumer sets optimization flags per TU; `OPTIMIZE_FOR_SPEED` (forced inlining) on hot paths.
1010
- **No comments** (except license/NOLINT). Allman braces, brace-init, PascalCase/camelCase.
1111
- **Tests** — `TEST_F` on `float`, `StrictMock` only, never plain `TEST()`, no redundant cases.
12-
- **No exceptions** — `std::optional`/status enums; interfaces `virtual ~I() = default`.
12+
- **No exceptions** — `std::optional`/status enums; interfaces get a `protected:` non-virtual `~I() = default`, never a virtual destructor.
1313
- **Be terse** — minimal prose; report file paths + pass/fail.

‎.github/instructions/numerical-cpp.instructions.md‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -68,7 +68,7 @@ Use `math::IsFinite` for finiteness checks: a consumer may still build with `-ff
6868
- SOLID principles — constructor injection, depend on abstractions
6969
- DRY — extract shared logic into helpers or templates
7070
- RAII for resource management
71-
- **No pure virtual destructors**: Never declare `virtual ~Interface() = 0`. Pure virtual destructors force the compiler to emit a thunk and a separate body, wasting memory on embedded targets. Use `virtual ~Interface() = default` if a virtual destructor is required, or omit it entirely when the class is never deleted through a base pointer.
71+
- **Protected, non-virtual interface destructors**: declare `~Interface() = default` in a `protected:` section, never `virtual ~Interface()` (defaulted or `= 0`). A virtual destructor puts a deleting destructor in every implementation's vtable, which links `operator delete` (and with it `malloc`/`free`) into heap-free firmware, and under multiple inheritance adds a thunk per base. The protected destructor makes deletion through an interface pointer a compile error. Concrete classes declare no destructor unless they need one, and callers hold the concrete type rather than a `std::unique_ptr` to the interface.
7272
- No virtual calls in ISR-callable or real-time critical paths
7373

7474
## Documentation — MANDATORY

‎AGENTS.md‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -44,7 +44,8 @@ resource-constrained embedded systems. Real-time, deterministic, no heap.
4444

4545
## Interfaces & errors
4646

47-
- Interfaces = pure virtual classes; `virtual ~I() = default` — **never** `= 0` destructors.
47+
- Interfaces = pure virtual classes with a `protected:`, non-virtual `~I() = default` — **never** a `virtual` or `= 0` destructor. A virtual one puts a deleting destructor in every vtable, which links `operator delete`/`free` into heap-free firmware.
48+
- Concrete classes declare no destructor unless they need one. Never own or delete through an interface pointer (no `std::unique_ptr<I>`): hold the concrete type, pass `I&`.
4849
- No exceptions. Use `std::optional<T>` or status enums. `assert`/`really_assert` for preconditions.
4950
- SOLID + DIP: constructor injection; depend on abstractions. RAII. No virtual calls in real-time paths.
5051

‎CLAUDE.md‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9,5 +9,5 @@ Essentials (full detail in AGENTS.md):
99
- **Embedded** — no `#pragma GCC optimize` / `optimize` attribute (they block inlining); the consumer sets optimization flags per TU; `OPTIMIZE_FOR_SPEED` (forced inlining) on hot paths.
1010
- **No comments** (except license/NOLINT). Allman braces, brace-init, PascalCase/camelCase.
1111
- **Tests** — `TEST_F` on `float`, `StrictMock` only, never plain `TEST()`, no redundant cases.
12-
- **No exceptions** — `std::optional`/status enums; interfaces `virtual ~I() = default`.
12+
- **No exceptions** — `std::optional`/status enums; interfaces get a `protected:` non-virtual `~I() = default`, never a virtual destructor.
1313
- **Be terse** — minimal prose; report file paths + pass/fail.

‎numerical/analysis/FastFourierTransform.hpp‎

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -9,8 +9,10 @@ namespace analysis
99
class TwiddleFactors
1010
{
1111
public:
12-
virtual ~TwiddleFactors() = default;
1312
virtual math::Complex<QNumberType>& operator[](std::size_t n) = 0;
13+
14+
protected:
15+
~TwiddleFactors() = default;
1416
};
1517

1618
template<typename QNumberType>
@@ -21,8 +23,6 @@ namespace analysis
2123
"FastFourierTransform can only be instantiated with math::QNumber types.");
2224

2325
public:
24-
virtual ~FastFourierTransform() = default;
25-
2626
static constexpr std::size_t Log2(std::size_t n)
2727
{
2828
std::size_t result = 0;
@@ -41,6 +41,8 @@ namespace analysis
4141
virtual VectorReal& Inverse(VectorComplex& input) = 0;
4242

4343
protected:
44+
~FastFourierTransform() = default;
45+
4446
template<typename T>
4547
inline T BitReverse(T x, T logr_n, T bits, T radix)
4648
{

‎numerical/analysis/windowing/Windowing.hpp‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -14,9 +14,11 @@ namespace windowing
1414
"Window can only be instantiated with math::QNumber types.");
1515

1616
public:
17-
virtual ~Window() = default;
1817
virtual QNumberType operator()(std::size_t n, std::size_t order) = 0;
1918
virtual QNumberType Power(std::size_t order) = 0;
19+
20+
protected:
21+
~Window() = default;
2022
};
2123

2224
namespace detail

‎numerical/analysis/windowing/test/TestWindowing.cpp‎

Lines changed: 13 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,6 @@ namespace
1111
{
1212
protected:
1313
static constexpr float kEpsilon = 1e-3f;
14-
std::unique_ptr<windowing::Window<QNumType>> window;
1514
};
1615

1716
using WindowingTypes = ::testing::Types<math::Q31, math::Q15, float>;
@@ -20,7 +19,7 @@ namespace
2019

2120
TYPED_TEST(WindowingTest, HammingWindowValues)
2221
{
23-
this->window = std::make_unique<windowing::HammingWindow<TypeParam>>();
22+
windowing::HammingWindow<TypeParam> window;
2423

2524
struct TestCase
2625
{
@@ -36,7 +35,7 @@ TYPED_TEST(WindowingTest, HammingWindowValues)
3635

3736
for (const auto& testCase : testCases)
3837
{
39-
TypeParam result = (*this->window)(testCase.n, testCase.order);
38+
TypeParam result = window(testCase.n, testCase.order);
4039
EXPECT_NEAR(math::ToFloat(result), testCase.expected, this->kEpsilon)
4140
<< "Failed for n=" << testCase.n
4241
<< ", order=" << testCase.order;
@@ -45,7 +44,7 @@ TYPED_TEST(WindowingTest, HammingWindowValues)
4544

4645
TYPED_TEST(WindowingTest, HanningWindowValues)
4746
{
48-
this->window = std::make_unique<windowing::HanningWindow<TypeParam>>();
47+
windowing::HanningWindow<TypeParam> window;
4948

5049
struct TestCase
5150
{
@@ -61,7 +60,7 @@ TYPED_TEST(WindowingTest, HanningWindowValues)
6160

6261
for (const auto& testCase : testCases)
6362
{
64-
TypeParam result = (*this->window)(testCase.n, testCase.order);
63+
TypeParam result = window(testCase.n, testCase.order);
6564
EXPECT_NEAR(math::ToFloat(result), testCase.expected, this->kEpsilon)
6665
<< "Failed for n=" << testCase.n
6766
<< ", order=" << testCase.order;
@@ -70,7 +69,7 @@ TYPED_TEST(WindowingTest, HanningWindowValues)
7069

7170
TYPED_TEST(WindowingTest, BlackmanWindowValues)
7271
{
73-
this->window = std::make_unique<windowing::BlackmanWindow<TypeParam>>();
72+
windowing::BlackmanWindow<TypeParam> window;
7473

7574
struct TestCase
7675
{
@@ -85,7 +84,7 @@ TYPED_TEST(WindowingTest, BlackmanWindowValues)
8584

8685
for (const auto& testCase : testCases)
8786
{
88-
TypeParam result = (*this->window)(testCase.n, testCase.order);
87+
TypeParam result = window(testCase.n, testCase.order);
8988
EXPECT_NEAR(math::ToFloat(result), testCase.expected, this->kEpsilon)
9089
<< "Failed for n=" << testCase.n
9190
<< ", order=" << testCase.order;
@@ -94,26 +93,26 @@ TYPED_TEST(WindowingTest, BlackmanWindowValues)
9493

9594
TYPED_TEST(WindowingTest, RectangularWindowValues)
9695
{
97-
this->window = std::make_unique<windowing::RectangularWindow<TypeParam>>();
96+
windowing::RectangularWindow<TypeParam> window;
9897

9998
for (std::size_t n = 0; n <= 8; n += 2)
10099
{
101-
TypeParam result = (*this->window)(n, 8);
100+
TypeParam result = window(n, 8);
102101
EXPECT_NEAR(math::ToFloat(result), 0.999f, this->kEpsilon)
103102
<< "Failed for n=" << n;
104103
}
105104
}
106105

107106
TYPED_TEST(WindowingTest, WindowSymmetry)
108107
{
109-
std::vector<std::unique_ptr<windowing::Window<TypeParam>>> windows;
110-
windows.push_back(std::make_unique<windowing::HammingWindow<TypeParam>>());
111-
windows.push_back(std::make_unique<windowing::HanningWindow<TypeParam>>());
112-
windows.push_back(std::make_unique<windowing::BlackmanWindow<TypeParam>>());
108+
windowing::HammingWindow<TypeParam> hamming;
109+
windowing::HanningWindow<TypeParam> hanning;
110+
windowing::BlackmanWindow<TypeParam> blackman;
111+
std::array<windowing::Window<TypeParam>*, 3> windows{ &hamming, &hanning, &blackman };
113112

114113
const std::size_t order = 16;
115114

116-
for (const auto& window : windows)
115+
for (auto* window : windows)
117116
{
118117
for (std::size_t n = 0; n < order / 2; ++n)
119118
{

‎numerical/controllers/implementations/Feedforward2Dof.hpp‎

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -12,9 +12,11 @@ namespace controllers
1212
public:
1313
static_assert(std::is_floating_point_v<T>, "Feedforward supports floating-point types");
1414

15-
virtual ~Feedforward() = default;
1615
virtual T Evaluate(T reference) = 0;
1716
virtual void Reset() = 0;
17+
18+
protected:
19+
~Feedforward() = default;
1820
};
1921

2022
template<typename T>
@@ -23,9 +25,11 @@ namespace controllers
2325
public:
2426
static_assert(std::is_floating_point_v<T>, "FeedbackLaw supports floating-point types");
2527

26-
virtual ~FeedbackLaw() = default;
2728
virtual T Process(T error) = 0;
2829
virtual void Reset() = 0;
30+
31+
protected:
32+
~FeedbackLaw() = default;
2933
};
3034

3135
template<typename T>

0 commit comments

Comments
 (0)