internal/core/adt: split ccArc

Use a separate type for the notify slice
and simplify the code.

Now the code is split, we can use the fact that
several of the arguments of addArcDepedency
and addNotificationDependency (which used to
be addDependency) are always the same or are
derived in a particular way. This allows us
to remove a bunch of assertions.

The TODO that was mentioned was actually
incorrect, and was computing the wrong
derivative, which is why that assertion
did not work.

Splitting addDepedency now also allows us
to merge addNotificationDependency with
linkNotify. Note that the uniqueness check
can now be removed and that the assertions
are enforced by the compiler (because of the
limitations of the function signature).

Signed-off-by: Marcel van Lohuizen <[email protected]>
Change-Id: I9042cb42be98d4e3f761f7a095f44348815a8dd2
Reviewed-on: https://review.gerrithub.io/c/cue-lang/cue/+/1207453
Reviewed-by: Daniel Martí <[email protected]>
Unity-Result: CUE porcuepine <[email protected]>
TryBot-Result: CUEcueckoo <[email protected]>

Author: mpvl

Diff for: internal/core/adt/conjunct.go

@@ -520,20 +520,19 @@ func (n *nodeContext) insertAndSkipConjuncts(c Conjunct, id CloseInfo, depth int
 	n.scheduleConjunct(c, id)
 }
 
