Skip to content

Commit f4aff0d

Browse files
committed
feat: Refactor ValueEvaluation and Evaluation structs for improved memory management and simplify assignment operations
1 parent 7263782 commit f4aff0d

File tree

6 files changed

+28
-208
lines changed

6 files changed

+28
-208
lines changed

docs/src/content/docs/guide/memory-management.mdx

Lines changed: 13 additions & 40 deletions
Original file line numberDiff line numberDiff line change
@@ -129,18 +129,18 @@ This happens automatically - **you don't need to call any special methods** when
129129

130130
### Nested Structs
131131

132-
When a struct contains members with postblit constructors, D automatically calls postblit on each member:
132+
When a struct contains members with postblit constructors, D automatically calls postblit on each member - **no explicit postblit is needed** in the containing struct:
133133

134134
```d
135135
struct ValueEvaluation {
136136
HeapString strValue; // Has postblit
137137
HeapString niceValue; // Has postblit
138+
HeapString fileName; // Has postblit
139+
HeapString prependText; // Has postblit
138140
139-
// D automatically calls strValue.this(this) and niceValue.this(this)
141+
// No explicit postblit needed!
142+
// D automatically calls the postblit for each HeapString field
140143
// when ValueEvaluation is blitted
141-
this(this) @trusted nothrow @nogc {
142-
// Nested postblits handle ref counting automatically
143-
}
144144
}
145145
```
146146

