Skip to content

Commit

Permalink
fixup! fixup! Fix Bug with valueLength being overwritten after Trim
Browse files Browse the repository at this point in the history
- add asserts to setAbsStartPos0bInBits and setAbsEndPos0bInBits to ensure they're not being overwritten
- undo change to capturedByValueParsers to put back im[liedRepresentation == Text
- do not capture the value lengths of choices in specified length parser base
- fix bug where padding is being added around prefixed length element (DAFFODIL-2943) by changing CaptureLengthRegion to wrap around contentlengthStart and padding
- fix bug where we use the valuelength to calculate the prefix length, according to the spec it should be the content length
- fix bug where we were missing return after PE for Out of Range Binary Integers (DAFFODIL-2942)
- fix bug where we were using the main element's qname instead of the prefixed element qname in the Unparse Error message
- refactor Prefixed parsers to use state's bitLimit to get the prefix length (PrefixedLengthParserMixin2) since the specifiedLengthPrefixedParser will take care of parsing the prefix length
- refactored Prefixed unparsers to not try to unparse prefix length since that is taken care of by SpecifiedLengthPrefixedUnparser
- refactored prefixed parsers and unparsers to remove unused prefixed length parser related members

DAFFODIL-2658
  • Loading branch information
olabusayoT committed Nov 1, 2024
1 parent bc68935 commit 78050c8
Show file tree
Hide file tree
Showing 28 changed files with 214 additions and 534 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,7 @@ import org.apache.daffodil.core.grammar.primitives.RightFill
import org.apache.daffodil.core.grammar.primitives.ScalarOrderedSequenceChild
import org.apache.daffodil.core.grammar.primitives.SpecifiedLengthExplicit
import org.apache.daffodil.core.grammar.primitives.SpecifiedLengthImplicit
import org.apache.daffodil.core.grammar.primitives.SpecifiedLengthPrefixed
import org.apache.daffodil.lib.api.Diagnostic
import org.apache.daffodil.lib.api.WarnID
import org.apache.daffodil.lib.schema.annotation.props.gen.FailureType
Expand Down Expand Up @@ -301,6 +302,7 @@ object DaffodilCCodeGenerator
case g: SeqComp => seqCompGenerateCode(g, cgState)
case g: SpecifiedLengthExplicit => specifiedLengthExplicit(g, cgState)
case g: SpecifiedLengthImplicit => specifiedLengthImplicit(g, cgState)
case g: SpecifiedLengthPrefixed => specifiedLengthPrefixed(g, cgState)
case _ => gram.SDE("Code generation not supported for: %s", Misc.getNameFromClass(gram))
}
}
Expand Down Expand Up @@ -409,4 +411,11 @@ object DaffodilCCodeGenerator
): Unit = {
DaffodilCCodeGenerator.generateCode(g.eGram, cgState)
}

private def specifiedLengthPrefixed(
g: SpecifiedLengthPrefixed,
cgState: CodeGeneratorState
): Unit = {
DaffodilCCodeGenerator.generateCode(g.eGram, cgState)
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -72,13 +72,21 @@ trait ElementBaseGrammarMixin
}
}

protected lazy val isDelimitedPrefixedPattern = {
protected lazy val isPrefixed: Boolean = {
import LengthKind._
lengthKind match {
case Prefixed => true
case _ => false
}
}

protected lazy val isDelimitedPrefixedPattern: Boolean = {
import LengthKind._
lengthKind match {
case Delimited =>
true // don't test for hasDelimiters because it might not be our delimiter, but a surrounding group's separator, or it's terminator, etc.
case Pattern => true
case Prefixed => true
case Prefixed => isPrefixed
case _ => false
}
}
Expand Down Expand Up @@ -474,12 +482,16 @@ trait ElementBaseGrammarMixin
lazy val leftPadding = leftPaddingArg
lazy val rightPadFill = rightPadFillArg
lazy val body = bodyArg
CaptureContentLengthStart(this) ~
leftPadding ~
CaptureValueLengthStart(this) ~
body ~
CaptureValueLengthEnd(this) ~
rightPadFill ~
specifiedLength(
CaptureContentLengthStart(this) ~
leftPadding ~
CaptureValueLengthStart(this) ~
body ~
CaptureValueLengthEnd(this) ~
rightPadFill
) ~
// CaptureContentLengthEnd must be outside the specified length so it can
// do any skipping of bits it needs to before capturing the end of content length
CaptureContentLengthEnd(this)
}

