You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@daffodil.apache.org by ja...@apache.org on 2020/07/31 14:41:16 UTC

[incubator-daffodil] branch master updated (38f6f3d -> 05d430d)

This is an automated email from the ASF dual-hosted git repository.

jadams pushed a change to branch master
in repository https://gitbox.apache.org/repos/asf/incubator-daffodil.git.


    from 38f6f3d  Added test case for DAFFODIL-2375
     new b4d9e0f  Fix variables getting reset incorrectly
     new 05d430d  Added comment about PState.Mark's

The 2 revisions listed above as "new" are entirely new to this
repository and will be described in separate emails.  The revisions
listed as "add" were already present in the repository and have only
been added to this reference.


Summary of changes:
 .../daffodil/processors/parsers/PState.scala       | 15 ++++----
 .../daffodil/section07/variables/variables.tdml    | 41 ++++++++++++++++++++++
 .../section07/variables/TestVariables.scala        |  3 ++
 3 files changed, 51 insertions(+), 8 deletions(-)


[incubator-daffodil] 01/02: Fix variables getting reset incorrectly

Posted by ja...@apache.org.
This is an automated email from the ASF dual-hosted git repository.

jadams pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/incubator-daffodil.git

commit b4d9e0fe1e2da4b55c875ddde58c5e105ed9798c
Author: Josh Adams <ja...@tresys.com>
AuthorDate: Mon Jul 27 15:27:50 2020 -0400

    Fix variables getting reset incorrectly
    
    When the variables infrastructure was changed to be mutable, a
    changedVariablesStack was introduced to track what variables had been
    changed in the current scope. These variables would be reset when the
    PState was reset. The problem was that new lists of variables would only
    be pushed when entering a point of uncertainty and popped when leaving,
    however the call to PState.reset() can happen outside of points of
    uncertainty, such as with nillable elements.
    
    This change makes it so that whenever a PState.Mark is created, a new
    list of changed variables is pushed on the stack. When a mark is
    discarded or the PState is reset, we pop the stack.
    
    DAFFODIL-2374
---
 .../daffodil/processors/parsers/PState.scala       | 10 ++----
 .../daffodil/section07/variables/variables.tdml    | 41 ++++++++++++++++++++++
 .../section07/variables/TestVariables.scala        |  3 ++
 3 files changed, 47 insertions(+), 7 deletions(-)

