mbeckerle commented on pull request #645: URL: https://github.com/apache/daffodil/pull/645#issuecomment-928250867
I actually believe the "fix" in this PR is wrong. The code was correct before. Just a little confusing. PState contains the changedVariablesStack, which must be maintained whenever variables are set or read or newInstances created. The variableMap.readVariable method was doing a callback to PState to maintain this changedVariablesStack. That means all uses of readVariable, including from DPath VRef, were doing this maintenance. However, variableMap.setVariable does no such callback. Instead, code that is setting variables is supposed to call the PState.setVariable method. which does this changedVariablesStack maintenance. The reality: There's more state to maintain having to do with variables than just the variableMap, so all manipulation of variables SHOULD go by way of PState/UState methods, and those should delegate things to the VariableMap object, and maintain other things like changedVariablesStack. So the change so that nobody uses variableMap directly, and instead uses variables by way of PState/UState is the way to go, and the uses of variableMap directly need to go away, and the callback from variableMap to PState also should go away. -- 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]
