Github user mbeckerle commented on a diff in the pull request:
https://github.com/apache/incubator-daffodil/pull/4#discussion_r149394482
--- Diff:
daffodil-runtime1/src/main/scala/edu/illinois/ncsa/daffodil/processors/parsers/RepParsers.scala
---
@@ -207,87 +253,129 @@ class RepUnboundedParser(occursCountKind:
OccursCountKind.Value, rParser: Parser
extends RepParser(-1, rParser, erd, "Unbounded") {
def parseAllRepeats(initialState: PState): Unit = {
+ var startState: PState.Mark = null
+ var priorState: PState.Mark = null
+ var markLeakCausedByException = false
+
Assert.invariant(initialState.processorStatus eq Success)
- val startState = initialState.mark("RepUnboundedParser1")
- val pstate = initialState
- var priorState = initialState.mark("RepUnboundedParser2")
- var returnFlag = false
- while (!returnFlag && (pstate.processorStatus eq Success)) {
-
- // erd.maxOccurs.foreach { maxOccurs =>
- // if ((occursCountKind == OccursCountKind.Implicit) &&
- // (maxOccurs == -1)) {
- // erd.minOccurs.foreach { minOccurs =>
- // if (pstate.mpstate.arrayPos - 1 <= minOccurs) {
- // // Is required element
- // // Need to trigger default value creation
- // // in right situations (like the element is
defaultable)
- // // This is relatively easy for simple types
- // // for complex types, defaulting is trickier as one
- // // must recursively traverse the type, and then
determine one
- // // has not advanced the data at all.
- // }
- // }
- // }
- // }
-
- //
- // Every parse is a new point of uncertainty.
- pstate.pushDiscriminator
- if (pstate.dataProc.isDefined)
pstate.dataProc.get.beforeRepetition(pstate, this)
-
- try {
- rParser.parse1(pstate)
- } catch {
- case sde: SchemaDefinitionDiagnosticBase => {
- pstate.discard(startState)
- throw sde
- }
- }
- if (pstate.dataProc.isDefined)
pstate.dataProc.get.afterRepetition(pstate, this)
- if (pstate.processorStatus ne Success) {
- //
- // Did not succeed
- //
- // Was a discriminator set?
+ try {
+ val pstate = initialState
+ startState = initialState.mark("RepUnboundedParser1")
+ priorState = initialState.mark("RepUnboundedParser2")
+ var returnFlag = false
+ while (!returnFlag && (pstate.processorStatus eq Success)) {
+
+ // erd.maxOccurs.foreach { maxOccurs =>
+ // if ((occursCountKind == OccursCountKind.Implicit) &&
+ // (maxOccurs == -1)) {
+ // erd.minOccurs.foreach { minOccurs =>
+ // if (pstate.mpstate.arrayPos - 1 <= minOccurs) {
+ // // Is required element
+ // // Need to trigger default value creation
+ // // in right situations (like the element is
defaultable)
+ // // This is relatively easy for simple types
+ // // for complex types, defaulting is trickier as one
+ // // must recursively traverse the type, and then
determine one
+ // // has not advanced the data at all.
+ // }
+ // }
+ // }
+ // }
+
//
- if (pstate.discriminator == true) {
- // we fail the whole RepUnbounded, because there was a
discriminator set
- // before the failure.
- pstate.reset(startState)
- // no need discard priorState, that is implicitly discarded by
resetting the startState
- } else {
+ // Every parse is a new point of uncertainty.
+ pstate.pushDiscriminator
+ if (pstate.dataProc.isDefined)
pstate.dataProc.get.beforeRepetition(pstate, this)
+
+ try {
+ rParser.parse1(pstate)
+ } catch {
+ case sde: SchemaDefinitionDiagnosticBase => {
+ pstate.discard(startState)
+ startState = null
+ priorState = null
+ throw sde
+ }
+ }
+
+ if (pstate.dataProc.isDefined)
pstate.dataProc.get.afterRepetition(pstate, this)
+ if (pstate.processorStatus ne Success) {
//
- // no discriminator, so suppress the failure. Loop terminated
with prior element.
+ // Did not succeed
+ //
+ // Was a discriminator set?
//
+ if (pstate.discriminator == true) {
+ // we fail the whole RepUnbounded, because there was a
discriminator set
+ // before the failure.
+ pstate.reset(startState)
+ startState = null
+ priorState = null
+ // no need discard priorState, that is implicitly discarded by
resetting the startState
+ } else {
+ //
+ // no discriminator, so suppress the failure. Loop terminated
with prior element.
+ //
- log(LogLevel.Debug, "Failure suppressed. This is normal
termination of a occursCountKind='parsed' array.")
- pstate.reset(priorState)
- pstate.discard(startState)
- }
- returnFlag = true
- } else {
- // Success
- // Need to check for forward progress
- if (pstate.bitPos =#= priorState.bitPos0b) {
- pstate.discard(priorState) // didn't move, but might have
assigned variables, have to undo those.
- pstate.discard(startState)
- PE(pstate,
- "RepUnbounded - No forward progress at byte %s. Attempt to
parse %s " +
- "succeeded but consumed no data.\nPlease re-examine your
schema to correct this infinite loop.",
- pstate.bytePos, erd.diagnosticDebugName)
+ log(LogLevel.Debug, "Failure suppressed. This is normal
termination of a occursCountKind='parsed' array.")
+ pstate.reset(priorState)
+ pstate.discard(startState)
+ startState = null
+ priorState = null
+ }
returnFlag = true
} else {
- pstate.discard(priorState)
- priorState = pstate.mark("RepUnboundedParser3")
- pstate.mpstate.moveOverOneArrayIndexOnly
- returnFlag = false
+ // Success
+ // Need to check for forward progress
+ if (pstate.bitPos =#= priorState.bitPos0b) {
+ pstate.discard(priorState) // didn't move, but might have
assigned variables, have to undo those.
+ pstate.discard(startState)
+ startState = null
+ priorState = null
+ PE(pstate,
+ "RepUnbounded - No forward progress at byte %s. Attempt to
parse %s " +
+ "succeeded but consumed no data.\nPlease re-examine your
schema to correct this infinite loop.",
+ pstate.bytePos, erd.diagnosticDebugName)
+ returnFlag = true
+ } else {
+ pstate.discard(priorState)
+ priorState = pstate.mark("RepUnboundedParser3")
+ pstate.mpstate.moveOverOneArrayIndexOnly
+ returnFlag = false
+ }
+ }
+ pstate.popDiscriminator
+ }
+ Assert.invariant(returnFlag == true)
+ } catch {
+ case t: Throwable => {
+ if (priorState != null || startState != null) {
+ markLeakCausedByException = true
+ if (!t.isInstanceOf[SchemaDefinitionDiagnosticBase] &&
!t.isInstanceOf[UnsuppressableException]) {
--- End diff --
I was going to comment on shared logic between this and the AltCompParser,
but really I think sharing would be more complex than duplicating in this case.
I suggest just a comment in the AltCompParser directing developers to the
corresponding part of Rep Parsers, and vice versa, so that nobody edits one
without also considering the other.
I had to think about whether there are other places that need to deal with
marks before I convinced myself there are only these two. It would be good to
write that down in a comment in here.
---
