[GitHub] [daffodil] tuxji commented on a change in pull request #674: Work around issue resetting predefined vars

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

tuxji commented on a change in pull request #674:
URL: https://github.com/apache/daffodil/pull/674#discussion_r742845282



##########
File path: daffodil-runtime1/src/main/scala/org/apache/daffodil/processors/VariableMap1.scala
##########
@@ -155,6 +155,7 @@ class VariableInstance private (val rd: VariableRuntimeData)
         this.setState(this.priorState)
         this.setValue(this.priorValue)
       }
+      case (VariableDefined, _, _) => // This should only occur for pre-defined variables

Review comment:
       This change definitely looks too simple to me.  At the very least it should verify some invariant or assumption so that we will be alerted if a deeper problem exists.  I'm not sure exactly what the original problem is or what invariant needs to be checked, but surely there must be something you can put into an `Assert.invariant(...)` call at this line.
   
   Perhaps the invariant should be whether the variable has ever been read?  Or whether it ever has been shadowed by a new variable instance?  Again, I'm not sure what problem is occurring or which part of the DFDL specification applies to this situation (the verb "reset" does not occur anywhere in the specification!), although I looked at [7.7 DFDL Variable Annotations](https://daffodil.apache.org/docs/dfdl/#_Toc62570090) and noted the following statements:
   
   > Variables are defined at the top-level of a schema and have a specific simple type. A distinction is made between the variable as defined, and an instance of the variable where a value can be stored. The dfdl:defineVariable also introduces a single unique global instance of the variable. Additional instances may be allocated in a stack-like fashion using dfdl:newVariableInstance which causes new instances to come into existence upon entry to the scope of a model group, and these instances go away on exit from the same. **[Is there where the function "reset" comes in?]**
   
   > DFDL variables only vary in the sense that different instances of the same variable can have different values. A single instance of a variable only ever takes on a single value. Each variable instance is a single-assignment location for a value[9]. Once a variable instance's value has been read, it can never be assigned again. If it has not yet been assigned, and its default value has not been read, then a variable instance can be assigned once using dfdl:setVariable. **[Can you find some invariant to check from this part of the specification?]**
   
   > The following variables are predefined, and their names are in the DFDL namespace (http://www.ogf.org/dfdl/dfdl-1.0/). These variables are expected to be commonly set externally so are predefined for convenience. **[Can a predefined variable ever take on a different value (I know it has a default value) if it was set externally but did not have its value read before a dfdl:setVariable?]**




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

To unsubscribe, e-mail: commits-unsubscribe@daffodil.apache.org

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