This is an automated email from the ASF dual-hosted git repository.

jadams pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/incubator-daffodil.git

commit fbefd3a2a8648e354d5c9bdcb2325bd1f84a8fb4
Author: Josh Adams <[email protected]>
AuthorDate: Mon Dec 7 14:58:36 2020 -0500

    Enabled non-constant expressions in defineVariable
    
    This involved a number of changes to allow these expressions to be
    evaluated correctly. First off, more laziness needed to be added to
    VariableInstance to delay the evaluation until the VariableMap has been
    created. In order to support serialization with these changes, we needed
    to move from MStackOf to an ArrayBuffer. Finally, we needed to make
    sure we trigger the evaluation of these expressions before parsing the
    infoset.
    
    DAFFODIL-2352
---
 .../apache/daffodil/dsom/DFDLDefineVariable.scala  |   2 +-
 .../externalvars/TestExternalVariablesLoader.scala |   8 +-
 .../scala/org/apache/daffodil/util/MStack.scala    |   2 +-
 .../unparsers/ExpressionEvaluatingUnparsers.scala  |   2 +-
 .../apache/daffodil/processors/DataProcessor.scala |   4 +
 .../apache/daffodil/processors/RuntimeData.scala   |  11 +-
 .../apache/daffodil/processors/VariableMap1.scala  | 249 ++++++++++++++++-----
 .../parsers/ExpressionEvaluatingParsers.scala      |   4 +-
 .../daffodil/processors/parsers/PState.scala       |   8 +-
 .../daffodil/processors/unparsers/UState.scala     |   4 +-
 .../daffodil/section07/variables/variables.tdml    | 139 +++++++++++-
 .../section07/variables/TestVariables.scala        |  12 +-
 12 files changed, 360 insertions(+), 85 deletions(-)

diff --git 
a/daffodil-core/src/main/scala/org/apache/daffodil/dsom/DFDLDefineVariable.scala
 
b/daffodil-core/src/main/scala/org/apache/daffodil/dsom/DFDLDefineVariable.scala
index a449592..41d7b97 100644
--- 
a/daffodil-core/src/main/scala/org/apache/daffodil/dsom/DFDLDefineVariable.scala
+++ 
b/daffodil-core/src/main/scala/org/apache/daffodil/dsom/DFDLDefineVariable.scala
@@ -62,7 +62,7 @@ class DFDLDefineVariable(node: Node, doc: SchemaDocument)
   final lazy val primType = PrimType.fromNameString(typeQName.local).getOrElse(
     this.SDE("Variables must have primitive type. Type was '%s'.", 
typeQName.toPrettyString))
 
-  final def createVariableInstance = 
variableRuntimeData.createVariableInstance()
+  final def createVariableInstance = variableRuntimeData.createVariableInstance
 
   final override lazy val runtimeData = variableRuntimeData
 
diff --git 
a/daffodil-core/src/test/scala/org/apache/daffodil/externalvars/TestExternalVariablesLoader.scala
 
