daffodil-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From GitBox <...@apache.org>
Subject [GitHub] [daffodil] mbeckerle commented on a change in pull request #510: New variable instances should inherit external values
Date Fri, 09 Apr 2021 16:16:43 GMT

mbeckerle commented on a change in pull request #510:
URL: https://github.com/apache/daffodil/pull/510#discussion_r610749034



##########
File path: daffodil-runtime1/src/main/scala/org/apache/daffodil/processors/VariableMap1.scala
##########
@@ -103,6 +104,9 @@ class VariableInstance private (
 
   lazy val defaultValueExpr = defaultValueExprArg
 
+  // This represents the default value at the start of processing, provided by

Review comment:
       The VariableInstance constructor takes only a VRD now. So this thread seems to be resolved.


##########
File path: daffodil-runtime1-unparser/src/main/scala/org/apache/daffodil/processors/unparsers/ExpressionEvaluatingUnparsers.scala
##########
@@ -103,13 +101,17 @@ class NewVariableInstanceStartUnparser(override val context: VariableRuntimeData
   override lazy val childProcessors = Vector()
 
   override def unparse(state: UState) = {
-    state.variableMap.newVariableInstance(context)
+    val nvi = state.variableMap.newVariableInstance(context)
+
     if (context.maybeDefaultValueExpr.isDefined) {
-      val maybeVar = state.variableMap.find(context.globalQName)
-      Assert.invariant(maybeVar.isDefined)
-      maybeVar.get.setState(VariableInProcess)
-      val suspendableExpression = new NewVariableInstanceSuspendableExpression(context.maybeDefaultValueExpr.get,
context)
+      val dve = context.maybeDefaultValueExpr.get
+      nvi.setState(VariableInProcess)
+      val suspendableExpression = new NewVariableInstanceDefaultValueSuspendableExpression(dve,
context, nvi)
       suspendableExpression.run(state)
+    } else if (nvi.firstInstanceInitialValue.isDefined) {
+      // The NVI will inherit the default value of the original variable instance
+      // This will also inherit any externally provided bindings.
+      nvi.setDefaultValue(nvi.firstInstanceInitialValue)

Review comment:
       Still need this test case. 

##########
File path: daffodil-runtime1/src/main/scala/org/apache/daffodil/processors/VariableMap1.scala
##########
@@ -328,10 +337,11 @@ class VariableMap private(vTable: Map[GlobalQName, ArrayBuffer[VariableInstance]
         // 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))
+          inst.setDefaultValue(DataValue.unsafeFromAnyRef(res))

Review comment:
       Please capture the comment above, clarifying the VariableDefined with DataValue.NoValue
- what that means and how used, in code comments please. I suggest a comment is needed where
VariableDefined is introduced. 




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



Mime
View raw message