This is an automated email from the ASF dual-hosted git repository.
slawrence pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/daffodil.git
The following commit(s) were added to refs/heads/main by this push:
new 5e6a11302 Store the last added child in the PState for use in
separator logic
5e6a11302 is described below
commit 5e6a11302ca27a8e78342c6539d3b6801645039c
Author: Steve Lawrence <[email protected]>
AuthorDate: Thu Dec 8 09:25:36 2022 -0500
Store the last added child in the PState for use in separator logic
In some cases, separator logic needs knowledge about the most recently
added child of an element. This is currently handled by examining the
infoset and finding its last child element using the
maybeMostRecentlyAddedChild function.
The problem with this approach is that the InfosetWalker is allowed to
"release" elements from the infoset that it thinks are no longer needed.
There isn't a good way to tell the infoset that these last children are
potentially still needed for separator logic, and so it could actually
release them prior to the separator logic trying to access them, which
leads to a null pointer exception.
To fix this, this modifies the PState to add a slot for the last
modified child of the current infoset element, and modifies the
ElementParser's to set this state appropriately. This way, the
InfosetWalker is free to remove elements as it normally does, but the
last child is still available when needed.
This also removes the maybeMostRecentlyAddedChild functions since this
kind of access to the infoset can lead to null pointers.
Also modifies the SAXInfosetOutputter to check isNilled correctly, which
supports checking if complex elements are nilled without being final
yet. For similar reasons, modifies the InfosetWalker so that it does not
walk into Complex elements if there is a chance that it could be nilled
and we might not be sure.
DAFFODIL-2755
---
.../org/apache/daffodil/infoset/InfosetImpl.scala | 28 ---------------
.../apache/daffodil/infoset/InfosetWalker.scala | 9 ++++-
.../daffodil/infoset/SAXInfosetOutputter.scala | 2 +-
.../processors/parsers/ElementCombinator1.scala | 8 +++--
.../parsers/ExpressionEvaluatingParsers.scala | 6 ++--
.../daffodil/processors/parsers/PState.scala | 42 +++++++++++++++++-----
.../SeparatedSequenceChildParseResultHelper.scala | 2 +-
.../parsers/SequenceChildParseResultHelper.scala | 2 +-
.../section00/general/TestInfosetWalker.scala | 2 +-
9 files changed, 54 insertions(+), 47 deletions(-)
diff --git
a/daffodil-runtime1/src/main/scala/org/apache/daffodil/infoset/InfosetImpl.scala
b/daffodil-runtime1/src/main/scala/org/apache/daffodil/infoset/InfosetImpl.scala
index 678fd2c19..fbb9845ff 100644
---
a/daffodil-runtime1/src/main/scala/org/apache/daffodil/infoset/InfosetImpl.scala
+++
b/daffodil-runtime1/src/main/scala/org/apache/daffodil/infoset/InfosetImpl.scala
@@ -1111,15 +1111,6 @@ final class DIArray(
final def length: Long = _contents.length
- final def maybeMostRecentlyAddedChild(): Maybe[DIElement] = {
- val len = contents.length
- if (len == 0) Maybe.Nope
- else {
- val e = _contents(len - 1)
- Maybe(e)
- }
- }
-
final def totalElementCount: Long = {
var a: Long = 0
_contents.foreach { c => a += c.totalElementCount }
@@ -1633,25 +1624,6 @@ sealed class DIComplex(override val erd:
ElementRuntimeData)
e.setParent(this)
}
- /**
- * Needed because at the point in the code where we need the
- * most recently added child, that node has already been popped from
- * the stack and the current node is its parent. This let's us get
- * a handle on the child just added without changing the invariants of
- * the way the node stack is handled in the PState/UState.
- */
- def maybeMostRecentlyAddedChild(): Maybe[DIElement] = {
- val len = contents.length
- if (len == 0) Maybe.Nope
- else {
- val lastChild = contents(len - 1)
- lastChild match {
- case a: DIArray => a.maybeMostRecentlyAddedChild()
- case e: DIElement => Maybe(e)
- }
- }
- }
-
def addChildToFastLookup(node: DINode): Unit = {
if (node.erd.dpathElementCompileInfo.isReferencedByExpressions) {
val name = node.namedQName
diff --git
a/daffodil-runtime1/src/main/scala/org/apache/daffodil/infoset/InfosetWalker.scala
b/daffodil-runtime1/src/main/scala/org/apache/daffodil/infoset/InfosetWalker.scala
index a03eaff91..e370b695a 100644
---
a/daffodil-runtime1/src/main/scala/org/apache/daffodil/infoset/InfosetWalker.scala
+++
b/daffodil-runtime1/src/main/scala/org/apache/daffodil/infoset/InfosetWalker.scala
@@ -380,7 +380,7 @@ class InfosetWalker private (
} else {
// no blocks on the container, figure out if we can take a step for the
- // element and the child index of this container
+ // element at the child index of this container
val children = containerNode.contents
@@ -399,6 +399,13 @@ class InfosetWalker private (
// doing some parsing for this simple element. Wait until we finish
// that parse before stepping into it.
false
+ } else if (elem.isComplex && elem.numChildren == 0 && !elem.isFinal) {
+ // This is a complex element that has no children and is not final.
+ // This means we don't yet know if it's just an empty complex element
+ // or if it's a complex element that is going to be nilled. We'll
+ // know for sure once it has children or is made final, but for now,
+ // do not step into it.
+ false
} else {
true
}
diff --git
a/daffodil-runtime1/src/main/scala/org/apache/daffodil/infoset/SAXInfosetOutputter.scala
b/daffodil-runtime1/src/main/scala/org/apache/daffodil/infoset/SAXInfosetOutputter.scala
index 65a584b86..9620a18f4 100644
---
a/daffodil-runtime1/src/main/scala/org/apache/daffodil/infoset/SAXInfosetOutputter.scala
+++
b/daffodil-runtime1/src/main/scala/org/apache/daffodil/infoset/SAXInfosetOutputter.scala
@@ -218,7 +218,7 @@ class SAXInfosetOutputter(xmlReader:
DFDL.DaffodilParseXMLReader,
}
// handle xsi:nil attribute
- if (diElem.isNilled) {
+ if (isNilled(diElem)) {
val isNilled = "true"
val nType: String = "CDATA"
val nValue: String = isNilled
diff --git
a/daffodil-runtime1/src/main/scala/org/apache/daffodil/processors/parsers/ElementCombinator1.scala
b/daffodil-runtime1/src/main/scala/org/apache/daffodil/processors/parsers/ElementCombinator1.scala
index e40e9eda8..9cd90b2c8 100644
---
a/daffodil-runtime1/src/main/scala/org/apache/daffodil/processors/parsers/ElementCombinator1.scala
+++
b/daffodil-runtime1/src/main/scala/org/apache/daffodil/processors/parsers/ElementCombinator1.scala
@@ -29,6 +29,8 @@ import org.apache.daffodil.processors.Success
import org.apache.daffodil.processors.TermRuntimeData
import org.apache.daffodil.util.Logger
import org.apache.daffodil.util.Maybe
+import org.apache.daffodil.util.Maybe.Nope
+import org.apache.daffodil.util.Maybe.One
abstract class ElementParserBase(
rd: TermRuntimeData,
@@ -252,7 +254,7 @@ class ElementParser(
}
}
Logger.log.debug(s"priorElement = ${priorElement}")
- pstate.setParent(currentElement)
+ pstate.setInfoset(currentElement, Nope)
}
def parseEnd(pstate: PState): Unit = {
@@ -266,7 +268,7 @@ class ElementParser(
// Execute checkConstraints
validate(pstate)
}
- if (priorElement ne null) pstate.setParent(priorElement)
+ if (priorElement ne null) pstate.setInfoset(priorElement,
One(currentElement))
move(pstate)
} else { // failure.
if (priorElement ne null) {
@@ -274,7 +276,7 @@ class ElementParser(
// But we do not remove the child here. That's done at the
// point of uncertainty when it restores the state of the
// element after a failure.
- pstate.setParent(priorElement)
+ pstate.setInfoset(priorElement, One(currentElement))
}
}
}
diff --git
a/daffodil-runtime1/src/main/scala/org/apache/daffodil/processors/parsers/ExpressionEvaluatingParsers.scala
b/daffodil-runtime1/src/main/scala/org/apache/daffodil/processors/parsers/ExpressionEvaluatingParsers.scala
index dab56a35a..be63333ba 100644
---
a/daffodil-runtime1/src/main/scala/org/apache/daffodil/processors/parsers/ExpressionEvaluatingParsers.scala
+++
b/daffodil-runtime1/src/main/scala/org/apache/daffodil/processors/parsers/ExpressionEvaluatingParsers.scala
@@ -35,6 +35,7 @@ import org.apache.daffodil.processors.TypeCalculator
import org.apache.daffodil.processors.VariableRuntimeData
import org.apache.daffodil.schema.annotation.props.gen.FailureType
import org.apache.daffodil.util.Logger
+import org.apache.daffodil.util.Maybe.Nope
/**
* Common parser base class for any parser that evaluates an expression.
@@ -103,7 +104,8 @@ trait WithDetachedParser {
*/
val priorElement = pstate.infoset
- pstate.setParent(pstate.infoset.diParent)
+ val priorElementLastChild = pstate.infosetLastChild
+ pstate.setInfoset(pstate.infoset.diParent, Nope)
// This isn't actually a point of uncertainty, we just use the logic to
// allow resetting the infoset after we create the detached parser
@@ -123,7 +125,7 @@ trait WithDetachedParser {
res
}
- pstate.setParent(priorElement)
+ pstate.setInfoset(priorElement, priorElementLastChild)
ans
}
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 8244576a8..ed80da48a 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
@@ -150,6 +150,7 @@ class MPState private () {
final class PState private (
var infoset: DIElement,
+ var infosetLastChild: Maybe[DIElement],
var dataInputStream: InputSourceDataInputStream,
val walker: InfosetWalker,
vmap: VariableMap,
@@ -296,17 +297,35 @@ final class PState private (
}
/**
- * This takes newParent as DIElement, not DIComplex, because there is code
- * where we don't know whether the node is simple or complex but we set it as
- * the parent anyway. If simple children will simply not be appended.
+ * Change the current infoset element that we are modifying and the last
+ * child of that element that has been added to that infoset element.
*
- * But this invariant that there is always a parent we could append a child
into
- * is being maintained. THis invariant starts at the very top as there is a
- * Document which is the parent of the root element. So there's no time when
there
- * isn't a parent there.
+ * This takes newInfoset as a DIElement, not DIComplex, because 'infoset'
+ * represents the current DIElement that Daffodil is working on, which could
+ * be either a complex or simple element.
+ *
+ * This also expects the last element that has been added as a child of
+ * newInfoset, sometimes needed for logic related to separated content.
+ *
+ * When starting a new infoset element, this function should be called with
+ * the new element passed as newInfoset (since it is the current infoset
+ * element being modified) and newInfosetLastChild set to Nope (because this
+ * new element does not have any children yet).
+ *
+ * When ending an element, this function should be called with the elements
+ * parent as newInfoset (because we are now modifying the parent, e.g.
+ * adding more children), and newInfosetLastChild should be set to the
element
+ * we just ended (because it is the last child added to newInfoset).
+ *
+ * Note that we must keep track of and store infosetLastChild because by the
+ * time Daffodil needs information about the last child added, the child
+ * could have been released by the InfosetWalker and no longer actually be
+ * part of the infoset. So we cannot query the infoset for this information,
+ * but must instead store it in, and retrieve it from, the PState.
*/
- def setParent(newParent: DIElement): Unit = {
- this.infoset = newParent
+ def setInfoset(newInfoset: DIElement, newInfosetLastChild:
Maybe[DIElement]): Unit = {
+ this.infoset = newInfoset
+ this.infosetLastChild = newInfosetLastChild
}
/**
@@ -561,6 +580,7 @@ object PState {
val simpleElementState = DISimpleState()
val complexElementState = DIComplexState()
var element: DIElement = _
+ var elementLastChild: Maybe[DIElement] = _
var disMark: DataInputStream.Mark = _
var variableMap: VariableMap = _
var processorStatus: ProcessorResult = _
@@ -573,6 +593,7 @@ object PState {
val mpStateMark = new MPState.Mark
def clear(): Unit = {
+ elementLastChild = Nope
simpleElementState.clear()
complexElementState.clear()
disMark = null
@@ -588,6 +609,7 @@ object PState {
def captureFrom(ps: PState, requestorID: String, context: RuntimeData):
Unit = {
this.element = ps.thisElement
+ this.elementLastChild = ps.infosetLastChild
if (element.isSimple)
simpleElementState.captureFrom(element)
else
@@ -619,6 +641,7 @@ object PState {
def restoreInto(ps: PState): Unit = {
restoreInfoset(ps)
+ ps.infosetLastChild = this.elementLastChild
ps.dataInputStream.reset(this.disMark)
ps.setVariableMap(this.variableMap)
ps._processorStatus = this.processorStatus
@@ -701,6 +724,7 @@ object PState {
dis.cst.setPriorBitOrder(root.defaultBitOrder)
val newState = new PState(
doc.asInstanceOf[DIElement],
+ Nope,
dis,
infosetWalker,
variables,
diff --git
a/daffodil-runtime1/src/main/scala/org/apache/daffodil/processors/parsers/SeparatedSequenceChildParseResultHelper.scala
b/daffodil-runtime1/src/main/scala/org/apache/daffodil/processors/parsers/SeparatedSequenceChildParseResultHelper.scala
index 572335ae6..550a3c718 100644
---
a/daffodil-runtime1/src/main/scala/org/apache/daffodil/processors/parsers/SeparatedSequenceChildParseResultHelper.scala
+++
b/daffodil-runtime1/src/main/scala/org/apache/daffodil/processors/parsers/SeparatedSequenceChildParseResultHelper.scala
@@ -304,7 +304,7 @@ class
NonPositionalGroupSeparatedSequenceChildParseResultHelper(
mgrd: ModelGroupRuntimeData,
requiredOptional: RequiredOptionalStatus): ParseAttemptStatus = {
if (pstate.isSuccess) {
- val maybeElem = pstate.infoset.asComplex.maybeMostRecentlyAddedChild()
+ val maybeElem = pstate.infosetLastChild
Assert.invariant(maybeElem.isDefined)
val elem = maybeElem.get
val maybeIsNilled = elem.maybeIsNilled // can't just call isNilled
because that throws exceptions on not defined
diff --git
a/daffodil-runtime1/src/main/scala/org/apache/daffodil/processors/parsers/SequenceChildParseResultHelper.scala
b/daffodil-runtime1/src/main/scala/org/apache/daffodil/processors/parsers/SequenceChildParseResultHelper.scala
index 5d2299848..98bfa3057 100644
---
a/daffodil-runtime1/src/main/scala/org/apache/daffodil/processors/parsers/SequenceChildParseResultHelper.scala
+++
b/daffodil-runtime1/src/main/scala/org/apache/daffodil/processors/parsers/SequenceChildParseResultHelper.scala
@@ -166,7 +166,7 @@ trait ElementSequenceChildParseResultHelper
currentBitPosAfterChild == prevBitPosBeforeChild
}
if (pstate.isSuccess) {
- val maybeElem = pstate.infoset.asComplex.maybeMostRecentlyAddedChild()
+ val maybeElem = pstate.infosetLastChild
Assert.invariant(maybeElem.isDefined)
val elem = maybeElem.get
val maybeIsNilled = elem.maybeIsNilled // can't just call isNilled
because that throws exceptions on not defined
diff --git
a/daffodil-test/src/test/scala/org/apache/daffodil/section00/general/TestInfosetWalker.scala
b/daffodil-test/src/test/scala/org/apache/daffodil/section00/general/TestInfosetWalker.scala
index d4d1dd4a5..8c82a6829 100644
---
a/daffodil-test/src/test/scala/org/apache/daffodil/section00/general/TestInfosetWalker.scala
+++
b/daffodil-test/src/test/scala/org/apache/daffodil/section00/general/TestInfosetWalker.scala
@@ -35,6 +35,6 @@ class TestInfosetWalker {
@Test def test_infosetWalker_01() = { runner2.runOneTest("infosetWalker_01")
}
// DAFFODIL-2755
- /*@Test*/ def test_infosetWalker_02() = {
runner2.runOneTest("infosetWalker_02") }
+ @Test def test_infosetWalker_02() = { runner2.runOneTest("infosetWalker_02")
}
@Test def test_infosetWalker_03() = { runner2.runOneTest("infosetWalker_03")
}
}
