Skip to content

Commit 162eb5a

Browse files
committed
revert: remove RadValue span tracking (perf fix)
The Span *Span field added to RadValue in 1a04b3c caused a 15-19% regression on loop-heavy benchmarks. Each newRadValue() call heap-allocated a 64-byte Span, and the struct grew from 16 to 24 bytes - 50% more data copied by value everywhere. The span tracking was infrastructure for future "assigned here" error context, but it's never read in production code. Errors at the usage site with actual types are sufficient - Python/Ruby/JS don't track assignment locations either. Removes Span field, HasSpan/GetSpan/WithSpan methods, spanFromNode helper, and associated tests.
1 parent 9d57d24 commit 162eb5a

File tree

2 files changed

+14
-81
lines changed

2 files changed

+14
-81
lines changed

core/testing/diagnostic_test.go

Lines changed: 0 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -225,30 +225,3 @@ func TestNewDiagnosticFromCheckNilCode(t *testing.T) {
225225
assert.Equal(t, rl.ErrGenericRuntime, coreDiag.Code)
226226
}
227227

228-
// Tests for RadValue span tracking
229-
230-
func TestRadValueHasSpan(t *testing.T) {
231-
// Value without span
232-
val := core.RadValue{Val: int64(42)}
233-
assert.False(t, val.HasSpan())
234-
assert.Nil(t, val.GetSpan())
235-
236-
// Value with span
237-
span := &core.Span{File: "test.rad", StartRow: 5}
238-
valWithSpan := val.WithSpan(span)
239-
assert.True(t, valWithSpan.HasSpan())
240-
assert.Equal(t, "test.rad", valWithSpan.GetSpan().File)
241-
assert.Equal(t, 5, valWithSpan.GetSpan().StartRow)
242-
}
243-
244-
func TestRadValueWithSpan(t *testing.T) {
245-
span := &core.Span{File: "test.rad", StartRow: 10, StartCol: 5}
246-
val := core.RadValue{Val: "hello"}
247-
248-
// WithSpan returns a copy with the span
249-
result := val.WithSpan(span)
250-
assert.Equal(t, span, result.Span)
251-
252-
// Original is also modified (value semantics - copy)
253-
// This is expected behavior for Go structs
254-
}

core/type_rad_value.go

Lines changed: 14 additions & 54 deletions
Original file line numberDiff line numberDiff line change
@@ -20,11 +20,6 @@ type RadValue struct {
2020
// nulls are RadNull
2121
// errors are *RadError
2222
Val interface{}
23-
24-
// Span tracks where this value originated in source code.
25-
// Used for "assigned here" context in error messages.
26-
// nil for intermediate computations (e.g., a + b).
27-
Span *Span
2823
}
2924

3025
func (v RadValue) Type() rl.RadType {
@@ -56,22 +51,6 @@ func (v RadValue) IsNull() bool {
5651
return v.Val == RAD_NULL
5752
}
5853

59-
// HasSpan returns true if this value has source location information.
60-
func (v RadValue) HasSpan() bool {
61-
return v.Span != nil
62-
}
63-
64-
// GetSpan returns the source location span, or nil if not available.
65-
func (v RadValue) GetSpan() *Span {
66-
return v.Span
67-
}
68-
69-
// WithSpan returns a copy of this value with the given span attached.
70-
func (v RadValue) WithSpan(span *Span) RadValue {
71-
v.Span = span
72-
return v
73-
}
74-
7554
func (v RadValue) Index(i *Interpreter, idxNode *ts.Node) RadValue {
7655
switch coerced := v.Val.(type) {
7756
case RadString:
@@ -474,67 +453,48 @@ func (v RadValue) ToCompatSubject(i *Interpreter) (out rl.TypingCompatVal) {
474453
return
475454
}
476455

477-
// spanFromNode creates a Span from a tree-sitter node for value tracking.
478-
// Returns nil if the node is nil (intermediate computations don't have spans).
479-
func spanFromNode(i *Interpreter, node *ts.Node) *Span {
480-
if node == nil {
481-
return nil
482-
}
483-
file := ""
484-
if i != nil {
485-
file = i.sd.ScriptName
486-
}
487-
span := NewSpanFromNode(node, file)
488-
return &span
489-
}
490-
491456
func newRadValue(i *Interpreter, node *ts.Node, value interface{}) RadValue {
492-
span := spanFromNode(i, node)
493457
switch coerced := value.(type) {
494458
case RadValue:
495-
// Preserve existing span if the value already has one
496-
if coerced.Span == nil {
497-
coerced.Span = span
498-
}
499459
return coerced
500460
case []RadValue: // todo risky to have this? might cover up issues
501461
list := NewRadList()
502462
list.Values = coerced
503463
return newRadValue(i, node, list)
504464
case RadString:
505-
return RadValue{Val: coerced, Span: span}
465+
return RadValue{Val: coerced}
506466
case string:
507-
return RadValue{Val: NewRadString(coerced), Span: span}
467+
return RadValue{Val: NewRadString(coerced)}
508468
case int:
509-
return RadValue{Val: int64(coerced), Span: span}
469+
return RadValue{Val: int64(coerced)}
510470
case int64, float64, bool:
511-
return RadValue{Val: coerced, Span: span}
471+
return RadValue{Val: coerced}
512472
case *RadList:
513-
return RadValue{Val: coerced, Span: span}
473+
return RadValue{Val: coerced}
514474
case RadList:
515-
return RadValue{Val: &coerced, Span: span}
475+
return RadValue{Val: &coerced}
516476
case *RadMap:
517-
return RadValue{Val: coerced, Span: span}
477+
return RadValue{Val: coerced}
518478
case RadMap:
519-
return RadValue{Val: &coerced, Span: span}
479+
return RadValue{Val: &coerced}
520480
case RadFn:
521-
return RadValue{Val: coerced, Span: span}
481+
return RadValue{Val: coerced}
522482
case *RadError:
523-
return RadValue{Val: coerced, Span: span}
483+
return RadValue{Val: coerced}
524484
case map[string]interface{}:
525485
radMap := NewRadMap()
526486
for key, val := range coerced {
527487
radMap.Set(newRadValue(i, node, key), newRadValue(i, node, val))
528488
}
529-
return RadValue{Val: radMap, Span: span}
489+
return RadValue{Val: radMap}
530490
case []interface{}:
531491
list := NewRadListFromGeneric(i, node, coerced)
532-
return RadValue{Val: list, Span: span}
492+
return RadValue{Val: list}
533493
case []string:
534494
list := NewRadListFromGeneric(i, node, coerced)
535-
return RadValue{Val: list, Span: span}
495+
return RadValue{Val: list}
536496
case RadNull, nil:
537-
return RadValue{Val: RAD_NULL, Span: span}
497+
return RadValue{Val: RAD_NULL}
538498
default:
539499
if i != nil && node != nil {
540500
i.emitErrorf(rl.ErrTypeMismatch, node, "Unsupported value type: %s", TypeAsString(coerced))

0 commit comments

Comments
 (0)