Skip to content

Commit b2ff013

Browse files
committed
Apply the rest of the review: fewer allocations, less repetition
Efficiency, measured with alloc profiles over the corpus tests: - internal/dump dominated the harness, not the parser. It rebuilt each node type's field layout per instance and formatted every scalar through fmt. The layout is cached per type and the leaves go straight to the builder; TestRoundTrip drops from 1.5s to 0.55s. - regenerate-parse started and tore down a pool of oracle processes per script, forking a few thousand times over a run. One pool for the whole run takes regeneration from 19s to 2.5s, corpus byte-identical. - parseTypeToken built a slice and joined it for every column type, when a type is almost always one word. - The %fallback set and the infix operator table are Kind-indexed arrays rather than maps; both are consulted per token. - Children() allocates only when a node has children, and the parser's named-parameter map only when there is a named parameter. Repetition: - parseOperators stated the precedence guard seventeen times, once per case, with half the operators tabled and half inline. One table, one guard; the operators with syntax of their own keep their cases. - The trigger body re-implemented UPDATE, INSERT and DELETE. It shares their cores now, so the three real differences -- no WITH, where_opt instead of where_opt_ret, no DEFAULT VALUES -- are visible lines rather than absences. - CreateTableStmt held Options and WithoutOpt as index-parallel slices, an invariant only the parser knew and only the renderer could break. - 27 Children() methods spelled out the same append loop; writeOnConflict and writeOrConflict differed only in a keyword. And the shift-then-decide rule that five comments were each re-arguing is now stated once in the package doc. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01JzBeCg7rjweVW3uGPg5G7T
1 parent 39dadb4 commit b2ff013

17 files changed

Lines changed: 363 additions & 370 deletions

File tree

ast/ddl.go

Lines changed: 25 additions & 38 deletions
Original file line numberDiff line numberDiff line change
@@ -14,23 +14,28 @@ type CreateTableStmt struct {
1414
Name *QualifiedName `json:"name"`
1515
Columns []*ColumnDef `json:"columns,omitempty"`
1616
Constraints []*TableConstraint `json:"constraints,omitempty"`
17-
Options []*Ident `json:"options,omitempty"` // WITHOUT ROWID, STRICT
18-
WithoutOpt []bool `json:"withoutOpt,omitempty"`
17+
Options []*TableOption `json:"options,omitempty"` // WITHOUT ROWID, STRICT
1918
Select *SelectStmt `json:"select,omitempty"`
2019
}
2120

21+
// TableOption is one entry of a table_option_set. SQLite rejects an unknown
22+
// one from a grammar action rather than the grammar, so any name parses.
23+
//
24+
// table_option ::= WITHOUT nm. / table_option ::= nm.
25+
type TableOption struct {
26+
Span
27+
Without bool `json:"without,omitempty"`
28+
Name *Ident `json:"name"`
29+
}
30+
31+
func (n *TableOption) Children() []Node { return nodes(n.Name) }
32+
2233
func (*CreateTableStmt) stmtNode() {}
2334
func (n *CreateTableStmt) Children() []Node {
2435
out := nodes(n.Name)
25-
for _, c := range n.Columns {
26-
out = append(out, c)
27-
}
28-
for _, c := range n.Constraints {
29-
out = append(out, c)
30-
}
31-
for _, o := range n.Options {
32-
out = append(out, o)
33-
}
36+
out = appendNodes(out, n.Columns)
37+
out = appendNodes(out, n.Constraints)
38+
out = appendNodes(out, n.Options)
3439
return append(out, nodes(n.Select)...)
3540
}
3641

