You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@daffodil.apache.org by GitBox <gi...@apache.org> on 2021/06/30 15:11:58 UTC

[GitHub] [daffodil] tuxji commented on a change in pull request #600: Remove by-name and transient params for runtime data objects

tuxji commented on a change in pull request #600:
URL: https://github.com/apache/daffodil/pull/600#discussion_r661514197



##########
File path: daffodil-runtime1/src/main/scala/org/apache/daffodil/dpath/DFDLXTypeCalcFunctions.scala
##########
@@ -44,67 +43,67 @@ case class DFDLXOutputTypeCalc(typedRecipes: List[(CompiledDPath, NodeInfo.Kind)
   }
 }
 
-case class DFDLXOutputTypeCalcNextSibling(a: CompiledDPath, b: NodeInfo.Kind) extends RecipeOp {
-
-  def run(dstate: DState): Unit = {
-    if (dstate.isCompile) {
-      //CompileDPath.runExpressionForConstant (in DPathRuntime.scala) determines
-      //that an expression is not constant by seeing if evalutating it throws an exception
-      throw new IllegalStateException()
-    }
-
-    val nextSibling = dstate.nextSibling.asSimple
-    val typeCalculator = nextSibling.erd.optSimpleTypeRuntimeData.get.typeCalculator.get
-    /*
-     * The compiler knows about all the potential typeCalculators we can see here
-     * so any validation of typeCalculator should go in Expression.scala as part of compilation
-     */
-
-    val primType = nextSibling.erd.optPrimType.get
-
-    val x = nextSibling.dataValue
-    Assert.invariant(x.isDefined)
-    typeCalculator.outputTypeCalcRun(dstate, x.getNonNullable, primType)
-  }
-}
-
-case class DFDLXRepTypeValue(a: CompiledDPath, b: NodeInfo.Kind)
-  extends RecipeOp {
-
-  override def run(dstate: DState): Unit = {
-    if (dstate.isCompile){
-      throw new IllegalStateException()
-    }
-    
-    if (!dstate.repValue.isDefined) {
-      /*
-     * In theory, we should be able to detect this error at compile time. In practice
-     * the compiler does not provide sufficient details to the expression compiler for it
-     * to notice.
-     */
-      dstate.SDE("dfdlx:repTypeValue() may only be called from within dfdlx:inputTypeCalc")
-    }
-    dstate.setCurrentValue(dstate.repValue)
-  }
-}
-
-case class DFDLXLogicalTypeValue(a: CompiledDPath, b: NodeInfo.Kind)
-  extends RecipeOp {
-
-  override def run(dstate: DState): Unit = {
-    
-    if (dstate.isCompile){
-      throw new IllegalStateException()
-    }
-    
-    if (!dstate.logicalValue.isDefined) {
-      /*
-     * In theory, we should be able to detect this error at compile time. In practice
-     * the compiler does not provide sufficient details to the expression compiler for it
-     * to notice.
-     */
-      dstate.SDE("dfdlx:logicalTypeValue() may only be called from within dfdlx:outputTypeCalc")
-    }
-    dstate.setCurrentValue(dstate.logicalValue)
-  }
-}
+//case class DFDLXOutputTypeCalcNextSibling(a: CompiledDPath, b: NodeInfo.Kind) extends RecipeOp {
+//
+//  def run(dstate: DState): Unit = {
+//    if (dstate.isCompile) {
+//      //CompileDPath.runExpressionForConstant (in DPathRuntime.scala) determines
+//      //that an expression is not constant by seeing if evalutating it throws an exception
+//      throw new IllegalStateException()
+//    }
+//
+//    val nextSibling = dstate.nextSibling.asSimple
+//    val typeCalculator = nextSibling.erd.optSimpleTypeRuntimeData.get.typeCalculator.get
+//    /*
+//     * The compiler knows about all the potential typeCalculators we can see here
+//     * so any validation of typeCalculator should go in Expression.scala as part of compilation
+//     */
+//
+//    val primType = nextSibling.erd.optPrimType.get
+//
+//    val x = nextSibling.dataValue
+//    Assert.invariant(x.isDefined)
+//    typeCalculator.outputTypeCalcRun(dstate, x.getNonNullable, primType)
+//  }
+//}
+//
+//case class DFDLXRepTypeValue(a: CompiledDPath, b: NodeInfo.Kind)
+//  extends RecipeOp {
+//
+//  override def run(dstate: DState): Unit = {
+//    if (dstate.isCompile){
+//      throw new IllegalStateException()
+//    }
+//
+//    if (!dstate.repValue.isDefined) {
+//      /*
+//     * In theory, we should be able to detect this error at compile time. In practice
+//     * the compiler does not provide sufficient details to the expression compiler for it
+//     * to notice.
+//     */
+//      dstate.SDE("dfdlx:repTypeValue() may only be called from within dfdlx:inputTypeCalc")
+//    }
+//    dstate.setCurrentValue(dstate.repValue)
+//  }
+//}
+//
+//case class DFDLXLogicalTypeValue(a: CompiledDPath, b: NodeInfo.Kind)
+//  extends RecipeOp {
+//
+//  override def run(dstate: DState): Unit = {
+//
+//    if (dstate.isCompile){
+//      throw new IllegalStateException()
+//    }
+//
+//    if (!dstate.logicalValue.isDefined) {
+//      /*
+//     * In theory, we should be able to detect this error at compile time. In practice
+//     * the compiler does not provide sufficient details to the expression compiler for it
+//     * to notice.
+//     */
+//      dstate.SDE("dfdlx:logicalTypeValue() may only be called from within dfdlx:outputTypeCalc")
+//    }
+//    dstate.setCurrentValue(dstate.logicalValue)
+//  }
+//}

