mbeckerle closed pull request #100: Moved pushDiscriminator to fix separators
bug.
URL: https://github.com/apache/incubator-daffodil/pull/100
This is a PR merged from a forked repository.
As GitHub hides the original diff on merge, it is displayed below for
the sake of provenance:
As this is a foreign pull request (from a fork), the diff is supplied
below (as it won't show otherwise due to GitHub magic):
diff --git
a/daffodil-core/src/main/scala/org/apache/daffodil/grammar/primitives/PrimitivesDelimiters.scala
b/daffodil-core/src/main/scala/org/apache/daffodil/grammar/primitives/PrimitivesDelimiters.scala
index 9aaf5f30d..ac3f387c2 100644
---
a/daffodil-core/src/main/scala/org/apache/daffodil/grammar/primitives/PrimitivesDelimiters.scala
+++
b/daffodil-core/src/main/scala/org/apache/daffodil/grammar/primitives/PrimitivesDelimiters.scala
@@ -80,57 +80,6 @@ abstract class Text(es: Term, e: Term, guard: Boolean)
extends StringDelimBase(e
""
}
}
-
- def getMatchedDelimiterInfo(remoteDelimRegex: Set[(String, String)],
foundDelimiter: String,
- delimiters: List[(List[String], String, String)]) = {
- val matchedDelim = remoteDelimRegex.find {
- case (delimRegex, _) => {
- foundDelimiter.matches("(?s)^(" + delimRegex + ")$")
- }
- } match {
- case Some((_, theValue)) => theValue
- case None => Assert.impossibleCase()
- }
-
- val (remoteDelimValue, remoteElemName, remoteElemPath, _) =
- {
- val findResult = delimiters.map {
- case (delimValueList, elemName, elemPath) => {
- delimValueList.find(delim => delim == matchedDelim) match {
- case Some(d) => (d, elemName, elemPath, true)
- case None => (delimValueList.mkString(","), elemName, elemPath,
false)
- }
- }
- }.toSet.filter { x => x._4 == true }
-
- if (findResult.size == 0) Assert.impossibleCase()
- findResult.head
- }
- (remoteDelimValue, remoteElemName, remoteElemPath)
- }
-
- def getMatchedDelimiterInfo(
- originalDelimRep: String,
- delimiters: List[(List[String], String, String)]) = {
-
- val (remoteDelimValue, remoteElemName, remoteElemPath, _) =
- {
- val findResult = delimiters.map {
- case (delimValueList, elemName, elemPath) => {
- val res = delimValueList.find(delim => delim == originalDelimRep)
match {
- case Some(d) => (d, elemName, elemPath, true)
- case None => (delimValueList.mkString(","), elemName, elemPath,
false)
- }
- res
- }
- }.toSet.filter { x => x._4 == true }
-
- if (findResult.size == 0) Assert.impossibleCase()
- findResult.head
- }
- (remoteDelimValue, remoteElemName, remoteElemPath)
- }
-
}
// NOTE: LiteralNil still uses this as it can only be Static/Constant
diff --git
a/daffodil-runtime1/src/main/scala/org/apache/daffodil/processors/parsers/SeparatedSequenceParsers.scala
b/daffodil-runtime1/src/main/scala/org/apache/daffodil/processors/parsers/SeparatedSequenceParsers.scala
index 889e5dda3..38e39cafd 100644
---
a/daffodil-runtime1/src/main/scala/org/apache/daffodil/processors/parsers/SeparatedSequenceParsers.scala
+++
b/daffodil-runtime1/src/main/scala/org/apache/daffodil/processors/parsers/SeparatedSequenceParsers.scala
@@ -299,6 +299,15 @@ final class OrderedSeparatedSequenceParser(
val bitPosBeforeSeparator = pstate.bitPos0b
+ //
+ // Note: Performance. All this conditional branch logic could in
principle be hoisted
+ // out and decided at schema compile time, as what is being tested here
is statically known.
+ //
+ // However, that's trading a few conditional branches for a virtual
function call, and that
+ // might or might not be an improvement worth making. Not worth
bothering with unless
+ // some profiling makes it look like this could help.
+ //
+
// parse prefix sep if any
val prefixSepSuccessful =
if (spos eq SeparatorPosition.Prefix) {
@@ -342,8 +351,6 @@ final class OrderedSeparatedSequenceParser(
//
val prevBitPosBeforeChild = pstate.bitPos0b
- pstate.pushDiscriminator
-
if (pstate.dataProc.isDefined)
pstate.dataProc.get.beforeRepetition(pstate, this)
parser.parse1(pstate)
@@ -496,7 +503,6 @@ final class OrderedSeparatedSequenceParser(
result
} // end if child success/fail
} // end val res
- pstate.popDiscriminator
res
} // end if postfix
} // end if infix
diff --git
a/daffodil-runtime1/src/main/scala/org/apache/daffodil/processors/parsers/SequenceParserBases.scala
b/daffodil-runtime1/src/main/scala/org/apache/daffodil/processors/parsers/SequenceParserBases.scala
index 541137a92..4de4a99c6 100644
---
a/daffodil-runtime1/src/main/scala/org/apache/daffodil/processors/parsers/SequenceParserBases.scala
+++
b/daffodil-runtime1/src/main/scala/org/apache/daffodil/processors/parsers/SequenceParserBases.scala
@@ -118,13 +118,6 @@ abstract class OrderedSequenceParserBase(
// different, or OCK=parsed.
//
- //
- // The beforeArrayState will be assigned the priorState before
the whole array
- // This is the same object as the priorState before the first
- // occurrence.
- //
- var beforeArrayState: PState.Mark = null
-
var resultOfTry: ParseAttemptStatus =
ParseAttemptStatus.Uninitialized
var ais: ArrayIndexStatus = null
@@ -155,8 +148,7 @@ abstract class OrderedSequenceParserBase(
//
val priorState =
if (isFirstIteration) {
- beforeArrayState = pstate.mark("before all occurrences")
- beforeArrayState
+ pstate.mark("before all occurrences")
} else {
pstate.mark("before second/subsequent occurrence")
}
@@ -166,9 +158,11 @@ abstract class OrderedSequenceParserBase(
var markLeakCausedByException = false
var wasThrow = true
try {
+ pstate.pushDiscriminator
resultOfTry =
parseOneWithPoU(parser, erd, pstate, priorState, goAIS,
isBounded)
wasThrow = false
+ pstate.popDiscriminator
//
// Now we handle the result of the parse attempt.
//
@@ -231,7 +225,6 @@ abstract class OrderedSequenceParserBase(
// discarded.
//
if (!isFirstIteration) pstate.discard(priorState)
- pstate.reset(beforeArrayState)
pstate.setFailed(cause)
resultOfTry = ParseAttemptStatus.Failed_EntireArray
}
@@ -284,7 +277,7 @@ abstract class OrderedSequenceParserBase(
}
}
// if it wasn't reset, we discard priorState here.
- if (!isFirstIteration && pstate.isInUse(priorState)) {
+ if (pstate.isInUse(priorState)) {
pstate.discard(priorState)
}
@@ -300,11 +293,6 @@ abstract class OrderedSequenceParserBase(
} // end while for each repeat
- // if it wasn't reset or discarded, discard here.
- if (pstate.isInUse(beforeArrayState)) {
- pstate.discard(beforeArrayState)
- }
-
} // end match case hasPoU = true
case false => {
diff --git
a/daffodil-runtime1/src/main/scala/org/apache/daffodil/processors/parsers/UnseparatedSequenceParsers.scala
b/daffodil-runtime1/src/main/scala/org/apache/daffodil/processors/parsers/UnseparatedSequenceParsers.scala
index 3210f7dcd..8ea27fe14 100644
---
a/daffodil-runtime1/src/main/scala/org/apache/daffodil/processors/parsers/UnseparatedSequenceParsers.scala
+++
b/daffodil-runtime1/src/main/scala/org/apache/daffodil/processors/parsers/UnseparatedSequenceParsers.scala
@@ -118,8 +118,6 @@ class OrderedUnseparatedSequenceParser(rd:
SequenceRuntimeData, childParsersArg:
Assert.invariant(erd.isArray || erd.isOptional) // not a PoU otherwise.
- pstate.pushDiscriminator
-
if (pstate.dataProc.isDefined)
pstate.dataProc.get.beforeRepetition(pstate, this)
val posBefore = pstate.bitPos0b
@@ -160,7 +158,6 @@ class OrderedUnseparatedSequenceParser(rd:
SequenceRuntimeData, childParsersArg:
}
}
}
- pstate.popDiscriminator
res
}
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
