internal/core/adt: merge OptionalField and Field types

Instead of introducing a new type for Required fields, we merge all these field types. They are nearly identical. Suffix is introduced to simplify debug printing. Issue #2003 Signed-off-by: Marcel van Lohuizen <mpvl@gmail.com> Change-Id: I44a5864e241e25dca1ec199d49bf2105683e61ca Reviewed-on: https://review.gerrithub.io/c/cue-lang/cue/+/551203 Reviewed-by: Roger Peppe <rogpeppe@gmail.com> Reviewed-by: Aram Hăvărneanu <aram@cue.works> TryBot-Result: CUEcueckoo <cueckoo@cuelang.org> Unity-Result: CUEcueckoo <cueckoo@cuelang.org>
cue-lang · Mar 22, 2023 · 1822d52 · 1822d52
1 parent e5cd9ac
commit 1822d52
Show file tree

Hide file tree

Showing 19 changed files with 67 additions and 140 deletions.
diff --git a/cue/testdata/cycle/compbottom2.txtar b/cue/testdata/cycle/compbottom2.txtar
@@ -270,7 +270,7 @@ Allocs: 10
 Retain: 76
 
 Unifications: 151
-Conjuncts:    166
+Conjuncts:    164
 Disjuncts:    205
 -- out/eval --
 (struct){
@@ -481,16 +481,16 @@ Disjuncts:    205
     }
     defCloseSuccess: (struct){
       #Example: (#struct){
+        ret?: (_){ _ }
         raises?: (#struct){
           runtime?: (string){ string }
         }
-        ret?: (_){ _ }
       }
       expr: (#struct){
+        ret: (int){ 2 }
         raises?: (#struct){
           runtime?: (string){ string }
         }
-        ret: (int){ 2 }
       }
     }
   }

diff --git a/cue/types.go b/cue/types.go
@@ -1724,19 +1724,17 @@ func (v Value) FillPath(p Path, x interface{}) Value {
 			expr = list
 
 		default:
-			var d adt.Decl
-			if sel.ConstraintType() == OptionalConstraint {
-				d = &adt.OptionalField{
-					Label: sel.sel.feature(v.idx),
-					Value: expr,
-				}
-			} else {
-				d = &adt.Field{
-					Label: sel.sel.feature(v.idx),
-					Value: expr,
-				}
+			f := &adt.Field{
+				Label:   sel.sel.feature(v.idx),
+				Value:   expr,
+				ArcType: adt.ArcMember,
 			}
-			expr = &adt.StructLit{Decls: []adt.Decl{d}}
+			switch sel.ConstraintType() {
+			case OptionalConstraint:
+				f.ArcType = adt.ArcOptional
+			}
+
+			expr = &adt.StructLit{Decls: []adt.Decl{f}}
 		}
 	}
 	n := &adt.Vertex{}

diff --git a/internal/core/adt/adt.go b/internal/core/adt/adt.go
@@ -234,8 +234,6 @@ func (*CallExpr) expr()      {}
 
 func (*Field) declNode()                {}
 func (x *Field) expr() Expr             { return x.Value }
-func (*OptionalField) declNode()        {}
-func (x *OptionalField) expr() Expr     { return x.Value }
 func (*LetField) declNode()             {}
 func (x *LetField) expr() Expr          { return x.Value }
 func (*BulkOptionalField) declNode()    {}
@@ -371,7 +369,6 @@ func (*BinaryExpr) node()        {}
 func (*CallExpr) node()          {}
 func (*DisjunctionExpr) node()   {}
 func (*Field) node()             {}
-func (*OptionalField) node()     {}
 func (*LetField) node()          {}
 func (*BulkOptionalField) node() {}
 func (*DynamicField) node()      {}

diff --git a/internal/core/adt/composite.go b/internal/core/adt/composite.go
@@ -281,6 +281,16 @@ const (
 	ArcVoid
 )
 
