Merge branch 'master' into smuv3.2

Author: ducky64

compiler/src/main/scala/edg/compiler/Compiler.scala

@@ -117,7 +117,7 @@ class AssignNamer() {
 }
 
 object Compiler {
-  final val kExpectedProtoVersion = 6
+  final val kExpectedProtoVersion = 7
 }
 
 /** Compiler for a particular design, with an associated library to elaborate references from.
@@ -258,7 +258,7 @@ class Compiler private (
   // Returns all errors, by scanning the design tree for errors and adding errors accumulated through the compile
   // process
   def getErrors(): Seq[CompilerError] = {
-    val pendingErrors = elaboratePending.getMissingValue.map { missingNode =>
+    val pendingErrors = elaboratePending.getMissingValues.map { missingNode =>
       CompilerError.Unelaborated(missingNode, elaboratePending.nodeMissing(missingNode))
     }.toSeq
 
@@ -1628,15 +1628,18 @@ class Compiler private (
     val partialCompileIgnoredRecords = mutable.Set[ElaborateRecord]()
 
     // repeat as long as there is work ready, and all the ready work isn't marked to be ignored
-    var readyList = Set[ElaborateRecord]()
+    var readyList = Iterable[ElaborateRecord]()
     do {
-      readyList = elaboratePending.getReady -- partialCompileIgnoredRecords
+      // TODO this is kind of ugly and expensive in that it keeps filtering the ignored records
+      // ideally this should be done at the getReady side, but these also need to be restored
+      // when the compiler forks
+      readyList = elaboratePending.getReady.filter(!partialCompileIgnoredRecords.contains(_))
       readyList.foreach { elaborateRecord =>
         try {
           elaborateRecord match {
             case elaborateRecord @ ElaborateRecord.ExpandBlock(blockPath, blockClass, blockProgress) =>
               if (partial.blocks.contains(blockPath) || partial.classes.contains(blockClass)) {
-                partialCompileIgnoredRecords.add(elaborateRecord)
+                partialCompileIgnoredRecords += elaborateRecord
               } else {
                 expandBlock(blockPath, blockProgress)
                 elaboratePending.setValue(elaborateRecord, None)
@@ -1653,7 +1656,7 @@ class Compiler private (
             case elaborateRecord @ ElaborateRecord.Parameter(root, rootClasses, postfix, param) =>
               val container = resolveBlock(root).asInstanceOf[wir.HasParams]
               if (paramMatchesPartial(root, rootClasses, postfix)) {
-                partialCompileIgnoredRecords.add(elaborateRecord)
+                partialCompileIgnoredRecords += elaborateRecord
               } else {
                 constProp.addDeclaration(root ++ postfix, param)
                 elaboratePending.setValue(elaborateRecord, None)
@@ -1695,7 +1698,7 @@ class Compiler private (
   }
 
   def evaluateExpr(root: DesignPath, value: expr.ValueExpr): ExprResult = {
-    new ExprEvaluatePartial(constProp, root).map(value)
+    new ExprEvaluatePartial(constProp.getValue, root).map(value)
   }
 
   // Primarily used for unit tests, TODO clean up this API?

compiler/src/main/scala/edg/compiler/CompilerError.scala

@@ -13,7 +13,7 @@ sealed trait CompilerError {
 }
 
 object CompilerError {
-  case class Unelaborated(record: ElaborateRecord, missing: Set[ElaborateRecord]) extends CompilerError {
+  case class Unelaborated(record: ElaborateRecord, missing: Iterable[ElaborateRecord]) extends CompilerError {
     // These errors may be redundant with below, but provides dependency data
     override def toString: String = s"Unelaborated missing dependencies $record:\n" +
       s"${missing.map(x => s"- $x").mkString("\n")}"
@@ -104,16 +104,15 @@ object CompilerError {
     }
   }
 
-  case class OverAssign(target: IndirectDesignPath, causes: Seq[OverAssignCause]) extends CompilerError {
-    override def toString: String = s"Overassign to $target:\n" +
-      s"${causes.map(x => s"- $x").mkString("\n")}"
+  case class ExprError(target: IndirectDesignPath, msg: String) extends CompilerError {
+    override def toString: String = s"Expr error at $target: $msg"
 
     override def toIr: edgcompiler.ErrorRecord = {
       edgcompiler.ErrorRecord(
         path = Some(target.toLocalPath),
-        kind = "Overassign",
+        kind = "Expr",
         name = "",
-        details = causes.map(_.toString).mkString(", ")
+        details = msg
       )
     }
   }
@@ -166,7 +165,7 @@ object CompilerError {
       root: DesignPath,
       constrName: String,
       value: expr.ValueExpr,
-      missing: Set[IndirectDesignPath]
+      missing: Iterable[IndirectDesignPath]
   ) extends AssertionError {
     override def toString: String =
       s"Unevaluated assertion: $root.$constrName: missing ${missing.mkString(", ")} in ${ExprToString.apply(value)}"

compiler/src/main/scala/edg/compiler/ConstProp.scala

@@ -1,17 +1,16 @@
 package edg.compiler
 
-import scala.collection.mutable
-import scala.collection.Set
+import edg.ExprBuilder
+import edg.compiler.CompilerError.ExprError
+import edg.util.DependencyGraph
+import edg.wir._
 import edgir.expr.expr
 import edgir.init.init
-import edg.wir._
-import edg.util.{DependencyGraph, MutableBiMap}
-import edg.ExprBuilder
 import edgir.ref.ref.LocalPath
 
-case class AssignRecord(target: IndirectDesignPath, root: DesignPath, value: expr.ValueExpr)
+import scala.collection.{Set, mutable}
 
-case class OverassignRecord(assigns: mutable.Set[(DesignPath, String, expr.ValueExpr)] = mutable.Set())
+case class AssignRecord(target: IndirectDesignPath, root: DesignPath, value: expr.ValueExpr)
 
 sealed trait ConnectedLinkRecord // a record in the connected link directed graph
 object ConnectedLinkRecord {
@@ -43,6 +42,7 @@ class ConstProp() {
   // Assign statements are added to the dependency graph only when arrays are ready
   // This is the authoritative source for the state of any param - in the graph (and its dependencies), or value solved
   // CONNECTED_LINK has an empty value but indicates that the path was resolved in that data structure
+  // NAME has an empty value but indicates declaration (existence in paramTypes)
   private val params = DependencyGraph[IndirectDesignPath, ExprValue]()
   // Parameter types are used to track declared parameters
   // Undeclared parameters cannot have values set, but can be forced (though the value is not effective until declared)
@@ -53,21 +53,20 @@ class ConstProp() {
   // Params that have a forced/override value, so they arent over-assigned.
   private val forcedParams = mutable.Set[IndirectDesignPath]()
 
-  // Overassigns, for error tracking
-  // This only tracks overassigns that were discarded, not including assigns that took effect.
-  // Additional analysis is needed to get the full set of conflicting assigns.
-  private val discardOverassigns = mutable.HashMap[IndirectDesignPath, OverassignRecord]()
+  // Errors that were generated during the process of resolving parameters, including overassigns
+  // A value may or may not exist (and may or not have been propagated) in the param dependency graph
+  private val paramErrors = mutable.HashMap[IndirectDesignPath, mutable.ListBuffer[ErrorValue]]()
 
   def initFrom(that: ConstProp): Unit = {
     require(paramAssign.isEmpty && paramSource.isEmpty && paramTypes.isEmpty && forcedParams.isEmpty
-      && discardOverassigns.isEmpty)
+      && paramErrors.isEmpty)
     paramAssign.addAll(that.paramAssign)
     paramSource.addAll(that.paramSource)
     params.initFrom(that.params)
     paramTypes.addAll(that.paramTypes)
     connectedLink.initFrom(that.connectedLink)
     forcedParams.addAll(that.forcedParams)
-    discardOverassigns.addAll(that.discardOverassigns)
+    paramErrors.addAll(that.paramErrors)
   }
 
   //
@@ -108,21 +107,20 @@ class ConstProp() {
       connectedLink.setValue(ready, DesignPath())
     }
 
-    var readyList = Set[IndirectDesignPath]()
+    var readyList = Iterable[IndirectDesignPath]()
     do {
-      // ignore params where we haven't seen the decl yet, to allow forced-assign when the block is expanded
-      // TODO support this for all params, including indirect ones (eg, name)
-      readyList = params.getReady.filter { elt =>
-        DesignPath.fromIndirectOption(elt) match {
-          case Some(elt) => paramTypes.keySet.contains(elt.asIndirect)
-          case None => true
-        }
-      }
+      readyList = params.getReady
       readyList.foreach { constrTarget =>
         val assign = paramAssign(constrTarget)
-        new ExprEvaluatePartial(this, assign.root).map(assign.value) match {
+        new ExprEvaluatePartial(getValue, assign.root).map(assign.value) match {
           case ExprResult.Result(result) =>
-            params.setValue(constrTarget, result)
+            result match {
+              case result @ ErrorValue(_) =>
+                paramErrors.getOrElseUpdate(constrTarget, mutable.ListBuffer()).append(result)
+                params.clearReadyNode(constrTarget)
+              case result => params.setValue(constrTarget, result)
+            }
+
             onParamSolved(constrTarget, result)
           case ExprResult.Missing(missing) => // account for CONNECTED_LINK prefix
             val missingCorrected = missing.map { path =>
@@ -161,6 +159,7 @@ class ConstProp() {
       case _ => throw new NotImplementedError(s"Unknown param declaration / init $decl")
     }
     paramTypes.put(target.asIndirect, paramType)
+    params.setValue(target.asIndirect + IndirectStep.Name, BooleanValue(false)) // dummy value
     update()
   }
 
@@ -192,22 +191,29 @@ class ConstProp() {
     require(target.splitConnectedLink.isEmpty, "cannot set CONNECTED_LINK")
     val paramSourceRecord = (root, constrName, targetExpr)
 
+    // ignore params where we haven't seen the decl yet, to allow forced-assign when the block is expanded
+    val paramTypesDep = DesignPath.fromIndirectOption(target) match {
+      case Some(path) => Seq(path.asIndirect + IndirectStep.Name)
+      case None => Seq() // has indirect step, no direct decl
+    }
     if (forced) {
       require(!forcedParams.contains(target), s"attempt to re-force $target")
       forcedParams.add(target)
       require(
         !params.valueDefinedAt(target),
         s"forced value must be set before value is resolved, prior ${paramSource(target)}"
       )
-      params.addNode(target, Seq(), overwrite = true) // forced can overwrite other records
+      params.addNode(target, paramTypesDep, overwrite = true) // forced can overwrite other records
     } else {
       if (!forcedParams.contains(target)) {
-        if (params.nodeDefinedAt(target)) {
-          val record = discardOverassigns.getOrElseUpdate(target, OverassignRecord())
-          record.assigns.add(paramSourceRecord)
+        if (params.nodeDefinedAt(target)) { // TODO add propagated assign
+          val (prevRoot, prevConstr, _) = paramSource.get(target).getOrElse("?", "?", "")
+          paramErrors.getOrElseUpdate(target, mutable.ListBuffer()).append(
+            ErrorValue(s"over-assign from $root.$constrName, prev assigned from $prevRoot.$prevConstr")
+          )
           return // first set "wins"
         }
-        params.addNode(target, Seq())
+        params.addNode(target, paramTypesDep)
       } else {
         return // ignored - param was forced, discard the new assign
       }
@@ -254,7 +260,7 @@ class ConstProp() {
   }
 
   /** Returns the value of a parameter, or None if it does not have a value (yet?). Can be used to check if parameters
-    * are resolved yet by testing against None.
+    * are resolved yet by testing against None. Cannot return an ErrorValue
     */
   def getValue(param: IndirectDesignPath): Option[ExprValue] = {
     resolveConnectedLink(param) match {
@@ -264,7 +270,6 @@ class ConstProp() {
 
   }
   def getValue(param: DesignPath): Option[ExprValue] = {
-    // TODO should this be an implicit conversion?
     getValue(param.asIndirect)
   }
 
@@ -282,20 +287,14 @@ class ConstProp() {
     * references.
     */
   def getUnsolved: Set[IndirectDesignPath] = {
-    paramTypes.keySet.toSet -- params.knownValueKeys
+    paramTypes.keySet.toSet -- params.knownValueKeys -- paramErrors.keys
   }
 
   def getAllSolved: Map[IndirectDesignPath, ExprValue] = params.toMap
 
-  def getErrors: Seq[CompilerError] = {
-    discardOverassigns.map { case (target, record) =>
-      val propagatedAssign = paramSource.get(target).map { case (root, constrName, value) =>
-        CompilerError.OverAssignCause.Assign(target, root, constrName, value)
-      }.toSeq
-      val discardedAssigns = record.assigns.map { case (root, constrName, value) =>
-        CompilerError.OverAssignCause.Assign(target, root, constrName, value)
-      }
-      CompilerError.OverAssign(target, propagatedAssign ++ discardedAssigns)
+  def getErrors: Seq[ExprError] = {
+    paramErrors.flatMap { case (target, errors) =>
+      errors.map(error => ExprError(target, error.msg))
     }.toSeq
   }
 }

compiler/src/main/scala/edg/compiler/ExprEvaluate.scala

@@ -56,6 +56,21 @@ object ExprEvaluate {
           case _ =>
             throw new ExprEvaluateException(s"Unknown binary operand types in $lhs ${binary.op} $rhs from $binary")
         }
+      case Op.SHRINK_MULT => (lhs, rhs) match {
+          case (RangeValue(targetMin, targetMax), RangeValue(contribMin, contribMax)) =>
+            val lower = contribMax * targetMin
+            val upper = contribMin * targetMax
+            if (lower > upper) {
+              ErrorValue(s"shrink_mult($lhs, $rhs) produces empty range, target $lhs tighter tol than contrib $rhs")
+            } else {
+              RangeValue(lower, upper)
+            }
+          case (RangeEmpty, RangeEmpty) => RangeEmpty
+          case (lhs: RangeValue, RangeEmpty) => RangeEmpty
+          case (RangeEmpty, rhs: RangeValue) => RangeEmpty
+          case _ =>
+            throw new ExprEvaluateException(s"Unknown binary operand types in $lhs ${binary.op} $rhs from $binary")
+        }
 
       case Op.AND => (lhs, rhs) match {
           case (BooleanValue(lhs), BooleanValue(rhs)) => BooleanValue(lhs && rhs)
@@ -270,10 +285,8 @@ object ExprEvaluate {
       case (Op.MIN, RangeValue(valMin, _)) => FloatValue(valMin)
       case (Op.MAX, RangeValue(_, valMax)) => FloatValue(valMax)
 
-      // TODO can we have stricter semantics to avoid min(RangeEmpty) and max(RangeEmpty)?
-      // This just NaNs out so at least it propagates
-      case (Op.MAX, RangeEmpty) => FloatValue(Float.NaN)
-      case (Op.MIN, RangeEmpty) => FloatValue(Float.NaN)
+      case (Op.MAX, RangeEmpty) => ErrorValue("max(RangeEmpty) is undefined")
+      case (Op.MIN, RangeEmpty) => ErrorValue("min(RangeEmpty) is undefined")
 
       case (Op.CENTER, RangeValue(valMin, valMax)) => FloatValue((valMin + valMax) / 2)
       case (Op.WIDTH, RangeValue(valMin, valMax)) => FloatValue(math.abs(valMax - valMin))
@@ -292,7 +305,7 @@ object ExprEvaluate {
       case (Op.SUM, ArrayValue.ExtractBoolean(vals)) => IntValue(vals.count(_ == true))
       case (Op.SUM, ArrayValue.UnpackRange(extracted)) => extracted match {
           case ArrayValue.UnpackRange.FullRange(valMins, valMaxs) => RangeValue(valMins.sum, valMaxs.sum)
-          case _ => RangeEmpty // TODO how should sum behave on empty ranges?
+          case _ => ErrorValue("unpack_range(empty) is undefined")
         }
 
       case (Op.ALL_TRUE, ArrayValue.Empty(_)) => BooleanValue(true)
@@ -311,13 +324,11 @@ object ExprEvaluate {
       case (Op.MINIMUM, ArrayValue.ExtractFloat(vals)) => FloatValue(vals.min)
       case (Op.MINIMUM, ArrayValue.ExtractInt(vals)) => IntValue(vals.min)
 
-      // TODO this should be a user-level assertion instead of a compiler error
-      case (Op.SET_EXTRACT, ArrayValue.Empty(_)) =>
-        throw new ExprEvaluateException(s"SetExtract with empty values from $unarySet")
+      case (Op.SET_EXTRACT, ArrayValue.Empty(_)) => ErrorValue("set_extract(empty) is undefined")
       case (Op.SET_EXTRACT, ArrayValue(vals)) => if (vals.forall(_ == vals.head)) {
           vals.head
         } else {
-          throw new ExprEvaluateException(s"SetExtract with non-equal values $vals from $unarySet")
+          ErrorValue(f"set_extract($vals) with non-equal values")
         }
 
       // Any empty value means the expression result is empty
@@ -327,19 +338,20 @@ object ExprEvaluate {
             if (maxMin <= minMax) {
               RangeValue(maxMin, minMax)
             } else { // does not intersect, null set
-              RangeEmpty
+              ErrorValue(f"intersection($extracted) produces empty set")
             }
           // The implicit initial value of intersect is the full range
           // TODO are these good semantics?
           case ArrayValue.UnpackRange.EmptyArray() => RangeValue(Float.NegativeInfinity, Float.PositiveInfinity)
-          case _ => RangeEmpty
+          case _ => ErrorValue(f"intersection($vals) is undefined")
         }
 
       // Any value is used (empty effectively discarded)
       case (Op.HULL, ArrayValue.UnpackRange(extracted)) => extracted match {
           case ArrayValue.UnpackRange.FullRange(valMins, valMaxs) => RangeValue(valMins.min, valMaxs.max)
           case ArrayValue.UnpackRange.RangeWithEmpty(valMins, valMaxs) => RangeValue(valMins.min, valMaxs.max)
-          case _ => RangeEmpty
+          case ArrayValue.UnpackRange.EmptyArray() => RangeEmpty // TODO: should this be an error?
+          case ArrayValue.UnpackRange.EmptyRange() => RangeEmpty
         }
       case (Op.HULL, ArrayValue.ExtractFloat(vals)) => RangeValue(vals.min, vals.max)
 
@@ -382,7 +394,7 @@ object ExprEvaluate {
     case (FloatPromotable(lhs), FloatPromotable(rhs)) => if (lhs <= rhs) {
         RangeValue(lhs, rhs)
       } else {
-        throw new ExprEvaluateException(s"Range with min $minimum not <= max $maximum from $range")
+        ErrorValue(s"range($minimum, $maximum) is malformed, $minimum > $maximum")
       }
     case _ => throw new ExprEvaluateException(s"Unknown range operands types $minimum $maximum from $range")
   }

compiler/src/main/scala/edg/compiler/ExprEvaluatePartial.scala

@@ -16,7 +16,8 @@ object ExprResult {
   * missing set may increase as more parameters are solved, because some expressions have dynamic references. For
   * example, if-then-else additionally depends on either true or false subexprs once the condition is known.
   */
-class ExprEvaluatePartial(refs: ConstProp, root: DesignPath) extends ValueExprMap[ExprResult] {
+class ExprEvaluatePartial(refResolver: IndirectDesignPath => Option[ExprValue], root: DesignPath)
+    extends ValueExprMap[ExprResult] {
   override def mapLiteral(literal: lit.ValueLit): ExprResult =
     ExprResult.Result(ExprEvaluate.evalLiteral(literal))
 
@@ -126,11 +127,11 @@ class ExprEvaluatePartial(refs: ConstProp, root: DesignPath) extends ValueExprMa
     val container = mapExtract.getContainer.expr.ref.getOrElse( // TODO restrict allowed types in proto
       throw new ExprEvaluateException(s"Non-ref container type in mapExtract $mapExtract"))
     val containerPath = root ++ container
-    refs.getValue(containerPath.asIndirect + IndirectStep.Elements) match {
+    refResolver(containerPath.asIndirect + IndirectStep.Elements) match {
       case Some(paramValue) =>
         val eltsVals = ArrayValue.ExtractText(paramValue).map { elt =>
           val eltPath = containerPath.asIndirect + elt ++ mapExtract.getPath
-          refs.getValue(eltPath) match {
+          refResolver(eltPath) match {
             case Some(value) => ExprResult.Result(value)
             case None => ExprResult.Missing(Set(eltPath))
           }
@@ -151,7 +152,7 @@ class ExprEvaluatePartial(refs: ConstProp, root: DesignPath) extends ValueExprMa
 
   override def mapRef(path: ref.LocalPath): ExprResult = {
     val refPath = root.asIndirect ++ path
-    refs.getValue(refPath) match {
+    refResolver(refPath) match {
       case Some(value) => ExprResult.Result(value)
       case None => ExprResult.Missing(Set(refPath))
     }