stevedlawrence commented on a change in pull request #212: Catch error when
delimiter expression fails
URL: https://github.com/apache/incubator-daffodil/pull/212#discussion_r278963668
##########
File path:
daffodil-runtime1/src/main/scala/org/apache/daffodil/processors/parsers/ElementKindParsers.scala
##########
@@ -57,26 +57,28 @@ class DelimiterStackParser(
val newLocalIndex = start.mpstate.delimiters.length
start.mpstate.delimitersLocalIndexStack.push(newLocalIndex)
- // evaluate and add delimiters to the stack
- var i: Int = 0
- while (i < delimiters.length) {
- start.mpstate.delimiters ++= delimiters(i).evaluate(start)
- i += 1
- }
-
- // set the index of the newly added delimiters
- val newDelimLen = start.mpstate.delimiters.length
- i = newLocalIndex
- while (i < newDelimLen) {
- start.mpstate.delimiters(i).indexInDelimiterStack = i
- i += 1
- }
+ try {
+ // evaluate and add delimiters to the stack
+ var i: Int = 0
+ while (i < delimiters.length) {
+ start.mpstate.delimiters ++= delimiters(i).evaluate(start)
+ i += 1
+ }
- // parse
- bodyParser.parse1(start)
+ // set the index of the newly added delimiters
+ val newDelimLen = start.mpstate.delimiters.length
+ i = newLocalIndex
+ while (i < newDelimLen) {
+ start.mpstate.delimiters(i).indexInDelimiterStack = i
+ i += 1
+ }
- // pop delimiters
-
start.mpstate.delimiters.reduceToSize(start.mpstate.delimitersLocalIndexStack.pop)
+ // parse
+ bodyParser.parse1(start)
+ } finally {
+ // pop delimiters
+
start.mpstate.delimiters.reduceToSize(start.mpstate.delimitersLocalIndexStack.pop)
+ }
Review comment:
This looks like the right fix to clean up the delimiter stack when evaluate
throws an exception.
However, the DFDL spec says this for dfdl:terminator:
> If dfdl:terminator is "" (the empty string), then the terminator region is
of length zero, and no terminator is expected. It is not permitted for an
expression to return an empty string, that is a schema definition error.
So we've fixed the unhelpful abort when .evaluate() fails and throws an
exception, but haven't resolved the issue where this error should be an SDE
instead of a PE. That issue is unrelated to this bug so might be worth just
creating a new ticket to track it.
+1 :+1:
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
[email protected]
With regards,
Apache Git Services