@@ -46,9 +51,7 @@ type ColumnDef struct {
4651

4752
func (n *ColumnDef) Children() []Node {
4853
out := nodes(n.Name, n.Type)
49-
for _, c := range n.Constraints {
50-
out = append(out, c)
51-
}
54+
out = appendNodes(out, n.Constraints)
5255
return out
5356
}
5457

@@ -132,13 +135,9 @@ type TableConstraint struct {
132135

133136
func (n *TableConstraint) Children() []Node {
134137
out := nodes(n.Name)
135-
for _, c := range n.Columns {
136-
out = append(out, c)
137-
}
138+
out = appendNodes(out, n.Columns)
138139
out = append(out, nodes(n.Expr)...)
139-
for _, c := range n.FKColumns {
140-
out = append(out, c)
141-
}
140+
out = appendNodes(out, n.FKColumns)
142141
return append(out, nodes(n.References, n.Deferrable)...)
143142
}
144143

@@ -179,12 +178,8 @@ type ForeignKeyClause struct {
179178

180179
func (n *ForeignKeyClause) Children() []Node {
181180
out := nodes(n.Table)
182-
for _, c := range n.Columns {
183-
out = append(out, c)
184-
}
185-
for _, a := range n.Args {
186-
out = append(out, a)
187-
}
181+
out = appendNodes(out, n.Columns)
182+
out = appendNodes(out, n.Args)
188183
return out
189184
}
190185

@@ -228,9 +223,7 @@ type CreateIndexStmt struct {
228223
func (*CreateIndexStmt) stmtNode() {}
229224
func (n *CreateIndexStmt) Children() []Node {
230225
out := nodes(n.Name, n.Table)
231-
for _, c := range n.Columns {
232-
out = append(out, c)
233-
}
226+
out = appendNodes(out, n.Columns)
234227
return append(out, nodes(n.Where)...)
235228
}
236229

@@ -249,9 +242,7 @@ type CreateViewStmt struct {
249242
func (*CreateViewStmt) stmtNode() {}
250243
func (n *CreateViewStmt) Children() []Node {
251244
out := nodes(n.Name)
252-
for _, c := range n.Columns {
253-
out = append(out, c)
254-
}
245+
out = appendNodes(out, n.Columns)
255246
return append(out, nodes(n.Select)...)
256247
}
257248

@@ -287,13 +278,9 @@ type CreateTriggerStmt struct {
287278
func (*CreateTriggerStmt) stmtNode() {}
288279
func (n *CreateTriggerStmt) Children() []Node {
289280
out := nodes(n.Name)
290-
for _, u := range n.UpdateOf {
291-
out = append(out, u)
292-
}
281+
out = appendNodes(out, n.UpdateOf)
293282
out = append(out, nodes(n.Table, n.When)...)
294-
for _, s := range n.Body {
295-
out = append(out, s)
296-
}
283+
out = appendNodes(out, n.Body)
297284
return out
298285
}
299286

ast/dml.go

Lines changed: 10 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -41,16 +41,10 @@ type InsertStmt struct {
4141
func (*InsertStmt) stmtNode() {}
4242
func (n *InsertStmt) Children() []Node {
4343
out := nodes(n.With, n.Table, n.Alias)
44-
for _, c := range n.Columns {
45-
out = append(out, c)
46-
}
44+
out = appendNodes(out, n.Columns)
4745
out = append(out, nodes(n.Select)...)
48-
for _, u := range n.Upserts {
49-
out = append(out, u)
50-
}
51-
for _, r := range n.Returning {
52-
out = append(out, r)
53-
}
46+
out = appendNodes(out, n.Upserts)
47+
out = appendNodes(out, n.Returning)
5448
return out
5549
}
5650

@@ -69,13 +63,9 @@ type Upsert struct {
6963

7064
func (n *Upsert) Children() []Node {
7165
var out []Node
72-
for _, t := range n.Target {
73-
out = append(out, t)
74-
}
66+
out = appendNodes(out, n.Target)
7567
out = append(out, nodes(n.TargetWhere)...)
76-
for _, s := range n.Set {
77-
out = append(out, s)
78-
}
68+
out = appendNodes(out, n.Set)
7969
return append(out, nodes(n.Where)...)
8070
}
8171

@@ -92,9 +82,7 @@ type SetPair struct {
9282

9383
func (n *SetPair) Children() []Node {
9484
out := make([]Node, 0, len(n.Columns)+1)
95-
for _, c := range n.Columns {
96-
out = append(out, c)
97-
}
85+
out = appendNodes(out, n.Columns)
9886
return append(out, nodes(n.Value)...)
9987
}
10088

@@ -123,16 +111,10 @@ type UpdateStmt struct {
123111
func (*UpdateStmt) stmtNode() {}
124112
func (n *UpdateStmt) Children() []Node {
125113
out := nodes(n.With, n.Table, n.Alias, n.IndexedBy)
126-
for _, s := range n.Set {
127-
out = append(out, s)
128-
}
129-
for _, f := range n.From {
130-
out = append(out, f)
131-
}
114+
out = appendNodes(out, n.Set)
115+
out = appendNodes(out, n.From)
132116
out = append(out, nodes(n.Where)...)
133-
for _, r := range n.Returning {
134-
out = append(out, r)
135-
}
117+
out = appendNodes(out, n.Returning)
136118
return out
137119
}
138120

@@ -155,8 +137,6 @@ type DeleteStmt struct {
155137
func (*DeleteStmt) stmtNode() {}
156138
func (n *DeleteStmt) Children() []Node {
157139
out := nodes(n.With, n.Table, n.Alias, n.IndexedBy, n.Where)
158-
for _, r := range n.Returning {
159-
out = append(out, r)
160-
}
140+
out = appendNodes(out, n.Returning)
161141
return out
162142
}

ast/expr.go

Lines changed: 19 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -292,12 +292,8 @@ type InExpr struct {
292292
func (*InExpr) exprNode() {}
293293
func (n *InExpr) Children() []Node {
294294
out := nodes(n.X, n.Select, n.Table)
295-
for _, e := range n.List {
296-
out = append(out, e)
297-
}
298-
for _, e := range n.Args {
299-
out = append(out, e)
300-
}
295+
out = appendNodes(out, n.List)
296+
out = appendNodes(out, n.Args)
301297
return out
302298
}
303299

@@ -314,9 +310,7 @@ type CaseExpr struct {
314310
func (*CaseExpr) exprNode() {}
315311
func (n *CaseExpr) Children() []Node {
316312
out := nodes(n.Operand)
317-
for _, w := range n.Whens {
318-
out = append(out, w)
319-
}
313+
out = appendNodes(out, n.Whens)
320314
return append(out, nodes(n.Else)...)
321315
}
322316

@@ -375,12 +369,8 @@ type FuncCall struct {
375369
func (*FuncCall) exprNode() {}
376370
func (n *FuncCall) Children() []Node {
377371
out := nodes(n.Name)
378-
for _, a := range n.Args {
379-
out = append(out, a)
380-
}
381-
for _, o := range n.OrderBy {
382-
out = append(out, o)
383-
}
372+
out = appendNodes(out, n.Args)
373+
out = appendNodes(out, n.OrderBy)
384374
return append(out, nodes(n.Filter, n.Over)...)
385375
}
386376

@@ -448,13 +438,25 @@ type TypeName struct {
448438

449439
func (n *TypeName) Children() []Node { return nil }
450440

451-
// nodes builds a Children() slice, dropping nil interfaces and nil pointers.
441+
// nodes builds a Children() slice, dropping nil interfaces and nil
442+
// pointers. Optional fields are common, so a node whose children are all
443+
// absent allocates nothing.
452444
func nodes(list ...Node) []Node {
453-
out := make([]Node, 0, len(list))
445+
var out []Node
454446
for _, n := range list {
455447
if !isNil(n) {
456448
out = append(out, n)
457449
}
458450
}
459451
return out
460452
}
453+
454+
// appendNodes appends a slice of concrete node pointers to a Children()
455+
// result. It exists because every node with a repeated field would
456+
// otherwise spell the same three-line loop out again.
457+
func appendNodes[T Node](out []Node, list []T) []Node {
458+
for _, n := range list {
459+
out = append(out, n)
460+
}
461+
return out
462+
}

0 commit comments

Comments
 (0)