mbeckerle commented on a change in pull request #369:
URL: https://github.com/apache/incubator-daffodil/pull/369#discussion_r413195711
##########
File path:
daffodil-core/src/main/scala/org/apache/daffodil/grammar/HasStatementsGrammarMixin.scala
##########
@@ -18,10 +18,11 @@
package org.apache.daffodil.grammar
import org.apache.daffodil.dsom.Term
+import org.apache.daffodil.dsom.DFDLNewVariableInstance
trait HasStatementsGrammarMixin extends GrammarMixin { self: Term =>
- private lazy val statementGrams = statements.map { _.gram(self) }
+ private lazy val statementGrams = statements.filter{ st =>
!st.isInstanceOf[DFDLNewVariableInstance] }.map { _.gram(self) }
// TODO: statements (but specifically not newVariableInstance) can appear on
simple type definitions as well as terms.
Review comment:
Remove "TODO" from comment?
##########
File path:
daffodil-core/src/main/scala/org/apache/daffodil/dsom/DFDLDefineVariable.scala
##########
@@ -109,13 +109,68 @@ abstract class VariableReference(node: Node, decl:
AnnotatedSchemaComponent)
final class DFDLNewVariableInstance(node: Node, decl: AnnotatedSchemaComponent)
extends VariableReference(node, decl) // with
NewVariableInstance_AnnotationMixin
{
- lazy val defaultValue = getAttributeOption("defaultValue")
+ // newVariableInstance only allowed within group ref, sequence, or choice
+ decl match {
+ case gr: GroupRef =>
+ case grl: GroupDefLike =>
+ case _ => decl.SDE("newVariableInstance may only be used on group
reference, sequence or choice")
+ }
+
+ private lazy val attrValue = getAttributeOption("value")
+
+ private lazy val <dfdl:setVariable>{ eltChildren @ _* }</dfdl:setVariable> =
node
+
+ private lazy val eltValue = eltChildren.text.trim
+
+ private lazy val defaultValueAsAttribute = getAttributeOption("defaultValue")
+ private lazy val defaultValueAsElement = node.child.text.trim
+
+ final lazy val defaultValue = (defaultValueAsAttribute,
defaultValueAsElement) match {
+ case (None, "") => defv.defaultValue
+ case (None, str) => Some(str)
+ case (Some(str), "") => Some(str)
+ case (Some(str), v) => schemaDefinitionError("Default value of variable
was supplied both as attribute and element value: %s", node.toString)
+ }
+
+ final lazy val value = (attrValue, eltValue) match {
+ case (None, v) if (v != "") => v
+ case (Some(v), "") => v
+ case (Some(v), ev) if (ev != "") => decl.SDE("Cannot have both a value
attribute and an element value: %s", node)
+ case (None, "") => decl.SDE("Must have either a value attribute or an
element value: %s", node)
+ }
- def gram(term: Term): Gram = Assert.nyi("dfdl:newVariableInstance")
- def endGram(term: Term): Gram = Assert.nyi("dfdl:newVariableInstance")
+ lazy val maybeDefaultValueExpr = {
+ val compilationTargetType = defv.primType
+ val qn = this.qNameForProperty("defaultValue", XMLUtils.dafintURI)
+ val defaultValExpr = defaultValue.map { e =>
+ ExpressionCompilers.AnyRef.compileProperty(qn, compilationTargetType,
Found(e, this.dpathCompileInfo, "defaultValue", false), this, dpathCompileInfo)
+ }
+
+ Maybe.toMaybe(defaultValExpr)
+ }
+
+ final override lazy val variableRuntimeData = {
Review comment:
Why are you constructing a new variableRuntimeData rather than just
accessing the one that is created from the definition?
I think two reasons. One is the default value can be different. Two is that
diagnostic messages want to point at this location, and blame the
newVariableInstance object, not the defineVariable declaration.
So please add a comment to that effect.
##########
File path:
daffodil-runtime1/src/main/scala/org/apache/daffodil/processors/DataProcessor.scala
##########
@@ -313,21 +309,19 @@ class DataProcessor private (
*/
@deprecated("Use withExternalVariables.", "2.6.0")
def setExternalVariables(extVars: File, tunable: DaffodilTunables): Unit = {
- val (newMap, newBindings) = newVariableMap(extVars)
- variableMap = newMap
+ val newBindings = loadExternalVariables(extVars)
externalVars = newBindings
}
@deprecated("Use withExternalVariables.", "2.6.0")
def setExternalVariables(extVars: Seq[Binding]): Unit = {
- val (newMap, newBindings) = newVariableMap(extVars)
- variableMap = newMap
+ val newBindings = loadExternalVariables(extVars)
externalVars = newBindings
}
def withExternalVariables(extVars: Seq[Binding]): DataProcessor = {
- val (newMap, newBindings) = newVariableMap(extVars)
- copy(variableMap = newMap, externalVars = newBindings)
+ val newBindings = loadExternalVariables(extVars)
+ copy(variableMap = variableMap, externalVars = newBindings)
Review comment:
And here also.
##########
File path:
daffodil-runtime1/src/main/scala/org/apache/daffodil/processors/DataProcessor.scala
##########
@@ -260,48 +258,46 @@ class DataProcessor private (
copy(areDebugging = flag, tunables = newTunables)
}
- private def newVariableMap(extVars: Map[String, String]): (VariableMap,
Queue[Binding]) = {
+ private def loadExternalVariables(extVars: Map[String, String]):
Queue[Binding] = {
val bindings = ExternalVariablesLoader.mapToBindings(extVars)
val newVars = externalVars ++ bindings
- val newMap = ExternalVariablesLoader.loadVariables(bindings, ssrd,
variableMap)
- (newMap, newVars)
+ ExternalVariablesLoader.loadVariables(bindings, ssrd, variableMap)
+ newVars
}
- private def newVariableMap(extVars: File): (VariableMap, Queue[Binding]) = {
+ private def loadExternalVariables(extVars: File): Queue[Binding] = {
val bindings = ExternalVariablesLoader.fileToBindings(extVars)
val newVars = externalVars ++ bindings
- val newMap = ExternalVariablesLoader.loadVariables(bindings, ssrd,
variableMap)
- (newMap, newVars)
+ ExternalVariablesLoader.loadVariables(bindings, ssrd, variableMap)
+ newVars
}
- private def newVariableMap(bindings: Seq[Binding]): (VariableMap,
Queue[Binding]) = {
+ private def loadExternalVariables(bindings: Seq[Binding]): Queue[Binding] =
{
val newVars = externalVars ++ bindings
- val newMap = ExternalVariablesLoader.loadVariables(bindings, ssrd,
variableMap)
- (newMap, newVars)
+ ExternalVariablesLoader.loadVariables(bindings, ssrd, variableMap)
+ newVars
}
@deprecated("Use withExternalVariables.", "2.6.0")
def setExternalVariables(extVars: Map[String, String]): Unit = {
- val (newMap, newBindings) = newVariableMap(extVars)
- variableMap = newMap
+ val newBindings = loadExternalVariables(extVars)
externalVars = newBindings
}
def withExternalVariables(extVars: Map[String, String]): DataProcessor = {
- val (newMap, newBindings) = newVariableMap(extVars)
- copy(variableMap = newMap, externalVars = newBindings)
+ val newBindings = loadExternalVariables(extVars)
+ copy(variableMap = variableMap, externalVars = newBindings)
Review comment:
If variableMap is unmodified here, then you don't need the copy call to
mention it.
##########
File path:
daffodil-runtime1/src/main/scala/org/apache/daffodil/processors/DataProcessor.scala
##########
@@ -260,48 +258,46 @@ class DataProcessor private (
copy(areDebugging = flag, tunables = newTunables)
}
- private def newVariableMap(extVars: Map[String, String]): (VariableMap,
Queue[Binding]) = {
+ private def loadExternalVariables(extVars: Map[String, String]):
Queue[Binding] = {
val bindings = ExternalVariablesLoader.mapToBindings(extVars)
val newVars = externalVars ++ bindings
- val newMap = ExternalVariablesLoader.loadVariables(bindings, ssrd,
variableMap)
- (newMap, newVars)
+ ExternalVariablesLoader.loadVariables(bindings, ssrd, variableMap)
+ newVars
}
- private def newVariableMap(extVars: File): (VariableMap, Queue[Binding]) = {
+ private def loadExternalVariables(extVars: File): Queue[Binding] = {
val bindings = ExternalVariablesLoader.fileToBindings(extVars)
val newVars = externalVars ++ bindings
- val newMap = ExternalVariablesLoader.loadVariables(bindings, ssrd,
variableMap)
- (newMap, newVars)
+ ExternalVariablesLoader.loadVariables(bindings, ssrd, variableMap)
+ newVars
}
- private def newVariableMap(bindings: Seq[Binding]): (VariableMap,
Queue[Binding]) = {
+ private def loadExternalVariables(bindings: Seq[Binding]): Queue[Binding] =
{
val newVars = externalVars ++ bindings
- val newMap = ExternalVariablesLoader.loadVariables(bindings, ssrd,
variableMap)
- (newMap, newVars)
+ ExternalVariablesLoader.loadVariables(bindings, ssrd, variableMap)
+ newVars
}
@deprecated("Use withExternalVariables.", "2.6.0")
def setExternalVariables(extVars: Map[String, String]): Unit = {
- val (newMap, newBindings) = newVariableMap(extVars)
- variableMap = newMap
+ val newBindings = loadExternalVariables(extVars)
externalVars = newBindings
}
def withExternalVariables(extVars: Map[String, String]): DataProcessor = {
- val (newMap, newBindings) = newVariableMap(extVars)
- copy(variableMap = newMap, externalVars = newBindings)
+ val newBindings = loadExternalVariables(extVars)
+ copy(variableMap = variableMap, externalVars = newBindings)
}
@deprecated("Use withExternalVariables.", "2.6.0")
def setExternalVariables(extVars: File): Unit = {
- val (newMap, newBindings) = newVariableMap(extVars)
- variableMap = newMap
+ val newBindings = loadExternalVariables(extVars)
externalVars = newBindings
}
def withExternalVariables(extVars: File): DataProcessor = {
- val (newMap, newBindings) = newVariableMap(extVars)
- copy(variableMap = newMap, externalVars = newBindings)
+ val newBindings = loadExternalVariables(extVars)
+ copy(variableMap = variableMap, externalVars = newBindings)
Review comment:
Here also. No new map, so drop the "variableMap = variableMap" from the
copy call.
##########
File path:
daffodil-runtime1/src/main/scala/org/apache/daffodil/processors/VariableMap1.scala
##########
@@ -53,28 +54,54 @@ case object VariableRead extends VariableState
case object VariableInProcess extends VariableState
/**
- * Core tuple of a pure functional "state" for variables.
+ * Core tuple of a "state" for variables.
Review comment:
Since this is real state, not some functional programming quasi-state,
we can remove the air-quotes around "state".
##########
File path:
daffodil-runtime1/src/main/scala/org/apache/daffodil/processors/VariableMap1.scala
##########
@@ -53,28 +54,54 @@ case object VariableRead extends VariableState
case object VariableInProcess extends VariableState
/**
- * Core tuple of a pure functional "state" for variables.
+ * Core tuple of a "state" for variables.
Review comment:
In the DFDL spec., I'm finding I need to use terms Variable and Variable
Instance to be clear about what is the information that is shared by all
instances, and what is a specific value-containing object.
I would suggest renaming this to VariableInstance.
##########
File path:
daffodil-runtime1/src/main/scala/org/apache/daffodil/processors/VariableMap1.scala
##########
@@ -295,14 +322,40 @@ class VariableMap private(vTable: Map[GlobalQName,
List[List[Variable]]])
} else {
bindingQName
}
- val optMapTiers = vTable.get(varQName.toGlobalQName)
- optMapTiers match {
- case None => referringContext.schemaDefinitionError("unknown variable
%s", varQName)
- // There should always be a list with at least one tier in it (the
global tier).
- case x @ Some(firstTier :: enclosingScopes) => {
- firstTier match {
-
- case Variable(VariableDefined, v, ctxt, defaultExpr) :: rest if
(v.isDefined && ctxt.external) => {
+ val stack = vTable.get(varQName.toGlobalQName)
+ if (!stack.isDefined)
+ referringContext.schemaDefinitionError("unknown variable %s", varQName)
+ else {
+ val variable = stack.get.top
+ variable.state match {
+ case VariableDefined if (!variable.rd.external) => {
+ referringContext.SDE("Cannot set variable %s externally. State was:
%s. Existing value: %s.",
+ variable.rd.globalQName, VariableDefined, variable.value)
+ }
+
+ case VariableUndefined if (!variable.rd.external) => {
+ referringContext.SDE("Cannot set variable %s externally. State was:
%s.", variable.rd.globalQName, VariableUndefined)
+ }
+
+ case VariableSet => {
+ referringContext.SDE("Cannot externally set variable %s twice. State
was: %s. Existing value: %s",
+ variable.rd.globalQName, VariableSet, variable.value)
+ }
+
+ case VariableRead => {
+ referringContext.SDE("Cannot externally set variable %s after
reading the default value. State was: %s. Existing value: %s",
+ variable.rd.globalQName, VariableSet, variable.value)
+ }
+
+ case _ => {
+ variable.setValue(VariableUtils.convert(newValue.getAnyRef.toString,
variable.rd))
+ variable.setState(VariableDefined)
+ }
+ }
+ }
+ }
+
+/* case Variable(VariableDefined, v, ctxt, defaultExpr) :: rest if
(v.isDefined && ctxt.external) => {
Review comment:
remove block of commented code here?
##########
File path:
daffodil-runtime1/src/main/scala/org/apache/daffodil/processors/parsers/PState.scala
##########
@@ -205,6 +210,12 @@ final class PState private (
m.restoreInto(this)
m.clear()
markPool.returnToPool(m)
+ changedVariablesStack.top.foreach { v => {
+ val variable = variableMap.find(v)
+ if (variable.isDefined)
+ variable.get.reset
+ }}
+ changedVariablesStack.top.clear
Review comment:
Is this clear needed? You are going to have to pop this stack, and when
you push, it will always be a new mutable list.
It's fine to do this clear, because if for example, this was changed to use
a pool of mutable lists (to avoid allocating them), then you *would* need the
clear.
##########
File path:
daffodil-runtime1/src/main/scala/org/apache/daffodil/processors/VariableMap1.scala
##########
@@ -129,54 +156,66 @@ 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.
*/
-class VariableMap private(vTable: Map[GlobalQName, List[List[Variable]]])
+class VariableMap private(vTable: Map[GlobalQName, MStackOf[Variable]])
extends Serializable {
def this(topLevelVRDs: Seq[VariableRuntimeData] = Nil) =
- this(topLevelVRDs.map {
+ this(Map(topLevelVRDs.map {
vrd =>
val variab = vrd.newVariableInstance
- (vrd.globalQName, List(List(variab)))
- }.toMap)
+ val stack = new MStackOf[Variable]
+ stack.push(variab)
+ (vrd.globalQName, stack)
+ }: _*))
override def toString(): String = {
"VariableMap(" + vTable.mkString(" | ") + ")"
}
+ def copy(): VariableMap = {
+ val table = Map[GlobalQName, MStackOf[Variable]]()
+ vTable.foreach { case (k: GlobalQName, s: MStackOf[Variable]) => {
+ val newStack= new MStackOf[Variable]()
+ newStack.push(s.top.copy())
+ table(k) = newStack
+ }}
+
+ new VariableMap(table)
+ }
+
def find(qName: GlobalQName): Option[Variable] = {
val optVrd = getVariableRuntimeData(qName)
- val optLists = optVrd.flatMap { vrd => vTable.get(vrd.globalQName) }
- val variab = optLists.flatMap { lists => lists.flatMap {
- _.toStream
- }.headOption
+ val optStack = optVrd.flatMap { vrd => vTable.get(vrd.globalQName) }
+ val variab = {
+ if (optStack.isDefined)
+ Some(optStack.get.top)
+ else
+ None
}
variab
}
def getVariableRuntimeData(qName: GlobalQName): Option[VariableRuntimeData]
= {
- val optLists = vTable.get(qName)
- optLists match {
+ val optStack = vTable.get(qName)
+ optStack match {
case None => None // no such variable.
- case Some(lists) => {
- val flatLists = lists.flatten
- Assert.invariant(flatLists.length > 0)
- val varObj = flatLists.head
+ case Some(stack) => {
+ val varObj = stack.top
Some(varObj.rd)
}
}
}
- lazy val context = Assert.invariantFailed("unused.")
-
- private def mkVMapForNewVariable(newVar: Variable, firstTier:
List[Variable], enclosingScopes: List[List[Variable]]) = {
- val newMap = vTable + ((newVar.rd.globalQName, (newVar :: firstTier) ::
enclosingScopes))
- new VariableMap(newMap)
+ def getVaraibleStack(qName: GlobalQName): Option[MStackOf[Variable]] = {
Review comment:
Misspelled method name "getVaraibleStack"
##########
File path:
daffodil-runtime1/src/main/scala/org/apache/daffodil/processors/parsers/PState.scala
##########
@@ -265,17 +276,29 @@ final class PState private (
}
def setVariable(vrd: VariableRuntimeData, newValue: DataValuePrimitive,
referringContext: VariableRuntimeData, pstate: PState) {
- this.setVariableMap(variableMap.setVariable(vrd, newValue,
referringContext, pstate))
+ variableMap.setVariable(vrd, newValue, referringContext, pstate)
+ changedVariablesStack.top += vrd.globalQName
+ }
+
+ def newVariableInstance(vrd: VariableRuntimeData) {
+ variableMap.newVariableInstance(vrd)
+ changedVariablesStack.top += vrd.globalQName
+ }
+
+ def removeVariableInstance(vrd: VariableRuntimeData) {
+ variableMap.removeVariableInstance(vrd)
}
def pushDiscriminator {
// threadCheck()
discriminatorStack.push(false)
+ changedVariablesStack.push(mutable.MutableList[GlobalQName]())
Review comment:
Suggest rename push/popDiscriminator to
pushPointOfUncertainty/popPointOfUncertainty. They are maintaining more
artifacts of state than just the discriminator stack now.
##########
File path:
daffodil-runtime1/src/main/scala/org/apache/daffodil/processors/parsers/PState.scala
##########
@@ -166,6 +168,9 @@ final class PState private (
private val discriminatorStack = MStackOfBoolean()
discriminatorStack.push(false)
+ private val changedVariablesStack = new
MStackOf[mutable.MutableList[GlobalQName]]()
Review comment:
Scala doc, please explain what this is for (which I assume is going to
be backtracking the state-changes to variable-instances.
Each mutable list of GlobalQNames is for one backtracking point, yes?
If I understand this correctly, the operations due to changedVariablesStack
are always going to be on the top of the variableMap stacks of variable
instance objects, because pushing/popping those stacks are about
adding/removing variable instances due to scopes that have newVariableInstance.
A further comment. Using an MStackOf for this, means when you push, you have
to allocate a new MutableList.
You clear these mutable lists later, but if each push is going to require
allocating one, no point in clearing them.
This is already going to be far more efficient than what it is replacing, so
probably this comment is not well motivated, but I think you could avoid
allocating mutable lists entirely if you didn't use an MStack here. That is, if
rather than doing a push, if you just moved to N+1 position of an ArrayBuffer,
then you could look to see if there already was a mutable list there, and only
allocate one if one wasn't already there. If there is one there it should be
empty (because you are clearing them on backtracking).
##########
File path:
daffodil-runtime1/src/main/scala/org/apache/daffodil/processors/unparsers/UState.scala
##########
@@ -676,7 +685,7 @@ object UState {
inputter: InfosetInputter): UStateMain = {
Assert.invariant(inputter.isInitialized)
- val variables = dataProc.variableMap
+ val variables = dataProc.variableMap.copy
Review comment:
Add a comment. Must be copied because variableMap is mutable.
##########
File path:
daffodil-runtime1/src/main/scala/org/apache/daffodil/processors/VariableMap1.scala
##########
@@ -129,54 +156,66 @@ 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.
*/
-class VariableMap private(vTable: Map[GlobalQName, List[List[Variable]]])
+class VariableMap private(vTable: Map[GlobalQName, MStackOf[Variable]])
extends Serializable {
def this(topLevelVRDs: Seq[VariableRuntimeData] = Nil) =
- this(topLevelVRDs.map {
+ this(Map(topLevelVRDs.map {
vrd =>
val variab = vrd.newVariableInstance
- (vrd.globalQName, List(List(variab)))
- }.toMap)
+ val stack = new MStackOf[Variable]
+ stack.push(variab)
+ (vrd.globalQName, stack)
+ }: _*))
override def toString(): String = {
"VariableMap(" + vTable.mkString(" | ") + ")"
}
+ def copy(): VariableMap = {
Review comment:
Please add scala doc as to 1) why this is needed at all 2) the specific
nature of the deep copy going on here and why it has to be that kind of deep
copy - which I believe is because the MStackOf[Variable] are mutable, so those
need to be disjoint.
##########
File path:
daffodil-test/src/test/resources/org/apache/daffodil/section07/variables/variables.tdml
##########
@@ -374,16 +509,111 @@
</tdml:parserTestCase>
- <tdml:parserTestCase name="varInstance" root="a"
- model="v">
+ <tdml:parserTestCase name="varInstance_01" root="nvi1"
+ model="v"
+ description="Simple demonstration of newVariableInstance">
+
+ <tdml:document />
+
+ <tdml:infoset>
+ <tdml:dfdlInfoset>
+ <nvi1 xmlns="http://example.com";>
+ <a xsi:type="xsd:int">7</a>
+ </nvi1>
+ </tdml:dfdlInfoset>
+ </tdml:infoset>
+ </tdml:parserTestCase>
+
+ <tdml:parserTestCase name="varInstance_02" root="nvi2"
+ model="v"
+ description="Demonstrates that newVariableInstance correctly goes into and
out of scope">
+
+ <tdml:document />
+
+ <tdml:infoset>
+ <tdml:dfdlInfoset>
+ <nvi2 xmlns="http://example.com";>
+ <a xsi:type="xsd:int">7</a>
+ <nest1>
+ <b xsi:type="xsd:int">8</b>
+ <nest2>
+ <c xsi:type="xsd:int">9</c>
+ </nest2>
+ <d xsi:type="xsd:int">8</d>
+ </nest1>
+ <e xsi:type="xsd:int">7</e>
+ </nvi2>
+ </tdml:dfdlInfoset>
+ </tdml:infoset>
+ </tdml:parserTestCase>
+
+ <tdml:parserTestCase name="varInstance_03" root="nvi3"
+ model="v"
+ description="Demonstrates using multiple newVariableInstance statements
within a sequence">
+
+ <tdml:document />
+
+ <tdml:infoset>
+ <tdml:dfdlInfoset>
+ <nvi3 xmlns="http://example.com";>
+ <a xsi:type="xsd:int">7</a>
+ <b xsi:type="xsd:int">17</b>
+ <c xsi:type="xsd:int">27</c>
+ </nvi3>
+ </tdml:dfdlInfoset>
+ </tdml:infoset>
+ </tdml:parserTestCase>
+
+ <tdml:parserTestCase name="varInstance_04" root="nvi4"
+ model="v"
+ description="SDE due to default value being set both as attribute and
element value">
+
+ <tdml:document />
+
+ <tdml:errors>
+ <tdml:error>Schema Definition Error</tdml:error>
+ <tdml:error>value of variable was supplied both as attribute and element
value</tdml:error>
+ </tdml:errors>
+
+ </tdml:parserTestCase>
+
+ <tdml:parserTestCase name="varInstance_05" root="nvi5"
+ model="v"
+ description="SDE due to multiple newVariableInstance calls within the same
scope to the same variable">
+
+ <tdml:document />
+
+ <tdml:errors>
+ <tdml:error>Schema Definition Error</tdml:error>
+ <tdml:error>newVariableInstances must all be distinct within the same
scope</tdml:error>
+ </tdml:errors>
+
+ </tdml:parserTestCase>
+
+ <tdml:parserTestCase name="varInstance_06" root="nvi6"
+ model="v"
+ description="SDE due to newVariableInstance being declared on an element
instead of group reference, sequence or choice">
+
+ <tdml:document />
+
+ <tdml:errors>
+ <tdml:error>Schema Definition Error</tdml:error>
+ <tdml:error>newVariableInstance may only be used on group reference,
sequence or choice</tdml:error>
+ </tdml:errors>
+
+ </tdml:parserTestCase>
+
Review comment:
I don't see tests which verifies that unwinding side-effects on
newVariableInstances works properly in the presence of backtracking.
I suggest a test with two defined variables.
You enter a scope that does NVI on each of them.
You enter another scope that does another NVI on each of them.
Then you enter a choice which sets one and reads the default of the other,
then fails, backtracks, and chooses the
next branch, which does the same thing.
The final branch can succeed.
then you unwind to the enclosing scope (different variable instances), and
you shoudl be alble to set and read-default-value of those instances, which
weren't affected by the backtracking on the inner instances.
Then when you finally unwind all the scopes, the global instances of the
variables should be able to set one and read-default from the other, as the
global ones should definitely not have been affected.
##########
File path:
daffodil-runtime1/src/main/scala/org/apache/daffodil/processors/parsers/PState.scala
##########
@@ -501,7 +524,7 @@ object PState {
output: InfosetOutputter,
dataProc: DFDL.DataProcessor): PState = {
- val variables = dataProc.variableMap
+ val variables = dataProc.variableMap.copy
Review comment:
Worth a comment here. This copy is really critical since variable maps
are mutable.
----------------------------------------------------------------
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:
[email protected]