Review comment:
       Please remove commented code.

##########
File path: daffodil-runtime1/src/main/scala/org/apache/daffodil/dsom/CompiledExpression1.scala
##########
@@ -341,9 +311,18 @@ class DPathCompileInfo(
  * structures are created which reference these.
  */
 class DPathElementCompileInfo(
+  // parentsArg is a transient due to serialization order issues,
+  // there is no delay/lazy/by-name involvement here.
   @TransientParam parentsArg: Seq[DPathElementCompileInfo],

Review comment:
       Is the "-Arg" suffix being used correctly here?

##########
File path: daffodil-runtime1/src/main/scala/org/apache/daffodil/processors/EncodingRuntimeData.scala
##########
@@ -121,9 +113,9 @@ trait KnownEncodingMixin { self: ThrowsSDE =>
  */
 
 final class EncodingRuntimeData(
-  @TransientParam charsetEvArg: => CharsetEv,
+  val charsetEv: CharsetEv,
   override val schemaFileLocation: SchemaFileLocation,
-  optionUTF16WidthArg: Option[UTF16Width],
+  @TransientParam optionUTF16WidthArg: Option[UTF16Width],

Review comment:
       Besides the "-Arg" suffix, why do we say `@TransientParam optionUTF16WidthArg` instead of `@TransientParam val optionUTF16WidthArg`?  Why do all the other parameters use val, but not this parameter?

##########
File path: daffodil-runtime1/src/main/scala/org/apache/daffodil/processors/RuntimeData.scala
##########
@@ -722,7 +647,11 @@ sealed abstract class ErrorERD(local: String, namespaceURI: String)
     new DPathElementCompileInfo(
       Nil, // parentsArg: => Seq[DPathElementCompileInfo],
       null, // variableMap: => VariableMap,
-      Nil, // elementChildrenCompileInfoArg: => Seq[DPathElementCompileInfo],
+      {
+        val d = Delay(Seq[DPathElementCompileInfo]()); // create Delay object so we have the right type
+        d.value ; // force evaluation which discards any closure stuff
+        d

Review comment:
       Which closure stuff is being passed?  I see only "()", or does even "()" capture the current context or objects in the current context?

##########
File path: daffodil-runtime1/src/main/scala/org/apache/daffodil/processors/RuntimeData.scala
##########
@@ -968,7 +872,7 @@ final class VariableRuntimeData(
     unqualifiedPathStepPolicyArg)
   with Serializable {
 
-  lazy val maybeDefaultValueExpr = maybeDefaultValueExprArg
+  lazy val maybeDefaultValueExpr = maybeDefaultValueExprArg.value

Review comment:
       I think the "-Arg" suffix definitely implies a Delay-type argument that has to be used by calling its "value" def.  Maybe change "-Arg" to "-Delay" everywhere to make that clearer and draw our attention to places where Delay-type args are used?
   
   I also see that we still have a preSerialization method below using maybeDefaultValueExpr.  Would it make sense or help find places using Delay-type args that can be removed in the future if the preSerialization method said maybeDefaultValueExprDelay.value instead?

##########
File path: daffodil-runtime1/src/main/scala/org/apache/daffodil/dsom/CompiledExpression1.scala
##########
@@ -201,26 +189,16 @@ final case class ConstantExpression[+T <: AnyRef](
  * into "passes".
  */
 class DPathCompileInfo(
+  // parentsArg is only a transient because of the issue below about
+  // serialization ordering. There is no delayed/lazy/by-name evaluation
+  // involved.
   @TransientParam parentsArg: Seq[DPathCompileInfo],

Review comment:
       Is `parentsArg` still an appropriate name for this parameter?  Doesn't the "-Arg" suffix imply delayed/lazy/by-name evaluation?

##########
File path: daffodil-runtime1/src/main/scala/org/apache/daffodil/dsom/CompiledExpression1.scala
##########
@@ -201,26 +189,16 @@ final case class ConstantExpression[+T <: AnyRef](
  * into "passes".
  */
 class DPathCompileInfo(
+  // parentsArg is only a transient because of the issue below about
+  // serialization ordering. There is no delayed/lazy/by-name evaluation
+  // involved.
   @TransientParam parentsArg: Seq[DPathCompileInfo],
-  @TransientParam variableMapArg: => VariableMap,
+  val variableMap: VariableMap,
   val namespaces: scala.xml.NamespaceBinding,
   val path: String,
   override val schemaFileLocation: SchemaFileLocation,
   val unqualifiedPathStepPolicy: UnqualifiedPathStepPolicy,
-  typeCalcMapArg: TypeCalcMap,
-  //
-  // lexicalContextRuntimeData is used to get the partialNextElementResolver which is used
-  // to get the next sibling info to support the outputTypeCalcNextSibling function.
-  //
-  // TODO: DAFFODIL-2326 do not pass runtime data as argment to DPathCompileInfo. The point
-  // is for DPathCompileInfo to NOT have all of the runtime data for the term, but just what
-  // is needed to compile DPath. If functions need info about next sibling, then we should
-  // compute what it needs and pass that here (e.g, sequence of possible next sibling DPathCompileInfos,
-  // or if this applies only to elements, then it should be on that object.
-  //
-  // We should not hook in the whole runtime data object here. This should be runtime independent code.
-  //
-  val lexicalContextRuntimeData: RuntimeData)
+  typeCalcMapArg: TypeCalcMap)

Review comment:
       Is the "-Arg" suffix being used correctly here?

##########
File path: daffodil-test/src/test/scala/org/apache/daffodil/extensions/TestInputTypeValueCalc.scala
##########
@@ -72,31 +72,33 @@ class TestInputTypeValueCalc {
   @Test def test_inputTypeCalcString_01(): Unit = { fnRunner.runOneTest("inputTypeCalcString_01") }
   @Test def test_outputTypeCalcInt_01(): Unit = { fnRunner.runOneTest("outputTypeCalcInt_01") }
   @Test def test_outputTypeCalcString_01(): Unit = { fnRunner.runOneTest("outputTypeCalcString_01") }
-  @Test def test_repTypeValueInt_01(): Unit = { fnRunner.runOneTest("repTypeValueInt_01") }
-  @Test def test_logicalTypeValueInt_01(): Unit = { fnRunner.runOneTest("logicalTypeValueInt_01") }
-  @Test def test_repTypeValueString_01(): Unit = { fnRunner.runOneTest("repTypeValueString_01") }
-  @Test def test_logicalTypeValueString_01(): Unit = { fnRunner.runOneTest("logicalTypeValueString_01") }
-  @Test def test_outputTypeCalcNextSiblingInt_01(): Unit = { fnRunner.runOneTest("outputTypeCalcNextSiblingInt_01") }
-  @Test def test_outputTypeCalcNextSiblingString_01(): Unit = { fnRunner.runOneTest("outputTypeCalcNextSiblingString_01") }
+//  @Test def test_repTypeValueInt_01(): Unit = { fnRunner.runOneTest("repTypeValueInt_01") }

Review comment:
       Why can we remove those tests?  Oh, because they were testing experimental features such as output type calc next sibling, etc.  Agreed, let's remove them all.

##########
File path: daffodil-core/src/main/scala/org/apache/daffodil/dpath/Expression.scala
##########
@@ -1693,96 +1693,96 @@ case class FunctionCallExpression(functionQNameString: String, expressions: List
           typeCalc.srcType, NodeInfo.String, typeCalc.dstType, DFDLXOutputTypeCalc(_, typeCalc))
       }
 
-      case (RefQName(_, "outputTypeCalcNextSibling", DFDLX), args) => {
-        val erd = compileInfo.lexicalContextRuntimeData match {
-          case erd: ElementRuntimeData => erd
-          case _ => SDE("dfdlx:outputTypeCalcNextSibling can only be defined on an element")
-        }
-        val resolver = erd.partialNextElementResolver
-        val followingERDs = resolver.currentPossibleNextElements.dropWhile { _ == erd }
-
-        // It is required that the next sibling be a peer "adjacent or downward", but not up-and-out
-        // from the location of the term where this resides.
-        //
-        //we keep the ERD to be able to produce better error messages
-        val dstTypes: Seq[(NodeInfo.Kind, ElementRuntimeData)] = followingERDs.map(erd => {
-          val strd = erd.optSimpleTypeRuntimeData match {
-            case Some(x) => x
-            case None => SDE("dfdlx:outputTypeCalcNextSibling: potential next sibling %s does not have a simple type def. This could be because it has a primitive type, or a complex type.", erd.namedQName)
-          }
-          val calc = strd.typeCalculator match {
-            case Some(x) => x
-            case None => SDE("dfdlx:outputTypeCalcNextSibling(): potential next sibling %s does not define a type calculator", erd.namedQName)
-          }
-          if (!calc.supportsUnparse) {
-            SDE("dfdlx:outputTypeCalcNextSibling(): potential next sibling %s does not have an outputTypeCalc", erd.namedQName)
-          }
-          (calc.srcType, erd)
-        })
-        val dstType = dstTypes match {
-          case Seq() => SDE("dfdlx:outputTypeCalcNextSibling() called where no suitable next sibling exists")
-          case Seq(x) => x._1
-          case x +: xs => {
-            val (ans, headQName) = x
-            /*
-             * Verify that all next siblings have the same dstType
-             * In theory, we could loosen the requirement to having the same primitive type,
-             * but in the interest of being conservative, we will stick to the stricter interperatation for now
-             */
-            xs.map(other => {
-              val (otherAns, otherQName) = other
-              if (otherAns != ans) {
-                SDE(
-                  "dfdlx:outputTypeCalcNextSibling() requires that all the possible next siblings have the same " +
-                    "repType. However, the potential next siblings %s and %s have repTypes %s and %s respectively",
-                  headQName, otherQName, ans, otherAns)
-              }
-            })
-            ans
-          }
-        }
-
-        // We need to mark all possible following compileInfos as possibly used
-        // in an expression since the DFDLXOutputTypeCalcNextSibling expression
-        // might need them. We also need to mark this compileInfo as used in an
-        // expression since the evaluate method of DFDLXTypeCalcNextSibilng
-        // uses this element to find the next sibling
-        val dpeci = erd.dpathCompileInfo.asInstanceOf[DPathElementCompileInfo]
-        followingERDs.foreach { erd =>
-          dpeci.indicateReferencedByExpression(Seq(erd.dpathElementCompileInfo))
-        }
-        dpeci.indicateReferencedByExpression(Seq(dpeci))
-
-        FNZeroArgExpr(functionQNameString, functionQName,
-          dstType, NodeInfo.AnyAtomic, DFDLXOutputTypeCalcNextSibling(_, _))
-      }
-
-      case (RefQName(_, "repTypeValue", DFDLX), args) => {
-        val strd = compileInfo.lexicalContextRuntimeData match {
-          case strd: SimpleTypeRuntimeData => strd
-          case _ => {
-            SDE("dfdlx:repTypeValue() can only be defined on a simple type")
-          }
-        }
-        val repPrimType = strd.optRepPrimType match {
-          case Some(x) => x
-          case None => SDE("dfdlx:repTypeValue() used on type that does not have a repType.")
-        }
-        FNZeroArgExpr(functionQNameString, functionQName,
-          repPrimType, NodeInfo.AnyAtomic, DFDLXRepTypeValue(_, _))
-      }
-
-      case (RefQName(_, "logicalTypeValue", DFDLX), args) => {
-        val strd = compileInfo.lexicalContextRuntimeData match {
-          case strd: SimpleTypeRuntimeData => strd
-          case _ => {
-            SDE("dfdlx:logicalTypeValue() can only be defined on a simple type")
-          }
-        }
-        val logicalType = strd.primType
-        FNZeroArgExpr(functionQNameString, functionQName,
-          logicalType, NodeInfo.AnyAtomic, DFDLXLogicalTypeValue(_, _))
-      }
+//      case (RefQName(_, "outputTypeCalcNextSibling", DFDLX), args) => {

Review comment:
       Please remove commented code as noted.

##########
File path: daffodil-runtime1/src/main/scala/org/apache/daffodil/processors/RuntimeData.scala
##########
@@ -101,27 +99,26 @@ object TermRuntimeData {
 
 sealed abstract class TermRuntimeData(
   /**
-   * These transient by-name args are part of how we
+   * These transient Delay-type args are part of how we
    * create a structure here which contains some objects that
    * somewhere within themselves, refer back to this structure.
    *
-   * These are passed by-name, and ultimately what is serialized is not
-   * these, but lazy vals that refer to them which are forced to have
-   * values at the time of object serialization.
+   * For any transient args, their values must be used in preSerialization
+   * method, unless they are passed to a super-class constructor.
    */

Review comment:
       Now that we have only one Delay-type arg here (partialNextElementResolverArg), I think your comment should explain why we still need to pass around a partialNextElementResolverArg Delay-type arg as well as what Delay-type means.

##########
File path: daffodil-lib/src/main/scala/org/apache/daffodil/util/Delay.scala
##########
@@ -28,17 +28,45 @@ package org.apache.daffodil.util
  * as Delay objects.
  *
  * For more info, this is used in the IncludeImport stuff in DSOM.
+ *
+ * This is NOT serializable, and shouldn't be made so.
  */
 
-class Delay[T] private (v: => T) {
-  lazy val value = v
-  private val hasValue: Boolean = false
+final class Delay[T] private (private var box: Delay.Box[T]) {
+  //
+  // This trick of taking another object on a var, and
+  // then clobbering the var after we demand the value
+  // eliminates holding onto a bunch of objects due
+  // to closures for the pass-by-name arguments.
+  //
+  lazy val value: T = {
+    val v = box.value
+    box = null // throws away box, which allows GC of all closures, etc.
+    v
+  }
+
+  def hasValue = box ne null

Review comment:
       Or remove hasValue and inline its body into the toString below.

##########
File path: daffodil-runtime1/src/main/scala/org/apache/daffodil/processors/EncodingRuntimeData.scala
##########
@@ -132,11 +124,7 @@ final class EncodingRuntimeData(
   val hasTextAlignment: Boolean)
   extends KnownEncodingMixin with ImplementsThrowsSDE with PreSerialization {
 
-  private val maybeUTF16Width_ = Maybe.toMaybe[UTF16Width](optionUTF16WidthArg)
-
-  def maybeUTF16Width = maybeUTF16Width_
-
-  lazy val charsetEv = charsetEvArg
+  val maybeUTF16Width = Maybe.toMaybe[UTF16Width](optionUTF16WidthArg)

Review comment:
       OK, make it so. :)

##########
File path: daffodil-runtime1/src/main/scala/org/apache/daffodil/processors/RuntimeData.scala
##########
@@ -539,41 +522,40 @@ final class SimpleTypeRuntimeData(
 
 sealed class ElementRuntimeData(
   /**
-   * These transient by-name args are part of how we
+   * These transient Delay-type args are part of how we
    * create a structure here which contains some objects that
    * somewhere within themselves, refer back to this structure.
    *
-   * These are passed by-name, and ultimately what is serialized is not
-   * these, but lazy vals that refer to them which are forced to have
-   * values at the time of object serialization.
-   *
-   * Note that all transient elements must be added to the preSerialization method below
-   * to allow parser serialization/deserialization to work.
+   * Note that all transient elements, their real values must
+   * be added to the preSerialization method below
+   * to allow parser serialization/deserialization to work, unless
+   * they are passed to a superclass constructor, in which case this
+   * becomes the superclass' responsibility.
    */

Review comment:
       How does partialNextElementResolverArg refer back to a containing object's structure?  This comment doesn't say.  Also, the comment says we should add partialNextElementResolverArg to the preSerialization method below, but I think from looking at the diff that you removed the preSerialization method below completely.  Is the comment still correct?

##########
File path: daffodil-test/src/test/resources/org/apache/daffodil/extensions/type_calc/typeCalcFunctionErrors.tdml
##########
@@ -53,60 +53,60 @@
     <xs:element name="typeCalcDispatch_typeError_01" type="xs:int" dfdl:inputValueCalc="{ dfdlx:inputTypeCalc('tns:intToString', 0) }" />
     <xs:element name="typeCalcDispatch_typeError_02" type="xs:int" dfdl:inputValueCalc="{ dfdlx:outputTypeCalc('tns:stringToInt', 0) }" />
 
-    <xs:element name="typeCalcDispatch_typeError_03" dfdlx:parseUnparsePolicy="unparseOnly">
-      <xs:complexType>
-        <xs:sequence>
-          <xs:element name="raw" dfdl:outputValueCalc="{ dfdlx:outputTypeCalcNextSibling() }" type="tns:uint8" dfdlx:parseUnparsePolicy="unparseOnly"/>
-          <xs:element name="stringToInt" type="stringToInt" dfdlx:parseUnparsePolicy="unparseOnly" dfdl:inputValueCalc="{ 0 }"/>
-        </xs:sequence>
-      </xs:complexType>
-    </xs:element>
-
-    <xs:element name="typeCalcDispatch_typeError_04" dfdlx:parseUnparsePolicy="unparseOnly">
-      <xs:complexType>
-        <xs:sequence>
-          <xs:element name="raw" dfdl:outputValueCalc="{ dfdlx:outputTypeCalcNextSibling() }" type="tns:uint8" dfdlx:parseUnparsePolicy="unparseOnly"/>
-          <xs:element name="stringToInt" type="stringToInt" dfdlx:parseUnparsePolicy="unparseOnly" dfdl:inputValueCalc="{ 0 }"/>
-        </xs:sequence>
-      </xs:complexType>
-    </xs:element>
-
-    <xs:element name="typeCalcDispatch_typeError_05">
-      <xs:simpleType dfdlx:repType="tns:string8" dfdlx:inputTypeCalc="{ dfdlx:repTypeValue() }">
-        <xs:restriction base="xs:int"/>
-      </xs:simpleType>
-    </xs:element>
-
-    <xs:element name="typeCalcDispatch_typeError_06">
-      <xs:simpleType dfdlx:repType="tns:string8" dfdlx:inputTypeCalc="{ dfdlx:repTypeValue() }">
-        <xs:restriction base="xs:int"/>
-      </xs:simpleType>
-    </xs:element>
-
-    <xs:element name="typeCalcDispatch_typeError_07" dfdlx:parseUnparsePolicy="unparseOnly" >
-      <xs:simpleType dfdlx:repType="tns:uint8" dfdlx:outputTypeCalc="{ dfdlx:logicalTypeValue() }">
-        <xs:restriction base="xs:string"/>
-      </xs:simpleType>
-    </xs:element>
-
-    <xs:element name="typeCalcDispatch_typeError_08" dfdlx:parseUnparsePolicy="unparseOnly">
-      <xs:simpleType dfdlx:repType="tns:uint8" dfdlx:outputTypeCalc="{ dfdlx:logicalTypeValue() }">
-        <xs:restriction base="xs:string"/>
-      </xs:simpleType>
-    </xs:element>
-
-    <xs:element name="repTypeValue_bad_context_01" type="xs:string" dfdl:inputValueCalc="{ dfdlx:repTypeValue() }"/>
-    <xs:element name="repTypeValue_bad_context_02" dfdlx:parseUnparsePolicy="unparseOnly">
-      <xs:simpleType dfdlx:repType="tns:uint8" dfdlx:outputTypeCalc="{ dfdlx:repTypeValue() }" dfdlx:inputTypeCalc="{ 0 }">
-        <xs:restriction base="xs:int"/>
-      </xs:simpleType>
-    </xs:element>
-    <xs:element name="logicalTypeValue_bad_context_01" type="xs:string" dfdl:inputValueCalc="{ dfdlx:logicalTypeValue() }"/>
-    <xs:element name="logicalTypeValue_bad_context_02">
-      <xs:simpleType dfdlx:repType="tns:uint8" dfdlx:inputTypeCalc="{ dfdlx:logicalTypeValue() }" dfdlx:outputTypeCalc="{ 0 }">
-        <xs:restriction base="xs:int"/>
-      </xs:simpleType>
-    </xs:element>
+<!--    <xs:element name="typeCalcDispatch_typeError_03" dfdlx:parseUnparsePolicy="unparseOnly">-->

Review comment:
       Remove commented code as noted.

##########
File path: daffodil-core/src/main/scala/org/apache/daffodil/runtime1/SequenceTermRuntime1Mixin.scala
##########
@@ -30,7 +30,7 @@ trait SequenceTermRuntime1Mixin { self: SequenceTermBase =>
   lazy val sequenceRuntimeData = {
     new SequenceRuntimeData(
       position,
-      partialNextElementResolver,
+      Delay(partialNextElementResolver),

Review comment:
       Be nice to remove if not needed.

##########
File path: daffodil-core/src/main/scala/org/apache/daffodil/runtime1/ElementBaseRuntime1Mixin.scala
##########
@@ -163,7 +164,7 @@ trait ElementBaseRuntime1Mixin { self: ElementBase =>
       position,
       childrenERDs,
       schemaSet.variableMap,
-      partialNextElementResolver,
+      Delay(partialNextElementResolver),

Review comment:
       Sure, one place is better unless the def becomes a hot spot.




-- 
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: commits-unsubscribe@daffodil.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org