b/daffodil-core/src/test/scala/org/apache/daffodil/externalvars/TestExternalVariablesLoader.scala
index 2b64618..60e68d9 100644
--- 
a/daffodil-core/src/test/scala/org/apache/daffodil/externalvars/TestExternalVariablesLoader.scala
+++ 
b/daffodil-core/src/test/scala/org/apache/daffodil/externalvars/TestExternalVariablesLoader.scala
@@ -89,11 +89,11 @@ class TestExternalVariablesLoader extends Logging {
     val (sd, sset) = generateSD(topLevelAnnotations)
     val initialVMap = sset.variableMap
 
-    val stk_v_no_default = initialVMap.getVariableBindings(v_no_default)
-    val stk_v_with_default = initialVMap.getVariableBindings(v_with_default)
+    val abuf_v_no_default = initialVMap.getVariableBindings(v_no_default)
+    val abuf_v_with_default = initialVMap.getVariableBindings(v_with_default)
 
-    val var_v_no_default = stk_v_no_default.top
-    val var_v_with_default = stk_v_with_default.top
+    val var_v_no_default = abuf_v_no_default.last
+    val var_v_with_default = abuf_v_with_default.last
 
     val v_no_default_vrd = var_v_no_default.rd
     val v_with_default_vrd = var_v_with_default.rd
diff --git a/daffodil-lib/src/main/scala/org/apache/daffodil/util/MStack.scala 
b/daffodil-lib/src/main/scala/org/apache/daffodil/util/MStack.scala
index 2b09746..acd511e 100644
--- a/daffodil-lib/src/main/scala/org/apache/daffodil/util/MStack.scala
+++ b/daffodil-lib/src/main/scala/org/apache/daffodil/util/MStack.scala
@@ -182,7 +182,7 @@ object MStackOfAnyRef {
  * things.
  */
 protected abstract class MStack[@specialized T] private[util] (
-  arrayAllocator: (Int) => Array[T], nullValue: T) extends Serializable {
+  arrayAllocator: (Int) => Array[T], nullValue: T) {
 
   private var index = 0
   private var table: Array[T] = null
diff --git 
a/daffodil-runtime1-unparser/src/main/scala/org/apache/daffodil/processors/unparsers/ExpressionEvaluatingUnparsers.scala
 
b/daffodil-runtime1-unparser/src/main/scala/org/apache/daffodil/processors/unparsers/ExpressionEvaluatingUnparsers.scala
index 855e54c..d0b7e0f 100644
--- 
a/daffodil-runtime1-unparser/src/main/scala/org/apache/daffodil/processors/unparsers/ExpressionEvaluatingUnparsers.scala
+++ 
b/daffodil-runtime1-unparser/src/main/scala/org/apache/daffodil/processors/unparsers/ExpressionEvaluatingUnparsers.scala
@@ -88,7 +88,7 @@ class NewVariableInstanceStartUnparser(override val context: 
RuntimeData)
 
   override def unparse(state: UState) = {
     val vrd = context.asInstanceOf[VariableRuntimeData]
-    state.newVariableInstance(vrd, vrd, state)
+    state.newVariableInstance(vrd)
   }
 }
 
diff --git 
a/daffodil-runtime1/src/main/scala/org/apache/daffodil/processors/DataProcessor.scala
 
b/daffodil-runtime1/src/main/scala/org/apache/daffodil/processors/DataProcessor.scala
index 3d60450..8661844 100644
--- 
a/daffodil-runtime1/src/main/scala/org/apache/daffodil/processors/DataProcessor.scala
+++ 
b/daffodil-runtime1/src/main/scala/org/apache/daffodil/processors/DataProcessor.scala
@@ -491,6 +491,10 @@ class DataProcessor private (
       try {
         Assert.usageErrorUnless(state.dataInputStreamIsValid, "Attempted to 
use an invalid input source. This can happen due to our position in the input 
source not being properly reset after failed parse could not backtrack to its 
original position")
 
+        // Force the evaluation of any defineVariable's with non-constant 
default
+        // value expressions
+        state.variableMap.forceExpressionEvaluations(state)
+
         this.startElement(state, p)
         p.parse1(state)
         this.endElement(state, p)
diff --git 
a/daffodil-runtime1/src/main/scala/org/apache/daffodil/processors/RuntimeData.scala
 
b/daffodil-runtime1/src/main/scala/org/apache/daffodil/processors/RuntimeData.scala
index c80e2c7..74ee236 100644
--- 
a/daffodil-runtime1/src/main/scala/org/apache/daffodil/processors/RuntimeData.scala
+++ 
b/daffodil-runtime1/src/main/scala/org/apache/daffodil/processors/RuntimeData.scala
@@ -987,8 +987,17 @@ final class VariableRuntimeData(
       }
     }
 
-  def createVariableInstance(defaultValue: DataValuePrimitiveNullable = 
value): VariableInstance = {
+  // Pass by name/lazy arg is needed here as we need to postpone evaluating the
+  // defaultValue, which is most likely an expression, until after the inital
+  // VariableMap is created. Otherwise when we attempt to evaluate the
+  // expression it will look in the VariableMap, which hasn't been fully
+  // created yet and will trigger an OOLAG Circular Definition Exception
+  def createVariableInstance(defaultValue: => DataValuePrimitiveNullable): 
VariableInstance = {
     VariableInstance(state, defaultValue, this, maybeDefaultValueExpr)
   }
 
+  def createVariableInstance(): VariableInstance = {
+    VariableInstance(state, value, this, maybeDefaultValueExpr)
+  }
+
 }
diff --git 
a/daffodil-runtime1/src/main/scala/org/apache/daffodil/processors/VariableMap1.scala
 
b/daffodil-runtime1/src/main/scala/org/apache/daffodil/processors/VariableMap1.scala
index 5c31336..35c2f73 100644
--- 
a/daffodil-runtime1/src/main/scala/org/apache/daffodil/processors/VariableMap1.scala
+++ 
b/daffodil-runtime1/src/main/scala/org/apache/daffodil/processors/VariableMap1.scala
@@ -29,11 +29,12 @@ import 
org.apache.daffodil.infoset.DataValue.DataValuePrimitiveNullable
 import org.apache.daffodil.infoset.RetryableException
 import org.apache.daffodil.util.Maybe
 import org.apache.daffodil.util.Maybe.Nope
-import org.apache.daffodil.util.MStackOf
+import org.apache.daffodil.util.PreSerialization
+import org.apache.daffodil.util.TransientParam
 import org.apache.daffodil.xml.{GlobalQName, UnspecifiedNamespace, NamedQName, 
RefQName}
 import org.apache.daffodil.processors.parsers.PState
 
-import scala.collection.mutable.Map
+import scala.collection.mutable.{Map, ArrayBuffer}
 
 sealed abstract class VariableState extends Serializable
 
@@ -56,44 +57,126 @@ case object VariableRead extends VariableState
 case object VariableInProcess extends VariableState
 
 /**
- * Core tuple of a state for variables.
+ * Class for maintaining the state of a variable
+ *
+ * Note: stateArg, valueArg, and defaultValueExprArg are pass by name/lazy
+ * values as it is necessary to postpone their evaluation beyond when the
+ * VariableInstance is created. Attempting to evaluate these arguments will
+ * likely trigger an expression to be evaluated, which will query the
+ * VariableMap if the expression references another variable. If this is
+ * occurring within a defineVariable call, the VariableMap hasn't been fully
+ * created and attempting to evaluate such an expression will result in an 
OOLAG
+ * Circular Definition Exception
  */
-case class VariableInstance(
-  var state: VariableState,
-  var value: DataValuePrimitiveNullable,
-  rd: VariableRuntimeData,
-  defaultValueExpr: Maybe[CompiledExpression[AnyRef]],
+object VariableInstance {
+  def apply(
+    stateArg: => VariableState,
+    valueArg: => DataValuePrimitiveNullable,
+    rd: VariableRuntimeData,
+    defaultValueExprArg: => Maybe[CompiledExpression[AnyRef]],
+    priorState: VariableState = VariableUndefined,
+    priorValue: DataValuePrimitiveNullable = DataValue.NoValue) = {
+
+    new VariableInstance(stateArg, valueArg, rd, defaultValueExprArg, 
priorState, priorValue)
+  }
+}
+
+/**
+ * See documentation for object VariableInstance
+ */
+class VariableInstance private (
+  @TransientParam stateArg: => VariableState,
+  @TransientParam valueArg: => DataValuePrimitiveNullable,
+  val rd: VariableRuntimeData,
+  @TransientParam defaultValueExprArg: => Maybe[CompiledExpression[AnyRef]],
   var priorState: VariableState = VariableUndefined,
   var priorValue: DataValuePrimitiveNullable = DataValue.NoValue)
-  extends Serializable {
+  extends Serializable
+  with PreSerialization {
 
-  def copy(
-    state: VariableState = state,
-    value: DataValuePrimitiveNullable = value,
-    rd: VariableRuntimeData = rd,
-    defaultValueExpr: Maybe[CompiledExpression[AnyRef]] = defaultValueExpr,
-    priorState: VariableState = priorState,
-    priorValue: DataValuePrimitiveNullable = priorValue) =
-      new VariableInstance(state, value, rd, defaultValueExpr, priorState, 
priorValue)
+  lazy val defaultValueExpr = defaultValueExprArg
+
+  private var _value: Option[DataValuePrimitiveNullable] = None
+  private var _state: VariableState = null
+
+  /**
+   * Returns the current value of the VariableInstance
+   *
+   * This function allows value to be set while also enabling valueArg to be
+   * lazily evaluated. This allows the VariableInstance to support setting the
+   * value later and allows the construction of the VariableMap containing the
+   * VariableInstance before evaluating the default value expression
+   */
+  def value = {
+    if (_value.isEmpty) {
+      _value = Some(valueArg)
+    }
+
+    _value.get
+  }
+
+  /**
+   * Returns the current state of the VariableInstance
+   *
+   * This function allows state to be set while also enabling stateArg to be
+   * lazily evaluated. This allows the VariableInstance to support setting the
+   * state later and allows the construction of the VariableMap containing the
+   * VariableInstance before evaluating the default value expression
+   */
+  def state = {
+    if (_state == null) {
+      _state = stateArg
+    }
+
+    _state
+  }
 
   def setState(s: VariableState) = {
     this.priorState = this.state
-    this.state = s
+    this._state = s
   }
 
   def setValue(v: DataValuePrimitiveNullable) = {
     this.priorValue = this.value
-    this.value = v
+    this._value = Some(v)
+  }
+
+  override def preSerialization: Unit = {
+    defaultValueExpr
+    value
+    state
+  }
+
+  override def toString: String = "VariableInstance(%s,%s,%s,%s,%s,%s".format(
+                                                    state,
+                                                    value,
+                                                    rd,
+                                                    defaultValueExpr,
+                                                    priorState,
+                                                    priorValue)
+
+  def copy(
+    state: VariableState = state,
+    value: DataValuePrimitiveNullable = value,
+    rd: VariableRuntimeData = rd,
+    defaultValueExpr: Maybe[CompiledExpression[AnyRef]] = defaultValueExpr,
+    priorState: VariableState = priorState,
+    priorValue: DataValuePrimitiveNullable = priorValue) = {
+      val inst = new VariableInstance(state, value, rd, defaultValueExpr, 
priorState, priorValue)
+      inst.init()
+      inst
   }
 
+  def init() = preSerialization
+
   def reset() = {
     Assert.invariant(this.state != VariableUndefined)
     (this.state, this.priorState, this.defaultValueExpr.isDefined) match {
-      case (VariableRead, VariableSet, _) => this.state = VariableSet
-      case (VariableRead, _, true) => this.state = VariableDefined
+      case (VariableRead, VariableSet, _) => this.setState(VariableSet)
+      case (VariableRead, _, true) => this.setState(VariableDefined)
       case (VariableSet, _, _) => {
-        this.state = this.priorState
-        this.value = this.priorValue
+        this.setState(this.priorState)
+        this.setValue(this.priorValue)
       }
       case (_, _, _) => Assert.impossible("Should have SDE before reaching 
this")
     }
@@ -129,6 +212,14 @@ class VariableHasNoValue(qname: NamedQName, context: 
VariableRuntimeData) extend
   with RetryableException
 
 /**
+ * This expression can be thrown either at the start of parsing is the
+ * expressions in a defineVariable are circular, or later during parsing if
+ * newVariableInstance contains a circular expression
+ */
+class VariableCircularDefinition(qname: NamedQName, context: 
VariableRuntimeData) extends VariableException(qname, context,
+  "Variable map (runtime): variable %s is part of a circular definition with 
other variables".format(qname))
+
+/**
  * Provides one more indirection to the variable map.
  *
  * Needed so that when unparsing multiple clones of a UState can share
@@ -166,17 +257,21 @@ final class VariableBox(initialVMap: VariableMap) {
  *
  * The DPath implementation must be made to implement the
  * no-set-after-default-value-has-been-read behavior. This requires that 
reading the variables causes a state transition.
+ *
+ * The implementation of the VariabeMap uses ArrayBuffers essentially as a
+ * stack, as they allow for easy serialization unlike the custom MStack classes
+ * we use elsewhere. Scala's mutable Stack is deprecated in 2.12
  */
-class VariableMap private(vTable: Map[GlobalQName, MStackOf[VariableInstance]])
+class VariableMap private(vTable: Map[GlobalQName, 
ArrayBuffer[VariableInstance]])
   extends Serializable {
 
   def this(topLevelVRDs: Seq[VariableRuntimeData] = Nil) =
     this(Map(topLevelVRDs.map {
       vrd =>
         val variab = vrd.createVariableInstance()
-        val stack = new MStackOf[VariableInstance]
-        stack.push(variab)
-        (vrd.globalQName, stack)
+        val abuf = new ArrayBuffer[VariableInstance]
+        abuf += variab
+        (vrd.globalQName, abuf)
     }: _*))
 
   override def toString(): String = {
@@ -188,24 +283,37 @@ class VariableMap private(vTable: Map[GlobalQName, 
MStackOf[VariableInstance]])
    * VariableInstances are mutable and cannot safely be shared across threads
    */
   def copy(): VariableMap = {
-    val table = Map[GlobalQName, MStackOf[VariableInstance]]()
-    vTable.foreach { case (k: GlobalQName, s: MStackOf[VariableInstance]) => {
-      val newStack= new MStackOf[VariableInstance]()
-
-      // toList provides a list in LIFO order, so we need to reverse it to
-      // maintain order
-      s.toList.reverse.foreach { case v: VariableInstance => 
newStack.push(v.copy()) }
-      table(k) = newStack
+    val table = vTable.map { case (k: GlobalQName, abuf: 
ArrayBuffer[VariableInstance]) => {
+      val newBuf = abuf.map { _.copy() }
+      (k, newBuf)
     }}
 
     new VariableMap(table)
   }
 
+  // For defineVariable's with non-constant expressions for default values, it
+  // is necessary to force the evaluation of the expressions after the
+  // VariableMap has been created and initialized, but before parsing begins. 
We
+  // must also ensure that the expressions only reference other variables that
+  // have default value expressions or are defined externally.
+  def forceExpressionEvaluations(state: ParseOrUnparseState): Unit = {
+    vTable.foreach { case (_, abuf) => { abuf.foreach { inst => {
+      (inst.state, inst.value, inst.defaultValueExpr.isDefined) match {
+        // Evaluate defineVariable statements with non-constant default value 
expressions
+        case (VariableDefined, DataValue.NoValue, true) => {
+          val res = inst.defaultValueExpr.get.evaluate(state)
+          inst.setValue(DataValue.unsafeFromAnyRef(res))
+        }
+        case (_, _, _) => // Do nothing
+      }
+    }}}}
+  }
+
   def find(qName: GlobalQName): Option[VariableInstance] = {
-    val optStack = vTable.get(qName)
+    val optBuf = vTable.get(qName)
     val variab = {
-      if (optStack.isDefined)
-        Some(optStack.get.top)
+      if (optBuf.isDefined)
+        Some(optBuf.get.last)
       else
         None
     }
@@ -220,9 +328,9 @@ class VariableMap private(vTable: Map[GlobalQName, 
MStackOf[VariableInstance]])
   lazy val context = Assert.invariantFailed("unused.")
 
   /**
-   * For testing mostly.
+   * Used only for testing.
    */
-  def getVariableBindings(qn: GlobalQName): MStackOf[VariableInstance] = {
+  def getVariableBindings(qn: GlobalQName): ArrayBuffer[VariableInstance] = {
     vTable.get(qn).get
   }
 
@@ -230,20 +338,39 @@ class VariableMap private(vTable: Map[GlobalQName, 
MStackOf[VariableInstance]])
    * Returns the value of a variable and sets the state of the variable to be
    * VariableRead.
    */
-  def readVariable(vrd: VariableRuntimeData, referringContext: ThrowsSDE, 
maybePstate: Maybe[ParseOrUnparseState]): DataValuePrimitive = {
+  def readVariable(vrd: VariableRuntimeData, referringContext: ThrowsSDE, 
maybeState: Maybe[ParseOrUnparseState]): DataValuePrimitive = {
     val varQName = vrd.globalQName
-    val stack = vTable.get(varQName)
-    if (stack.isDefined) {
-      val variable = stack.get.top
+    val abuf = vTable.get(varQName)
+    if (abuf.isDefined) {
+      val variable = abuf.get.last
       variable.state match {
         case VariableRead if (variable.value.isDefined) => 
variable.value.getNonNullable
         case VariableDefined | VariableSet if (variable.value.isDefined) => {
-          if (maybePstate.isDefined && maybePstate.get.isInstanceOf[PState])
-            maybePstate.get.asInstanceOf[PState].markVariableRead(vrd)
+          if (maybeState.isDefined && maybeState.get.isInstanceOf[PState])
+            maybeState.get.asInstanceOf[PState].markVariableRead(vrd)
 
           variable.setState(VariableRead)
           variable.value.getNonNullable
         }
+        // This case is only hit for defineVariable's who's expression 
reference
+        // other defineVariables with expressions. It will be hit at the start
+        // of parsing, before an infoset is generated, via the
+        // forceExpressionEvaluations function after which all variables should
+        // have a defined value
+        case VariableDefined if (!variable.value.isDefined && 
variable.defaultValueExpr.isDefined) => {
+          Assert.invariant(maybeState.isDefined)
+          variable.setState(VariableInProcess)
+          val state = maybeState.get
+          val res = 
DataValue.unsafeFromAnyRef(variable.defaultValueExpr.get.evaluate(state))
+
+          // Need to update the variable's value with the result of the
+          // expression
+          variable.setState(VariableRead)
+          variable.setValue(res)
+
+          res
+        }
+        case VariableInProcess => throw new 
VariableCircularDefinition(varQName, vrd)
         case _ => throw new VariableHasNoValue(varQName, vrd)
       }
     } else
@@ -255,9 +382,9 @@ class VariableMap private(vTable: Map[GlobalQName, 
MStackOf[VariableInstance]])
    */
   def setVariable(vrd: VariableRuntimeData, newValue: DataValuePrimitive, 
referringContext: ThrowsSDE, pstate: ParseOrUnparseState) = {
     val varQName = vrd.globalQName
-    val stack = vTable.get(varQName)
-    if (stack.isDefined) {
-      val variable = stack.get.top
+    val abuf = vTable.get(varQName)
+    if (abuf.isDefined) {
+      val variable = abuf.get.last
       variable.state match {
         case VariableSet => {
           referringContext.SDE("Cannot set variable %s twice. State was: %s. 
Existing value: %s",
@@ -286,28 +413,28 @@ class VariableMap private(vTable: Map[GlobalQName, 
MStackOf[VariableInstance]])
   /**
    * Creates a new instance of a variable
    */
-  def newVariableInstance(vrd: VariableRuntimeData, referringContext: 
ThrowsSDE, state: ParseOrUnparseState) = {
+  def newVariableInstance(vrd: VariableRuntimeData, state: 
ParseOrUnparseState) = {
     val varQName = vrd.globalQName
-    val stack = vTable.get(varQName)
-    Assert.invariant(stack.isDefined)
+    val abuf = vTable.get(varQName)
+    Assert.invariant(abuf.isDefined)
 
     if (vrd.maybeDefaultValueExpr.isDefined) {
       val defaultValue = 
DataValue.unsafeFromAnyRef(vrd.maybeDefaultValueExpr.get.evaluate(state))
-      
stack.get.push(vrd.createVariableInstance(VariableUtils.convert(defaultValue.getAnyRef.toString,
 vrd, referringContext)))
+      abuf.get += 
vrd.createVariableInstance(VariableUtils.convert(defaultValue.getAnyRef.toString,
 vrd, vrd))
     } else
-      stack.get.push(vrd.createVariableInstance())
+      abuf.get += vrd.createVariableInstance()
   }
 
   def removeVariableInstance(vrd: VariableRuntimeData): Unit = {
     val varQName = vrd.globalQName
-    val stack = vTable.get(varQName)
-    Assert.invariant(stack.isDefined)
-    stack.get.pop
+    val abuf = vTable.get(varQName)
+    Assert.invariant(abuf.isDefined)
+    abuf.get.trimEnd(1)
   }
 
 
   private lazy val externalVarGlobalQNames: Seq[GlobalQName] =
-    vTable.map { case (_, stack) if (stack.top.rd.external) => 
stack.top.rd.globalQName }.toSeq
+    vTable.map { case (_, abuf) if (abuf.last.rd.external) => 
abuf.last.rd.globalQName }.toSeq
 
   /**
    * Assigns an external variable and sets the variables state to VariableSet
@@ -334,11 +461,11 @@ class VariableMap private(vTable: Map[GlobalQName, 
MStackOf[VariableInstance]])
         bindingQName
       }
 
-    val stack = vTable.get(varQName.toGlobalQName)
-    if (!stack.isDefined)
+    val abuf = vTable.get(varQName.toGlobalQName)
+    if (!abuf.isDefined)
       referringContext.schemaDefinitionError("unknown variable %s", varQName)
     else {
-      val variable = stack.get.top
+      val variable = abuf.get.last
       variable.state match {
         case VariableDefined if (!variable.rd.external) => {
           referringContext.SDE("Cannot set variable %s externally. State was: 
%s. Existing value: %s.",
diff --git 
a/daffodil-runtime1/src/main/scala/org/apache/daffodil/processors/parsers/ExpressionEvaluatingParsers.scala
 
b/daffodil-runtime1/src/main/scala/org/apache/daffodil/processors/parsers/ExpressionEvaluatingParsers.scala
index 2dc1c1c..c187005 100644
--- 
a/daffodil-runtime1/src/main/scala/org/apache/daffodil/processors/parsers/ExpressionEvaluatingParsers.scala
+++ 
b/daffodil-runtime1/src/main/scala/org/apache/daffodil/processors/parsers/ExpressionEvaluatingParsers.scala
@@ -156,7 +156,7 @@ class SetVariableParser(expr: CompiledExpression[AnyRef], 
decl: VariableRuntimeD
     log(LogLevel.Debug, "This is %s", toString) // important. Don't toString 
unless we have to log.
     val res = eval(start)
     if (start.processorStatus.isInstanceOf[Failure]) return
-    start.setVariable(decl, res, decl, start)
+    start.setVariable(decl, res, decl)
   }
 }
 
@@ -165,7 +165,7 @@ class NewVariableInstanceStartParser(override val context: 
VariableRuntimeData)
   override lazy val runtimeDependencies = Vector()
 
   def parse(start: PState): Unit = {
-    start.newVariableInstance(context, context, start)
+    start.newVariableInstance(context)
   }
 }
 
diff --git 
a/daffodil-runtime1/src/main/scala/org/apache/daffodil/processors/parsers/PState.scala
 
b/daffodil-runtime1/src/main/scala/org/apache/daffodil/processors/parsers/PState.scala
index 09018d0..45292dc 100644
--- 
a/daffodil-runtime1/src/main/scala/org/apache/daffodil/processors/parsers/PState.scala
+++ 
b/daffodil-runtime1/src/main/scala/org/apache/daffodil/processors/parsers/PState.scala
@@ -330,8 +330,8 @@ final class PState private (
     this.infoset = newParent
   }
 
-  def setVariable(vrd: VariableRuntimeData, newValue: DataValuePrimitive, 
referringContext: VariableRuntimeData, pstate: PState): Unit = {
-    variableMap.setVariable(vrd, newValue, referringContext, pstate)
+  def setVariable(vrd: VariableRuntimeData, newValue: DataValuePrimitive, 
referringContext: VariableRuntimeData): Unit = {
+    variableMap.setVariable(vrd, newValue, referringContext, this)
     changedVariablesStack.top += vrd.globalQName
   }
 
@@ -343,8 +343,8 @@ final class PState private (
     changedVariablesStack.top += vrd.globalQName
   }
 
-  def newVariableInstance(vrd: VariableRuntimeData, referringContext: 
VariableRuntimeData, pstate: PState): Unit = {
-    variableMap.newVariableInstance(vrd, referringContext, pstate)
+  def newVariableInstance(vrd: VariableRuntimeData): Unit = {
+    variableMap.newVariableInstance(vrd, this)
     changedVariablesStack.top += vrd.globalQName
   }
 
diff --git 
a/daffodil-runtime1/src/main/scala/org/apache/daffodil/processors/unparsers/UState.scala
 
b/daffodil-runtime1/src/main/scala/org/apache/daffodil/processors/unparsers/UState.scala
index f033e1b..1fb399f 100644
--- 
a/daffodil-runtime1/src/main/scala/org/apache/daffodil/processors/unparsers/UState.scala
+++ 
b/daffodil-runtime1/src/main/scala/org/apache/daffodil/processors/unparsers/UState.scala
@@ -343,8 +343,8 @@ abstract class UState(
 
   def documentElement: DIDocument
 
-  def newVariableInstance(vrd: VariableRuntimeData, referringContext: 
VariableRuntimeData, ustate: UState): Unit = {
-    variableMap.newVariableInstance(vrd, referringContext, ustate)
+  def newVariableInstance(vrd: VariableRuntimeData): Unit = {
+    variableMap.newVariableInstance(vrd, this)
   }
 
   def removeVariableInstance(vrd: VariableRuntimeData): Unit = {
diff --git 
a/daffodil-test/src/test/resources/org/apache/daffodil/section07/variables/variables.tdml
 
b/daffodil-test/src/test/resources/org/apache/daffodil/section07/variables/variables.tdml
index 453c2ec..4c79505 100644
--- 
a/daffodil-test/src/test/resources/org/apache/daffodil/section07/variables/variables.tdml
+++ 
b/daffodil-test/src/test/resources/org/apache/daffodil/section07/variables/variables.tdml
@@ -33,10 +33,12 @@
       defaultValue="16" />
     <dfdl:defineVariable name="myVar3" type="xs:int"
       defaultValue="26" />
-    <!--<dfdl:defineVariable name="myVar4" type="xs:int"
+    <dfdl:defineVariable name="myVar4" type="xs:int"
       defaultValue="36" />
     <dfdl:defineVariable name="nonConstVar" type="xs:int"
-      defaultValue="{ $ex:myVar4 }" />-->
+      defaultValue="{ $ex:myVar4 }" />
+    <dfdl:defineVariable name="nonConstVar2" type="xs:int"
+      defaultValue="{ $ex:nonConstVar }" />
     <xs:include 
schemaLocation="org/apache/daffodil/xsd/DFDLGeneralFormat.dfdl.xsd"/>
     <dfdl:format ref="ex:GeneralFormat" />
     <xs:element name="c">
@@ -435,13 +437,15 @@
       </xs:complexType>
     </xs:element>
 
-    <!--<xs:element name="defNonConst">
+    <xs:element name="defNonConst">
       <xs:complexType>
         <xs:sequence>
+          <xs:element name="test" type="xsd:int" dfdl:lengthKind="explicit" 
dfdl:length="1" />
           <xs:element name="value" type="xsd:int" dfdl:inputValueCalc="{ 
$ex:nonConstVar }" />
+          <xs:element name="value2" type="xsd:int" dfdl:inputValueCalc="{ 
$ex:nonConstVar2 }" />
         </xs:sequence>
       </xs:complexType>
-    </xs:element>-->
+    </xs:element>
 
     <xs:element name="e1">
       <xs:complexType>
@@ -966,18 +970,39 @@
     </tdml:infoset>
   </tdml:parserTestCase>
 
-  <!--<tdml:parserTestCase name="defineVariable_nonConstantExpression" 
root="defNonConst"
+  <tdml:parserTestCase name="defineVariable_nonConstantExpression" 
root="defNonConst"
     model="v"
     description="defineVariable with a non-constant expression as 
defaultValue">
 
+    <tdml:document><![CDATA[7]]></tdml:document>
+
+    <tdml:infoset>
+      <tdml:dfdlInfoset>
+        <defNonConst xmlns="http://example.com";>
+          <test>7</test>
+          <value>36</value>
+          <value2>36</value2>
+        </defNonConst>
+      </tdml:dfdlInfoset>
+    </tdml:infoset>
+  </tdml:parserTestCase>
+
+  <tdml:unparserTestCase name="defineVariable_nonConstantExpression_unp" 
root="defNonConst"
+    model="v"
+    description="defineVariable with a non-constant expression as 
defaultValue" roundTrip="false">
+
     <tdml:infoset>
       <tdml:dfdlInfoset>
         <defNonConst xmlns="http://example.com";>
+          <test>7</test>
           <value>36</value>
+          <value2>36</value2>
         </defNonConst>
       </tdml:dfdlInfoset>
     </tdml:infoset>
-  </tdml:parserTestCase>-->
+
+    <tdml:document><![CDATA[7]]></tdml:document>
+  </tdml:unparserTestCase>
 
   <tdml:parserTestCase name="varAsSeparator" root="e1"
     model="v"
@@ -1951,4 +1976,106 @@
     </tdml:infoset>
   </tdml:parserTestCase>
 
+  <tdml:defineSchema name="circular_defineVariable">
+    <xs:include 
schemaLocation="org/apache/daffodil/xsd/DFDLGeneralFormat.dfdl.xsd"/>
+    <dfdl:format ref="ex:GeneralFormat" />
+
+    <dfdl:defineVariable name="circular1" type="xs:int"
+      defaultValue="{ $ex:circular2 }" />
+    <dfdl:defineVariable name="circular2" type="xs:int"
+      defaultValue="{ $ex:circular1 }" />
+    <xs:element name="root" type="xs:int" dfdl:inputValueCalc="{ $ex:circular1 
}" />
+
+  </tdml:defineSchema>
+
+  <tdml:parserTestCase name="circular_defineVariable_err" root="root"
+    model="circular_defineVariable" description="Section 7 - ">
+    <tdml:document/>
+    <tdml:errors>
+      <tdml:error>Runtime Schema Definition Error</tdml:error>
+      <tdml:error>part of a circular definition</tdml:error>
+    </tdml:errors>
+  </tdml:parserTestCase>
+
+  <tdml:defineSchema name="defineVariable_ref_infoset">
+    <xs:include 
schemaLocation="org/apache/daffodil/xsd/DFDLGeneralFormat.dfdl.xsd"/>
+    <dfdl:format ref="ex:GeneralFormat" />
+
+    <dfdl:defineVariable name="badRef" type="xs:int"
+      defaultValue="{ ex:root }" />
+    <xs:element name="root" type="xs:int" dfdl:inputValueCalc="{ 42 }" />
+
+  </tdml:defineSchema>
+
+  <tdml:parserTestCase name="defineVariable_ref_infoset_err" root="root"
+    model="defineVariable_ref_infoset" description="Section 7 - ">
+    <tdml:document/>
+    <tdml:errors>
+      <tdml:error>????</tdml:error>
+    </tdml:errors>
+  </tdml:parserTestCase>
+
+  <tdml:defineSchema name="defineVariable_ref_noDefault">
+    <xs:include 
schemaLocation="org/apache/daffodil/xsd/DFDLGeneralFormat.dfdl.xsd"/>
+    <dfdl:format ref="ex:GeneralFormat" />
+
+    <dfdl:defineVariable name="noDefault" type="xs:int" />
+    <dfdl:defineVariable name="badRef" type="xs:int"
+      defaultValue="{ $ex:noDefault }" />
+    <xs:element name="root" type="xs:int" dfdl:inputValueCalc="{ $ex:badRef }" 
/>
+
+  </tdml:defineSchema>
+
+  <tdml:parserTestCase name="defineVariable_ref_noDefault_err" root="root"
+    model="defineVariable_ref_noDefault" description="Section 7 - ">
+    <tdml:document/>
+    <tdml:errors>
+      <tdml:error>Runtime Schema Definition Error</tdml:error>
+      <tdml:error>has no value. It was not set</tdml:error>
+    </tdml:errors>
+  </tdml:parserTestCase>
+
+  <tdml:defineSchema name="defineVariable_nonConstantExpression_setVar">
+    <xs:include 
schemaLocation="org/apache/daffodil/xsd/DFDLGeneralFormat.dfdl.xsd"/>
+    <dfdl:format ref="ex:GeneralFormat" />
+
+    <dfdl:defineVariable name="var1" type="xs:int"
+      defaultValue="{ 5 }" />
+    <dfdl:defineVariable name="var2" type="xs:int"
+      defaultValue="{ $ex:var1 }" />
+    <xs:element name="root">
+      <xs:complexType>
+        <xs:sequence>
+          <xs:annotation>
+            <xs:appinfo source="http://www.ogf.org/dfdl/";>
+              <dfdl:setVariable ref="ex:var2">2</dfdl:setVariable>
+              <dfdl:setVariable ref="ex:var1">1</dfdl:setVariable>
+            </xs:appinfo>
+          </xs:annotation>
+          <xs:element name="e1" type="xs:int" dfdl:inputValueCalc="{ $ex:var1 
}" />
+          <xs:element name="e2" type="xs:int" dfdl:inputValueCalc="{ $ex:var2 
}" />
+        </xs:sequence>
+      </xs:complexType>
+    </xs:element>
+
+  </tdml:defineSchema>
+
+  <tdml:parserTestCase name="defineVariable_nonConstantExpression_setVar_err" 
root="root"
+    model="defineVariable_nonConstantExpression_setVar" description="Section 7 
- ">
+    <tdml:document/>
+    <tdml:warnings>
+      <tdml:warning>Runtime Schema Definition Warning</tdml:warning>
+      <tdml:warning>Cannot set variable</tdml:warning>
+      <tdml:warning>after reading the default value</tdml:warning>
+    </tdml:warnings>
+    <tdml:infoset>
+      <tdml:dfdlInfoset>
+        <root>
+          <e1>1</e1>
+          <e2>2</e2>
+        </root>
+      </tdml:dfdlInfoset>
+    </tdml:infoset>
+  </tdml:parserTestCase>
+
 </tdml:testSuite>
diff --git 
a/daffodil-test/src/test/scala/org/apache/daffodil/section07/variables/TestVariables.scala
 
b/daffodil-test/src/test/scala/org/apache/daffodil/section07/variables/TestVariables.scala
index 9f81647..0a29c75 100644
--- 
a/daffodil-test/src/test/scala/org/apache/daffodil/section07/variables/TestVariables.scala
+++ 
b/daffodil-test/src/test/scala/org/apache/daffodil/section07/variables/TestVariables.scala
@@ -60,8 +60,16 @@ class TestVariables {
   @Test def test_varInstance_10(): Unit = { 
runner.runOneTest("varInstance_10") }
   @Test def test_varInstance_11(): Unit = { 
runner.runOneTest("varInstance_11") }
   @Test def test_varInstance_12(): Unit = { 
runner.runOneTest("varInstance_12") }
-  // Causes OOLAG Circular Definition
-  // @Test def test_defineVariable_nonConstantExpression(): Unit = { 
runner.runOneTest("defineVariable_nonConstantExpression") }
+
+  @Test def test_defineVariable_nonConstantExpression(): Unit = { 
runner.runOneTest("defineVariable_nonConstantExpression") }
+  @Test def test_defineVariable_nonConstantExpression_unp(): Unit = { 
runner.runOneTest("defineVariable_nonConstantExpression_unp") }
+  @Test def test_circular_defineVariable_err(): Unit = { 
runner.runOneTest("circular_defineVariable_err") }
+  @Test def test_defineVariable_ref_noDefault_err(): Unit = { 
runner.runOneTest("defineVariable_ref_noDefault_err") }
+  @Test def test_defineVariable_nonConstantExpression_setVar_err(): Unit = { 
runner.runOneTest("defineVariable_nonConstantExpression_setVar_err") }
+
+  // This test triggers an unhandled NoSuchElement exception, which if handled 
then runs into an Assert.invariant
+  //@Test def test_defineVariable_ref_infoset_err(): Unit = { 
runner.runOneTest("defineVariable_ref_infoset_err") }
+
   @Test def test_setVarChoice(): Unit = { runner.runOneTest("setVarChoice") }
   @Test def test_unparse_setVarChoice(): Unit = { 
runner.runOneTest("unparse_setVarChoice") }
   @Test def test_setVarOnSeqAndElemRef(): Unit = { 
runner.runOneTest("setVarOnSeqAndElemRef") }

Reply via email to