Expand Down Expand Up @@ -623,14 +635,14 @@ trait ElementBaseGrammarMixin

private lazy val stringPrim = {
lengthKind match {
case LengthKind.Explicit => specifiedLength(StringOfSpecifiedLength(this))
case LengthKind.Prefixed => specifiedLength(StringOfSpecifiedLength(this))
case LengthKind.Explicit => StringOfSpecifiedLength(this)
case LengthKind.Prefixed => StringOfSpecifiedLength(this)
case LengthKind.Delimited => stringDelimitedEndOfData
case LengthKind.Pattern => specifiedLength(StringOfSpecifiedLength(this))
case LengthKind.Pattern => StringOfSpecifiedLength(this)
case LengthKind.Implicit => {
val pt = this.simpleType.primType
Assert.invariant(pt == PrimType.String)
specifiedLength(StringOfSpecifiedLength(this))
StringOfSpecifiedLength(this)
}
case LengthKind.EndOfParent if isComplexType =>
notYetImplemented("lengthKind='endOfParent' for complex type")
Expand All @@ -645,7 +657,7 @@ trait ElementBaseGrammarMixin
}

private lazy val hexBinaryLengthPattern = prod("hexBinaryLengthPattern") {
new SpecifiedLengthPattern(this, new HexBinaryEndOfBitLimit(this))
new HexBinaryEndOfBitLimit(this)
}

private lazy val hexBinaryLengthPrefixed = prod("hexBinaryLengthPrefixed") {
Expand Down Expand Up @@ -1226,7 +1238,7 @@ trait ElementBaseGrammarMixin
}

private lazy val nilLitSimple = prod("nilLitSimple", isSimpleType) {
captureLengthRegions(leftPadding, specifiedLength(nilLitContent), rightPadding ~ rightFill)
captureLengthRegions(leftPadding, nilLitContent, rightPadding ~ rightFill)
}

private lazy val nilLitComplex = prod("nilLitComplex", isComplexType) {
Expand Down Expand Up @@ -1329,7 +1341,10 @@ trait ElementBaseGrammarMixin
* as well, by not enclosing the body in a specified length enforcer.
*/
private def specifiedLength(bodyArg: => Gram) = {
lazy val body = bodyArg
// we need this to evaluate before we wrap in specified length parser,
// so it can do any internal checks for example blobValue's check for
// non-explicit lengthKind
val body = bodyArg
lazy val bitsMultiplier = lengthUnits match {
case LengthUnits.Bits => 1
case LengthUnits.Bytes => 8
Expand All @@ -1341,7 +1356,13 @@ trait ElementBaseGrammarMixin
case LengthKind.Delimited => body
case LengthKind.Pattern => new SpecifiedLengthPattern(this, body)
case LengthKind.Explicit if bitsMultiplier != 0 =>
new SpecifiedLengthExplicit(this, body, bitsMultiplier)
if (isSimpleType && primType == PrimType.HexBinary) {
// hexBinary has some checks that need to be done that SpecifiedLengthExplicit
// gets in the way of
body
} else {
new SpecifiedLengthExplicit(this, body, bitsMultiplier)
}
case LengthKind.Explicit => {
Assert.invariant(!knownEncodingIsFixedWidth)
Assert.invariant(lengthUnits eq LengthUnits.Characters)
Expand Down Expand Up @@ -1403,7 +1424,7 @@ trait ElementBaseGrammarMixin
private lazy val sharedComplexContentRegion: Gram =
schemaSet.sharedComplexContentFactory.getShared(
shareKey,
captureLengthRegions(EmptyGram, specifiedLength(complexContent), elementUnused) ~
captureLengthRegions(EmptyGram, complexContent, elementUnused) ~
terminatorRegion
)

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -52,20 +52,10 @@ class BCDIntegerKnownLength(val e: ElementBase, lengthInBits: Long) extends Term

class BCDIntegerPrefixedLength(val e: ElementBase) extends Terminal(e, true) {

override lazy val parser = new BCDIntegerPrefixedLengthParser(
e.elementRuntimeData,
e.prefixedLengthBody.parser,
e.prefixedLengthElementDecl.elementRuntimeData,
e.lengthUnits,
e.prefixedLengthAdjustmentInUnits
)
override lazy val parser = new BCDIntegerPrefixedLengthParser(e.elementRuntimeData)

override lazy val unparser: Unparser = new BCDIntegerPrefixedLengthUnparser(
e.elementRuntimeData,
e.prefixedLengthBody.unparser,
e.prefixedLengthElementDecl.elementRuntimeData,
e.lengthUnits,
e.prefixedLengthAdjustmentInUnits
e.elementRuntimeData
)
}

Expand Down Expand Up @@ -102,21 +92,9 @@ class BCDDecimalKnownLength(val e: ElementBase, lengthInBits: Long) extends Term

class BCDDecimalPrefixedLength(val e: ElementBase) extends Terminal(e, true) {

override lazy val parser = new BCDDecimalPrefixedLengthParser(
e.elementRuntimeData,
e.prefixedLengthBody.parser,
e.prefixedLengthElementDecl.elementRuntimeData,
e.binaryDecimalVirtualPoint,
e.lengthUnits,
e.prefixedLengthAdjustmentInUnits
)
override lazy val parser =
new BCDDecimalPrefixedLengthParser(e.elementRuntimeData, e.binaryDecimalVirtualPoint)

override lazy val unparser: Unparser = new BCDDecimalPrefixedLengthUnparser(
e.elementRuntimeData,
e.prefixedLengthBody.unparser,
e.prefixedLengthElementDecl.elementRuntimeData,
e.binaryDecimalVirtualPoint,
e.lengthUnits,
e.prefixedLengthAdjustmentInUnits
)
override lazy val unparser: Unparser =
new BCDDecimalPrefixedLengthUnparser(e.elementRuntimeData, e.binaryDecimalVirtualPoint)
}
Original file line number Diff line number Diff line change
Expand Up @@ -48,21 +48,15 @@ class BinaryBoolean(val e: ElementBase) extends Terminal(e, true) {
class BinaryBooleanPrefixedLength(val e: ElementBase) extends Terminal(e, true) {
override lazy val parser = new BinaryBooleanPrefixedLengthParser(
e.elementRuntimeData,
e.prefixedLengthBody.parser,
e.prefixedLengthElementDecl.elementRuntimeData,
e.binaryBooleanTrueRep,
e.binaryBooleanFalseRep,
e.lengthUnits,
e.prefixedLengthAdjustmentInUnits
e.lengthUnits
)

override lazy val unparser: Unparser = new BinaryBooleanPrefixedLengthUnparser(
e.elementRuntimeData,
e.prefixedLengthBody.unparser,
e.prefixedLengthElementDecl.elementRuntimeData,
e.binaryBooleanTrueRep,
e.binaryBooleanFalseRep,
e.lengthUnits,
e.prefixedLengthAdjustmentInUnits
e.lengthUnits
)
}
Original file line number Diff line number Diff line change
Expand Up @@ -77,14 +77,7 @@ class BinaryIntegerPrefixedLength(val e: ElementBase, signed: Boolean)
private lazy val pladj = e.prefixedLengthAdjustmentInUnits

override lazy val parser =
new BinaryIntegerPrefixedLengthParser(
erd,
e.prefixedLengthBody.parser,
plerd,
signed,
e.lengthUnits,
pladj
)
new BinaryIntegerPrefixedLengthParser(erd, signed)

override lazy val unparser: Unparser = {
val maybeNBits = e.primType match {
Expand All @@ -96,15 +89,7 @@ class BinaryIntegerPrefixedLength(val e: ElementBase, signed: Boolean)
case _ =>
Assert.invariantFailed("Only integer base types should be used for this primitive")
}
new BinaryIntegerPrefixedLengthUnparser(
erd,
e.prefixedLengthBody.unparser,
plerd,
maybeNBits,
signed,
e.lengthUnits,
pladj
)
new BinaryIntegerPrefixedLengthUnparser(erd, maybeNBits, signed)
}
}

Expand Down Expand Up @@ -151,23 +136,15 @@ class BinaryDecimalPrefixedLength(val e: ElementBase) extends Terminal(e, true)
override lazy val parser =
new BinaryDecimalPrefixedLengthParser(
e.elementRuntimeData,
e.prefixedLengthBody.parser,
e.prefixedLengthElementDecl.elementRuntimeData,
e.decimalSigned,
e.binaryDecimalVirtualPoint,
e.lengthUnits,
e.prefixedLengthAdjustmentInUnits
e.binaryDecimalVirtualPoint
)

override lazy val unparser: Unparser =
new BinaryDecimalPrefixedLengthUnparser(
e.elementRuntimeData,
e.prefixedLengthBody.unparser,
e.prefixedLengthElementDecl.elementRuntimeData,
e.decimalSigned,
e.binaryDecimalVirtualPoint,
e.lengthUnits,
e.prefixedLengthAdjustmentInUnits
e.binaryDecimalVirtualPoint
)

}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -61,21 +61,11 @@ class IBM4690PackedIntegerKnownLength(val e: ElementBase, signed: Boolean, lengt

class IBM4690PackedIntegerPrefixedLength(val e: ElementBase, signed: Boolean)
extends Terminal(e, true) {
override lazy val parser = new IBM4690PackedIntegerPrefixedLengthParser(
e.elementRuntimeData,
e.prefixedLengthBody.parser,
e.prefixedLengthElementDecl.elementRuntimeData,
signed,
e.lengthUnits,
e.prefixedLengthAdjustmentInUnits
)
override lazy val parser =
new IBM4690PackedIntegerPrefixedLengthParser(e.elementRuntimeData, signed)

override lazy val unparser: Unparser = new IBM4690PackedIntegerPrefixedLengthUnparser(
e.elementRuntimeData,
e.prefixedLengthBody.unparser,
e.prefixedLengthElementDecl.elementRuntimeData,
e.lengthUnits,
e.prefixedLengthAdjustmentInUnits
e.elementRuntimeData
)
}

Expand Down Expand Up @@ -114,19 +104,11 @@ class IBM4690PackedDecimalKnownLength(val e: ElementBase, lengthInBits: Long)
class IBM4690PackedDecimalPrefixedLength(val e: ElementBase) extends Terminal(e, true) {
override lazy val parser = new IBM4690PackedDecimalPrefixedLengthParser(
e.elementRuntimeData,
e.prefixedLengthBody.parser,
e.prefixedLengthElementDecl.elementRuntimeData,
e.binaryDecimalVirtualPoint,
e.lengthUnits,
e.prefixedLengthAdjustmentInUnits
e.binaryDecimalVirtualPoint
)

override lazy val unparser: Unparser = new IBM4690PackedDecimalPrefixedLengthUnparser(
e.elementRuntimeData,
e.prefixedLengthBody.unparser,
e.prefixedLengthElementDecl.elementRuntimeData,
e.binaryDecimalVirtualPoint,
e.lengthUnits,
e.prefixedLengthAdjustmentInUnits
e.binaryDecimalVirtualPoint
)
}
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,6 @@ import org.apache.daffodil.runtime1.processors.unparsers.{ Unparser => DaffodilU
import org.apache.daffodil.unparsers.runtime1.BCDDecimalDelimitedUnparser
import org.apache.daffodil.unparsers.runtime1.BCDIntegerDelimitedUnparser
import org.apache.daffodil.unparsers.runtime1.BlobSpecifiedLengthUnparser
import org.apache.daffodil.unparsers.runtime1.HexBinaryLengthPrefixedUnparser
import org.apache.daffodil.unparsers.runtime1.HexBinaryMinLengthInBytesUnparser
import org.apache.daffodil.unparsers.runtime1.HexBinarySpecifiedLengthUnparser
import org.apache.daffodil.unparsers.runtime1.IBM4690PackedDecimalDelimitedUnparser
Expand Down Expand Up @@ -186,21 +185,11 @@ case class HexBinaryEndOfBitLimit(e: ElementBase) extends Terminal(e, true) {
case class HexBinaryLengthPrefixed(e: ElementBase) extends Terminal(e, true) {

override lazy val parser: DaffodilParser = new HexBinaryLengthPrefixedParser(
e.elementRuntimeData,
e.prefixedLengthBody.parser,
e.prefixedLengthElementDecl.elementRuntimeData,
e.lengthUnits,
e.prefixedLengthAdjustmentInUnits
e.elementRuntimeData
)

override lazy val unparser: DaffodilUnparser = new HexBinaryLengthPrefixedUnparser(
e.elementRuntimeData,
e.prefixedLengthBody.unparser,
e.prefixedLengthElementDecl.elementRuntimeData,
e.minLength.longValue,
e.lengthUnits,
e.prefixedLengthAdjustmentInUnits
)
override lazy val unparser: DaffodilUnparser =
new HexBinaryMinLengthInBytesUnparser(e.minLength.longValue, e.elementRuntimeData)
}

abstract class PackedIntegerDelimited(
Expand Down
Loading

0 comments on commit 78050c8

Please sign in to comment.