Skip to content

Commit 7ccd57b

Browse files
jfhamlinjfhamlin
committed
Fix nullable records and owned-vector evaluation order
1 parent f0e4685 commit 7ccd57b

6 files changed

Lines changed: 151 additions & 55 deletions

File tree

‎pkg/compiler/record_specialization_ir.go‎

Lines changed: 17 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -20,12 +20,14 @@ const (
2020
)
2121

2222
type IRRecordSpecializedType struct {
23-
Kind IRRecordSpecializedKind
24-
Record *lang.RecordType
23+
Kind IRRecordSpecializedKind
24+
Record *lang.RecordType
25+
Nullable bool
2526
}
2627

2728
func (t IRRecordSpecializedType) Equal(other IRRecordSpecializedType) bool {
28-
return t.Kind == other.Kind && t.Record == other.Record
29+
return t.Kind == other.Kind && t.Record == other.Record &&
30+
t.Nullable == other.Nullable
2931
}
3032

3133
type IRRecordShape struct {
@@ -108,8 +110,9 @@ func InferRecursiveRecordProducerWithParams(
108110
}
109111
if merged.Kind == IRRecordSpecializedNil {
110112
merged = IRRecordSpecializedType{
111-
Kind: IRRecordSpecializedRecord,
112-
Record: record,
113+
Kind: IRRecordSpecializedRecord,
114+
Record: record,
115+
Nullable: true,
113116
}
114117
}
115118
fields[index] = merged
@@ -250,7 +253,7 @@ func (a *irRecordFunctionAnalyzer) expr(
250253
case ast.OpKeywordLookup:
251254
lookup := node.Sub.(*ast.KeywordLookupNode)
252255
target := a.expr(lookup.Target, locals)
253-
if target.Kind != IRRecordSpecializedRecord {
256+
if target.Kind != IRRecordSpecializedRecord || target.Nullable {
254257
a.valid = false
255258
break
256259
}
@@ -421,17 +424,20 @@ func mergeIRRecordFieldTypes(
421424
if left.Kind == IRRecordSpecializedInvalid {
422425
return right, right.Kind != IRRecordSpecializedInvalid
423426
}
424-
if left.Equal(right) {
427+
if left.Kind == right.Kind && left.Record == right.Record {
428+
left.Nullable = left.Nullable || right.Nullable
425429
return left, true
426430
}
427431
if left.Kind == IRRecordSpecializedNil &&
428432
right.Kind == IRRecordSpecializedRecord &&
429433
right.Record == record {
434+
right.Nullable = true
430435
return right, true
431436
}
432437
if right.Kind == IRRecordSpecializedNil &&
433438
left.Kind == IRRecordSpecializedRecord &&
434439
left.Record == record {
440+
left.Nullable = true
435441
return left, true
436442
}
437443
return IRRecordSpecializedType{}, false
@@ -440,15 +446,18 @@ func mergeIRRecordFieldTypes(
440446
func mergeIRRecordResultTypes(
441447
left, right IRRecordSpecializedType,
442448
) (IRRecordSpecializedType, bool) {
443-
if left.Equal(right) {
449+
if left.Kind == right.Kind && left.Record == right.Record {
450+
left.Nullable = left.Nullable || right.Nullable
444451
return left, left.Kind != IRRecordSpecializedInvalid
445452
}
446453
if left.Kind == IRRecordSpecializedNil &&
447454
right.Kind == IRRecordSpecializedRecord {
455+
right.Nullable = true
448456
return right, true
449457
}
450458
if right.Kind == IRRecordSpecializedNil &&
451459
left.Kind == IRRecordSpecializedRecord {
460+
left.Nullable = true
452461
return left, true
453462
}
454463
return IRRecordSpecializedType{}, false

‎pkg/compiler/record_specialization_ir_test.go‎

Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,35 @@
1+
package compiler
2+
3+
import (
4+
"testing"
5+
6+
"github.com/glojurelang/glojure/pkg/lang"
7+
)
8+
9+
func TestRecordSpecializationMergesNilAsNullableRecord(t *testing.T) {
10+
record := lang.InternRecordType(
11+
"compiler.record-specialization-test",
12+
"NullableNode",
13+
"next",
14+
)
15+
recordType := IRRecordSpecializedType{
16+
Kind: IRRecordSpecializedRecord,
17+
Record: record,
18+
}
19+
nilType := IRRecordSpecializedType{Kind: IRRecordSpecializedNil}
20+
21+
field, ok := mergeIRRecordFieldTypes(nilType, recordType, record)
22+
if !ok || field.Kind != IRRecordSpecializedRecord ||
23+
field.Record != record || !field.Nullable {
24+
t.Fatalf("nil/record field merge = %#v, %v", field, ok)
25+
}
26+
27+
result, ok := mergeIRRecordResultTypes(recordType, nilType)
28+
if !ok || result.Kind != IRRecordSpecializedRecord ||
29+
result.Record != record || !result.Nullable {
30+
t.Fatalf("record/nil result merge = %#v, %v", result, ok)
31+
}
32+
if result.Equal(recordType) {
33+
t.Fatal("nullable record compared equal to a non-null record")
34+
}
35+
}

‎pkg/runtime/codegen_internal_test.go‎

Lines changed: 60 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -635,10 +635,10 @@ func TestGenerateRecursiveRecordSpecialization(t *testing.T) {
635635
expected, generated)
636636
}
637637
}
638-
if got := strings.Count(generated, "var aotRecordFn"); got != 3 {
638+
if got := strings.Count(generated, "var aotRecordFn"); got != 2 {
639639
t.Fatalf(
640-
"generated %d record-specialized functions, want 3; "+
641-
"record equality must retain generic semantics:\n%s",
640+
"generated %d record-specialized functions, want 2; "+
641+
"nullable recursive fields must retain generic semantics:\n%s",
642642
got,
643643
generated,
644644
)
@@ -670,6 +670,40 @@ func TestGenerateRecursiveRecordSpecialization(t *testing.T) {
670670
}
671671
}
672672

673+
func TestNullableRecordProducerUsesGenericFallback(t *testing.T) {
674+
ns := lang.FindOrCreateNamespace(
675+
lang.NewSymbol("codegen.nullable-record-producer"),
676+
)
677+
ns.ReferAllSnapshot(lang.NSCore, nil)
678+
lang.PushThreadBindings(lang.NewMap(lang.VarCurrentNS, ns))
679+
defer lang.PopThreadBindings()
680+
681+
ReadEval(`
682+
(defrecord MaybeLink [value next])
683+
(defn maybe-link [value]
684+
(if (zero? value)
685+
nil
686+
(->MaybeLink value (maybe-link (dec value)))))
687+
(defn sample-maybe-link []
688+
(maybe-link 0))`)
689+
690+
var output bytes.Buffer
691+
if err := NewGenerator(&output).Generate(ns); err != nil {
692+
t.Fatalf("generate nullable record producer: %v", err)
693+
}
694+
if strings.Contains(output.String(), "var aotRecordFn") {
695+
t.Fatalf(
696+
"nullable record producer received an unsound pointer-returning specialization:\n%s",
697+
output.String(),
698+
)
699+
}
700+
if got := ns.FindInternedVar(
701+
lang.NewSymbol("sample-maybe-link"),
702+
).Invoke(); got != nil {
703+
t.Fatalf("nullable record producer returned %#v, want nil", got)
704+
}
705+
}
706+
673707
func TestGenerateBooleanRecordSpecialization(t *testing.T) {
674708
ns := lang.FindOrCreateNamespace(lang.NewSymbol("codegen.record-bool-specialization"))
675709
ns.ReferAllSnapshot(lang.NSCore, nil)
@@ -2958,14 +2992,18 @@ func TestGenerateOwnedNestedVectorUpdateRegion(t *testing.T) {
29582992
(mapv
29592993
(fn [row]
29602994
(assoc row 0 (+ (nth row 0) delta)))
2961-
updated)))`)
2995+
updated)))
2996+
(defn ordered-assoc [values divisor]
2997+
(let [row (nth values 0)
2998+
updated (assoc row -777777 99 1 (quot 888888 divisor))]
2999+
(assoc-in values [0 0] (nth updated 0))))`)
29623000

29633001
var output bytes.Buffer
29643002
generator := NewGenerator(&output)
29653003
if err := generator.Generate(ns); err != nil {
29663004
t.Fatalf("generate owned nested vector region: %v", err)
29673005
}
2968-
for _, name := range []string{"update-cell", "update-all"} {
3006+
for _, name := range []string{"update-cell", "update-all", "ordered-assoc"} {
29693007
vr := ns.FindInternedVar(lang.NewSymbol(name))
29703008
target := generator.aotCallTargets[vr]
29713009
if target == nil || target.ownedVectorAnalysis == nil {
@@ -2997,6 +3035,23 @@ func TestGenerateOwnedNestedVectorUpdateRegion(t *testing.T) {
29973035
expected, generated)
29983036
}
29993037
}
3038+
valueIndex := strings.Index(
3039+
generated,
3040+
"lang.Numbers.Quotient(int64(888888)",
3041+
)
3042+
assocIndex := strings.Index(
3043+
generated,
3044+
".AssocCopy(lang.IntCast(int64(-777777))",
3045+
)
3046+
if valueIndex < 0 || assocIndex < 0 || valueIndex > assocIndex {
3047+
t.Fatalf(
3048+
"owned vector assoc did not evaluate all operands before mutation "+
3049+
"(value=%d assoc=%d):\n%s",
3050+
valueIndex,
3051+
assocIndex,
3052+
generated,
3053+
)
3054+
}
30003055

30013056
update := ns.FindInternedVar(lang.NewSymbol("update-all"))
30023057
original := lang.NewVector(

‎pkg/runtime/codegen_owned_vector.go‎

Lines changed: 10 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -808,17 +808,22 @@ func (g *Generator) generateOwnedVectorAOTAssoc(
808808
}
809809
assoc := node.Sub.(*ast.AssocNode)
810810
target := g.generateASTNode(assoc.Target)
811-
for index, entry := range assoc.Entries {
812-
key := g.generateASTNode(entry.Key)
813-
value := g.generateASTNode(entry.Val)
811+
keys := make([]string, len(assoc.Entries))
812+
values := make([]string, len(assoc.Entries))
813+
for index := range assoc.Entries {
814+
entry := assoc.Entries[index]
815+
keys[index] = g.generateASTNode(entry.Key)
816+
values[index] = g.generateASTNode(entry.Val)
817+
}
818+
for index := range assoc.Entries {
814819
if copyTarget && index == 0 {
815820
updated := g.allocateTempVar()
816821
g.writef("%s := %s.AssocCopy(lang.IntCast(%s), %s)\n",
817-
updated, target, key, value)
822+
updated, target, keys[index], values[index])
818823
target = updated
819824
} else {
820825
g.writef("%s.Assoc(lang.IntCast(%s), %s)\n",
821-
target, key, value)
826+
target, keys[index], values[index])
822827
}
823828
}
824829
return target, true

‎pkg/runtime/codegen_record_specialization.go‎

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -572,7 +572,16 @@ func (g *Generator) generateRecordSpecializedFixedFn(
572572
if len(guards) > 0 {
573573
g.writef("if %s {\n", strings.Join(guards, " && "))
574574
}
575-
g.writef("return %s(%s)\n", helper, strings.Join(args, ", "))
575+
call := helper + "(" + strings.Join(args, ", ") + ")"
576+
if analysis.Signature.Result.Kind == compiler.IRRecordSpecializedRecord &&
577+
analysis.Signature.Result.Nullable {
578+
result := g.allocateTempVar()
579+
g.writef("%s := %s\n", result, call)
580+
g.writef("if %s == nil { return nil }\n", result)
581+
g.writef("return %s\n", result)
582+
} else {
583+
g.writef("return %s\n", call)
584+
}
576585
if len(guards) > 0 {
577586
g.writef("}\n")
578587
}

‎pkg/runtime/testdata/codegen/test/record_specialization/load.go.out‎

Lines changed: 19 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -322,7 +322,6 @@ var aotDirectFn4 lang.FnFunc1
322322
var aotRecordFn1 func(*aotRecord0Flag) int64
323323
var aotRecordFn2 func(int64) *aotRecord0Flag
324324
var aotRecordFn3 func(int64, int64) *aotRecord1Node
325-
var aotRecordFn4 func(*aotRecord1Node) int64
326325

327326
var aotKeywordMapShape0 = lang.NewKeywordMapShape("value", "left", "right", "extra")
328327

@@ -748,14 +747,14 @@ func LoadNS() {
748747
}
749748
v7 := p0
750749
_ = v7
751-
var tmp8 any
750+
var tmp8 int64
752751
tmp9 := aotKeywordLookup3(v7, nil)
753752
if lang.IsTruthy(tmp9) {
754753
tmp8 = int64(1)
755754
} else {
756755
tmp8 = int64(0)
757756
}
758-
return tmp8
757+
return lang.BoxInt64(tmp8)
759758
})
760759
aotDirectFn1 = tmp1
761760
var_codegen_DOT_test_DOT_record_DASH_specialization_flag_DASH_score = ns.InternWithValue(tmp0, tmp1, true)
@@ -881,42 +880,26 @@ func LoadNS() {
881880
{
882881
tmp0 := sym_sum_DASH_node
883882
var tmp1 lang.FnFunc1
884-
var tmp2 func(tmp3 *aotRecord1Node) int64
885-
tmp2 = func(tmp3 *aotRecord1Node) int64 {
886-
var tmp4 int64
887-
if tmp3.f1 == nil {
888-
tmp4 = tmp3.f0
889-
} else {
890-
tmp4 = lang.CheckedAddInt64(lang.CheckedAddInt64(tmp3.f0, tmp2(tmp3.f1)), tmp2(tmp3.f2))
891-
}
892-
return tmp4
893-
}
894-
aotRecordFn4 = tmp2
895883
tmp1 = lang.FnFunc1(func(p0 any) any {
896-
tmp5, tmp6 := p0.(*aotRecord1Node)
897-
tmp6 = tmp6 && tmp5.aotRecordFast()
898-
if tmp6 {
899-
return tmp2(tmp5)
900-
}
901-
v7 := p0
902-
_ = v7
903-
var tmp8 any
904-
tmp9 := aotKeywordLookup1(v7, nil)
905-
tmp10 := lang.Identical(tmp9, nil)
906-
if lang.IsTruthy(tmp10) {
907-
tmp11 := aotKeywordLookup0(v7, nil)
908-
tmp8 = tmp11
884+
v2 := p0
885+
_ = v2
886+
var tmp3 any
887+
tmp4 := aotKeywordLookup1(v2, nil)
888+
tmp5 := lang.Identical(tmp4, nil)
889+
if lang.IsTruthy(tmp5) {
890+
tmp6 := aotKeywordLookup0(v2, nil)
891+
tmp3 = tmp6
909892
} else {
910-
tmp12 := aotKeywordLookup0(v7, nil)
911-
tmp13 := aotKeywordLookup1(v7, nil)
912-
tmp14 := aotDirectFn4(tmp13)
913-
tmp15 := lang.Numbers.Add(tmp12, tmp14)
914-
tmp16 := aotKeywordLookup2(v7, nil)
915-
tmp17 := aotDirectFn4(tmp16)
916-
tmp18 := lang.Numbers.Add(tmp15, tmp17)
917-
tmp8 = tmp18
893+
tmp7 := aotKeywordLookup0(v2, nil)
894+
tmp8 := aotKeywordLookup1(v2, nil)
895+
tmp9 := aotDirectFn4(tmp8)
896+
tmp10 := lang.Numbers.Add(tmp7, tmp9)
897+
tmp11 := aotKeywordLookup2(v2, nil)
898+
tmp12 := aotDirectFn4(tmp11)
899+
tmp13 := lang.Numbers.Add(tmp10, tmp12)
900+
tmp3 = tmp13
918901
}
919-
return tmp8
902+
return tmp3
920903
})
921904
aotDirectFn4 = tmp1
922905
var_codegen_DOT_test_DOT_record_DASH_specialization_sum_DASH_node = ns.InternWithValue(tmp0, tmp1, true)

0 commit comments

Comments
 (0)