[GitHub] [daffodil] mbeckerle commented on a change in pull request #510: New variable instances should inherit external values

[GitBox <gi...@apache.org>]

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



##########
File path: daffodil-runtime1-unparser/src/main/scala/org/apache/daffodil/processors/unparsers/ExpressionEvaluatingUnparsers.scala
##########
@@ -104,7 +104,10 @@ class NewVariableInstanceStartUnparser(override val context: VariableRuntimeData
 
   override def unparse(state: UState) = {
     state.variableMap.newVariableInstance(context)
-    if (context.maybeDefaultValueExpr.isDefined) {
+
+    // Only process the default value expression if it is defined and only if
+    // the variable is non-external or has a local default
+    if (context.maybeDefaultValueExpr.isDefined && (!context.external || context.hasLocalDefault)) {

Review comment:
       I think given an NVI without a default value, and a defineVariable with an expression for the default value, the assumption should be that the defineVariable default value is computed once, at start of processing, and that value is reused. The expression would not be re-evaluated when the NVI instance is created. 
   
   Suppose your var2 was also external="true", so that the default value can be overridden externally. In that case the default value expression would never be evaluated. 
   
   There is no place in DFDL where "inherit the expression" semantics is used I think. 
   
   E.g., suppose you use a property from a dfdl:format default format and there is an expression used there. That expression is evaluated once only. Not every time that property is referenced. If that expression references variables then the values of those variables are only the global values of them. 
   
   The "inherit the expression" semantics can be obtained by creating new variable instances of both your var1 and var2 where var2 has its own default value expression of "{ $var1 }".  
   
   `<dfdl:newVariableInstance ref="var1" />
   <dfd:setVariable ref="var1" value="Y" />
   <dfdl:newVariableInstance ref="var2" defaultValue="{ $var1 }"/>`
   
   Here var2's default value would be Y. 




----------------------------------------------------------------
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