-func (n *nodeContext) addNotify2(v *Vertex, c CloseInfo) []receiver {
+func (n *nodeContext) addNotify2(v *Vertex, c CloseInfo) {
 	// scheduleConjunct should ensure that the closeContext of of c is aligned
 	// with v. We rely on this to be the case here. We enforce this invariant
 	// here for clarity and to ensure correctness.
 	n.ctx.Assertf(token.NoPos, c.cc.src == v, "close context not aligned with vertex")
 
 	// No need to do the notification mechanism if we are already complete.
-	old := n.notify
 	switch {
 	case n.node.isFinal():
-		return old
+		return
 	case !n.node.isInProgress():
 	case n.meets(allAncestorsProcessed):
-		return old
+		return
 	}
 
 	// Create a "root" closeContext to reflect the entry point of the
@@ -544,22 +543,22 @@ func (n *nodeContext) addNotify2(v *Vertex, c CloseInfo) []receiver {
 	// is even possible by adding a panic.
 	root := n.node.rootCloseContext(n.ctx)
 	if root.isDecremented {
-		return old
+		return
 	}
 
 	for _, r := range n.notify {
 		if r.cc == c.cc {
-			return old
+			return
 		}
 	}
 
 	cc := c.cc
 
-	if root.linkNotify(n.ctx, cc) {
+	if root.addNotifyDependency(n.ctx, cc) {
+		// TODO: this is mostly identical to the slice in the root closeContext.
+		// Use only one once V2 is removed.
 		n.notify = append(n.notify, receiver{cc.src, cc})
 	}
-
-	return old
 }
 
 // Literal conjuncts

Diff for: internal/core/adt/dep.go

@@ -133,94 +133,62 @@ func (c *closeContext) matchDecrement(ctx *OpContext, v *Vertex, kind depKind, d
 	}
 }
 
-// A ccArcRef x refers to the x.src.[arcs|notify][x.index]
+// A ccDepRef x refers to the x.src.[arcs|notify][x.index]
 //
 // We use this instead of pointers, because the address may change when
 // growing a slice. We use this instead mechanism instead of a pointers so
 // that we do not need to maintain separate free buffers once we use pools of
 // closeContext.
-type ccArcRef struct {
+type ccDepRef struct {
 	src   *closeContext
 	kind  depKind
 	index int
 }
 
 // addArc adds a dependent arc to c. If child is an arc, child.src == key
-func (c *closeContext) addArcDependency(ctx *OpContext, matched bool, key, child, root *closeContext) {
-	const kind = ARC
+func (c *closeContext) addArcDependency(ctx *OpContext, matched bool, child *closeContext) {
+	root := child.src.cc()
 
 	// NOTE: do not increment
 	// - either root closeContext or otherwise resulting from sub closeContext
 	//   all conjuncts will be added now, notified, or scheduled as task.
 	for _, a := range c.arcs {
-		if a.key == key {
+		if a.root == root {
 			panic("addArc: Label already exists")
 		}
 	}
-	child.incDependent(ctx, kind, c) // matched in decDependent REF(arcs)
+	child.incDependent(ctx, ARC, c) // matched in decDependent REF(arcs)
 
 	c.arcs = append(c.arcs, ccArc{
 		matched: matched,
-		key:     key,
+		root:    root,
 		dst:     child,
 	})
 
-	// TODO: this tests seems sensible, but panics. Investigate what could
-	// trigger this.
-	// if child.src.Parent != c.src {
-	// 	panic("addArc: inconsistent parent")
-	// }
-	if child.src.cc() != root.src.cc() {
-		panic("addArc: inconsistent root")
-	}
-
-	root.externalDeps = append(root.externalDeps, ccArcRef{
+	root.externalDeps = append(root.externalDeps, ccDepRef{
 		src:   c,
-		kind:  kind,
+		kind:  ARC,
 		index: len(c.arcs) - 1,
 	})
 }
 
-func (cc *closeContext) linkNotify(ctx *OpContext, key *closeContext) bool {
-	for _, a := range cc.notify {
-		if a.key == key {
-			return false
-		}
-	}
-
-	cc.addNotificationDependency(ctx, false, key, key, key.src.cc())
-	return true
-}
-
-func (c *closeContext) addNotificationDependency(ctx *OpContext, matched bool, key, child, root *closeContext) {
-	const kind = NOTIFY
+func (c *closeContext) addNotifyDependency(ctx *OpContext, dst *closeContext) bool {
 	for _, a := range c.notify {
-		if a.key == key {
-			panic("addArc: Label already exists")
+		if a.dst == dst {
+			return false
 		}
 	}
-	child.incDependent(ctx, kind, c) // matched in decDependent REF(arcs)
-
-	c.notify = append(c.notify, ccArc{
-		matched: matched,
-		key:     key,
-		dst:     child,
-	})
+	dst.incDependent(ctx, NOTIFY, c) // matched in decDependent REF(arcs)
 
-	// TODO: this tests seems sensible, but panics. Investigate what could
-	// trigger this.
-	// if child.src.Parent != c.src {
-	// 	panic("addArc: inconsistent parent")
-	// }
-	if child.src.cc() != root.src.cc() {
-		panic("addArc: inconsistent root")
-	}
+	c.notify = append(c.notify, ccNotify{dst: dst})
 
-	root.externalDeps = append(root.externalDeps, ccArcRef{
+	root := dst.src.cc()
+	root.externalDeps = append(root.externalDeps, ccDepRef{
 		src:   c,
-		kind:  kind,
+		kind:  NOTIFY,
 		index: len(c.notify) - 1,
 	})
+	return true
 }
 
 // incDisjunct increases disjunction-related counters. We require kind to be

Diff for: internal/core/adt/disjunct2.go

@@ -738,7 +738,7 @@ func equalPartialNode(ctx *OpContext, x, y *closeContext) bool {
 outer:
 	for _, a := range x.arcs {
 		for _, b := range y.arcs {
-			if a.key.src.Label != b.key.src.Label {
+			if a.root.src.Label != b.root.src.Label {
 				continue
 			}
 			if !equalPartialNode(ctx, a.dst, b.dst) {

Diff for: internal/core/adt/fields.go

@@ -182,7 +182,7 @@ type closeContext struct {
 	//
 	// This is only used for root closedContext and only for debugging.
 	// TODO: move to nodeContext.
-	externalDeps []ccArcRef
+	externalDeps []ccDepRef
 
 	// child links to a sequence which additional patterns need to be verified
 	// against (&&). If there are more than one, these additional nodes are
@@ -273,7 +273,7 @@ type closeContext struct {
 	// originate from a more specific closeContext, allowing it to stopped
 	// sooner and possibly even remove the need for breaking dependency
 	// cycles.
-	notify []ccArc
+	notify []ccNotify
 
 	// parentIndex is the position in the parent's arcs slice that corresponds
 	// to this closeContext. This is currently unused. The intention is to use
@@ -329,9 +329,17 @@ type ccArc struct {
 	// matched pattern and that it is not explicitly defined as a field.
 	// This is only used for arcs and not for notify.
 	matched bool
-	// key is the closeContext is used to find the destination of the arc, which
-	// is the root context.
-	key *closeContext
+	// root is dst.src.cc(). TODO: remove and use dst directly.
+	root *closeContext
+	// dst is the closeContext for which the counters are incremented and
+	// decremented and which is the actual destination of the dependency.
+	dst *closeContext
+}
+
+type ccNotify struct {
+	// decremented indicates whether [decDependant] has been called for this
+	// dependency.
+	decremented bool
 	// dst is the closeContext for which the counters are incremented and
 	// decremented and which is the actual destination of the dependency.
 	dst *closeContext
@@ -403,7 +411,7 @@ func (v *Vertex) assignConjunct(ctx *OpContext, root *closeContext, c Conjunct,
 func (cc *closeContext) getKeyedCC(ctx *OpContext, key *closeContext, c CycleInfo, mode ArcType, checkClosed bool) *closeContext {
 	for i := range cc.arcs {
 		a := &cc.arcs[i]
-		if a.key == key {
+		if a.root == key {
 			a.matched = a.matched && !checkClosed
 			a.dst.updateArcType(ctx, mode)
 			return a.dst
@@ -454,7 +462,7 @@ func (cc *closeContext) getKeyedCC(ctx *OpContext, key *closeContext, c CycleInf
 		// prevent a dependency on self.
 		if key.src != cc.src {
 			matched := !checkClosed
-			cc.addArcDependency(ctx, matched, key, arc, key)
+			cc.addArcDependency(ctx, matched, arc)
 		}
 	}
 

Diff for: internal/core/adt/overlay.go

@@ -441,22 +441,19 @@ func (ctx *overlayContext) initCloneCC(x *closeContext) {
 		if a.dst.overlay == nil {
 			continue
 		}
-		if a.key.overlay != nil {
-			a.key = a.key.overlay // TODO: is this necessary?
+		if a.root.overlay != nil {
+			a.root = a.root.overlay // TODO: is this necessary?
 		}
 		a.dst = a.dst.overlay
 		o.arcs = append(o.arcs, a)
 	}
 
 	for _, a := range x.notify {
-		// If an arc does not have an overlay, we should not decrement the
+		// If a notification does not have an overlay, we should not decrement the
 		// dependency counter. We simply remove the dependency in that case.
 		if a.dst.overlay == nil {
 			continue
 		}
-		if a.key.overlay != nil {
-			a.key = a.key.overlay // TODO: is this necessary?
-		}
 		a.dst = a.dst.overlay
 		o.notify = append(o.notify, a)
 	}