+// Suffix reports the field suffix for the given ArcType if it is a
+// constraint or the empty string otherwise.
+func (a ArcType) Suffix() string {
+	switch a {
+	case ArcOptional:
+		return "?"
+	}
+	return ""
+}
+
 func (v *Vertex) Clone() *Vertex {
 	c := *v
 	c.state = nil
@@ -814,8 +824,12 @@ func (v *Vertex) AddConjunct(c Conjunct) *Bottom {
 }
 
 func (v *Vertex) hasConjunct(c Conjunct) (added bool) {
-	switch c.x.(type) {
-	case *OptionalField, *BulkOptionalField, *Ellipsis:
+	switch f := c.x.(type) {
+	case *BulkOptionalField, *Ellipsis:
+	case *Field:
+		if f.ArcType == ArcMember {
+			v.ArcType = ArcMember
+		}
 	default:
 		v.ArcType = ArcMember
 	}

diff --git a/internal/core/adt/comprehension.go b/internal/core/adt/comprehension.go
@@ -176,6 +176,7 @@ func (n *nodeContext) insertComprehension(
 					Syntax:  c.Syntax,
 					Clauses: c.Clauses,
 					Value:   f,
+					arcType: f.ArcType,
 
 					comp:   ec,
 					parent: c,
@@ -405,7 +406,7 @@ func (n *nodeContext) injectComprehensions(allP *[]envYield, allowCycle bool, st
 
 		v := n.node
 		for c := d.leaf; c.parent != nil; c = c.parent {
-			v.ArcType = ArcMember
+			v.UpdateArcType(c.arcType)
 			v = c.arc
 		}
 

diff --git a/internal/core/adt/eval.go b/internal/core/adt/eval.go
@@ -1906,7 +1906,7 @@ func (n *nodeContext) addStruct(
 
 	for _, d := range s.Decls {
 		switch x := d.(type) {
-		case *Field, *OptionalField, *LetField:
+		case *Field, *LetField:
 			// handle in next iteration.
 
 		case *DynamicField:
@@ -1947,14 +1947,11 @@ func (n *nodeContext) addStruct(
 	for _, d := range s.Decls {
 		switch x := d.(type) {
 		case *Field:
-			if x.Label.IsString() {
+			if x.Label.IsString() && x.ArcType == ArcMember {
 				n.aStruct = s
 				n.aStructID = closeInfo
 			}
-			n.insertField(x.Label, ArcMember, MakeConjunct(childEnv, x, closeInfo))
-
-		case *OptionalField:
-			n.insertField(x.Label, ArcOptional, MakeConjunct(childEnv, x, closeInfo))
+			n.insertField(x.Label, x.ArcType, MakeConjunct(childEnv, x, closeInfo))
 
 		case *LetField:
 			arc := n.insertField(x.Label, ArcMember, MakeConjunct(childEnv, x, closeInfo))

diff --git a/internal/core/adt/expr.go b/internal/core/adt/expr.go
@@ -102,13 +102,9 @@ func (o *StructLit) Init() {
 			if o.fieldIndex(x.Label) < 0 {
 				o.Fields = append(o.Fields, FieldInfo{Label: x.Label})
 			}
-
-		case *OptionalField:
-			p := o.fieldIndex(x.Label)
-			if p < 0 {
-				o.Fields = append(o.Fields, FieldInfo{Label: x.Label})
+			if x.ArcType > ArcMember {
+				o.types |= HasField
 			}
-			o.types |= HasField
 
 		case *LetField:
 			if o.fieldIndex(x.Label) >= 0 {
@@ -174,8 +170,8 @@ func (o *StructLit) OptionalTypes() OptionalType {
 // Fields can also be used as expressions whereby the value field is the
 // expression this allows retaining more context.
 
-// Field represents a field with a fixed label. It can be a regular field,
-// definition or hidden field.
+// Field represents a regular field or field constraint with a fixed label.
+// The label can be a regular field, definition or hidden field.
 //
 //	foo: bar
 //	#foo: bar
@@ -187,8 +183,9 @@ func (o *StructLit) OptionalTypes() OptionalType {
 type Field struct {
 	Src *ast.Field
 
-	Label Feature
-	Value Expr
+	ArcType ArcType
+	Label   Feature
+	Value   Expr
 }
 
 func (x *Field) Source() ast.Node {
@@ -198,22 +195,6 @@ func (x *Field) Source() ast.Node {
 	return x.Src
 }
 
-// An OptionalField represents an optional regular field.
-//
-//	foo?: expr
-type OptionalField struct {
-	Src   *ast.Field
-	Label Feature
-	Value Expr
-}
-
-func (x *OptionalField) Source() ast.Node {
-	if x.Src == nil {
-		return nil
-	}
-	return x.Src
-}
-
 // A LetField represents a field that is only visible in the local scope.
 //
 //	let X = expr
@@ -1822,6 +1803,9 @@ type Comprehension struct {
 	// information as possible.
 	Value Node
 
+	// The type of field as which the comprehension is added.
+	arcType ArcType
+
 	// Only used for partial comprehensions.
 	comp   *envComprehension
 	parent *Comprehension // comprehension from which this one was derived, if any

diff --git a/internal/core/adt/expr_test.go b/internal/core/adt/expr_test.go
@@ -55,7 +55,6 @@ func TestNilSource(t *testing.T) {
 		&NodeLink{},
 		&Null{},
 		&Num{},
-		&OptionalField{},
 		&SelectorExpr{},
 		&SliceExpr{},
 		&String{},

diff --git a/internal/core/compile/compile.go b/internal/core/compile/compile.go
@@ -595,18 +595,15 @@ func (c *compiler) decl(d ast.Decl) adt.Decl {
 				return c.errf(x, "cannot use _ as label")
 			}
 
-			if x.Optional == token.NoPos {
-				return &adt.Field{
-					Src:   x,
-					Label: label,
-					Value: value,
-				}
-			} else {
-				return &adt.OptionalField{
-					Src:   x,
-					Label: label,
-					Value: value,
-				}
+			t := adt.ArcMember
+			if x.Optional != token.NoPos {
+				t = adt.ArcOptional
+			}
+			return &adt.Field{
+				Src:     x,
+				Label:   label,
+				ArcType: t,
+				Value:   value,
 			}
 
 		case *ast.ListLit:

diff --git a/internal/core/debug/compact.go b/internal/core/debug/compact.go
@@ -64,9 +64,7 @@ func (w *compactPrinter) node(n adt.Node) {
 					w.node(a)
 				} else {
 					w.label(a.Label)
-					if a.IsConstraint() {
-						w.string("?")
-					}
+					w.string(a.ArcType.Suffix())
 					w.string(":")
 					w.node(a)
 				}
@@ -116,15 +114,10 @@ func (w *compactPrinter) node(n adt.Node) {
 	case *adt.Field:
 		s := w.labelString(x.Label)
 		w.string(s)
+		w.string(x.ArcType.Suffix())
 		w.string(":")
 		w.node(x.Value)
 
-	case *adt.OptionalField:
-		s := w.labelString(x.Label)
-		w.string(s)
-		w.string("?:")
-		w.node(x.Value)
-
 	case *adt.LetField:
 		w.string("let ")
 		s := w.labelString(x.Label)

diff --git a/internal/core/debug/debug.go b/internal/core/debug/debug.go
@@ -27,7 +27,6 @@ import (
 
 	"cuelang.org/go/cue/errors"
 	"cuelang.org/go/cue/literal"
-	"cuelang.org/go/internal"
 	"cuelang.org/go/internal/core/adt"
 )
 
@@ -237,9 +236,7 @@ func (w *printer) node(n adt.Node) {
 			} else {
 				w.string("\n")
 				w.label(a.Label)
-				if a.IsConstraint() {
-					w.string("?")
-				}
+				w.string(a.ArcType.Suffix())
 				w.string(": ")
 				w.node(a)
 			}
@@ -298,20 +295,8 @@ func (w *printer) node(n adt.Node) {
 	case *adt.Field:
 		s := w.labelString(x.Label)
 		w.string(s)
+		w.string(x.ArcType.Suffix())
 		w.string(":")
-		if x.Label.IsDef() && !internal.IsDef(s) {
-			w.string(":")
-		}
-		w.string(" ")
-		w.node(x.Value)
-
-	case *adt.OptionalField:
-		s := w.labelString(x.Label)
-		w.string(s)
-		w.string("?:")
-		if x.Label.IsDef() && !internal.IsDef(s) {
-			w.string(":")
-		}
 		w.string(" ")
 		w.node(x.Value)
 

diff --git a/internal/core/dep/dep.go b/internal/core/dep/dep.go
@@ -291,11 +291,6 @@ func (c *visitor) markDecl(env *adt.Environment, d adt.Decl) {
 	case *adt.Field:
 		c.markSubExpr(env, x.Value)
 
-	case *adt.OptionalField:
-		// when dynamic, only continue if there is evidence of
-		// the field in the parallel actual evaluation.
-		c.markSubExpr(env, x.Value)
-
 	case *adt.BulkOptionalField:
 		c.markExpr(env, x.Filter)
 		// when dynamic, only continue if there is evidence of

diff --git a/internal/core/dep/mixed.go b/internal/core/dep/mixed.go
@@ -82,9 +82,6 @@ func (m marked) markExpr(x adt.Expr) {
 			case *adt.Field:
 				m.markExpr(x.Value)
 
-			case *adt.OptionalField:
-				m.markExpr(x.Value)
-
 			case *adt.BulkOptionalField:
 				m.markExpr(x.Value)
 

diff --git a/internal/core/export/adt.go b/internal/core/export/adt.go
@@ -420,21 +420,10 @@ func (e *exporter) decl(env *adt.Environment, d adt.Decl) ast.Decl {
 		f := &ast.Field{
 			Label: e.stringLabel(x.Label),
 		}
-
-		e.setField(x.Label, f)
-
-		f.Value = e.expr(env, x.Value)
-		f.Attrs = extractFieldAttrs(nil, x)
-
-		return f
-
-	case *adt.OptionalField:
-		e.setDocs(x)
-		f := &ast.Field{
-			Label:    e.stringLabel(x.Label),
-			Optional: token.NoSpace.Pos(),
+		if x.ArcType != adt.ArcMember {
+			// Backwards compatibility.
+			f.Optional = token.NoSpace.Pos()
 		}
-
 		e.setField(x.Label, f)
 
 		f.Value = e.expr(env, x.Value)

diff --git a/internal/core/export/expr.go b/internal/core/export/expr.go
@@ -375,9 +375,7 @@ func (e *conjuncts) addExpr(env *adt.Environment, src *adt.Vertex, x adt.Elem, i
 			switch f := d.(type) {
 			case *adt.Field:
 				label = f.Label
-			case *adt.OptionalField:
-				// TODO: mark optional here.
-				label = f.Label
+				// TODO: mark optional here, if needed.
 			case *adt.LetField:
 				continue
 			case *adt.Ellipsis:
@@ -509,13 +507,6 @@ func isComplexStruct(s *adt.StructLit) bool {
 				}
 			}
 
-		case *adt.OptionalField:
-			if x.Src != nil {
-				if _, ok := x.Src.Label.(*ast.Alias); ok {
-					return ok
-				}
-			}
-
 		case *adt.LetField:
 
 		case adt.Expr: