diff --git a/CHANGELOG.md b/CHANGELOG.md index f057ad70bc..36e846ace2 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -665,6 +665,26 @@ release is described in [docs/project/releasing.md](docs/project/releasing.md). - **`ParseSources` re-analyzes only what an edit reaches when a client parses the same documents again.** From the second parse of a document set, the service answers from an incremental workspace held for it: unchanged documents keep their trees and diagnostics, and only the changed documents and those whose analysis read them are analyzed again. The answer is the one a fresh parse gives. On a 186-document model a re-parse after a one-line edit takes about 0.9 s instead of 1.7 s. +- **Recognising `this`, `that` and `isActive` no longer builds a qualified name.** Each read of a feature chain in a calculation or action body asked whether its target was `Occurrences::Occurrence::this`, `Base::things::that` or `StateActivity::isActive` by assembling the target's fully-qualified name and comparing it; the owner chain is now compared against the known name segment by segment, so a conformant body allocates nothing for the question, and every answer is as before. + +- **Compiled calcs pass their Integers, Rationals, Reals and Booleans in three words.** The + compiled calc tier held each scalar in 56 bytes and, for every parameter, local and result, + boxed it back into the evaluator's constant to decide whether the declaration holds it, and + built the diagnostic a refusal would name whether or not one followed. A scalar is now the + constant packed into 24 bytes (`semantics.Packed`), unpacked without allocating; which kinds + a declaration holds whatever the value is decided once when the calc is compiled; the + diagnostic is built only on refusal; and the step charge inlines into every node. + `Fib(25)` interpreted runs in a fifth of the time with a quarter of the allocations; every + result, promotion to a big value, refusal, error message and step count is unchanged. + +- **The gRPC service converts a cached model's diagnostics to protobuf once.** `ParseFile`, `ParseSources` and `GetDiagnostics` on a model parsed before answered the same diagnostics converted again on every call, two allocations per warning; each document's are now converted on first use and shared by every later response, and a model answered from a lineage shares those of every document the edit did not reach. The responses are unchanged. + +- **A body statement's write no longer pays for its diagnostic description when it conforms.** The binding check of an assignment or declaration in an action, state or calculation body took the description of where it ran (`action node step`, `state behavior s`, `action A`) as a string built on every write; it now takes a function and formats the description only in the refusal it reports. An assignment step of an action loop allocates about a thousand fewer objects per thousand iterations, and every diagnostic reads as before. + +- **Replacing a document no longer copies the re-export claims of every name it re-registers.** Before a name's registration changes, the index notes what a lookup of it reads so that one registered again exactly as it was does not count as a change; the note held a copy of every document's claims and routes on the name, taken once per name touched. It now shares the index's own claim tables, which a write replaces rather than changes, so an edit beside a large model allocates a fraction of what it did. What counts as a change is as before. + +- Constraint checks and other runs no longer walk the succession graph for a cycle among held behaviors on every quiescent scan: the runtime remembers that no behavior is held until one enters held, by instantiation, image or snapshot restore, which cut `BenchmarkBatchConstraints` per-check time. + ## 0.9.2 — 2026-10-05 ### Added diff --git a/internal/exec/runtime/action_control_objects.go b/internal/exec/runtime/action_control_objects.go index 1797694c8c..097949987b 100644 --- a/internal/exec/runtime/action_control_objects.go +++ b/internal/exec/runtime/action_control_objects.go @@ -134,7 +134,7 @@ func arrivingObject(frame *actionFrame, graph *lower.ActionGraph, node ast.Node, func (e *performances) queueControlObject(frame *actionFrame, graph *lower.ActionGraph, flow lower.ObjectFlow, value Value) error { for _, f := range graph.Features[flow.Target] { if f.Name == flow.TargetPin && f.Direction == ast.DirIn { - if err := e.ctx.checkNamedWrite(graph.Scopes[flow.Target], nodeDescription(flow.Target), f.Name, &value); err != nil { + if err := e.ctx.checkNamedWriteAs(graph.Scopes[flow.Target], func() string { return nodeDescription(flow.Target) }, f.Name, &value); err != nil { return fmt.Errorf("%s: %w", flowDescription(flow), err) } frame.hold(flow.Target, f.Name, nodeObject{value: value, flow: flow.Decl, carried: flow.Kind == lower.FlowSuccession}) diff --git a/internal/exec/runtime/action_executor.go b/internal/exec/runtime/action_executor.go index a6d92c2fb3..55dfcbd49c 100644 --- a/internal/exec/runtime/action_executor.go +++ b/internal/exec/runtime/action_executor.go @@ -1537,7 +1537,7 @@ func (e *ActionExecutor) setFeature(name string, value Value) error { if value, err = e.mirrorOccurrence(name, value); err != nil { return err } - } else if err := e.ctx.checkNamedWrite(e.graph.Scope, actionLabelPrefix+symbolText(e.action), name, &value); err != nil { + } else if err := e.ctx.checkNamedWriteAs(e.graph.Scope, func() string { return actionLabelPrefix + symbolText(e.action) }, name, &value); err != nil { // No occurrence holds this feature, so its declaration is checked here // rather than by the write to that occurrence. return err diff --git a/internal/exec/runtime/action_frame.go b/internal/exec/runtime/action_frame.go index d52fd2307d..42e4d9d7d8 100644 --- a/internal/exec/runtime/action_frame.go +++ b/internal/exec/runtime/action_frame.go @@ -422,7 +422,7 @@ func (e *performances) seedPerformance(parent *actionFrame, flow *lower.ActionGr continue } check := func(value *Value) error { - return e.ctx.checkBodyDeclaration(feature.Scope, perf.describe(), feature.Name, value) + return e.ctx.checkBodyDeclarationAs(feature.Scope, perf.describe, feature.Name, value) } context := func(scope *symbols.Scope) *EvalContext { return e.evalBindingContext(perf, scope, feature.Name, activation) @@ -497,7 +497,7 @@ func (e *performances) seedDeclaredValues(perf *actionFrame, features []lower.Fe if feature.Binding { cells := e.bodyCells(perf) check := func(value *Value) error { - return e.ctx.checkBodyDeclaration(feature.Scope, perf.describe(), feature.Name, value) + return e.ctx.checkBodyDeclarationAs(feature.Scope, perf.describe, feature.Name, value) } name := perf.key(feature.Name) cell := cells.existingCell(name) @@ -514,7 +514,7 @@ func (e *performances) seedDeclaredValues(perf *actionFrame, features []lower.Fe } else { value, err = ec.Eval(feature.Value) if err == nil { - err = e.ctx.checkBodyDeclaration(feature.Scope, perf.describe(), feature.Name, &value) + err = e.ctx.checkBodyDeclarationAs(feature.Scope, perf.describe, feature.Name, &value) } if err == nil { e.ctx.writeBodyValue(perf.cells, perf.data, perf.key(feature.Name), value) @@ -894,7 +894,7 @@ func (e *performances) deliver(f *actionFrame, flow *lower.ActionGraph, node ast if !pins.declares(pin) { return fmt.Errorf("%w: %s declares no %s", ErrNodePin, nodeDescription(node), pin) } - if err := e.ctx.checkNamedWrite(flow.Scopes[node], nodeDescription(node), pin, &value); err != nil { + if err := e.ctx.checkNamedWriteAs(flow.Scopes[node], func() string { return nodeDescription(node) }, pin, &value); err != nil { return err } f.queue(node, canonical(pins.aliases, pin), value) @@ -952,7 +952,7 @@ func (e *performances) checkNestedDelivery(flow *lower.ActionGraph, node ast.Nod if !pins.declares(pin) { return fmt.Errorf("%w: %s declares no %s", ErrNodePin, nodeDescription(node), pin) } - return e.ctx.checkNamedWrite(flow.Scopes[node], nodeDescription(node), pin, value) + return e.ctx.checkNamedWriteAs(flow.Scopes[node], func() string { return nodeDescription(node) }, pin, value) } // takeDeliveries moves the oldest delivery at each pin of node into perf, so that @@ -1035,7 +1035,7 @@ func (e *performances) setFrameFeature(f *actionFrame, name string, value Value) e.noteFrameWrite(f, name, value) return nil } - if err := e.ctx.checkNamedWrite(f.scope, f.describe(), name, &value); err != nil { + if err := e.ctx.checkNamedWriteAs(f.scope, f.describe, name, &value); err != nil { return err } e.ctx.writeBodyValue(f.cells, f.data, f.key(name), value) @@ -1101,7 +1101,7 @@ func (e *performances) streamFlow( if !pins.declares(flow.TargetPin) { return fmt.Errorf("%s: %w: %s declares no %s", flowDescription(flow), ErrNodePin, nodeDescription(flow.Target), flow.TargetPin) } - if err := e.ctx.checkNamedWrite(graph.Scopes[flow.Target], nodeDescription(flow.Target), flow.TargetPin, &value); err != nil { + if err := e.ctx.checkNamedWriteAs(graph.Scopes[flow.Target], func() string { return nodeDescription(flow.Target) }, flow.TargetPin, &value); err != nil { return fmt.Errorf("%s: %w", flowDescription(flow), err) } pin := canonical(pins.aliases, flow.TargetPin) diff --git a/internal/exec/runtime/compile.go b/internal/exec/runtime/compile.go index 4a98ac4a80..864cfa6fbc 100644 --- a/internal/exec/runtime/compile.go +++ b/internal/exec/runtime/compile.go @@ -107,21 +107,46 @@ type scalarCheck struct { holdsReal bool // least is the smallest integer a Natural-holding declaration takes: 1 for Positive. least int64 + // plain has bit 1< 0) + switch v.Kind { + case semantics.ValInt: + if i, ok := v.smallInt(); ok { + return c.integerOK || (c.naturalOK && i >= c.least) } - return c.integerOK || (c.naturalOK && v.int() >= c.least) - case scalarBool: + return c.integerOK || (c.naturalOK && v.Unpack().BigIntView().Sign() > 0) + case semantics.ValBool: return c.boolOK } return c.acceptsReal(v) @@ -138,15 +163,21 @@ func (c *scalarCheck) acceptsReal(v scalar) bool { return false } +// holds reports whether the declaration takes v as it is, so held has nothing to do. +func (c *scalarCheck) holds(v scalar) bool { + return !c.holdsReal || v.Kind != semantics.ValRational +} + // held is v as the declaration holds it: a Rational a Real declaration takes as its // nearest binary64, refused when no finite Real is; what, if any, names the declaration. func (c *scalarCheck) held(v scalar, what func() string) (scalar, error) { - if !c.holdsReal || v.kind != scalarRational { + if c.holds(v) { return v, nil } - real, err := semantics.RealOf(v.exact) + exact := v.Unpack() + real, err := semantics.RealOf(exact) if err != nil { - err = fmt.Errorf("%s as a Real: %w", v.exact.FormatRational(), err) + err = fmt.Errorf("%s as a Real: %w", exact.FormatRational(), err) if what != nil { err = fmt.Errorf("%s: %w", what(), err) } @@ -388,11 +419,13 @@ func (c *calcCompiler) scalarCheckFor(decl *calcMemberDecl) (scalarCheck, bool) check := scalarCheck{decl: decl, countOK: true, boolOK: true, naturalOK: true, integerOK: true, rationalOK: true, realOK: true} if decl.Target == nil { + check.settle() return check, true } one := Value{Kind: ValConst} check.countOK = c.ctx.writeCountRefusal(decl.Target, &one) == "" if decl.Target.typ == nil { + check.settle() return check, true } if decl.Target.typ.Kind == symbols.SymbolEnumerationDef { @@ -412,6 +445,7 @@ func (c *calcCompiler) scalarCheckFor(decl *calcMemberDecl) (scalarCheck, bool) if c.ctx.positiveScalar(decl.Target.typ) { check.least = 1 } + check.settle() return check, true } @@ -810,26 +844,19 @@ func (c *compiledCalc) invoke(ctx *Context, base int, bound paramSet) (scalar, e for i := range c.params { p := &c.params[i] v := frame[i] - source := "argument" - if !bound.has(i) { + defaulted := !bound.has(i) + if defaulted { var err error if v, err = p.dflt(ctx, frame); err != nil { ctx.leaveCalc() return scalar{}, calcDefaultError(c.kind, c.name, p.name, err) } - source = "default" frame[i] = v } - what := func() string { - return fmt.Sprintf("%s %s: %s for parameter %q", c.kind, c.name, source, p.name) - } - if !p.check.accepts(v) { - if err := p.check.refuse(ctx, v, what); err != nil { - ctx.leaveCalc() - return scalar{}, err - } + if p.check.accepts(v) && p.check.holds(v) { + continue } - held, err := p.check.held(v, what) + held, err := c.holdParam(ctx, p, v, defaulted) if err != nil { ctx.leaveCalc() return scalar{}, err @@ -841,7 +868,7 @@ func (c *compiledCalc) invoke(ctx *Context, base int, bound paramSet) (scalar, e if err != nil { return scalar{}, calcFrame(c.kind, c.name, fmt.Errorf("%s%w", c.bodyErr, err)) } - if c.result != nil { + if c.result != nil && !(c.result.accepts(result) && c.result.holds(result)) { what := func() string { return c.resultWhat } if !c.result.accepts(result) { if err := c.result.refuse(ctx, result, what); err != nil { @@ -855,6 +882,24 @@ func (c *compiledCalc) invoke(ctx *Context, base int, bound paramSet) (scalar, e return result, nil } +// holdParam settles a parameter value its check did not take as it is: the +// reference's verdict on a value accepts declined, then the declaration's hold. +func (c *compiledCalc) holdParam(ctx *Context, p *compiledParam, v scalar, defaulted bool) (scalar, error) { + source := "argument" + if defaulted { + source = "default" + } + what := func() string { + return fmt.Sprintf("%s %s: %s for parameter %q", c.kind, c.name, source, p.name) + } + if !p.check.accepts(v) { + if err := p.check.refuse(ctx, v, what); err != nil { + return scalar{}, err + } + } + return p.check.held(v, what) +} + // frameAt extends the scalar stack to hold a frame of size slots at base; the // local slots are written by their declarations before any read. func (ctx *Context) frameAt(base, size int) []scalar { diff --git a/internal/exec/runtime/compiled_ops.go b/internal/exec/runtime/compiled_ops.go index 291b7f11f3..d0f417fbbd 100644 --- a/internal/exec/runtime/compiled_ops.go +++ b/internal/exec/runtime/compiled_ops.go @@ -3,93 +3,60 @@ package runtime import ( "fmt" "math" - "math/big" "slices" "github.com/Open-MBEE/OpenSysML/internal/semantic/semantics" "github.com/Open-MBEE/OpenSysML/internal/syntax/ast" ) -// scalar is an unboxed Integer, Rational, Real or Boolean, the values the -// compiled calc tier computes over; it is boxed into a Value only at an -// invocation boundary. An Integer beyond int64 is held in big, immutable, with -// bits zero; a Rational is held exactly in exact. -type scalar struct { - kind scalarKind - bits uint64 - big *big.Int - exact semantics.Value -} - -type scalarKind uint8 - -const ( - scalarInt scalarKind = iota - scalarReal - scalarBool - scalarRational -) +// scalar is an Integer, Rational, Real or Boolean constant packed into three +// words, the value the compiled calc tier passes between nodes and holds in frames. +type scalar struct{ semantics.Packed } // #nosec G115 -- the Integer's two's-complement bits are stored, not its magnitude. -func intScalar(i int64) scalar { return scalar{kind: scalarInt, bits: uint64(i)} } +func intScalar(i int64) scalar { + return scalar{semantics.Packed{Kind: semantics.ValInt, Bits: uint64(i)}} +} -func realScalar(f float64) scalar { return scalar{kind: scalarReal, bits: math.Float64bits(f)} } +func realScalar(f float64) scalar { + return scalar{semantics.Packed{Kind: semantics.ValReal, Bits: math.Float64bits(f)}} +} func boolScalar(b bool) scalar { if b { - return scalar{kind: scalarBool, bits: 1} + return scalar{semantics.Packed{Kind: semantics.ValBool, Bits: 1}} } - return scalar{kind: scalarBool, bits: 0} + return scalar{semantics.Packed{Kind: semantics.ValBool}} } // #nosec G115 -- the inverse of intScalar: the same bits read back as an Integer. -func (s scalar) int() int64 { return int64(s.bits) } +func (s scalar) int() int64 { return int64(s.Bits) } -func (s scalar) real() float64 { return math.Float64frombits(s.bits) } +func (s scalar) truth() bool { return s.Bits != 0 } -func (s scalar) truth() bool { return s.bits != 0 } +// smallInt answers s as an int64 when it is an Integer within int64. +func (s scalar) smallInt() (int64, bool) { + return s.int(), s.Kind == semantics.ValInt && s.Ext == nil +} -// smallInts reports whether l and r are both Integers within int64. -func smallInts(l, r scalar) bool { - return l.kind == scalarInt && r.kind == scalarInt && l.big == nil && r.big == nil +// smallInts answers l and r as int64 when both are Integers within int64. +func smallInts(l, r scalar) (a, b int64, ok bool) { + return l.int(), r.int(), l.Kind == semantics.ValInt && r.Kind == semantics.ValInt && l.Ext == nil && r.Ext == nil } // semantic is the constant the scalar stands for. -func (s scalar) semantic() semantics.Value { - switch s.kind { - case scalarInt: - if s.big != nil { - return semantics.BigIntValue(s.big) - } - return semantics.IntValue(s.int()) - case scalarReal: - return semantics.Value{Kind: semantics.ValReal, Real: s.real()} - case scalarRational: - return s.exact - default: - return semantics.Value{Kind: semantics.ValBool, Bool: s.truth()} - } -} +func (s scalar) semantic() semantics.Value { return s.Unpack() } // boxed is the scalar as the evaluator's Value. func (s scalar) boxed() Value { - return Value{Kind: ValConst, Const: s.semantic()} + return Value{Kind: ValConst, Const: s.Unpack()} } // scalarOfConst unboxes a scalar constant, declining any other constant kind. func scalarOfConst(c semantics.Value) (scalar, bool) { switch c.Kind { - case semantics.ValInt: - if i, ok := c.Int64(); ok { - return intScalar(i), true - } - return scalar{kind: scalarInt, big: c.BigIntView()}, true - case semantics.ValReal: - return realScalar(c.Real), true - case semantics.ValRational: - return scalar{kind: scalarRational, exact: c}, true - case semantics.ValBool: - return boolScalar(c.Bool), true + case semantics.ValInt, semantics.ValReal, semantics.ValRational, semantics.ValBool: + return scalar{c.Pack()}, true } return scalar{}, false } @@ -110,16 +77,24 @@ type compiledExpr func(ctx *Context, args []scalar) (scalar, error) // which can fail. It leaves the counter where the evaluator's would stop. func (ctx *Context) chargeSteps(n int64) error { if n > ctx.maxSteps-ctx.run.steps { - ctx.run.steps = ctx.maxSteps - if ctx.run.steps < math.MaxInt64 { - ctx.run.steps++ - } - return ctx.stepLimitExceeded() + return ctx.stepsExhausted() } ctx.run.steps += n return nil } +// stepsExhausted is the refusal of a charge the budget cannot meet, kept out +// of line so chargeSteps inlines into every compiled node. +// +//go:noinline +func (ctx *Context) stepsExhausted() error { + ctx.run.steps = ctx.maxSteps + if ctx.run.steps < math.MaxInt64 { + ctx.run.steps++ + } + return ctx.stepLimitExceeded() +} + // binaryOp combines two evaluated operands. type binaryOp func(ctx *Context, l, r scalar) (scalar, error) @@ -136,8 +111,7 @@ func arithmeticOp(op ast.OperatorKind) binaryOp { } func addScalars(ctx *Context, l, r scalar) (scalar, error) { - if smallInts(l, r) { - a, b := l.int(), r.int() + if a, b, ok := smallInts(l, r); ok { if res := a + b; (b <= 0 || res > a) && (b >= 0 || res < a) { return intScalar(res), nil } @@ -146,8 +120,7 @@ func addScalars(ctx *Context, l, r scalar) (scalar, error) { } func subScalars(ctx *Context, l, r scalar) (scalar, error) { - if smallInts(l, r) { - a, b := l.int(), r.int() + if a, b, ok := smallInts(l, r); ok { if res := a - b; (b >= 0 || res > a) && (b <= 0 || res < a) { return intScalar(res), nil } @@ -181,29 +154,29 @@ func comparisonOp(op ast.OperatorKind) binaryOp { switch op { case ast.OpLt: return func(_ *Context, l, r scalar) (scalar, error) { - if smallInts(l, r) { - return boolScalar(l.int() < r.int()), nil + if a, b, ok := smallInts(l, r); ok { + return boolScalar(a < b), nil } return compareScalars(op, l, r) } case ast.OpLe: return func(_ *Context, l, r scalar) (scalar, error) { - if smallInts(l, r) { - return boolScalar(l.int() <= r.int()), nil + if a, b, ok := smallInts(l, r); ok { + return boolScalar(a <= b), nil } return compareScalars(op, l, r) } case ast.OpGt: return func(_ *Context, l, r scalar) (scalar, error) { - if smallInts(l, r) { - return boolScalar(l.int() > r.int()), nil + if a, b, ok := smallInts(l, r); ok { + return boolScalar(a > b), nil } return compareScalars(op, l, r) } case ast.OpGe: return func(_ *Context, l, r scalar) (scalar, error) { - if smallInts(l, r) { - return boolScalar(l.int() >= r.int()), nil + if a, b, ok := smallInts(l, r); ok { + return boolScalar(a >= b), nil } return compareScalars(op, l, r) } @@ -242,7 +215,7 @@ func unaryScalar(op ast.OperatorKind, v scalar) (scalar, error) { // scalarTruth reads a Boolean operand, reporting any other scalar as the // evaluator's boolOperand does. func scalarTruth(what string, v scalar) (bool, error) { - if v.kind != scalarBool { + if v.Kind != semantics.ValBool { return boolOperand(what, v.boxed()) } return v.truth(), nil diff --git a/internal/exec/runtime/compiled_stmts.go b/internal/exec/runtime/compiled_stmts.go index 0ebf833bae..51ff74d420 100644 --- a/internal/exec/runtime/compiled_stmts.go +++ b/internal/exec/runtime/compiled_stmts.go @@ -4,6 +4,7 @@ import ( "fmt" "github.com/Open-MBEE/OpenSysML/internal/ir/lower" + "github.com/Open-MBEE/OpenSysML/internal/semantic/semantics" "github.com/Open-MBEE/OpenSysML/internal/semantic/symbols" "github.com/Open-MBEE/OpenSysML/internal/syntax/ast" ) @@ -228,8 +229,10 @@ func declareStmt(s lower.Declare, slot int, value compiledExpr, check *scalarChe return scalar{}, false, err } } - if v, err = check.held(v, nil); err != nil { - return scalar{}, false, err + if !check.holds(v) { + if v, err = check.held(v, nil); err != nil { + return scalar{}, false, err + } } } frame[slot] = v @@ -245,7 +248,7 @@ func returnStmt(value compiledExpr, result *scalarCheck) compiledStmt { if err != nil { return scalar{}, false, fmt.Errorf("evaluating the returned expression: %w", err) } - if result != nil { + if result != nil && !(result.accepts(v) && result.holds(v)) { what := func() string { return "result" } if !result.accepts(v) { if err := result.refuse(ctx, v, what); err != nil { @@ -268,7 +271,7 @@ func ifStmt(cond compiledExpr, then, els compiledStmt) compiledStmt { if err != nil { return scalar{}, false, fmt.Errorf("eval condition of 'if': %w", err) } - if cv.kind != scalarBool { + if cv.Kind != semantics.ValBool { return scalar{}, false, fmt.Errorf("%s: condition of 'if' must evaluate to a Boolean, got %s", calcBodyDescription, ValConst) } diff --git a/internal/exec/runtime/context.go b/internal/exec/runtime/context.go index 909bb16ad6..b028d53019 100644 --- a/internal/exec/runtime/context.go +++ b/internal/exec/runtime/context.go @@ -303,6 +303,10 @@ type Context struct { // quiescent is the memo a full scan leaves when it finds them all idle. work uint64 quiescent quiescence + // held counts the behaviors that entered held on a succession; noneHeld is + // the memo the cycle check leaves when it finds none held at that count. + held uint64 + noneHeld noneHeldMemo // onStack lists the runs of the executors whose calls are under way, outermost first. onStack []*executorRun diff --git a/internal/exec/runtime/held_image_behavior.go b/internal/exec/runtime/held_image_behavior.go index 6377ff78bc..51a159558a 100644 --- a/internal/exec/runtime/held_image_behavior.go +++ b/internal/exec/runtime/held_image_behavior.go @@ -833,7 +833,7 @@ func (m *materializing) frameCells( continue } check := func(value *Value) error { - return m.dst.checkBodyDeclaration(state.scope, perf.describe(), state.name, value) + return m.dst.checkBodyDeclarationAs(state.scope, perf.describe, state.name, value) } var onDerived func(*Value) error if perf.node == nil && perf.parent == nil { diff --git a/internal/exec/runtime/occurrence_terminate.go b/internal/exec/runtime/occurrence_terminate.go index ccfcfb9f23..dba75a6835 100644 --- a/internal/exec/runtime/occurrence_terminate.go +++ b/internal/exec/runtime/occurrence_terminate.go @@ -110,7 +110,7 @@ func (ctx *Context) noteEndingUndo(b *ObjectBehavior) { captured.captureState(b.State) captured.captureRunState(b.State.driven.state) } - ctx.noteProbeUndo(captured.restore) + ctx.noteProbeUndo(func() { captured.restore(ctx) }) } // endsWith reports whether the executor's performance ends with the objects ended: diff --git a/internal/exec/runtime/snapshot.go b/internal/exec/runtime/snapshot.go index 6f05ad23aa..433043d37c 100644 --- a/internal/exec/runtime/snapshot.go +++ b/internal/exec/runtime/snapshot.go @@ -219,7 +219,7 @@ func (s *Snapshot) Restore() { ctx.snapshots = ctx.snapshots[:at+1] ctx.rollbackJournal(s.journal) s.run.restore(ctx) - s.executorCaptures.restore() + s.executorCaptures.restore(ctx) } // captureAction captures an action executor once, with the executors the work of @@ -267,7 +267,7 @@ func (s *executorCaptures) captureBehavior(behavior *ObjectBehavior) { }) } -func (s *executorCaptures) restore() { +func (s *executorCaptures) restore(ctx *Context) { for _, capture := range s.runStates { capture.restore() } @@ -285,6 +285,9 @@ func (s *executorCaptures) restore() { capture.behavior.State = capture.state capture.behavior.Err = capture.err capture.behavior.deferred = capture.deferred + if capture.deferred != nil { + ctx.behaviorHeld() + } } } @@ -1033,5 +1036,5 @@ func (m *moveMark) undo() { m.run.steps, m.run.elements, m.run.notes = m.steps, m.elements, m.notes e.ctx.choices, e.ctx.draws = e.ctx.choices[:m.choices], e.ctx.draws[:m.draws] m.trace.restore(e.ctx.trace) - m.state.restore() + m.state.restore(e.ctx) } diff --git a/internal/exec/runtime/state_executor.go b/internal/exec/runtime/state_executor.go index 2f851fcc5b..db4effa1cc 100644 --- a/internal/exec/runtime/state_executor.go +++ b/internal/exec/runtime/state_executor.go @@ -639,11 +639,11 @@ func (e *StateExecutor) assignAttribute(name string, value Value) error { } else { // No occurrence holds this feature, so its declaration is checked here // rather than by the write to that occurrence. - where := "state machine " + symbolText(e.stateMachine) - if err := e.ctx.checkMutable(e.graph.Scope, func() string { return where + ": assignment to " + name }, name); err != nil { + where := func() string { return "state machine " + symbolText(e.stateMachine) } + if err := e.ctx.checkMutable(e.graph.Scope, func() string { return where() + ": assignment to " + name }, name); err != nil { return err } - if err := e.ctx.checkNamedWrite(e.graph.Scope, where, name, &value); err != nil { + if err := e.ctx.checkNamedWriteAs(e.graph.Scope, where, name, &value); err != nil { return err } } diff --git a/internal/exec/runtime/state_statements.go b/internal/exec/runtime/state_statements.go index 4ba9de1c6e..e4d4cdcedb 100644 --- a/internal/exec/runtime/state_statements.go +++ b/internal/exec/runtime/state_statements.go @@ -396,7 +396,7 @@ func (h *stateStmtHost) assignStateAttribute(name string, value Value) (bool, er if !ok { return false, nil } - if err := h.exec.ctx.checkNamedWrite(scope, h.describe(), name, &value); err != nil { + if err := h.exec.ctx.checkNamedWriteAs(scope, h.describe, name, &value); err != nil { return true, err } h.exec.ctx.writeBodyValue(cells, data, name, value) @@ -512,7 +512,7 @@ func (h *stateStmtHost) runFlow(block lower.Block) (stmtFlow, error) { // the body's flow, else what is around it. func (h *stateStmtHost) setFeature(name string, value Value) error { if root := h.perfs.root; root.holds(name) { - if err := h.exec.ctx.checkNamedWrite(root.scope, h.describe(), name, &value); err != nil { + if err := h.exec.ctx.checkNamedWriteAs(root.scope, h.describe, name, &value); err != nil { return err } h.exec.ctx.writeBodyValue(root.cells, root.data, root.key(name), value) diff --git a/internal/exec/runtime/statements.go b/internal/exec/runtime/statements.go index 78fff53a07..1f1d515993 100644 --- a/internal/exec/runtime/statements.go +++ b/internal/exec/runtime/statements.go @@ -833,7 +833,7 @@ func (e *stmtEngine) execute(stmt lower.Statement) (stmtFlow, error) { if s.Target == "" { return flowNext, fmt.Errorf("%s: unsupported assignment target", e.host.describe()) } - if err := e.ctx.checkAssignable(e.host.describe(), s); err != nil { + if err := e.ctx.checkAssignable(e.host.describe, s); err != nil { return flowNext, err } value, err := e.evalIn(s.Scope).Eval(s.Value) @@ -895,7 +895,7 @@ func (e *stmtEngine) execute(stmt lower.Statement) (stmtFlow, error) { cells = e.ensureRootCells() } check := func(value *Value) error { - return e.ctx.checkBodyDeclaration(s.Scope, e.host.describe(), name, value) + return e.ctx.checkBodyDeclarationAs(s.Scope, e.host.describe, name, value) } bindingContext := e.evalIn(s.Scope) visible := make([]map[string]bool, len(bindingContext.frames)) @@ -930,7 +930,7 @@ func (e *stmtEngine) execute(stmt lower.Statement) (stmtFlow, error) { if err != nil { return flowNext, fmt.Errorf("eval declaration %s: %w", s.Name, err) } - if err := e.ctx.checkBodyDeclaration(s.Scope, e.host.describe(), s.Name, &evaluated); err != nil { + if err := e.ctx.checkBodyDeclarationAs(s.Scope, e.host.describe, s.Name, &evaluated); err != nil { return flowNext, err } value = evaluated @@ -945,7 +945,7 @@ func (e *stmtEngine) execute(stmt lower.Statement) (stmtFlow, error) { if err != nil { return flowNext, fmt.Errorf("eval declaration %s: %w", s.Name, err) } - if err := e.ctx.checkBodyDeclaration(s.Scope, e.host.describe(), s.Name, &evaluated); err != nil { + if err := e.ctx.checkBodyDeclarationAs(s.Scope, e.host.describe, s.Name, &evaluated); err != nil { return flowNext, err } value = evaluated @@ -958,7 +958,7 @@ func (e *stmtEngine) execute(stmt lower.Statement) (stmtFlow, error) { if err != nil { return flowNext, fmt.Errorf("eval declaration %s: %w", s.Name, err) } - if err := e.ctx.checkBodyDeclaration(s.Scope, e.host.describe(), s.Name, &evaluated); err != nil { + if err := e.ctx.checkBodyDeclarationAs(s.Scope, e.host.describe, s.Name, &evaluated); err != nil { return flowNext, err } e.env.declare(e.ctx, s.Name, evaluated) diff --git a/internal/exec/runtime/succession_cycle_memo_test.go b/internal/exec/runtime/succession_cycle_memo_test.go new file mode 100644 index 0000000000..81c2f7bb5f --- /dev/null +++ b/internal/exec/runtime/succession_cycle_memo_test.go @@ -0,0 +1,86 @@ +package runtime + +import ( + "errors" + "testing" +) + +// TestSuccessionCycleMemo: the none-held memo answers the cycle check only while +// no behavior has entered held; one entering, by instantiation or by a snapshot +// restore, sends the check back to the walk, whose verdict it must then match. +func TestSuccessionCycleMemo(t *testing.T) { + src := `package test { + private import ScalarValues::*; + part def Plain { + attribute n : Integer = 0; + perform action step { assign n := 1; } + } + part def Ring { + perform action a { action stepA; } + perform action b { action stepB; } + first a then b; + first b then a; + } + }` + idx, _, ctx := buildRuntimeWithLibraries(t, "", parseAndBuild(t, src)) + if _, err := ctx.Instantiate(oneSymbol(t, idx, "test::Plain")); err != nil { + t.Fatalf("instantiate Plain: %v", err) + } + requireSameCycleVerdict(t, ctx) + if !ctx.noneHeld.answers(ctx) { + t.Fatal("cycle check over no held behavior left no memo") + } + + _, cached := ctx.Instantiate(oneSymbol(t, idx, "test::Ring")) + requireSuccessionError(t, cached, ErrSuccessionOrderCycle, successionOrderCycleCode) + if ctx.noneHeld.answers(ctx) { + t.Fatal("memo still answers after behaviors entered held") + } + requireSameCycleVerdict(t, ctx) + + // The same objects in a context whose check never answered from the memo. + freshIdx, _, fresh := buildRuntimeWithLibraries(t, "", parseAndBuild(t, src)) + if _, err := fresh.Instantiate(oneSymbol(t, freshIdx, "test::Plain")); err != nil { + t.Fatalf("instantiate Plain afresh: %v", err) + } + fresh.noneHeld = noneHeldMemo{} + _, uncached := fresh.Instantiate(oneSymbol(t, freshIdx, "test::Ring")) + if uncached == nil || uncached.Error() != cached.Error() { + t.Fatalf("cycle after memo = %v, want the uncached %v", cached, uncached) + } + + timed, inst := instantiateTimedSuccession(t) + snapshot, err := timed.Snapshot() + if err != nil { + t.Fatalf("snapshot: %v", err) + } + if _, err := timed.Advance(1); err != nil { + t.Fatalf("advance clock: %v", err) + } + if behaviorNamed(t, inst, "later").deferred != nil { + t.Fatal("Advance did not release later") + } + requireSameCycleVerdict(t, timed) + if !timed.noneHeld.answers(timed) { + t.Fatal("cycle check over released behaviors left no memo") + } + snapshot.Restore() + if behaviorNamed(t, inst, "later").deferred == nil { + t.Fatal("restore did not return later to held") + } + if timed.noneHeld.answers(timed) { + t.Fatal("memo still answers after a restore returned a behavior to held") + } + requireSameCycleVerdict(t, timed) +} + +func requireSameCycleVerdict(t *testing.T, ctx *Context) { + t.Helper() + got, want := ctx.successionCycle(), ctx.walkSuccessionCycle() + if (got == nil) != (want == nil) || (got != nil && got.Error() != want.Error()) { + t.Fatalf("memoized cycle check = %v, want the walk's %v", got, want) + } + if !errors.Is(got, ErrSuccessionOrderCycle) && want != nil { + t.Fatalf("memoized cycle check lost the sentinel: %v", got) + } +} diff --git a/internal/exec/runtime/succession_order.go b/internal/exec/runtime/succession_order.go index 9da593cd91..a92df0571f 100644 --- a/internal/exec/runtime/succession_order.go +++ b/internal/exec/runtime/succession_order.go @@ -317,6 +317,7 @@ func (ctx *Context) deferredBehaviorFor(inst *Instance, decl classifierBehaviorD return nil, err } deferred := decl + ctx.behaviorHeld() return &ObjectBehavior{ Name: decl.behavior.Name, Kind: decl.behavior.Kind, @@ -584,7 +585,34 @@ func (ctx *Context) releaseDeferredBehavior(behavior *ObjectBehavior) error { return nil } +// noneHeldMemo remembers a cycle check that found no behavior held; it answers +// until a behavior enters held, the one move that can put a cycle in the graph. +type noneHeldMemo struct { + taken bool + held uint64 +} + +func (m noneHeldMemo) answers(ctx *Context) bool { return m.taken && m.held == ctx.held } + +// behaviorHeld records a behavior entering held, retiring the none-held memo. +func (ctx *Context) behaviorHeld() { ctx.held++ } + +// successionCycle is walkSuccessionCycle, answered from the memo while no behavior +// is held: the walk visits held behaviors only, so with none it finds no cycle. func (ctx *Context) successionCycle() error { + if ctx.noneHeld.answers(ctx) { + return nil + } + for _, behavior := range ctx.objectBehaviors { + if behavior.deferred != nil { + return ctx.walkSuccessionCycle() + } + } + ctx.noneHeld = noneHeldMemo{taken: true, held: ctx.held} + return nil +} + +func (ctx *Context) walkSuccessionCycle() error { var visit func(*ObjectBehavior) bool state := make(map[*ObjectBehavior]uint8) stack := make([]*ObjectBehavior, 0) diff --git a/internal/exec/runtime/write_conformance.go b/internal/exec/runtime/write_conformance.go index b05782d955..31d56b2703 100644 --- a/internal/exec/runtime/write_conformance.go +++ b/internal/exec/runtime/write_conformance.go @@ -102,14 +102,14 @@ func (ctx *Context) readOnlyRefusal(what func() string, name string, feature *sy // statement was written — before its value is evaluated. A chained target is // judged on the object its chain reaches (writeThroughChain), unless it ends in // a `featured by` feature no object holds a slot for (featuredChainTarget). -func (ctx *Context) checkAssignable(where string, s lower.Assign) error { +func (ctx *Context) checkAssignable(where func() string, s lower.Assign) error { if s.Chain != nil { if feature, ok := ctx.featuredChainTarget(s); ok { - return ctx.readOnlyRefusal(func() string { return fmt.Sprintf("%s: assignment to %s", where, s.Chain.Text) }, s.Chain.Text, feature) + return ctx.readOnlyRefusal(func() string { return fmt.Sprintf("%s: assignment to %s", where(), s.Chain.Text) }, s.Chain.Text, feature) } return nil } - what := func() string { return fmt.Sprintf("%s: assignment to %s", where, s.Target) } + what := func() string { return fmt.Sprintf("%s: assignment to %s", where(), s.Target) } if s.Qualified { if s.Feature == nil { return nil @@ -182,17 +182,23 @@ func (ctx *Context) writeCountRefusal(target *writeTarget, value *Value) string // block-local, a parameter, an output - against the declaration of the name it // writes, before that value is stored. func (ctx *Context) checkBodyWrite(host stmtHost, s lower.Assign, value *Value) error { - return ctx.checkNamedWrite(s.Scope, host.describe(), s.Target, value) + return ctx.checkNamedWriteAs(s.Scope, host.describe, s.Target, value) } // checkNamedWrite checks a write of a name resolved in scope, for a path that // stores the value itself rather than reaching Instance.SetFeatureValue. func (ctx *Context) checkNamedWrite(scope *symbols.Scope, where, name string, value *Value) error { + return ctx.checkNamedWriteAs(scope, func() string { return where }, name, value) +} + +// checkNamedWriteAs is checkNamedWrite naming where the write ran only when it is +// refused, so a conformant write on a hot path does not pay for the description. +func (ctx *Context) checkNamedWriteAs(scope *symbols.Scope, where func() string, name string, value *Value) error { target, ok := ctx.writeTargetIn(scope, name) if !ok { return nil } - what := func() string { return fmt.Sprintf("%s: assignment to %s", where, name) } + what := func() string { return fmt.Sprintf("%s: assignment to %s", where(), name) } return ctx.checkTargetAs(scope, what, target, value, admitWritten, true) } @@ -203,13 +209,19 @@ func (ctx *Context) checkNamedWrite(scope *symbols.Scope, where, name string, va // holds the count its initializer has and, as a namespace-level one, is judged for // uniqueness only where it declares more than one value. func (ctx *Context) checkBodyDeclaration(scope *symbols.Scope, where, name string, value *Value) error { + return ctx.checkBodyDeclarationAs(scope, func() string { return where }, name, value) +} + +// checkBodyDeclarationAs is checkBodyDeclaration naming where the declaration ran +// only when it is refused, so a conformant binding does not pay for the description. +func (ctx *Context) checkBodyDeclarationAs(scope *symbols.Scope, where func() string, name string, value *Value) error { target, ok := ctx.writeTargetIn(scope, name) if !ok { return nil } declared := *target declared.unique = target.unique && multiValued(target.mult) - what := func() string { return fmt.Sprintf("%s: declaration of %s", where, name) } + what := func() string { return fmt.Sprintf("%s: declaration of %s", where(), name) } return ctx.checkTargetAs(scope, what, &declared, value, admitDeclared, target.countJudged) } diff --git a/internal/frontend/grpc/cache.go b/internal/frontend/grpc/cache.go index de86a341ef..411a22ee71 100644 --- a/internal/frontend/grpc/cache.go +++ b/internal/frontend/grpc/cache.go @@ -8,6 +8,7 @@ import ( "connectrpc.com/connect" + pb "github.com/Open-MBEE/OpenSysML/api/proto" "github.com/Open-MBEE/OpenSysML/internal/check/passes" "github.com/Open-MBEE/OpenSysML/internal/exec/analysis" "github.com/Open-MBEE/OpenSysML/internal/exec/runtime" @@ -34,6 +35,35 @@ type CachedDocument struct { // and the passes' own findings when the model parsed clean and was analyzed. Diagnostics []diag.Diagnostic Warnings []string + // proto is Diagnostics and Warnings converted to protobuf, once per document; + // a lineage hands the same cell to the next model when neither changed. + proto *protoDiagnostics +} + +// protoDiagnostics is one document's diagnostics as protobuf, built once (capability +// filter applied) and shared read-only by every response that reports them. +type protoDiagnostics struct { + once sync.Once + diags []*pb.Diagnostic +} + +// newCachedDocument is a document whose diagnostics are converted at most once. +func newCachedDocument(root *ast.RootNamespace, sf *source.SourceFile, parseDiags []parser.Diagnostic, + diagnostics []diag.Diagnostic, warnings []string) *CachedDocument { + return &CachedDocument{ + Root: root, Source: sf, ParseDiags: parseDiags, Diagnostics: diagnostics, + Warnings: append([]string(nil), warnings...), proto: &protoDiagnostics{}, + } +} + +// protoDiagnostics is the document's diagnostics as build converts them, built on +// first use and shared read-only across responses: never mutate a returned message. +func (d *CachedDocument) protoDiagnostics(build func(*CachedDocument) []*pb.Diagnostic) []*pb.Diagnostic { + if d.proto == nil { + return build(d) + } + d.proto.once.Do(func() { d.proto.diags = build(d) }) + return d.proto.diags } // CachedModel holds parsed model data with semantic analysis results diff --git a/internal/frontend/grpc/diagnostics_converted_once_test.go b/internal/frontend/grpc/diagnostics_converted_once_test.go new file mode 100644 index 0000000000..3d117e6b32 --- /dev/null +++ b/internal/frontend/grpc/diagnostics_converted_once_test.go @@ -0,0 +1,223 @@ +package grpc + +import ( + "context" + "testing" + + pb "github.com/Open-MBEE/OpenSysML/api/proto" + "google.golang.org/protobuf/proto" +) + +// warningLibrary, warningTop and warningOther are a model whose top document +// parses clean with a warning, so a parse of it reports diagnostics. +const ( + warningLibrary = "package Lib {\n\tpart def Engine;\n}\n" + warningTop = "package Top {\n\tprivate import Lib::*;\n\tcalc def F { 1 /* c */ }\n\tpart def Car {\n\t\tpart motor : Engine;\n\t}\n}\n" + warningOther = "package Other {\n\tpart def Wheel;\n}\n" +) + +// requireWarnings fails unless diags hold a warning and nothing more severe. +func requireWarnings(t *testing.T, diags []*pb.Diagnostic) { + t.Helper() + if len(diags) == 0 { + t.Fatal("no diagnostics to compare") + } + for _, d := range diags { + if d.Severity != "warning" { + t.Fatalf("diagnostic %v is not a warning", d) + } + } +} + +// requireSameDiagnostics fails unless got repeats want message for message. +func requireSameDiagnostics(t *testing.T, got, want []*pb.Diagnostic) { + t.Helper() + if len(got) != len(want) { + t.Fatalf("%d diagnostics, want %d:\n%v\n%v", len(got), len(want), got, want) + } + for i := range got { + if !proto.Equal(got[i], want[i]) { + t.Errorf("diagnostic %d is %v, want %v", i, got[i], want[i]) + } + } +} + +// A parse of a model parsed before answers the diagnostics the first parse +// did, converted once: the messages are the same, so a service seeing the model +// for the first time, a repeated parse and GetDiagnostics all agree, and a +// service that withholds diagnostic codes withholds them on every call. +func TestParseFileConvertsACachedModelsDiagnosticsOnce(t *testing.T) { + ctx := context.Background() + req := &pb.ParseFileRequest{Source: &pb.ParseFileRequest_Content{Content: misplacedComment}} + fresh := mustNewService(t, 10) + defer fresh.Close() + want, err := fresh.ParseFile(ctx, req) + if err != nil { + t.Fatalf("ParseFile: %v", err) + } + requireWarnings(t, want.Diagnostics) + requireEveryCoded(t, want.Diagnostics) + + srv := mustNewService(t, 10) + defer srv.Close() + first, err := srv.ParseFile(ctx, req) + if err != nil { + t.Fatalf("ParseFile: %v", err) + } + second, err := srv.ParseFile(ctx, req) + if err != nil { + t.Fatalf("ParseFile again: %v", err) + } + if !proto.Equal(first, want) { + t.Errorf("first parse is\n%v\nfresh parse is\n%v", first, want) + } + if !proto.Equal(second, want) { + t.Errorf("repeated parse is\n%v\nfresh parse is\n%v", second, want) + } + for i := range first.Diagnostics { + if first.Diagnostics[i] != second.Diagnostics[i] { + t.Errorf("diagnostic %d was converted again for the repeated parse", i) + } + } + // The list is the response's own: growing one leaves the other as it was. + second.Diagnostics = append(second.Diagnostics, &pb.Diagnostic{Severity: "error"}) + diags, err := srv.GetDiagnostics(ctx, &pb.DiagnosticsRequest{ModelHash: first.ModelHash}) + if err != nil { + t.Fatalf("GetDiagnostics: %v", err) + } + requireSameDiagnostics(t, diags.Diagnostics, want.Diagnostics) + requireSameDiagnostics(t, first.Diagnostics, want.Diagnostics) + + withheld, err := NewServiceWithUnavailableCapabilitiesForTesting(10, "test", []string{CapabilityDiagnosticCodes}) + if err != nil { + t.Fatal(err) + } + defer withheld.Close() + for call := 0; call < 2; call++ { + resp, err := withheld.ParseFile(ctx, req) + if err != nil { + t.Fatalf("ParseFile: %v", err) + } + if len(resp.Diagnostics) != len(want.Diagnostics) { + t.Fatalf("withheld service reported %d diagnostics, want %d", len(resp.Diagnostics), len(want.Diagnostics)) + } + for i, d := range resp.Diagnostics { + uncoded := proto.Clone(want.Diagnostics[i]).(*pb.Diagnostic) + uncoded.Code = "" + if !proto.Equal(d, uncoded) { + t.Errorf("call %d: withheld diagnostic = %v, want %v", call, d, uncoded) + } + } + } +} + +// documentDiagnosticsOf are the diagnostics of resp located in the document named. +func documentDiagnosticsOf(resp *pb.ParseSourcesResponse, name string) []*pb.Diagnostic { + var out []*pb.Diagnostic + for _, d := range resp.Diagnostics { + if d.GetSpan().GetFile() == name { + out = append(out, d) + } + } + return out +} + +// A model answered from its lineage shares the converted diagnostics of every +// document the edit before it did not reach, and converts afresh those of the +// documents it did; either way it reports what a fresh parse of the same +// documents reports. +func TestParseSourcesSharesConvertedDiagnosticsOfDocumentsAnEditDidNotReach(t *testing.T) { + ctx := context.Background() + srv := mustNewService(t, 32) + defer srv.Close() + parse := func(srv *Service, documents []*pb.SourceDocument) *pb.ParseSourcesResponse { + t.Helper() + resp, err := srv.ParseSources(ctx, &pb.ParseSourcesRequest{Documents: documents}) + if err != nil { + t.Fatalf("ParseSources: %v", err) + } + for _, d := range resp.Diagnostics { + if d.Severity != "warning" { + t.Fatalf("diagnostic %v is not a warning", d) + } + } + return resp + } + documents := func(other string) []*pb.SourceDocument { + return inlineDocuments("lib.sysml", warningLibrary, "top.sysml", warningTop, "other.sysml", other) + } + // Parsed twice so the edits are answered from the set's lineage. + parse(srv, documents(warningOther+"// first\n")) + base := parse(srv, documents(warningOther)) + requireWarnings(t, documentDiagnosticsOf(base, "top.sysml")) + + untouched := parse(srv, documents(warningOther+"// a comment\n")) + requireSameDiagnostics(t, untouched.Diagnostics, base.Diagnostics) + before, after := documentDiagnosticsOf(base, "top.sysml"), documentDiagnosticsOf(untouched, "top.sysml") + for i := range before { + if before[i] != after[i] { + t.Errorf("top.sysml diagnostic %d was converted again though the edit did not reach the document", i) + } + } + + reached := inlineDocuments("lib.sysml", warningLibrary, "top.sysml", warningTop+"// edited\n", "other.sysml", warningOther) + edited := parse(srv, reached) + fresh := mustNewService(t, 10) + defer fresh.Close() + requireSameDiagnostics(t, edited.Diagnostics, parse(fresh, reached).Diagnostics) + for _, d := range documentDiagnosticsOf(edited, "top.sysml") { + for _, was := range before { + if d == was { + t.Errorf("top.sysml diagnostic %v is the base model's though the edit reached the document", d) + } + } + } +} + +// freshlyConverted converts the cached model's diagnostics again, past the cache. +func freshlyConverted(t *testing.T, srv *Service, hash string) []*pb.Diagnostic { + t.Helper() + cached, ok := srv.cache.Get(hash) + if !ok { + t.Fatalf("model %s is not cached", hash) + } + var fresh []*pb.Diagnostic + for _, doc := range cached.Documents { + fresh = append(fresh, srv.documentDiagnostics(doc)...) + } + return fresh +} + +// The shared messages are never written after they are built: every later +// response still equals a conversion made afresh from the cached model. +func TestCachedDiagnosticsStayEqualToAFreshConversion(t *testing.T) { + ctx := context.Background() + req := &pb.ParseFileRequest{Source: &pb.ParseFileRequest_Content{Content: misplacedComment}} + withheld, err := NewServiceWithUnavailableCapabilitiesForTesting(10, "test", []string{CapabilityDiagnosticCodes}) + if err != nil { + t.Fatal(err) + } + defer withheld.Close() + for name, srv := range map[string]*Service{"coded": mustNewService(t, 10), "codes withheld": withheld} { + t.Run(name, func(t *testing.T) { + defer srv.Close() + first, err := srv.ParseFile(ctx, req) + if err != nil { + t.Fatalf("ParseFile: %v", err) + } + requireWarnings(t, first.Diagnostics) + second, err := srv.ParseFile(ctx, req) + if err != nil { + t.Fatalf("ParseFile again: %v", err) + } + diags, err := srv.GetDiagnostics(ctx, &pb.DiagnosticsRequest{ModelHash: first.ModelHash}) + if err != nil { + t.Fatalf("GetDiagnostics: %v", err) + } + fresh := freshlyConverted(t, srv, first.ModelHash) + requireSameDiagnostics(t, first.Diagnostics, fresh) + requireSameDiagnostics(t, second.Diagnostics, fresh) + requireSameDiagnostics(t, diags.Diagnostics, fresh) + }) + } +} diff --git a/internal/frontend/grpc/lineage.go b/internal/frontend/grpc/lineage.go index ca7caf48e6..ed98edefe7 100644 --- a/internal/frontend/grpc/lineage.go +++ b/internal/frontend/grpc/lineage.go @@ -2,6 +2,7 @@ package grpc import ( "container/list" + "slices" "sort" "strings" "sync" @@ -42,6 +43,15 @@ type lineage struct { library libs.Source // the files the workspace's library was built from held map[string]string // document name to the content the workspace holds versions int // the version the next update is given + // answered is each document of the model last answered, with the stamp of its + // analysis; the next model shares its converted diagnostics when both are unchanged. + answered map[string]answeredDocument +} + +// answeredDocument is one document of the model a lineage last answered. +type answeredDocument struct { + doc *CachedDocument + stamp uint64 } // lineages holds the most recently used lineages, a few at most: a lineage @@ -171,7 +181,7 @@ func (s *Service) parseFromLineage(inputs []sourceInput, mode diag.ConformanceMo } } if len(standIns) > 0 { - l.fresh, l.standIns, l.ws, l.held = true, standIns, nil, nil + l.fresh, l.standIns, l.ws, l.held, l.answered = true, standIns, nil, nil, nil return nil, false } names := make([]string, len(inputs)) @@ -199,24 +209,37 @@ func (s *Service) parseFromLineage(inputs []sourceInput, mode diag.ConformanceMo documents := make([]*CachedDocument, len(inputs)) for i, input := range inputs { doc := l.ws.Document(input.name) - documents[i] = &CachedDocument{ - Root: doc.AST, - Source: detached.Source(input.name), - ParseDiags: doc.ParseDiagnostics, - Diagnostics: diagnostics[i], - Warnings: append([]string(nil), input.warnings...), - } + documents[i] = newCachedDocument(doc.AST, detached.Source(input.name), doc.ParseDiagnostics, + diagnostics[i], input.warnings) } stamps := make(map[string]uint64, len(names)) for _, name := range names { stamps[name] = l.ws.AnalysisStamp(name) } + l.shareConvertedDiagnostics(documents, stamps) return &CachedModel{ Documents: documents, Index: detached.Index(), Library: l.library, Mode: mode, analysis: &analysisSnapshot{workspace: l.ws, stamps: stamps, scoped: declaresIdentityScope(inputs)}, }, true } +// shareConvertedDiagnostics hands a document the converted diagnostics answered under +// its name before when stamp, source and warnings are unchanged, i.e. no edit reached it. +func (l *lineage) shareConvertedDiagnostics(documents []*CachedDocument, stamps map[string]uint64) { + if l.answered == nil { + l.answered = make(map[string]answeredDocument, len(documents)) + } + for _, doc := range documents { + name := doc.Source.Name() + stamp := stamps[name] + if last, ok := l.answered[name]; ok && stamp != 0 && last.stamp == stamp && + last.doc.Source == doc.Source && slices.Equal(last.doc.Warnings, doc.Warnings) { + doc.proto = last.doc.proto + } + l.answered[name] = answeredDocument{doc: doc, stamp: stamp} + } +} + // analysisSnapshot is which analysis of each document a model was answered // with, as the lineage's workspace numbers them (model.Workspace.AnalysisStamp). // A stamp changes exactly when the document is analyzed afresh, which happens diff --git a/internal/frontend/grpc/service.go b/internal/frontend/grpc/service.go index 2dda0747ce..d9d8ac2ed3 100644 --- a/internal/frontend/grpc/service.go +++ b/internal/frontend/grpc/service.go @@ -873,13 +873,8 @@ func (s *Service) parseModel(inputs []sourceInput, mode diag.ConformanceMode, in if len(p.Diagnostics) > 0 { parsedClean = false } - documents = append(documents, &CachedDocument{ - Root: root, - Source: srcFile, - ParseDiags: p.Diagnostics, - Diagnostics: parser.AsDiagnostics(p.Diagnostics, p.Warnings), - Warnings: append([]string(nil), input.warnings...), - }) + documents = append(documents, newCachedDocument(root, srcFile, p.Diagnostics, + parser.AsDiagnostics(p.Diagnostics, p.Warnings), input.warnings)) } // Registers what a wildcard import re-exports, so a qualified name reaches a @@ -944,22 +939,32 @@ func (s *Service) GetDiagnostics(ctx context.Context, req *pb.DiagnosticsRequest }, nil } -// modelDiagnostics are every document's diagnostics, document by document, each -// located in the source it came from. The parse's are among them once: the -// passes report them, escalated where a pass judged the notation. +// modelDiagnostics are every document's diagnostics in document order, each in its own +// source, the parse's among them once as the passes report them; the list is the response's own. func (s *Service) modelDiagnostics(model *CachedModel) []*pb.Diagnostic { var pbDiags []*pb.Diagnostic for _, doc := range model.Documents { - for _, diag := range doc.Diagnostics { - pbDiags = append(pbDiags, DiagnosticToProto(diag, doc.Source)) - } - for _, warning := range doc.Warnings { - pbDiags = append(pbDiags, &pb.Diagnostic{ - Severity: diag.SeverityWarning.String(), - Message: warning, - Span: &pb.Span{File: doc.Source.Name()}, - }) - } + pbDiags = append(pbDiags, doc.protoDiagnostics(s.documentDiagnostics)...) + } + return pbDiags +} + +// documentDiagnostics converts one document's diagnostics, then its warnings, to +// protobuf, filtered by the service's capabilities, which are fixed when it is built. +func (s *Service) documentDiagnostics(doc *CachedDocument) []*pb.Diagnostic { + if len(doc.Diagnostics)+len(doc.Warnings) == 0 { + return nil + } + pbDiags := make([]*pb.Diagnostic, 0, len(doc.Diagnostics)+len(doc.Warnings)) + for _, diag := range doc.Diagnostics { + pbDiags = append(pbDiags, DiagnosticToProto(diag, doc.Source)) + } + for _, warning := range doc.Warnings { + pbDiags = append(pbDiags, &pb.Diagnostic{ + Severity: diag.SeverityWarning.String(), + Message: warning, + Span: &pb.Span{File: doc.Source.Name()}, + }) } return s.filterDiagnosticCapabilities(pbDiags) } diff --git a/internal/semantic/resolve/document.go b/internal/semantic/resolve/document.go index cc8db81ada..230350b665 100644 --- a/internal/semantic/resolve/document.go +++ b/internal/semantic/resolve/document.go @@ -1918,7 +1918,7 @@ func (r *Resolver) IsBaseThat(sym *symbols.Symbol) bool { if sym == nil { return false } - return sym.Name == baseThatFQN || r.registeredFQN(sym) == baseThatFQN + return sym.Name == baseThatFQN || (r.idx != nil && symbols.HasFQN(sym, baseThatFQN)) } // getUsageType returns the type symbol of a usage by resolving its typing relationship. diff --git a/internal/semantic/resolve/frames_test.go b/internal/semantic/resolve/frames_test.go index f9b6a6b171..6acc0d34ca 100644 --- a/internal/semantic/resolve/frames_test.go +++ b/internal/semantic/resolve/frames_test.go @@ -224,3 +224,37 @@ func TestFramesReadingTheWholeIndexSurviveAJudgmentChange(t *testing.T) { t.Fatalf("the refreshed table still declares X as %v", got) } } + +// A document replaced by one stating the same wildcard import registers the +// names the import surfaces again, as they were; a document that resolved +// through one keeps what it memoized. Stated otherwise, the import moves them. +func TestFramesSurviveAReexportRegisteredAgainAsItWas(t *testing.T) { + w := newTrackedIndex(t, map[string]string{ + "lib.sysml": "package Lib { part def Base; }", + "gate.sysml": "package P { public import Lib::*; }", + "user.sysml": "package P { part def Derived :> P::Base; }", + }) + w.idx.ExpandWildcardImports() + w.idx.TakeChanges() + w.analyzeAll() + // As the workspace does: the imports are expanded again before the changes + // are read. + edit := func(src string) []string { + w.roots["gate.sysml"] = parsedRoot(t, "gate.sysml", src) + w.idx.AddDocument("gate.sysml", w.roots["gate.sysml"]) + w.idx.ExpandWildcardImports() + ch := w.idx.TakeChanges() + ch.Docs = map[string]bool{"gate.sysml": true} + return w.r.Invalidate(ch) + } + if dropped := edit("package P { public import Lib::*; } // edited"); !reflect.DeepEqual(dropped, []string{"gate.sysml"}) { + t.Fatalf("dropped %v, want [gate.sysml]", dropped) + } + if !w.owned("user.sysml") { + t.Fatal("user.sysml, resolving through a re-export registered again as it was, lost what it memoized") + } + w.analyzeAll() + if dropped := edit("package P { private import Lib::*; }"); !reflect.DeepEqual(dropped, []string{"gate.sysml", "user.sysml"}) { + t.Fatalf("dropped %v, want [gate.sysml user.sysml]", dropped) + } +} diff --git a/internal/semantic/resolve/this_context.go b/internal/semantic/resolve/this_context.go index e77382a203..e04b387d6d 100644 --- a/internal/semantic/resolve/this_context.go +++ b/internal/semantic/resolve/this_context.go @@ -19,7 +19,12 @@ func (r *Resolver) IsOccurrenceThis(sym *symbols.Symbol) bool { if sym == nil { return false } - return sym.Name == thisFeatureName || strings.HasSuffix(sym.Name, thisFeatureSuffix) || + if sym.Name == thisFeatureName || strings.HasSuffix(sym.Name, thisFeatureSuffix) { + return true + } + // Only a name that is itself a tail of `::this` can qualify to that suffix, + // so the qualified name is built for those alone. + return strings.HasSuffix(thisFeatureSuffix, sym.Name) && strings.HasSuffix(r.registeredFQN(sym), thisFeatureSuffix) } diff --git a/internal/semantic/semantics/eval.go b/internal/semantic/semantics/eval.go index 6331d0eee3..381a3f0ecc 100644 --- a/internal/semantic/semantics/eval.go +++ b/internal/semantic/semantics/eval.go @@ -172,6 +172,45 @@ type Value struct { ext *big.Rat } +// Packed is an Integer, Rational, Real or Boolean Value in three words, for a +// holder that keeps or passes many; Unpack restores the Value it was packed from. +type Packed struct { + Kind ValueKind + Den uint32 + Bits uint64 + Ext *big.Rat +} + +// Pack is v as a Packed. +func (v Value) Pack() Packed { + p := Packed{Kind: v.Kind, Den: v.den, Ext: v.ext} + switch v.Kind { + case ValReal: + p.Bits = math.Float64bits(v.Real) + case ValBool: + if v.Bool { + p.Bits = 1 + } + default: + p.Bits = uint64(v.Int) // #nosec G115 -- the Integer's two's-complement bits are stored, not its magnitude. + } + return p +} + +// Unpack is the Value p packs. +func (p Packed) Unpack() Value { + v := Value{Kind: p.Kind, den: p.Den, ext: p.Ext} + switch p.Kind { + case ValReal: + v.Real = math.Float64frombits(p.Bits) + case ValBool: + v.Bool = p.Bits != 0 + default: + v.Int = int64(p.Bits) // #nosec G115 -- the inverse of Pack: the same bits read back as an Integer. + } + return v +} + // IsNumeric reports whether the value is an Integer, a Rational or a Real. func (v Value) IsNumeric() bool { return v.Kind == ValInt || v.Kind == ValReal || v.Kind == ValRational diff --git a/internal/semantic/semantics/packed_test.go b/internal/semantic/semantics/packed_test.go new file mode 100644 index 0000000000..c275bf6b23 --- /dev/null +++ b/internal/semantic/semantics/packed_test.go @@ -0,0 +1,48 @@ +package semantics + +import ( + "math" + "math/big" + "testing" +) + +// Every Value a Packed can hold unpacks to the Value it was packed from, field +// for field, the big form sharing its immutable big.Rat. +func TestPackedRoundTrip(t *testing.T) { + bigRat, _ := new(big.Rat).SetString("12345678901234567890123/7") + values := []Value{ + {}, + {Kind: ValInfinity}, + IntValue(0), IntValue(1), IntValue(-1), IntValue(math.MaxInt64), IntValue(math.MinInt64), + bigVal("1180591620717411303424"), bigVal("-1180591620717411303424"), + mustFrac(t, 1, 3), mustFrac(t, -7, 2), mustFrac(t, 6, 3), mustFrac(t, math.MaxInt64, math.MaxUint32), + RatValue(bigRat), + {Kind: ValReal, Real: 0}, {Kind: ValReal, Real: math.Copysign(0, -1)}, {Kind: ValReal, Real: 1.5}, + {Kind: ValReal, Real: math.Inf(1)}, {Kind: ValReal, Real: math.Inf(-1)}, {Kind: ValReal, Real: math.NaN()}, + {Kind: ValBool, Bool: true}, {Kind: ValBool, Bool: false}, + } + for _, v := range values { + got := v.Pack().Unpack() + if v.Kind == ValReal && math.IsNaN(v.Real) { + if got.Kind != ValReal || !math.IsNaN(got.Real) || math.Float64bits(got.Real) != math.Float64bits(v.Real) { + t.Errorf("NaN round-trips to %+v", got) + } + continue + } + if got != v { + t.Errorf("%+v round-trips to %+v", v, got) + } + if got.ext != v.ext { + t.Errorf("%+v round-trips to another big.Rat", v) + } + } +} + +func mustFrac(t *testing.T, num, den int64) Value { + t.Helper() + v, ok := FracValue(num, den) + if !ok { + t.Fatalf("FracValue(%d, %d) declined", num, den) + } + return v +} diff --git a/internal/semantic/semantics/state_activity.go b/internal/semantic/semantics/state_activity.go index cdd15e631b..8507ec3126 100644 --- a/internal/semantic/semantics/state_activity.go +++ b/internal/semantic/semantics/state_activity.go @@ -14,7 +14,7 @@ const StateActivityFQN = StateActivityPackage + "::isActive" // IsStateActivity reports whether sym is the StateActivity::isActive feature. func IsStateActivity(sym *symbols.Symbol) bool { - return sym != nil && symbols.FQNOf(sym) == StateActivityFQN + return sym != nil && symbols.HasFQN(sym, StateActivityFQN) } // FeaturingTypes are the types sym declares itself `featured by` (KerML diff --git a/internal/semantic/symbols/changes.go b/internal/semantic/symbols/changes.go index 0acecfa54a..f725d3157f 100644 --- a/internal/semantic/symbols/changes.go +++ b/internal/semantic/symbols/changes.go @@ -80,13 +80,15 @@ func (idx *Index) noteBefore(fqn string) { // tables: the symbols registered under it, in order, each with its re-export // and hidden marks and the claims and routes that surfaced it. It holds the // symbols themselves, so one dropped since cannot have its address reused by -// a new symbol that would then compare equal to it. +// a new symbol that would then compare equal to it. The claims are the index's +// own maps, not copies: a write replaces a claims map rather than changing it +// (see Index.reexportDocs), so the one noted stays as it was read. type registration []registeredSymbol type registeredSymbol struct { sym *Symbol reexported, hidden bool - claims map[string]reexportClaim + claims map[string]*reexportClaim } func registrationOf(idx *Index, fqn string) registration { @@ -98,12 +100,11 @@ func registrationOf(idx *Index, fqn string) registration { hidden, _ := idx.hidden.get(fqn) out := make(registration, len(syms)) for i, sym := range syms { - out[i] = registeredSymbol{sym: sym, reexported: reexported.has(sym), hidden: hidden.has(sym)} - if claims := idx.reexportDocs.at(reexportKey{fqn: fqn, sym: sym}); len(claims) > 0 { - out[i].claims = make(map[string]reexportClaim, len(claims)) - for doc, claim := range claims { - out[i].claims[doc] = reexportClaim{public: claim.public, routes: append([]gateRoute(nil), claim.routes...)} - } + out[i] = registeredSymbol{ + sym: sym, + reexported: reexported.has(sym), + hidden: hidden.has(sym), + claims: idx.reexportDocs.at(reexportKey{fqn: fqn, sym: sym}), } } return out diff --git a/internal/semantic/symbols/changes_test.go b/internal/semantic/symbols/changes_test.go index 2f3795fe75..6dd02204dc 100644 --- a/internal/semantic/symbols/changes_test.go +++ b/internal/semantic/symbols/changes_test.go @@ -145,3 +145,68 @@ func TestTakeChangesLeavesOutWhatIsRegisteredAgainAsItWas(t *testing.T) { t.Errorf("a library parsed again leaves its declarations and their re-exports unchanged: %v", keys(ch.Names)) } } + +// A re-export two documents surface outlives the replacement of one of them +// under the other's claim. The replacement's claim is compared with the one it +// held before: the same import is no change, a differently stated one is. +func TestTakeChangesComparesAReplacedClaimBesideTheOnesThatStayed(t *testing.T) { + idx := buildIndex(t, map[string]string{ + "base.sysml": "package L { metadata def M; part def A; }", + "a.sysml": "package P { public import L::*; }", + "b.sysml": "package P { public import L::*; }", + }) + idx.TrackChanges() + replace := func(src string) Changes { + addDoc(t, idx, "b.sysml", src) + idx.ExpandWildcardImports() + return idx.TakeChanges() + } + if ch := replace("package P { public import L::*; } // edited"); ch.Names["P::A"] { + t.Errorf("a re-export claimed again as it was is a change: %v", keys(ch.Names)) + } + // Claimed privately beside a public claim, the name stays exported; the + // claims alone read differently. + if ch := replace("package P { private import L::*; }"); !ch.Names["P::A"] { + t.Errorf("a re-export claimed privately where it was public is not a change: %v", keys(ch.Names)) + } + if ch := replace("package P { private import L::*; } // edited"); ch.Names["P::A"] { + t.Errorf("a re-export claimed privately again is a change: %v", keys(ch.Names)) + } + // A second import widens the claim's routes after the first recorded them. + if ch := replace("package P { private import L::*; public import L::*; }"); !ch.Names["P::A"] { + t.Errorf("a re-export whose routes widened is not a change: %v", keys(ch.Names)) + } + if ch := replace("package P { private import L::*; public import L::*; } // edited"); ch.Names["P::A"] { + t.Errorf("a re-export whose routes were recorded again as they were is a change: %v", keys(ch.Names)) + } + // A filter's condition is the expression declaring it, so one parsed again + // is another condition and the route it gates another route. + if ch := replace("package P { private import L::*[@M]; }"); !ch.Names["P::A"] { + t.Errorf("a re-export gated where it was not is not a change: %v", keys(ch.Names)) + } + if ch := replace("package P { private import L::*[@M]; } // edited"); !ch.Names["P::A"] { + t.Errorf("a re-export gated by a condition parsed again is not a change: %v", keys(ch.Names)) + } +} + +// What a registration noted before a write reads is what the index held then, +// whatever the writes since did to the claims under the same key. +func TestNotedRegistrationIsUnmovedByLaterClaims(t *testing.T) { + idx := buildIndex(t, map[string]string{ + "base.sysml": "package L { part def A; }", + "a.sysml": "package P { public import L::*; }", + "b.sysml": "package P { private import L::*; }", + }) + idx.TrackChanges() + before := registrationOf(idx, "P::A") + addDoc(t, idx, "b.sysml", "package P { private import L::*; public import L::*; }") + idx.ExpandWildcardImports() + if before.same(registrationOf(idx, "P::A")) { + t.Fatal("the registration noted before b.sysml widened its claim reads as the one after") + } + addDoc(t, idx, "b.sysml", "package P { private import L::*; }") + idx.ExpandWildcardImports() + if !before.same(registrationOf(idx, "P::A")) { + t.Fatal("b.sysml claiming as it first did does not read as the registration noted then") + } +} diff --git a/internal/semantic/symbols/index.go b/internal/semantic/symbols/index.go index b855d9a4c7..0c3539dc41 100644 --- a/internal/semantic/symbols/index.go +++ b/internal/semantic/symbols/index.go @@ -81,6 +81,10 @@ type Index struct { // This is deliberately not folded into contributions: that is an append-only // slice per document, and a re-export has to be found by (FQN, symbol) to // drop one document's claim, or purged across all documents at once. + // + // A claims map and the claims in it are never written once installed: a + // write installs a new map, so a registration noted before it (noteBefore) + // keeps reading what it read without copying. reexportDocs *layer[reexportKey, map[string]*reexportClaim] docReexports *layer[string, map[reexportKey]bool] @@ -1221,14 +1225,10 @@ func filtersSubsume(a, b []ElementFilter) bool { // a name is a member of the importing namespace when any one of them admits it, // so an unfiltered import re-exports it whatever another route filters out. func (idx *Index) reexportGated(fqn string, sym *Symbol, doc string, private bool, gates [][]ElementFilter) { - claim := idx.reexport(fqn, sym, doc, private) - if claim == nil { + if !idx.reexport(fqn, sym) { return // the namespace declares it; nothing was borrowed } - widened := false - for _, gate := range gates { - widened = claim.record(gateRoute{private: private, filters: gate}) || widened - } + widened := idx.claimReexport(reexportKey{fqn: fqn, sym: sym}, doc, !private, gates) if widened { idx.changedName(fqn) } @@ -1244,14 +1244,15 @@ func (idx *Index) reexportGated(fqn string, sym *Symbol, doc string, private boo // beside it redundant, and re-expanding an importer records nothing new. Keeping // only the routes no other subsumes also bounds the set, which is what lets a // cycle of filtered imports settle. It reports whether the claim now admits more -// than it did. +// than it did. The routes are replaced, not written in place: a claim installed +// in the index may share them (see claimReexport). func (c *reexportClaim) record(route gateRoute) bool { for _, have := range c.routes { if have.private == route.private && filtersSubsume(have.filters, route.filters) { return false } } - kept := c.routes[:0] + kept := make([]gateRoute, 0, len(c.routes)+1) for _, have := range c.routes { if have.private != route.private || !filtersSubsume(route.filters, have.filters) { kept = append(kept, have) @@ -1333,58 +1334,80 @@ func nonZeroFilters(filters []ElementFilter) []ElementFilter { return out } -// reexport registers sym under fqn on doc's behalf and returns doc's writable -// claim on it, or nil when the namespace declares sym there itself. -func (idx *Index) reexport(fqn string, sym *Symbol, doc string, private bool) *reexportClaim { +// reexport registers sym under fqn as a re-export and reports whether it is +// one, which it is not when the namespace declares sym there itself. +func (idx *Index) reexport(fqn string, sym *Symbol) bool { if idx.hasFQN(fqn, sym) { - if !idx.reexported.at(fqn).has(sym) { - return nil // declared here, not borrowed - } - } else { - idx.link(fqn, sym) // claimReexport notes the gain + return idx.reexported.at(fqn).has(sym) // else declared here, not borrowed } - return idx.claimReexport(reexportKey{fqn: fqn, sym: sym}, doc, !private) + idx.link(fqn, sym) // claimReexport notes the gain + return true } // claimReexport records that doc's wildcard import surfaces the re-export key, -// publicly or not, and updates the marks a lookup reads. A name is exported when -// any import that surfaced it was public, so a public claim clears the hidden -// mark a private one left. It returns doc's writable claim. -func (idx *Index) claimReexport(key reexportKey, doc string, public bool) *reexportClaim { - docs := idx.writableClaims(key) - claim, claimed := docs[doc] - if claimed && (claim.public || !public) { - return claim // nothing new - } - if !claimed { - claim = &reexportClaim{} - docs[doc] = claim +// publicly or not and by the given routes, and updates the marks a lookup reads. +// A name is exported when any import that surfaced it was public, so a public +// claim clears the hidden mark a private one left. It reports whether the routes +// now admit more than they did. +// +// The claims on key are replaced rather than written: the map and the claims in +// it may be what a registration noted before this write reads (noteBefore). +func (idx *Index) claimReexport(key reexportKey, doc string, public bool, gates [][]ElementFilter) bool { + idx.noteBefore(key.fqn) + docs := idx.reexportDocs.at(key) + have, claimed := docs[doc] + var claim reexportClaim + if claimed { + claim = *have } + upgraded := !claimed || (public && !claim.public) claim.public = claim.public || public - idx.changedName(key.fqn) - writableMap(idx.docReexports, doc)[key] = true - idx.applyReexportMarks(key, docs) - parent, _ := splitFQN(key.fqn) - idx.markGained(parent) // a public claim can un-hide it, which exports it onward - return claim + widened := false + for _, gate := range gates { + widened = claim.record(gateRoute{private: !public, filters: gate}) || widened + } + if !upgraded && !widened { + return false // nothing new + } + next := make(map[string]*reexportClaim, len(docs)+1) + for other, c := range docs { + next[other] = c + } + next[doc] = &claim + idx.reexportDocs.set(key, next) + if upgraded { + idx.changedName(key.fqn) + writableMap(idx.docReexports, doc)[key] = true + idx.applyReexportMarks(key, next) + parent, _ := splitFQN(key.fqn) + idx.markGained(parent) // a public claim can un-hide it, which exports it onward + } + return widened } // dropClaim forgets doc's claim on a re-export, deregistering the name once no // document surfaces it any more and re-hiding it when only private imports -// remain. +// remain. The surviving claims go into a new map (see claimReexport). func (idx *Index) dropClaim(key reexportKey, doc string) { - if _, claimed := idx.reexportDocs.at(key)[doc]; !claimed { + docs := idx.reexportDocs.at(key) + if _, claimed := docs[doc]; !claimed { return } - docs := idx.writableClaims(key) - delete(docs, doc) // the routes this document recorded go with its claim + idx.noteBefore(key.fqn) idx.changedName(key.fqn) - if len(docs) == 0 { + if len(docs) == 1 { idx.reexportDocs.del(key) idx.deregister(key.fqn, key.sym) return } - idx.applyReexportMarks(key, docs) + kept := make(map[string]*reexportClaim, len(docs)-1) + for other, c := range docs { + if other != doc { // the routes this document recorded go with its claim + kept[other] = c + } + } + idx.reexportDocs.set(key, kept) + idx.applyReexportMarks(key, kept) parent, _ := splitFQN(key.fqn) idx.markLost(parent) // only private imports may remain, hiding it again } @@ -1404,26 +1427,6 @@ func (idx *Index) purgeReexport(key reexportKey) { idx.deregister(key.fqn, key.sym) } -// writableClaims returns the claims on key that this index may write to. A claim -// the frozen base recorded is copied with them: recording a route on it would -// otherwise change what every index over that base re-exports. -func (idx *Index) writableClaims(key reexportKey) map[string]*reexportClaim { - idx.noteBefore(key.fqn) - if docs, owned := idx.reexportDocs.own[key]; owned { - idx.reexportDocs.gen.bump() - return docs - } - shared, _ := idx.reexportDocs.below(key) - docs := make(map[string]*reexportClaim, len(shared)+1) - for doc, claim := range shared { - copied := *claim - copied.routes = append([]gateRoute(nil), claim.routes...) - docs[doc] = &copied - } - idx.reexportDocs.set(key, docs) - return docs -} - // applyReexportMarks brings the reexported and hidden marks in line with docs, // the claims on key: a claimed name is re-exported, and hidden while every // document that surfaced it did so with a private import (KerML 8.2.3.3).