From 8231c44fa230b5c479eb97959c073398cbd7844e Mon Sep 17 00:00:00 2001 From: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> Date: Thu, 24 Sep 2026 18:32:23 +0000 Subject: [PATCH 1/3] fix(check): read the ~ operator sites the parser records instead of walking the tree Co-Authored-By: jason.han --- ...fined-operator-parse-record.performance.md | 1 + internal/check/passes/undefined_operator.go | 29 ++++----- internal/syntax/ast/astcodec/codec.go | 16 +++++ internal/syntax/ast/astcodec/nodes.go | 2 + internal/syntax/ast/namespace.go | 3 + internal/syntax/parser/expr.go | 3 + internal/syntax/parser/parser.go | 28 ++++++--- .../syntax/parser/undefined_operators_test.go | 53 ++++++++++++++++ internal/workspace/libs/stdlib.snapshot | Bin 3699318 -> 3699424 bytes .../testdata/parse/undefined-operator.golden | 15 +++++ .../testdata/parse/undefined-operator.sysml | 4 ++ tests/parser/undefined_operators_test.go | 58 ++++++++++++++++++ 12 files changed, 186 insertions(+), 26 deletions(-) create mode 100644 changes/unreleased/undefined-operator-parse-record.performance.md create mode 100644 internal/syntax/parser/undefined_operators_test.go create mode 100644 tests/parser/testdata/parse/undefined-operator.golden create mode 100644 tests/parser/testdata/parse/undefined-operator.sysml create mode 100644 tests/parser/undefined_operators_test.go diff --git a/changes/unreleased/undefined-operator-parse-record.performance.md b/changes/unreleased/undefined-operator-parse-record.performance.md new file mode 100644 index 000000000..ef93698d0 --- /dev/null +++ b/changes/unreleased/undefined-operator-parse-record.performance.md @@ -0,0 +1 @@ +- The `~` undefined-operator warning now reads the operator sites the parser records instead of walking every node of every document, removing about 8% from load and validation time. diff --git a/internal/check/passes/undefined_operator.go b/internal/check/passes/undefined_operator.go index 9f04d4aa4..d43a5c992 100644 --- a/internal/check/passes/undefined_operator.go +++ b/internal/check/passes/undefined_operator.go @@ -19,25 +19,22 @@ type UndefinedOperatorPass struct{} // later-tier failure never hides the warning. func (UndefinedOperatorPass) Level() PassLevel { return LevelSyntax } -// Run walks the parsed tree and warns at each `~` operator expression. It stays -// a warning in every conformance mode: the specification asks for a warning, -// not a rejection. +// Run warns at each `~` operator expression the parser recorded on the root. +// It stays a warning in every conformance mode: the specification asks for a +// warning, not a rejection. func (UndefinedOperatorPass) Run(ctx *Context, name string, root *ast.RootNamespace) []diag.Diagnostic { if root == nil { return nil } - var diags []diag.Diagnostic - ast.Inspect(root, func(n ast.Node) bool { - if e, ok := n.(*ast.OperatorExpr); ok && e.Operator == ast.OpBitNot { - diags = append(diags, diag.Diagnostic{ - Severity: diag.SeverityWarning, - Span: e.Span(), - Message: msgUndefinedOperator, - Code: codeUndefinedOperator, - Source: "syntax", - }) - } - return true - }) + diags := make([]diag.Diagnostic, 0, len(root.UndefinedOperators)) + for _, e := range root.UndefinedOperators { + diags = append(diags, diag.Diagnostic{ + Severity: diag.SeverityWarning, + Span: e.Span(), + Message: msgUndefinedOperator, + Code: codeUndefinedOperator, + Source: "syntax", + }) + } return diags } diff --git a/internal/syntax/ast/astcodec/codec.go b/internal/syntax/ast/astcodec/codec.go index 0b6a31c76..820c06dc8 100644 --- a/internal/syntax/ast/astcodec/codec.go +++ b/internal/syntax/ast/astcodec/codec.go @@ -166,6 +166,13 @@ func (e *Encoder) ends(ns []*ast.ConnectorEnd) { } } +func (e *Encoder) operatorExprs(ns []*ast.OperatorExpr) { + e.w.Len(len(ns)) + for _, n := range ns { + e.node(n) + } +} + func (e *Encoder) segments(segs []ast.NameSegment) { e.w.Len(len(segs)) for _, s := range segs { @@ -213,6 +220,7 @@ type Decoder struct { nameSlices pack.Arena[*ast.QualifiedName] regSlices pack.Arena[*ast.StateRegion] endSlices pack.Arena[*ast.ConnectorEnd] + opSlices pack.Arena[*ast.OperatorExpr] segArena pack.Arena[ast.NameSegment] argArena pack.Arena[ast.NamedArg] paramArena pack.Arena[ast.BodyParam] @@ -390,6 +398,14 @@ func (d *Decoder) ends() []*ast.ConnectorEnd { return out } +func (d *Decoder) operatorExprs() []*ast.OperatorExpr { + out := d.opSlices.Take(d.r.Len()) + for i := range out { + out[i] = typed[*ast.OperatorExpr](d) + } + return out +} + func (d *Decoder) segments() []ast.NameSegment { return d.segmentsN(d.r.Len()) } diff --git a/internal/syntax/ast/astcodec/nodes.go b/internal/syntax/ast/astcodec/nodes.go index 60303f43a..166d09e98 100644 --- a/internal/syntax/ast/astcodec/nodes.go +++ b/internal/syntax/ast/astcodec/nodes.go @@ -1001,6 +1001,7 @@ func (e *Encoder) encodeFields(node ast.Node) { case *ast.RootNamespace: e.base(&n.NodeBase) e.nodes(n.Members) + e.operatorExprs(n.UndefinedOperators) case *ast.SelectExpr: e.base(&n.NodeBase) e.node(n.Operand) @@ -1504,6 +1505,7 @@ func (d *Decoder) decodeFields(node ast.Node) { case *ast.RootNamespace: d.base(&n.NodeBase) n.Members = d.nodes() + n.UndefinedOperators = d.operatorExprs() case *ast.SelectExpr: d.base(&n.NodeBase) n.Operand = d.node() diff --git a/internal/syntax/ast/namespace.go b/internal/syntax/ast/namespace.go index 6c3fd8133..ed5cb6e16 100644 --- a/internal/syntax/ast/namespace.go +++ b/internal/syntax/ast/namespace.go @@ -249,6 +249,9 @@ type Membership struct { type RootNamespace struct { NodeBase Members []Node // *Membership | *Import | *Alias | *ErrorNode + // UndefinedOperators are the `~` operator expressions of the document in + // source order: KerML leaves DataFunctions::'~' undefined, so a tool warns at each. + UndefinedOperators []*OperatorExpr } // PrefixMetadata records a `# QualifiedName` metadata annotation reference. diff --git a/internal/syntax/parser/expr.go b/internal/syntax/parser/expr.go index cc789a6d3..e79e524c0 100644 --- a/internal/syntax/parser/expr.go +++ b/internal/syntax/parser/expr.go @@ -188,6 +188,9 @@ func (p *Parser) parseUnary() ast.Node { operand := p.parseUnary() e := &ast.OperatorExpr{Operator: op, Operands: []ast.Node{operand}} e.NodeSpan = p.spanFrom(start) + if op == ast.OpBitNot { + p.undefinedOps = append(p.undefinedOps, e) + } return e } diff --git a/internal/syntax/parser/parser.go b/internal/syntax/parser/parser.go index de0f6f5fe..1ea05599a 100644 --- a/internal/syntax/parser/parser.go +++ b/internal/syntax/parser/parser.go @@ -36,6 +36,10 @@ type Parser struct { // as a declaration name. Warnings []Diagnostic + // undefinedOps are the `~` operator expressions the parse built, in source + // order; ParseFile hands them to the root. + undefinedOps []*ast.OperatorExpr + // calcBodyDepth counts the calculation bodies being parsed, so a `return` // reached in a statement position inside one is read as the result // parameter it declares rather than as an unknown action keyword. @@ -117,9 +121,10 @@ func (p *Parser) bodyContext() bodyContext { // parseCheckpoint captures parser state for backtracking. type parseCheckpoint struct { - pos int - diagnosticLen int - warningLen int + pos int + diagnosticLen int + warningLen int + undefinedOpLen int // triv is a copy of the pending trivia at the checkpoint; trivLogLen is // how much of trivLog was already lexed then. triv []ast.Trivia @@ -489,6 +494,7 @@ func (p *Parser) ParseFile() *ast.RootNamespace { } } root.NodeSpan = p.spanFrom(start) + root.UndefinedOperators = p.undefinedOps return root } @@ -497,13 +503,14 @@ func (p *Parser) ParseFile() *ast.RootNamespace { func (p *Parser) checkpoint() parseCheckpoint { p.checkpoints++ return parseCheckpoint{ - pos: p.pos, - diagnosticLen: len(p.Diagnostics), - warningLen: len(p.Warnings), - triv: slices.Clone(p.triv), - trivLogLen: len(p.trivLog), - pendingSpan: p.pendingComment, - hadPending: p.hasPendingComment, + pos: p.pos, + diagnosticLen: len(p.Diagnostics), + warningLen: len(p.Warnings), + undefinedOpLen: len(p.undefinedOps), + triv: slices.Clone(p.triv), + trivLogLen: len(p.trivLog), + pendingSpan: p.pendingComment, + hadPending: p.hasPendingComment, } } @@ -515,6 +522,7 @@ func (p *Parser) restore(cp parseCheckpoint) { p.pos = cp.pos p.Diagnostics = p.Diagnostics[:cp.diagnosticLen] p.Warnings = p.Warnings[:cp.warningLen] + p.undefinedOps = p.undefinedOps[:cp.undefinedOpLen] p.pendingComment = cp.pendingSpan p.hasPendingComment = cp.hadPending p.triv = append(cp.triv, p.trivLog[cp.trivLogLen:]...) diff --git a/internal/syntax/parser/undefined_operators_test.go b/internal/syntax/parser/undefined_operators_test.go new file mode 100644 index 000000000..7a16d2dc5 --- /dev/null +++ b/internal/syntax/parser/undefined_operators_test.go @@ -0,0 +1,53 @@ +package parser + +import ( + "testing" + + "github.com/Open-MBEE/OpenSysML/internal/syntax/ast" + "github.com/Open-MBEE/OpenSysML/internal/syntax/source" +) + +// The parser records every `~` operator expression on the root in source +// order, wherever the expression sits. +func TestUndefinedOperatorsRecordsEveryTilde(t *testing.T) { + src := "package p {\n" + + "attribute a = ~1;\n" + + "calc def C { return ~x; }\n" + + "attribute b = f(~y, 2);\n" + + "}" + root := New(source.New("t.sysml", []byte(src))).ParseFile() + ops := root.UndefinedOperators + if len(ops) != 3 { + t.Fatalf("UndefinedOperators len = %d, want 3", len(ops)) + } + for i := 1; i < len(ops); i++ { + if ops[i].Span().Offset <= ops[i-1].Span().Offset { + t.Fatalf("UndefinedOperators not in source order at %d", i) + } + } + for _, e := range ops { + if e.Operator != ast.OpBitNot { + t.Fatalf("recorded operator = %v, want OpBitNot", e.Operator) + } + } +} + +// `~~x` records both the inner and the outer `~` expression. +func TestUndefinedOperatorsRecordsNestedTildes(t *testing.T) { + root := New(source.New("t.sysml", []byte("package p { attribute a = ~~x; }"))).ParseFile() + if len(root.UndefinedOperators) != 2 { + t.Fatalf("UndefinedOperators len = %d, want 2 for ~~x", len(root.UndefinedOperators)) + } +} + +// A checkpoint restore drops the `~` sites the abandoned attempt recorded. +func TestUndefinedOperatorsFollowsRestore(t *testing.T) { + // `attribute a = ~1` inside the first package parses; a second top-level + // member cannot start mid-expression, so anything the abandoned try-parse + // of a mistaken shape recorded must not leak into the root list. + src := "package p { attribute a = ~1; } package q { attribute b = ~2; }" + root := New(source.New("t.sysml", []byte(src))).ParseFile() + if len(root.UndefinedOperators) != 2 { + t.Fatalf("UndefinedOperators len = %d, want 2", len(root.UndefinedOperators)) + } +} diff --git a/internal/workspace/libs/stdlib.snapshot b/internal/workspace/libs/stdlib.snapshot index 3b61e0bc383d0a2b6eb09cda34d703786a2ccca2..78a089a4bc00cc0f0aa4d43bb182e42346928192 100644 GIT binary patch delta 1327 zcmW;MTS!xJ0LSqi&8}*8&#v12e$OHzDyX2RUV4n+JO@Efy~O6~xX>i&qFtuR5Sdz9 zVp_IziJ6(1rLOF{EgNPfCMsc3eSdrS{`vU#zn!!5|8FBbAtU%DWaJ!)>pPnlOiVW| z)3D4K%Z#K#|WpFoK;nUZJIm58o+V`USQqe&)C{?XQE*HA640)uM!dU1N!$(nB z7K)^x&r<(VG%U4EMecK`MgJ|8g3?pzW-f1IHv)3MUrQs|aHNjGh+JZfbSA?3%dq(U zc$jwudP>H@E;oFetp$}g;K=znnC^wS`SI{~2^>tH01GPMo4{CDT?Ko8>K(i_(C$cv z0}o(jTMTshVDkaweu#$lpgL(ZtWTY#yUW>%PCr6!J(OW-)WcxpH!wE6)yi*Tc)eCW zLCz(pTI!pI8YG8lv)hc9qA!8Tzmk7G%4$Zx7NYCYnaSv}RHX}cOVx|fZ>e}Yy7d&b z=px@G&k9u5f^O>rMx>_IDEk?5?m*3weN>bG#M)J;p;Zg@V(U37-H+;|T5V8H8*(P0 zo6@gSs8MRvg^ORHYdcZBRH)a6Bi%v5y7-T{NU#cPuHIPyx3+Jf&= zdHWdX>4dde3`RJ>=s$)kUZcXpS}8fS<%Z`CW`2 z8!xOyVD<#?@{!8h)(T{;}hy#fbt}VcA->??oor}oz2`!so)2L z5r|IebPfgi+nuES|0DM3oMwcOPkWsy73;BFluA>PNBXG4QyyKX@914ASD)*>RJj`s zO1(Ny`C(M0^Hd}a>27MGb9!V=(#|G*gS*AR7W7TZ4Cw&7KEr-puj30E(8=qOGWCn$ zj2Jf8Kvy!&p}91V=FM)e&e;z&eVzV>ehdPVV|xtzg|q^OKtTTh1Y4T zhhkEffJ*RbI;aP33v_^*SnwAF7l4c=F+&O93OGC27?(Ui0iC5AR4xW1plCa|(M%C_ z=r6#x99(}S=BDnY4>YaB2=O2nxV5|`z%^R>dgPzOrw&l?RoCL^ zC%s&QJ&dlU`U86PlBIFCHB3Vclk;P;QNg_+hGP0bkAS=l#vYM Date: Thu, 24 Sep 2026 19:29:49 +0000 Subject: [PATCH 2/3] fix(parser): record nested ~ operators in source order Co-Authored-By: jason.han --- internal/syntax/parser/expr.go | 10 ++++++++-- internal/syntax/parser/undefined_operators_test.go | 13 ++++++++++--- 2 files changed, 18 insertions(+), 5 deletions(-) diff --git a/internal/syntax/parser/expr.go b/internal/syntax/parser/expr.go index e79e524c0..63eabeb62 100644 --- a/internal/syntax/parser/expr.go +++ b/internal/syntax/parser/expr.go @@ -185,11 +185,17 @@ func (p *Parser) parseUnary() ast.Node { return p.parsePrimary() } p.advance() // prefix operator + // Reserve the slot before the operand so nested `~~x` records in source order. + slot := -1 + if op == ast.OpBitNot { + slot = len(p.undefinedOps) + p.undefinedOps = append(p.undefinedOps, nil) + } operand := p.parseUnary() e := &ast.OperatorExpr{Operator: op, Operands: []ast.Node{operand}} e.NodeSpan = p.spanFrom(start) - if op == ast.OpBitNot { - p.undefinedOps = append(p.undefinedOps, e) + if slot >= 0 { + p.undefinedOps[slot] = e } return e } diff --git a/internal/syntax/parser/undefined_operators_test.go b/internal/syntax/parser/undefined_operators_test.go index 7a16d2dc5..65f5eb830 100644 --- a/internal/syntax/parser/undefined_operators_test.go +++ b/internal/syntax/parser/undefined_operators_test.go @@ -1,6 +1,7 @@ package parser import ( + "strings" "testing" "github.com/Open-MBEE/OpenSysML/internal/syntax/ast" @@ -34,9 +35,15 @@ func TestUndefinedOperatorsRecordsEveryTilde(t *testing.T) { // `~~x` records both the inner and the outer `~` expression. func TestUndefinedOperatorsRecordsNestedTildes(t *testing.T) { - root := New(source.New("t.sysml", []byte("package p { attribute a = ~~x; }"))).ParseFile() - if len(root.UndefinedOperators) != 2 { - t.Fatalf("UndefinedOperators len = %d, want 2 for ~~x", len(root.UndefinedOperators)) + src := "package p { attribute a = ~~x; }" + root := New(source.New("t.sysml", []byte(src))).ParseFile() + ops := root.UndefinedOperators + if len(ops) != 2 { + t.Fatalf("UndefinedOperators len = %d, want 2 for ~~x", len(ops)) + } + outer := strings.Index(src, "~~") + if got := []int{ops[0].Span().Offset, ops[1].Span().Offset}; got[0] != outer || got[1] != outer+1 { + t.Fatalf("offsets = %v, want outer %d then inner %d", got, outer, outer+1) } } From ab95f7dd8bbab83630ddc9e6856dc10521b57b7d Mon Sep 17 00:00:00 2001 From: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> Date: Thu, 24 Sep 2026 19:32:11 +0000 Subject: [PATCH 3/3] test(parser): exercise a checkpoint restore over recorded ~ operators Co-Authored-By: jason.han --- .../syntax/parser/undefined_operators_test.go | 24 +++++++++++++------ 1 file changed, 17 insertions(+), 7 deletions(-) diff --git a/internal/syntax/parser/undefined_operators_test.go b/internal/syntax/parser/undefined_operators_test.go index 65f5eb830..3773d2ab2 100644 --- a/internal/syntax/parser/undefined_operators_test.go +++ b/internal/syntax/parser/undefined_operators_test.go @@ -49,12 +49,22 @@ func TestUndefinedOperatorsRecordsNestedTildes(t *testing.T) { // A checkpoint restore drops the `~` sites the abandoned attempt recorded. func TestUndefinedOperatorsFollowsRestore(t *testing.T) { - // `attribute a = ~1` inside the first package parses; a second top-level - // member cannot start mid-expression, so anything the abandoned try-parse - // of a mistaken shape recorded must not leak into the root list. - src := "package p { attribute a = ~1; } package q { attribute b = ~2; }" - root := New(source.New("t.sysml", []byte(src))).ParseFile() - if len(root.UndefinedOperators) != 2 { - t.Fatalf("UndefinedOperators len = %d, want 2", len(root.UndefinedOperators)) + p := newParser("~x + ~y") + if p.parseUnary(); len(p.undefinedOps) != 1 { + t.Fatalf("after ~x: len = %d, want 1", len(p.undefinedOps)) + } + cp := p.checkpoint() + p.advance() // '+' + if p.parseUnary(); len(p.undefinedOps) != 2 { + t.Fatalf("after ~y: len = %d, want 2", len(p.undefinedOps)) + } + p.restore(cp) + p.release() + if len(p.undefinedOps) != 1 || p.undefinedOps[0].Span().Offset != 0 { + t.Fatalf("after restore: %d ops, want only the ~ at offset 0", len(p.undefinedOps)) + } + p.advance() // '+' + if p.parseUnary(); len(p.undefinedOps) != 2 || p.undefinedOps[1].Span().Offset != 5 { + t.Fatalf("after reparse: %d ops, want the ~ at offset 5 second", len(p.undefinedOps)) } }