stevedlawrence commented on code in PR #1338:
URL: https://github.com/apache/daffodil/pull/1338#discussion_r1803544680
##########
daffodil-runtime1/src/main/scala/org/apache/daffodil/runtime1/processors/parsers/SpecifiedLengthParsers.scala:
##########
@@ -85,7 +85,9 @@ sealed abstract class SpecifiedLengthParserBase(eParser:
Parser, erd: RuntimeDat
if (pState.processorStatus ne Success) return
val finalEndPos0b = startingBitPos0b + nBits
- captureValueLength(pState, ULong(startingBitPos0b), ULong(dis.bitPos0b))
+ // if we haven't already set the value length, set it now
+ if (pState.infoset.valueLength.isEndUndef)
+ captureValueLength(pState, ULong(startingBitPos0b), ULong(dis.bitPos0b))
Review Comment:
The `isReferenced` variable is used to avoid capturing value lengths that
will never be used. For example, if we have something like this:
```xml
<element name="a" type="xs:int" ... />
```
If nothing ever references the value length of `a, then we shouldn't insert
`CaptureValueLengthStart/End` parsers, regardless of the value of
`capturedByParsers`. But if something does reference the length, e.g.
```xml
<element name="len" type="xs:int" dfdl:inputValueCalc="{
dfdl:valueLength(../a, 'bytes')) } />
```
The we do need to capture the value length of `a`. So `isReferenced` is
needed to disabled capturing when it's not needed. I don't think we can get rid
of it.
Usually we capture the length via the `CaptureValueLengthStart/End` parsers,
but sometimes those parsers aren't smart enough so it must be done by a
separate parser that has enough information to capture length correctly (e.g.
StringOfSpecifiedLength that knows about pad/trim stuff and how to exclude it
from the value length). The `capturedByParsers` val is supposed to figure out
which are those special cases so we don't unnecessarily insert
`CaptureValueLengthStart/End` parsers for parsers that do it themselves.
It seems like SpecifiedLengthParserBase doesn't have enough information to
know how to correctly capture value length (since sometimes the subparser is
the thing that knows how to do it). So that probably means it shouldn't be
capturing the length at all. But that then means other parsers that relied on
that behavior either need to call `captureValueLength` themselves, or
`capturedByParsers` needs to be false in some cases so that we insert the
`CaptureValueLengthStart/End` parsers.
--
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.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