diff --git a/daffodil-runtime1/src/main/scala/org/apache/daffodil/processors/parsers/PState.scala b/daffodil-runtime1/src/main/scala/org/apache/daffodil/processors/parsers/PState.scala
index 1ce5002..6cf5d64 100644
--- a/daffodil-runtime1/src/main/scala/org/apache/daffodil/processors/parsers/PState.scala
+++ b/daffodil-runtime1/src/main/scala/org/apache/daffodil/processors/parsers/PState.scala
@@ -197,6 +197,7 @@ final class PState private (
 
   def mark(requestorID: String): PState.Mark = {
     // threadCheck()
+    changedVariablesStack.push(mutable.MutableList[GlobalQName]())
     val m = markPool.getFromPool(requestorID)
     m.captureFrom(this, requestorID)
     m
@@ -218,19 +219,14 @@ final class PState private (
       if (variable.isDefined)
         variable.get.reset
     }}
-    /* When parsing choices or unordered sequences it is necessary to clear the
-     * list at the top of the stack because it is possible for multiple
-     * PState.reset calls to occur withing the same point of uncertainty. If we
-     * do not clear the list, the changes made in failed branches of the choice
-     * will accumulate even though their effects have already been reset.
-     */
-    changedVariablesStack.top.clear
+    changedVariablesStack.pop
   }
 
   def discard(m: PState.Mark): Unit = {
     dataInputStream.discard(m.disMark)
     m.clear()
     markPool.returnToPool(m)
+    changedVariablesStack.pop
   }
 
   override def toString() = {
diff --git a/daffodil-test/src/test/resources/org/apache/daffodil/section07/variables/variables.tdml b/daffodil-test/src/test/resources/org/apache/daffodil/section07/variables/variables.tdml
index 35eb691..d7fc1c9 100644
--- a/daffodil-test/src/test/resources/org/apache/daffodil/section07/variables/variables.tdml
+++ b/daffodil-test/src/test/resources/org/apache/daffodil/section07/variables/variables.tdml
@@ -1754,4 +1754,45 @@
     </tdml:infoset>
   </tdml:parserTestCase>
 
+  <tdml:defineSchema name="variables_nilled_element">
+    <xs:include schemaLocation="org/apache/daffodil/xsd/DFDLGeneralFormat.dfdl.xsd"/>
+    <dfdl:format ref="ex:GeneralFormat"
+      nilKind="literalValue" nilValue="%ES;" useNilForDefault="no" nilValueDelimiterPolicy="none" />
+
+    <dfdl:defineVariable name="var1" type="xs:string" defaultValue="false" />
+    <xs:element name="root">
+      <xs:complexType>
+        <xs:sequence dfdl:separator=",">
+          <xs:element name="id" type="xs:unsignedShort" dfdl:lengthKind="explicit" dfdl:length="1">
+            <xs:annotation>
+              <xs:appinfo source="http://www.ogf.org/dfdl/">
+                <dfdl:setVariable ref="var1" value="true" />
+              </xs:appinfo>
+            </xs:annotation>
+          </xs:element>
+          <xs:element name="nillable_but_present" type="xs:string" nillable="true" dfdl:lengthKind="explicit" dfdl:length="4" />
+          <xs:choice dfdl:choiceDispatchKey="{ $ex:var1 }">
+            <xs:element name="var1_true" type="xs:string" dfdl:choiceBranchKey="true" dfdl:lengthKind="explicit" dfdl:length="7" />
+            <xs:element name="var1_false" type="xs:string" dfdl:choiceBranchKey="false" dfdl:lengthKind="explicit" dfdl:length="7" />
+          </xs:choice>
+        </xs:sequence>
+      </xs:complexType>
+    </xs:element>
+
+  </tdml:defineSchema>
+
+  <tdml:parserTestCase name="variables_nilled_element" root="root"
+    model="variables_nilled_element" description="Section 7 - setVariable - verify that variables aren't getting reset incorrectly - DAFFODIL-2374">
+    <tdml:document>1,test,is set?</tdml:document>
+    <tdml:infoset>
+      <tdml:dfdlInfoset>
+        <root>
+          <id>1</id>
+          <nillable_but_present>test</nillable_but_present>
+          <var1_true>is set?</var1_true>
+        </root>
+      </tdml:dfdlInfoset>
+    </tdml:infoset>
+  </tdml:parserTestCase>
+
 </tdml:testSuite>
diff --git a/daffodil-test/src/test/scala/org/apache/daffodil/section07/variables/TestVariables.scala b/daffodil-test/src/test/scala/org/apache/daffodil/section07/variables/TestVariables.scala
index 79d62b8..0a330af 100644
--- a/daffodil-test/src/test/scala/org/apache/daffodil/section07/variables/TestVariables.scala
+++ b/daffodil-test/src/test/scala/org/apache/daffodil/section07/variables/TestVariables.scala
@@ -93,6 +93,9 @@ class TestVariables {
 
   @Test def test_unsignedIntVarCast(): Unit = { runner.runOneTest("unsignedIntVarCast") }
 
+  // DFDL-2374
+  @Test def test_variables_nilled_element(): Unit = { runner.runOneTest("variables_nilled_element") }
+
   // DFDL-2375
   //@Test def test_choiceBranchVariables(): Unit = { runner.runOneTest("choiceBranchVariables") }
 


[incubator-daffodil] 02/02: Added comment about PState.Mark's

Posted by ja...@apache.org.
This is an automated email from the ASF dual-hosted git repository.

jadams pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/incubator-daffodil.git

commit 05d430d2b6f9c9c8c654f08a96d08ca630d0a5b6
Author: Josh Adams <ja...@tresys.com>
AuthorDate: Fri Jul 31 09:57:26 2020 -0400

    Added comment about PState.Mark's
---
 .../main/scala/org/apache/daffodil/processors/parsers/PState.scala   | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/daffodil-runtime1/src/main/scala/org/apache/daffodil/processors/parsers/PState.scala b/daffodil-runtime1/src/main/scala/org/apache/daffodil/processors/parsers/PState.scala
index 6cf5d64..8b3d8d7 100644
--- a/daffodil-runtime1/src/main/scala/org/apache/daffodil/processors/parsers/PState.scala
+++ b/daffodil-runtime1/src/main/scala/org/apache/daffodil/processors/parsers/PState.scala
@@ -169,7 +169,10 @@ final class PState private (
   /**
    * This stack tracks variables that have changed within the current point of
    * uncertainty. This tracking is necessary to revert changes made to variables
-   * when the parser needs to backtrack.
+   * when the parser needs to backtrack. This stack needs to be pushed when a
+   * new mark is created and popped when it is discarded or reset. It is
+   * necessary for every mark to either be discarded or reset to inorder for
+   * this stack to funciton correctly.
    */
   private val changedVariablesStack = new MStackOf[mutable.MutableList[GlobalQName]]()
   changedVariablesStack.push(mutable.MutableList[GlobalQName]())