You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@daffodil.apache.org by ja...@apache.org on 2022/02/18 20:36:48 UTC

[daffodil] branch main updated: Check for RepType when getting element length

This is an automated email from the ASF dual-hosted git repository.

jadams pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/daffodil.git


The following commit(s) were added to refs/heads/main by this push:
     new f3ef3b9  Check for RepType when getting element length
f3ef3b9 is described below

commit f3ef3b9f5a5cddaf74c04d8d3026f8b99d72cbff
Author: Josh Adams <ja...@owlcyberdefense.com>
AuthorDate: Wed Dec 1 22:07:32 2021 -0500

    Check for RepType when getting element length
    
    Now using a repElement value that is either the element itself or, if
    optRepTypeElement is defined, the repTypeElement.  This avoids the
    proliferating conditionals where we needed to check to see if
    optRepTypeElement was defined or not.
    
    There may be more places where repElement should be used, but this
    covers all of our existing needs.
    
    DAFFODIL-2596
---
 .../org/apache/daffodil/dsom/ElementBase.scala     |  36 +++----
 .../daffodil/dsom/RuntimePropertyMixins.scala      | 113 +++++++++------------
 .../daffodil/grammar/ElementBaseGrammarMixin.scala |  12 +--
 .../extensions/type_calc/inputTypeCalc.tdml        |  36 +++++++
 .../extensions/TestInputTypeValueCalc.scala        |   3 +
 5 files changed, 110 insertions(+), 90 deletions(-)

diff --git a/daffodil-core/src/main/scala/org/apache/daffodil/dsom/ElementBase.scala b/daffodil-core/src/main/scala/org/apache/daffodil/dsom/ElementBase.scala
index 36dd7dc..0f82ccc 100644
--- a/daffodil-core/src/main/scala/org/apache/daffodil/dsom/ElementBase.scala
+++ b/daffodil-core/src/main/scala/org/apache/daffodil/dsom/ElementBase.scala
@@ -650,13 +650,13 @@ trait ElementBase
     // lengthUnits is bytes/bits, but the fixed length is given in characters
     // of a variable-width charset, so greater than 0, but we don't know
     // exactly.
-    Assert.usage(isFixedLength)
-    if (lengthKind =:= LengthKind.Explicit) lengthEv.optConstant.get
+    Assert.usage(repElement.isFixedLength)
+    if (repElement.lengthKind =:= LengthKind.Explicit) repElement.lengthEv.optConstant.get
     else {
-      Assert.invariant(lengthKind =:= LengthKind.Implicit)
+      Assert.invariant(repElement.lengthKind =:= LengthKind.Implicit)
       // it's a string with implicit length. get from facets
-      schemaDefinitionUnless(this.hasMaxLength, "String with dfdl:lengthKind='implicit' must have an XSD maxLength facet value.")
-      val ml = this.maxLength
+      schemaDefinitionUnless(repElement.hasMaxLength, "String with dfdl:lengthKind='implicit' must have an XSD maxLength facet value.")
+      val ml = repElement.maxLength
       ml.longValue()
     }
   }