@@ -203,40 +203,9 @@ static HeapData create(size_t initialCapacity = 0) @trusted @nogc nothrow {
203203
}
204204
```
205205

206-
## Assignment Operators
207-
208-
HeapData provides both lvalue and rvalue assignment operators:
209-
210-
### Lvalue Assignment (from another variable)
211-
212-
```d
213-
void opAssign(ref HeapData rhs) @trusted @nogc nothrow {
214-
// Handle self-assignment
215-
if (isHeap() && rhs.isHeap() && _payload.heap is rhs._payload.heap) {
216-
return;
217-
}
218-
219-
// Decrement old ref count and free if needed
220-
if (isHeap() && _payload.heap) {
221-
if (--_payload.heap.refCount == 0) {
222-
free(_payload.heap);
223-
}
224-
}
225-
226-
// Copy from rhs
227-
_length = rhs._length;
228-
_flags = rhs._flags;
206+
## Assignment Operator
229207

230-
if (rhs.isHeap()) {
231-
_payload.heap = rhs._payload.heap;
232-
if (_payload.heap) _payload.heap.refCount++;
233-
} else {
234-
_payload.small = rhs._payload.small; // Copy SBO data
235-
}
236-
}
237-
```
238-
239-
### Rvalue Assignment (from temporary)
208+
HeapData provides a single assignment operator that handles both lvalue and rvalue assignments efficiently:
240209

241210
```d
242211
void opAssign(HeapData rhs) @trusted @nogc nothrow {
@@ -247,7 +216,7 @@ void opAssign(HeapData rhs) @trusted @nogc nothrow {
247216
}
248217
}
249218
250-
// Take ownership from rhs
219+
// Take ownership from rhs (rhs was copied via postblit)
251220
_length = rhs._length;
252221
_flags = rhs._flags;
253222
@@ -256,11 +225,13 @@ void opAssign(HeapData rhs) @trusted @nogc nothrow {
256225
rhs._payload.heap = null; // Prevent rhs destructor from freeing
257226
rhs.setHeap(false);
258227
} else {
259-
_payload.small = rhs._payload.small;
228+
_payload.small = rhs._payload.small; // Copy SBO data
260229
}
261230
}
262231
```
263232

233+
When called with an lvalue, D invokes the postblit constructor on `rhs` first, incrementing the ref count. The assignment then takes ownership of the copied reference. This unified approach keeps the code simpler while handling all cases correctly.
234+
264235
## Accessing HeapString Content
265236

266237
Use the slice operator `[]` to get a `const(char)[]` from a HeapString:
@@ -319,6 +290,8 @@ assert(hs.isValid(), "HeapString memory corruption detected");
319290

320291
With the current implementation, you **don't need to**:
321292
- Call `incrementRefCount()` before returning structs (postblit handles this)
293+
- Write explicit postblit or copy constructors for structs containing HeapString (D calls nested postblits automatically)
294+
- Write explicit assignment operators for structs containing HeapString (D handles this)
322295
- Worry about heap allocation for short strings (SBO handles this)
323296
- Make two allocations for ref count and data (combined allocation)
324297
- Manually track reference counts when passing structs through containers

operation-snapshots.md

0 Bytes
Binary file not shown.

source/fluentasserts/core/evaluation.d

Lines changed: 10 additions & 149 deletions
Original file line numberDiff line numberDiff line change
@@ -54,94 +54,26 @@ struct ValueEvaluation {
5454
string[string] meta;
5555

5656
/// The file name containing the evaluated value
57-
string fileName;
57+
HeapString fileName;
5858

5959
/// The line number of the evaluated value
6060
size_t line;
6161

6262
/// a custom text to be prepended to the value
63-
string prependText;
64-
65-
/// Copy constructor
66-
this(ref return scope ValueEvaluation other) @trusted nothrow {
67-
this.throwable = other.throwable;
68-
this.duration = other.duration;
69-
this.gcMemoryUsed = other.gcMemoryUsed;
70-
this.nonGCMemoryUsed = other.nonGCMemoryUsed;
71-
this.strValue = other.strValue;
72-
this.proxyValue = other.proxyValue;
73-
this.niceValue = other.niceValue;
74-
this.typeNames = other.typeNames;
75-
this.meta = other.meta;
76-
this.fileName = other.fileName;
77-
this.line = other.line;
78-
this.prependText = other.prependText;
79-
}
80-
81-
/// Assignment operator (properly handles HeapString ref counting)
82-
void opAssign(ref ValueEvaluation other) @trusted nothrow {
83-
this.throwable = other.throwable;
84-
this.duration = other.duration;
85-
this.gcMemoryUsed = other.gcMemoryUsed;
86-
this.nonGCMemoryUsed = other.nonGCMemoryUsed;
87-
this.strValue = other.strValue;
88-
this.proxyValue = other.proxyValue;
89-
this.niceValue = other.niceValue;
90-
this.typeNames = other.typeNames;
91-
this.meta = other.meta;
92-
this.fileName = other.fileName;
93-
this.line = other.line;
94-
this.prependText = other.prependText;
95-
}
96-
97-
/// Assignment operator for rvalues
98-
void opAssign(ValueEvaluation other) @trusted nothrow {
99-
this.throwable = other.throwable;
100-
this.duration = other.duration;
101-
this.gcMemoryUsed = other.gcMemoryUsed;
102-
this.nonGCMemoryUsed = other.nonGCMemoryUsed;
103-
this.strValue = other.strValue;
104-
this.proxyValue = other.proxyValue;
105-
this.niceValue = other.niceValue;
106-
this.typeNames = other.typeNames;
107-
this.meta = other.meta;
108-
this.fileName = other.fileName;
109-
this.line = other.line;
110-
this.prependText = other.prependText;
111-
}
112-
113-
/// Postblit - called after D blits this struct.
114-
/// HeapString members have their own postblit that increments ref counts.
115-
/// This is called automatically by D when the struct is copied via blit.
116-
this(this) @trusted nothrow @nogc {
117-
// HeapString's postblit handles ref counting automatically
118-
// No additional work needed here, but having an explicit postblit
119-
// ensures the struct is properly marked as having postblit semantics
120-
}
121-
122-
/// Increment HeapString ref counts to survive blit operations.
123-
/// D's blit (memcpy) doesn't call copy constructors.
124-
///
125-
/// IMPORTANT: Call this IMMEDIATELY before returning a ValueEvaluation
126-
/// or any struct containing it from a function.
127-
void prepareForBlit() @trusted nothrow @nogc {
128-
strValue.incrementRefCount();
129-
niceValue.incrementRefCount();
130-
}
63+
HeapString prependText;
64+
65+
// HeapString has proper postblit/opAssign, so D handles copying automatically
13166

13267
/// Returns true if this ValueEvaluation's HeapString fields are valid.
133-
/// Use this in debug assertions to catch memory corruption early.
13468
bool isValid() @trusted nothrow @nogc const {
13569
return strValue.isValid() && niceValue.isValid();
13670
}
13771

13872
/// Returns the primary type name of the evaluated value.
139-
/// Returns: The first type name, or "unknown" if no types are available.
14073
string typeName() @safe nothrow @nogc {
141-
if(typeNames.length == 0) {
74+
if (typeNames.length == 0) {
14275
return "unknown";
14376
}
144-
14577
return typeNames[0];
14678
}
14779
}
@@ -153,74 +85,7 @@ struct Evaluation {
15385
/// The id of the current evaluation
15486
size_t id;
15587

156-
/// Copy constructor
157-
this(ref return scope Evaluation other) @trusted nothrow {
158-
this.id = other.id;
159-
this.currentValue = other.currentValue;
160-
this.expectedValue = other.expectedValue;
161-
this._operationCount = other._operationCount;
162-
foreach (i; 0 .. other._operationCount) {
163-
this._operationNames[i] = other._operationNames[i];
164-
}
165-
this.isNegated = other.isNegated;
166-
this.source = other.source;
167-
this.throwable = other.throwable;
168-
this.isEvaluated = other.isEvaluated;
169-
this.result = other.result;
170-
}
171-
172-
/// Assignment operator (properly handles HeapString ref counting)
173-
void opAssign(ref Evaluation other) @trusted nothrow {
174-
this.id = other.id;
175-
this.currentValue = other.currentValue;
176-
this.expectedValue = other.expectedValue;
177-
this._operationCount = other._operationCount;
178-
foreach (i; 0 .. other._operationCount) {
179-
this._operationNames[i] = other._operationNames[i];
180-
}
181-
this.isNegated = other.isNegated;
182-
this.source = other.source;
183-
this.throwable = other.throwable;
184-
this.isEvaluated = other.isEvaluated;
185-
this.result = other.result;
186-
}
187-
188-
/// Assignment operator for rvalues
189-
void opAssign(Evaluation other) @trusted nothrow {
190-
this.id = other.id;
191-
this.currentValue = other.currentValue;
192-
this.expectedValue = other.expectedValue;
193-
this._operationCount = other._operationCount;
194-
foreach (i; 0 .. other._operationCount) {
195-
this._operationNames[i] = other._operationNames[i];
196-
}
197-
this.isNegated = other.isNegated;
198-
this.source = other.source;
199-
this.throwable = other.throwable;
200-
this.isEvaluated = other.isEvaluated;
201-
this.result = other.result;
202-
}
203-
204-
/// Postblit - called after D blits this struct.
205-
/// Nested structs with postblit (ValueEvaluation, HeapString) have
206-
/// their postblits called automatically by D.
207-
this(this) @trusted nothrow @nogc {
208-
// Nested postblits handle ref counting automatically
209-
}
210-
211-
/// Increment HeapString ref counts to survive blit operations.
212-
/// D's blit (memcpy) doesn't call copy constructors.
213-
///
214-
/// NOTE: With postblit constructors, this method may no longer be needed
215-
/// in most cases. Keep it for backwards compatibility and edge cases.
216-
void prepareForBlit() @trusted nothrow @nogc {
217-
currentValue.prepareForBlit();
218-
expectedValue.prepareForBlit();
219-
foreach (i; 0 .. _operationCount) {
220-
_operationNames[i].incrementRefCount();
221-
}
222-
result.prepareForBlit();
223-
}
88+
// HeapString has proper postblit/opAssign, so D handles copying automatically
22489

22590
/// The value that will be validated
22691
ValueEvaluation currentValue;
@@ -409,12 +274,10 @@ auto evaluate(T)(lazy T testData, const string file = __FILE__, const size_t lin
409274
valueEvaluation.proxyValue = equableValue(value, niceValueStr);
410275
valueEvaluation.niceValue = toHeapString(niceValueStr);
411276
valueEvaluation.typeNames = extractTypes!TT;
412-
valueEvaluation.fileName = file;
277+
valueEvaluation.fileName = toHeapString(file);
413278
valueEvaluation.line = line;
414-
valueEvaluation.prependText = prependText;
279+
valueEvaluation.prependText = toHeapString(prependText);
415280

416-
// Increment HeapString ref counts to survive the blit on return
417-
valueEvaluation.prepareForBlit();
418281
return Result(value, valueEvaluation);
419282
} catch(Throwable t) {
420283
T result;
@@ -435,12 +298,10 @@ auto evaluate(T)(lazy T testData, const string file = __FILE__, const size_t lin
435298
valueEvaluation.proxyValue = equableValue(result, resultStr);
436299
valueEvaluation.niceValue = toHeapString(resultStr);
437300
valueEvaluation.typeNames = extractTypes!T;
438-
valueEvaluation.fileName = file;
301+
valueEvaluation.fileName = toHeapString(file);
439302
valueEvaluation.line = line;
440-
valueEvaluation.prependText = prependText;
303+
valueEvaluation.prependText = toHeapString(prependText);
441304

442-
// Increment HeapString ref counts to survive the blit on return
443-
valueEvaluation.prepareForBlit();
444305
return Result(result, valueEvaluation);
445306
}
446307
}

source/fluentasserts/core/expect.d

Lines changed: 5 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -56,7 +56,7 @@ import std.conv;
5656
this(ValueEvaluation value) @trusted {
5757
_evaluation.id = Lifecycle.instance.beginEvaluation(value);
5858
_evaluation.currentValue = value;
59-
_evaluation.source = SourceResult.create(value.fileName, value.line);
59+
_evaluation.source = SourceResult.create(value.fileName[].idup, value.line);
6060

6161
try {
6262
auto sourceValue = _evaluation.source.getValue;
@@ -72,8 +72,8 @@ import std.conv;
7272

7373
_evaluation.result.addText(" should");
7474

75-
if(value.prependText) {
76-
_evaluation.result.addText(value.prependText);
75+
if(value.prependText.length > 0) {
76+
_evaluation.result.addText(value.prependText[].idup);
7777
}
7878
}
7979

@@ -530,13 +530,11 @@ Expect expect(void delegate() callable, const string file = __FILE__, const size
530530
value.meta["Throwable"] = "yes";
531531
}
532532

533-
value.fileName = file;
533+
value.fileName = toHeapString(file);
534534
value.line = line;
535-
value.prependText = prependText;
535+
value.prependText = toHeapString(prependText);
536536

537537
auto result = Expect(value);
538-
// Increment HeapString ref counts to survive the blit on return.
539-
result._evaluation.prepareForBlit();
540538
return result;
541539
}
542540

@@ -549,9 +547,5 @@ Expect expect(void delegate() callable, const string file = __FILE__, const size
549547
/// Returns: An Expect struct for fluent assertions
550548
Expect expect(T)(lazy T testedValue, const string file = __FILE__, const size_t line = __LINE__, string prependText = null) @trusted {
551549
auto result = Expect(testedValue.evaluate(file, line, prependText).evaluation);
552-
// Increment HeapString ref counts to survive the blit on return.
553-
// D's blit (memcpy) doesn't call copy constructors, so we must manually
554-
// ensure ref counts are incremented before the original is destroyed.
555-
result._evaluation.prepareForBlit();
556550
return result;
557551
}

source/fluentasserts/core/lifecycle.d

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -170,8 +170,6 @@ Evaluation recordEvaluation(void delegate() assertion) @trusted {
170170

171171
assertion();
172172

173-
// Increment HeapString ref counts to survive the blit on return
174-
Lifecycle.instance.lastEvaluation.prepareForBlit();
175173
return Lifecycle.instance.lastEvaluation;
176174
}
177175

source/fluentasserts/results/asserts.d

Lines changed: 0 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -85,12 +85,6 @@ struct AssertResult {
8585
|| diff.length > 0 || extra.length > 0 || missing.length > 0;
8686
}
8787

88-
/// No-op for AssertResult (no HeapString fields).
89-
/// Required for Evaluation.prepareForBlit() compatibility.
90-
void prepareForBlit() @trusted nothrow @nogc {
91-
// AssertResult uses FixedAppender, not HeapString, so nothing to do
92-
}
93-
9488
/// Formats a value for display, replacing special characters with glyphs.
9589
string formatValue(string value) nothrow inout {
9690
return value

0 commit comments

Comments
 (0)