mbeckerle commented on code in PR #1338:
URL: https://github.com/apache/daffodil/pull/1338#discussion_r1909594142
##########
daffodil-core/src/main/scala/org/apache/daffodil/core/grammar/ElementBaseGrammarMixin.scala:
##########
@@ -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
Review Comment:
I would not refactor this now. This code has gotten kind of undisciplined
and been that way for a long time now.
Our performance last we checked was in the ballpark of 2x what a programmer
would write by hand for simple binary data.
If we start reorganizing this sort of thing we need to rerun those tests.
We're too close to a release for that.
Fixing these bugs does seem worth it however.
##########
daffodil-core/src/main/scala/org/apache/daffodil/core/grammar/ElementBaseGrammarMixin.scala:
##########
@@ -1387,14 +1381,15 @@ trait ElementBaseGrammarMixin
}
case LengthKind.Implicit if isSimpleType && primType == PrimType.String
=>
new SpecifiedLengthImplicitCharacters(this, body,
this.maxLength.longValue)
-
- case LengthKind.Implicit if isSimpleType && primType ==
PrimType.HexBinary =>
- new SpecifiedLengthImplicit(this, body, this.maxLength.longValue *
bitsMultiplier)
case LengthKind.Implicit
- if isSimpleType && impliedRepresentation == Representation.Binary =>
+ if isSimpleType &&
+ impliedRepresentation == Representation.Binary &&
+ primType != PrimType.HexBinary =>
new SpecifiedLengthImplicit(this, body, implicitBinaryLengthInBits)
Review Comment:
I suggest parking this cleanup as a separate ticket. Unnecessary parsers are
not optimized out in many cases I bet, and this is just one such case where
specified length stuff is unnecessary but not removed.
--
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]