@@ -680,7 +680,7 @@ trait ElementBase
   }
 
   final lazy val fixedLength = {
-    if (isFixedLength) lengthEv.optConstant.get.longValue() else -1L
+    if (isFixedLength) repElement.lengthEv.optConstant.get.longValue() else -1L
     // FIXME: shouldn't even be asking for this if not isFixedLength
     // try changing to Assert.usage(isFixedLength)
     // FIXME: needs to return fixed length in lengthUnits.
@@ -693,23 +693,19 @@ trait ElementBase
 
   // FIXME: bless this method. Deprecate and remove other less reliable things.
   final lazy val maybeFixedLengthInBits: MaybeULong = {
-    if (optRepTypeElement.isDefined) {
-      optRepTypeElement.get.maybeFixedLengthInBits
-    } else {
-      if (isRepresented && isFixedLength) {
-        val bitsMultiplier = lengthUnits match {
-          case LengthUnits.Bits => 1
-          case LengthUnits.Bytes => 8
-          case LengthUnits.Characters => if (knownEncodingIsFixedWidth) knownEncodingWidthInBits else -1
-        }
-        if (bitsMultiplier > 0) {
-          MaybeULong(fixedLengthValue * bitsMultiplier)
-        } else {
-          MaybeULong.Nope
-        }
+    if (isRepresented && repElement.isFixedLength) {
+      val bitsMultiplier = repElement.lengthUnits match {
+        case LengthUnits.Bits => 1
+        case LengthUnits.Bytes => 8
+        case LengthUnits.Characters => if (knownEncodingIsFixedWidth) knownEncodingWidthInBits else -1
+      }
+      if (bitsMultiplier > 0) {
+        MaybeULong(repElement.fixedLengthValue * bitsMultiplier)
       } else {
         MaybeULong.Nope
       }
+    } else {
+      MaybeULong.Nope
     }
   }
 
diff --git a/daffodil-core/src/main/scala/org/apache/daffodil/dsom/RuntimePropertyMixins.scala b/daffodil-core/src/main/scala/org/apache/daffodil/dsom/RuntimePropertyMixins.scala
index 388c079..1581429 100644
--- a/daffodil-core/src/main/scala/org/apache/daffodil/dsom/RuntimePropertyMixins.scala
+++ b/daffodil-core/src/main/scala/org/apache/daffodil/dsom/RuntimePropertyMixins.scala
@@ -277,23 +277,23 @@ trait ElementRuntimeValuedPropertiesMixin
   }
 
   private lazy val explicitLengthEv: ExplicitLengthEv = {
-    Assert.usage(lengthKind eq LengthKind.Explicit)
-    val ev = new ExplicitLengthEv(lengthExpr, eci)
+    Assert.usage(repElement.lengthKind eq LengthKind.Explicit)
+    val ev = new ExplicitLengthEv(repElement.lengthExpr, eci)
     ev.compile(tunable)
     ev
   }
 
   private lazy val implicitLengthEv: LengthEv = {
-    Assert.usage(lengthKind eq LengthKind.Implicit)
+    Assert.usage(repElement.lengthKind eq LengthKind.Implicit)
     import Representation._
     import NodeInfo._
-    lazy val maxLengthLong = maxLength.longValueExact
-    val ev = (impliedRepresentation, typeDef.typeNode) match {
+    lazy val maxLengthLong = repElement.maxLength.longValueExact
+    val ev = (repElement.impliedRepresentation, repElement.typeDef.typeNode) match {
       case (Text, String) => new ImplicitLengthEv(maxLengthLong, eci)
       case (Binary, HexBinary) => new ImplicitLengthEv(maxLengthLong, eci)
       case (Binary, _) => new ImplicitLengthEv(implicitBinaryLengthInBits, eci)
       case (Text, _) =>
-        SDE("Type %s with dfdl:representation='text' cannot have dfdl:lengthKind='implicit'", typeDef.typeNode.name)
+        SDE("Type %s with dfdl:representation='text' cannot have dfdl:lengthKind='implicit'", repElement.typeDef.typeNode.name)
     }
     ev.compile(tunable)
     ev
@@ -334,21 +334,21 @@ trait ElementRuntimeValuedPropertiesMixin
    * is constant.
    */
   lazy val elementLengthInBitsEv: LengthInBitsEv = {
-    Assert.usage((lengthKind eq LengthKind.Implicit) || (lengthKind eq LengthKind.Explicit))
+    Assert.usage((repElement.lengthKind eq LengthKind.Implicit) || (repElement.lengthKind eq LengthKind.Explicit))
     import LengthKind._
     import Representation._
     import NodeInfo._
     val (units: LengthUnits, lenEv: LengthEv) =
-      (lengthKind, impliedRepresentation, typeDef.typeNode) match {
-        case (Explicit, Binary, HexBinary) => (lengthUnits, explicitLengthEv)
+      (repElement.lengthKind, repElement.impliedRepresentation, repElement.typeDef.typeNode) match {
+        case (Explicit, Binary, HexBinary) => (repElement.lengthUnits, explicitLengthEv)
         case (Implicit, Binary, HexBinary) => (LengthUnits.Bytes, implicitLengthEv)
-        case (Explicit, Binary, _) => (lengthUnits, explicitLengthEv)
+        case (Explicit, Binary, _) => (repElement.lengthUnits, explicitLengthEv)
         case (Implicit, Binary, _) => (LengthUnits.Bits, implicitLengthEv)
-        case (Explicit, Text, _) => (lengthUnits, explicitLengthEv)
-        case (Implicit, Text, _) => (lengthUnits, implicitLengthEv)
+        case (Explicit, Text, _) => (repElement.lengthUnits, explicitLengthEv)
+        case (Implicit, Text, _) => (repElement.lengthUnits, implicitLengthEv)
         case _ => Assert.invariantFailed("not Implicit or Explicit")
       }
-    val ev = new LengthInBitsEv(units, lengthKind, maybeCharsetEv, lenEv, eci)
+    val ev = new LengthInBitsEv(units, repElement.lengthKind, repElement.maybeCharsetEv, lenEv, eci)
     ev.compile(tunable)
     ev
   }
@@ -363,7 +363,7 @@ trait ElementRuntimeValuedPropertiesMixin
    *
    */
   private lazy val minLengthInBitsEv: MinLengthInBitsEv = {
-    val ev = new MinLengthInBitsEv(minLenUnits, lengthKind, maybeCharsetEv, minLen, eci)
+    val ev = new MinLengthInBitsEv(repElement.minLenUnits, repElement.lengthKind, repElement.maybeCharsetEv, repElement.minLen, eci)
     ev.compile(tunable)
     ev
   }
@@ -435,17 +435,13 @@ trait ElementRuntimeValuedPropertiesMixin
   }
 
   final lazy val maybeUnparseTargetLengthInBitsEv: Maybe[UnparseTargetLengthInBitsEv] = {
-    if (this.isSimpleType && this.simpleType.optRepTypeElement.isDefined) {
-      this.simpleType.optRepTypeElement.get.maybeUnparseTargetLengthInBitsEv
-    } else {
-      if ((this.optionLengthRaw.isDefined &&
-        (lengthKind _eq_ LengthKind.Explicit)) ||
-        ((lengthKind _eq_ LengthKind.Implicit) && isSimpleType)) {
-        val ev = unparseTargetLengthInBitsEv
-        One(ev)
-      } else
-        Nope
-    }
+    if ((repElement.optionLengthRaw.isDefined &&
+      (repElement.lengthKind _eq_ LengthKind.Explicit)) ||
+      ((repElement.lengthKind _eq_ LengthKind.Implicit) && repElement.isSimpleType)) {
+      val ev = repElement.unparseTargetLengthInBitsEv
+      One(ev)
+    } else
+      Nope
   }
 
   /**
@@ -453,53 +449,42 @@ trait ElementRuntimeValuedPropertiesMixin
    * for delimited.
    */
   final lazy val maybeUnparseMinOrTargetLengthInBitsEv: Maybe[Evaluatable[MaybeJULong]] = {
-    if (this.isSimpleType && this.simpleType.optRepTypeElement.isDefined) {
-      this.simpleType.optRepTypeElement.get.maybeUnparseMinOrTargetLengthInBitsEv
-    } else {
-      if ((this.optionLengthRaw.isDefined &&
-        (lengthKind _eq_ LengthKind.Explicit)) ||
-        ((lengthKind _eq_ LengthKind.Implicit) && isSimpleType)) {
-        maybeUnparseTargetLengthInBitsEv
-      } else if (this.isDelimitedPrefixedPatternWithPadding) {
-        //
-        // if delimited but there is a min length, we just need the min
-        // length. There is no target length other than it.
-        //
-        val ev = minLengthInBitsEv
-        One(ev)
-      } else
-        Nope
-    }
+    if ((repElement.optionLengthRaw.isDefined &&
+      (repElement.lengthKind _eq_ LengthKind.Explicit)) ||
+      ((repElement.lengthKind _eq_ LengthKind.Implicit) && repElement.isSimpleType)) {
+      repElement.maybeUnparseTargetLengthInBitsEv
+    } else if (this.isDelimitedPrefixedPatternWithPadding) {
+      //
+      // if delimited but there is a min length, we just need the min
+      // length. There is no target length other than it.
+      //
+      val ev = minLengthInBitsEv
+      One(ev)
+    } else
+      Nope
   }
 
   final lazy val unparseTargetLengthInBitsEv: UnparseTargetLengthInBitsEv = {
-    if (this.isSimpleType && this.simpleType.optRepTypeElement.isDefined) {
-      this.simpleType.optRepTypeElement.get.unparseTargetLengthInBitsEv
-    } else {
-      val ev = new UnparseTargetLengthInBitsEv(elementLengthInBitsEv, minLengthInBitsEv, eci)
-      ev.compile(tunable)
-      ev
-    }
+    val ev = new UnparseTargetLengthInBitsEv(repElement.elementLengthInBitsEv, minLengthInBitsEv, eci)
+    ev.compile(tunable)
+    ev
   }
 
+
   final lazy val maybeUnparseTargetLengthInCharactersEv: Maybe[UnparseTargetLengthInCharactersEv] = {
-    if (this.isSimpleType && this.simpleType.optRepTypeElement.isDefined) {
-      this.simpleType.optRepTypeElement.get.maybeUnparseTargetLengthInCharactersEv
-    } else {
-      if ((lengthUnits eq LengthUnits.Characters) &&
-        (this.optionLengthRaw.isDefined &&
-          (lengthKind _eq_ LengthKind.Explicit)) ||
-          ((lengthKind _eq_ LengthKind.Implicit) && isSimpleType)) {
-        val optCs = charsetEv.optConstant
-        if (optCs.isEmpty || optCs.get.maybeFixedWidth.isEmpty) {
-          val ev = new UnparseTargetLengthInCharactersEv(lengthEv, charsetEv, minLen, eci)
-          ev.compile(tunable)
-          One(ev)
-        } else
-          Nope
+    if ((repElement.lengthUnits eq LengthUnits.Characters) &&
+      (this.optionLengthRaw.isDefined &&
+        (repElement.lengthKind _eq_ LengthKind.Explicit)) ||
+        ((repElement.lengthKind _eq_ LengthKind.Implicit) && repElement.isSimpleType)) {
+      val optCs = repElement.charsetEv.optConstant
+      if (optCs.isEmpty || optCs.get.maybeFixedWidth.isEmpty) {
+        val ev = new UnparseTargetLengthInCharactersEv(repElement.lengthEv, repElement.charsetEv, repElement.minLen, eci)
+        ev.compile(tunable)
+        One(ev)
       } else
         Nope
-    }
+    } else
+      Nope
   }
 
   //
diff --git a/daffodil-core/src/main/scala/org/apache/daffodil/grammar/ElementBaseGrammarMixin.scala b/daffodil-core/src/main/scala/org/apache/daffodil/grammar/ElementBaseGrammarMixin.scala
index 4e6e9f7..767ea3f 100644
--- a/daffodil-core/src/main/scala/org/apache/daffodil/grammar/ElementBaseGrammarMixin.scala
+++ b/daffodil-core/src/main/scala/org/apache/daffodil/grammar/ElementBaseGrammarMixin.scala
@@ -526,10 +526,9 @@ trait ElementBaseGrammarMixin
    * SDE if the lengthKind is inconsistent with binary numbers, not yet implemented
    * for binary numbers, or not supported by Daffodil.
    */
-  protected lazy val binaryNumberKnownLengthInBits: Long = lengthKind match {
-    case _ if optRepTypeElement.isDefined => optRepTypeElement.get.binaryNumberKnownLengthInBits
-    case LengthKind.Implicit => implicitBinaryLengthInBits
-    case LengthKind.Explicit if (lengthEv.isConstant) => explicitBinaryLengthInBits()
+  protected lazy val binaryNumberKnownLengthInBits: Long = repElement.lengthKind match {
+    case LengthKind.Implicit => repElement.implicitBinaryLengthInBits
+    case LengthKind.Explicit if (repElement.lengthEv.isConstant) => explicitBinaryLengthInBits()
     case LengthKind.Explicit => -1 // means must be computed at runtime.
     case LengthKind.Prefixed => -1 // means must be computed at runtime
     case LengthKind.Delimited => primType match {
@@ -545,8 +544,8 @@ trait ElementBaseGrammarMixin
   }
 
   private def explicitBinaryLengthInBits() = {
-    val lengthFromProp: JLong = lengthEv.optConstant.get
-    val nbits = lengthUnits match {
+    val lengthFromProp: JLong = repElement.lengthEv.optConstant.get
+    val nbits = repElement.lengthUnits match {
       case LengthUnits.Bits => lengthFromProp.longValue()
       case LengthUnits.Bytes => lengthFromProp.longValue() * 8
       case LengthUnits.Characters => SDE("The lengthUnits for the binary type %s must be either 'bits' or 'bytes'. Not 'characters'.", primType.name)
@@ -1149,6 +1148,7 @@ trait ElementBaseGrammarMixin
       Some(simpleType.optRepTypeElement.get)
     else
       None
+  lazy val repElement = optRepTypeElement.getOrElse(this)
 
   /**
    * the element left framing does not include the initiator nor the element right framing the terminator
diff --git a/daffodil-test/src/test/resources/org/apache/daffodil/extensions/type_calc/inputTypeCalc.tdml b/daffodil-test/src/test/resources/org/apache/daffodil/extensions/type_calc/inputTypeCalc.tdml
index 74c6ec9..17bab5f 100644
--- a/daffodil-test/src/test/resources/org/apache/daffodil/extensions/type_calc/inputTypeCalc.tdml
+++ b/daffodil-test/src/test/resources/org/apache/daffodil/extensions/type_calc/inputTypeCalc.tdml
@@ -50,6 +50,9 @@
       </xs:complexType>
     </xs:element>
 
+    <!-- lengthKind="implicit" triggers DAFFODIL-2596 -->
+    <xs:element name="keysetValue_03" type="tns:keysetValue_01" dfdl:lengthKind="implicit"/>
+
     <xs:simpleType name="uint8" dfdl:lengthKind="explicit" dfdl:length="1">
       <xs:restriction base="xs:unsignedInt"/>
     </xs:simpleType>
@@ -180,6 +183,23 @@
     </tdml:infoset>
   </tdml:parserTestCase>
 
+  <tdml:parserTestCase name="RepType_lengthKind_inherited_01"
+    root="keysetValue_03" model="inputTypeCalc-Embedded.dfdl.xsd"
+    description="Verify that the lengthKind property is inherited from the repType, not the element using the repType">
+
+    <tdml:document>
+    <tdml:documentPart type="byte">
+    01
+    </tdml:documentPart>
+    </tdml:document>
+    <tdml:infoset>
+      <tdml:dfdlInfoset xmlns:xs="http://www.w3.org/2001/XMLSchema"
+        xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance">
+        <keysetValue_03>one</keysetValue_03>
+      </tdml:dfdlInfoset>
+    </tdml:infoset>
+  </tdml:parserTestCase>
+
   <tdml:unparserTestCase name="InputTypeCalc_unparse_keysetValue_00"
     root="keysetValue_00" model="inputTypeCalc-Embedded.dfdl.xsd" description="Extensions - inputTypeCalc keysetValue transform">
 
@@ -236,6 +256,22 @@
     </tdml:infoset>
   </tdml:unparserTestCase>
 
+  <tdml:unparserTestCase name="InputTypeCalc_unparse_keysetValue_03"
+    root="keysetValue_03" model="inputTypeCalc-Embedded.dfdl.xsd" description="Extensions - inputTypeCalc keysetValue transform">
+
+    <tdml:document>
+    <tdml:documentPart type="byte">
+    01
+    </tdml:documentPart>
+    </tdml:document>
+    <tdml:infoset>
+      <tdml:dfdlInfoset xmlns:xs="http://www.w3.org/2001/XMLSchema"
+        xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance">
+        <keysetValue_03>one</keysetValue_03>
+      </tdml:dfdlInfoset>
+    </tdml:infoset>
+  </tdml:unparserTestCase>
+
   <tdml:parserTestCase name="InputTypeCalc_unionOfKeysetValueCalcs_01"
     root="inputTypeCalc_unionOfKeysetValueCalcs_01" model="inputTypeCalc-Embedded.dfdl.xsd" description="Extensions - repType with union of keysetValue types">
 
diff --git a/daffodil-test/src/test/scala/org/apache/daffodil/extensions/TestInputTypeValueCalc.scala b/daffodil-test/src/test/scala/org/apache/daffodil/extensions/TestInputTypeValueCalc.scala
index 571a6f1..2ec6fe2 100644
--- a/daffodil-test/src/test/scala/org/apache/daffodil/extensions/TestInputTypeValueCalc.scala
+++ b/daffodil-test/src/test/scala/org/apache/daffodil/extensions/TestInputTypeValueCalc.scala
@@ -44,9 +44,12 @@ class TestInputTypeValueCalc {
   @Test def test_InputTypeCalc_keysetValue_01(): Unit = { runner.runOneTest("InputTypeCalc_keysetValue_01") }
   @Test def test_InputTypeCalc_keysetValue_02(): Unit = { runner.runOneTest("InputTypeCalc_keysetValue_02") }
 
+  @Test def test_RepType_lengthKind_inherited_01(): Unit = { runner.runOneTest("RepType_lengthKind_inherited_01") }
+
   @Test def test_InputTypeCalc_unparse_keysetValue_00(): Unit = { runner.runOneTest("InputTypeCalc_unparse_keysetValue_00") }
   @Test def test_InputTypeCalc_unparse_keysetValue_01(): Unit = { runner.runOneTest("InputTypeCalc_unparse_keysetValue_01") }
   @Test def test_InputTypeCalc_unparse_keysetValue_02(): Unit = { runner.runOneTest("InputTypeCalc_unparse_keysetValue_02") }
+  @Test def test_InputTypeCalc_unparse_keysetValue_03(): Unit = { runner.runOneTest("InputTypeCalc_unparse_keysetValue_03") }
 
   @Test def test_InputTypeCalc_unionOfKeysetValueCalcs_01(): Unit = { runner.runOneTest("InputTypeCalc_unionOfKeysetValueCalcs_01") }
   @Test def test_InputTypeCalc_unparse_unionOfKeysetValueCalcs_01(): Unit = { runner.runOneTest("InputTypeCalc_unparse_unionOfKeysetValueCalcs_01") }