Skip to content

Commit

Permalink
internal/core/adt: closedness improvements
Browse files Browse the repository at this point in the history
Differentiate between the one-level closedness
of the closed builtin and that of definitions.

Keep track of whether nodes use ellipses or top
values.

Issue #2884

Signed-off-by: Marcel van Lohuizen <mpvl@gmail.com>
Change-Id: I561f813b8384af7eccdfbe2d22900037b13d1229
  • Loading branch information
mpvl committed Mar 7, 2024
1 parent cdcb6fa commit 2e0f947
Show file tree
Hide file tree
Showing 2 changed files with 40 additions and 1 deletion.
2 changes: 2 additions & 0 deletions internal/core/adt/debug.go
Original file line number Diff line number Diff line change
Expand Up @@ -622,6 +622,8 @@ func (m *mermaidContext) pstr(cc *closeContext) string {
addFlag(cc.isDef, '#')
addFlag(cc.isEmbed, 'E')
addFlag(cc.isClosed, 'c')
addFlag(cc.isClosedOnce, 'C')
addFlag(cc.hasEllipsis, 'o')
io.Copy(w, flags)

w.WriteString(close)
Expand Down
39 changes: 38 additions & 1 deletion internal/core/adt/fields.go
Original file line number Diff line number Diff line change
Expand Up @@ -180,6 +180,19 @@ type closeContext struct {
// definition.
isDef bool

// hasEllipsis indicates whether the node contains an ellipsis.
hasEllipsis bool

// hasTop indicates a node has at least one top conjunct.
hasTop bool

// hasNonTop indicates a node has at least one conjunct that is not top.
hasNonTop bool

// isClosedOnce is true if this closeContext is the result of calling the
// close builtin.
isClosedOnce bool

// isEmbed indicates whether the closeContext is created as part of an
// embedding.
isEmbed bool
Expand Down Expand Up @@ -415,6 +428,13 @@ func (c *closeContext) addDependency(kind depKind, key, child *closeContext) {
panic("addArc: Label already exists")
}
}

// TODO: this tests seems sensible, but panics. Investigate what could
// trigger this.
// if child.src.Parent != c.src {
// panic("addArc: inconsistent parent")
// }

c.arcs = append(c.arcs, ccArc{kind: kind, key: key, cc: child})
}

Expand Down Expand Up @@ -460,13 +480,20 @@ func (c *closeContext) decDependent(ctx *OpContext, kind depKind, dependant *clo

p := c.parent

if c.isDef {
if c.isDef && !c.hasEllipsis && (!c.hasTop || c.hasNonTop) {
c.isClosed = true
if p != nil {
p.isDef = true
}
}

if c.isClosedOnce {
c.isClosed = true
if p != nil {
p.isClosedOnce = true
}
}

for _, a := range c.arcs {
cc := a.cc
cc.decDependent(ctx, a.kind, c) // REF(arcs)
Expand All @@ -486,6 +513,16 @@ func (c *closeContext) decDependent(ctx *OpContext, kind depKind, dependant *clo
return
}

if c.hasEllipsis {
p.hasEllipsis = true
}
if c.hasTop {
p.hasTop = true
}
if c.hasNonTop {
p.hasNonTop = true
}

if !c.isEmbed && c.isClosed {
// Merge the two closeContexts and ensure that the patterns and fields
// are mutually compatible according to the closedness rules.
Expand Down

0 comments on commit 2e0f947

Please sign in to comment.