stevedlawrence commented on code in PR #1097:
URL: https://github.com/apache/daffodil/pull/1097#discussion_r1367068944
##########
daffodil-runtime1/src/main/scala/org/apache/daffodil/runtime1/processors/VariableMap1.scala:
##########
@@ -384,41 +376,34 @@ class VariableMap private (vTable: Map[GlobalQName,
ArrayBuffer[VariableInstance
case _ => // Do nothing
}
- val variableInstances = vTable.get(varQName)
- if (variableInstances.isDefined) {
- val variable = variableInstances.get.last
- variable.state match {
- case VariableRead if (variable.value.isDefined) =>
variable.value.getNonNullable
- case VariableDefined | VariableSet if (variable.value.isDefined) => {
- 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 VariableUndefined if
(variable.rd.maybeDefaultValueExpr.isDefined) => {
- variable.setState(VariableBeingDefined)
- val res =
-
DataValue.unsafeFromAnyRef(variable.rd.maybeDefaultValueExpr.get.evaluate(state))
-
- // Need to update the variable's value with the result of the
- // expression
- variable.setState(VariableRead)
- variable.setValue(res)
-
- res
- }
- case VariableBeingDefined => throw new
VariableCircularDefinition(varQName, vrd)
- case VariableInProcess => throw new VariableSuspended(varQName, vrd)
- case _ => throw new VariableHasNoValue(varQName, vrd)
+ val variable = vTable(vrd.vmapIndex).head
+ variable.state match {
+ case VariableRead if (variable.value.isDefined) =>
variable.value.getNonNullable
+ case VariableDefined | VariableSet if (variable.value.isDefined) => {
+ 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 VariableUndefined if (variable.rd.maybeDefaultValueExpr.isDefined)
=> {
+ variable.setState(VariableBeingDefined)
+ val res =
+
DataValue.unsafeFromAnyRef(variable.rd.maybeDefaultValueExpr.get.evaluate(state))
+
+ // Need to update the variable's value with the result of the
+ // expression
+ variable.setState(VariableRead)
+ variable.setValue(res)
Review Comment:
> There is a codecov warning saying that
daffodil-runtime1/src/main/scala/org/apache/daffodil/runtime1/processors/VariableMap1.scala#L398-L399
were not covered by tests. This warning may or may not be spurious because
these 2 lines are preceded by 2 lines which received no warnings and these 4
lines should have been executed together or not at all.
According to codecov it does look like these two lines are currently covered
in the main branch:
https://app.codecov.io/gh/apache/daffodil/blob/main/daffodil-runtime1%2Fsrc%2Fmain%2Fscala%2Forg%2Fapache%2Fdaffodil%2Fruntime1%2Fprocessors%2FVariableMap1.scala#L408
And I've added an Assert that confirms that these lines are in fact not
being hit now.
The only thing I can think of is maybe the order of things have changed (for
example, looping over a Map will give a different order than looping over an
Array), and now the `evaluate()` call right before these lines throws an
exception earlier than it did before and doesn't give a chance to hit these
lines. I'll see if I can modify a test that get this coverage back.
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
