You are viewing a plain text version of this content. The canonical link for it is here.

Posted to commits@daffodil.apache.org by "stevedlawrence (via GitHub)" <gi...@apache.org> on 2023/08/18 12:31:27 UTC

[GitHub] [daffodil] stevedlawrence opened a new pull request, #1070: Remove dfdlx:inputTypeCalc and dfdlx:outputTypeCalc

stevedlawrence opened a new pull request, #1070:
URL: https://github.com/apache/daffodil/pull/1070

This removes the dfdlx:inputTypeCalc and dfdlx:outputTypeCalc properties and DPath functions. This allows for removal of a number of related member variables and functions used only by the expression type calc functions. For example, the typeCalcMap is only used by the functions to lookup a calculator by QName at runtime. All type calculators are known statically at compile time, so everything related to runtime type calculator maps goes away.

Also does some minor refactoring:
- Moves code around so similar logic is all in one place or removes conditionals
- Changes a tuple where only element has a meaningful value to an Either so we enforce that restriction
- Fixes typo in IdentityTypeCalc name
- Changes some error messages so they are a bit more less developer centric

DAFFODIL-2211

----
Note that this is only part of of DAFFODIL-2211. I'm making this a separate PR so it's more clear which parts are about removing input/outputValueCalc and which parts are about updating the diagnostics. It also means if we ever want to bring back typeValueCalc, we can just revert this PR without undoing the diagnostic changes

--
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

[GitHub] [daffodil] tuxji commented on a diff in pull request #1070: Remove dfdlx:inputTypeCalc and dfdlx:outputTypeCalc

Posted by "tuxji (via GitHub)" <gi...@apache.org>.

tuxji commented on code in PR #1070:
URL: https://github.com/apache/daffodil/pull/1070#discussion_r1298603950


##########
daffodil-runtime1/src/main/scala/org/apache/daffodil/runtime1/processors/TypeCalculator.scala:
##########
@@ -40,116 +35,72 @@ abstract class TypeCalculator(val srcType: NodeInfo.Kind, val dstType: NodeInfo.
   extends Serializable {

Review Comment:
   I would also add a scaladoc comment to this class giving an overview of what type calculations are used for in Daffodil schema compilation.



##########
daffodil-core/src/main/scala/org/apache/daffodil/core/dsom/SimpleTypes.scala:
##########
@@ -312,40 +308,49 @@ abstract class SimpleTypeDefBase(xml: Node, lexicalParent: SchemaComponent)
     }
   }
 
-  lazy val optInputTypeCalc = findPropertyOption("inputTypeCalc", expressionAllowed = true)
-  lazy val optOutputTypeCalc = findPropertyOption("outputTypeCalc", expressionAllowed = true)
-
   lazy val optTypeCalculator: Option[TypeCalculator] = LV('optTypeCalculator) {

Review Comment:
   I haven't read about type calculations before your PR.  Please add a comment here explaining what a type calculator does for DFDL schema compilation.  I see 5 uses of optTypeCalculator in my checkout and 2 uses left in your branch, so I wonder what prevented you from simply removing everything to do with type calculations (although I assume they have something to do with reptypes).



-- 
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

[GitHub] [daffodil] mbeckerle commented on a diff in pull request #1070: Remove dfdlx:inputTypeCalc and dfdlx:outputTypeCalc

Posted by "mbeckerle (via GitHub)" <gi...@apache.org>.

mbeckerle commented on code in PR #1070:
URL: https://github.com/apache/daffodil/pull/1070#discussion_r1303615295


##########
daffodil-runtime1/src/main/scala/org/apache/daffodil/runtime1/processors/TypeCalculator.scala:
##########
@@ -223,159 +186,34 @@ class KeysetValueTypeCalculatorUnordered(
   override def inputTypeCalc(
     x: DataValuePrimitive,
     xType: NodeInfo.Kind,
-  ): (DataValuePrimitiveNullable, Maybe[Error]) = {
+  ): Either[Error, DataValuePrimitiveNullable] = {
     valueMap.get(x) match {
-      case Some(a) => (a, Maybe.Nope)
-      case None =>
-        (DataValue.NoValue, One(s"Value ${x} not found in enumeration dfdlx:repValues"))
+      case Some(a) => Right(a)
+      case None => Left(s"Value ${x} not found in enumeration dfdlx:repValues")
     }
   }
 
   override def outputTypeCalc(
     x: DataValuePrimitive,
     xType: NodeInfo.Kind,
-  ): (DataValuePrimitiveNullable, Maybe[Error]) = {
+  ): Either[Error, DataValuePrimitiveNullable] = {
     unparseMap.get(x) match {
-      case Some(v) => (v, Maybe.Nope)
-      case None => (DataValue.NoValue, One(s"Value ${x} not found in enumeration"))
+      case Some(v) => Right(v)
+      case None => Left(s"Value ${x} not found in enumeration")
     }
   }
 
 }
 
-class ExpressionTypeCalculator(
-  private val maybeInputTypeCalcDelay: Delay[Maybe[CompiledExpression[AnyRef]]],
-  private val maybeOutputTypeCalcDelay: Delay[Maybe[CompiledExpression[AnyRef]]],
-  srcType: NodeInfo.Kind,
-  dstType: NodeInfo.Kind,
-) extends TypeCalculator(srcType, dstType) {
-
-  /*
-   * objects with Delay arguments for functional programming construction of
-   * cyclic graphs, need a way to force the delays, resulting in an ordinary
-   * (though cyclic) data structure.
-   */
-  final override def initialize(): Unit = {
-    super.initialize()
-    maybeInputTypeCalc
-    maybeOutputTypeCalc
-  }
-
-  override def supportsParse = maybeInputTypeCalc.isDefined
-  override def supportsUnparse = maybeOutputTypeCalc.isDefined
-
-  /*
-   * Compiling DPath expressions may need to evaluate typeCalculators in order to lookup their srcType and dstType.
-   * To prevent circular dependencies, this means that the underlying expressions must be lazy.
-   *
-   * Since these fields must be lazy, we cannot use them to determine supportsParse or supportUnparse
-   */
-  lazy val maybeInputTypeCalc = maybeInputTypeCalcDelay.value
-  lazy val maybeOutputTypeCalc = maybeOutputTypeCalcDelay.value
-
-  // The class TypeValueCalc will verify that supports(Un)Parse is true when nessasary
-  // Therefore, if we ever call the below functions, we know that the relevent Maybe object is defined.
-
-  // TODO, pass x into DPath state
-
+class IdentityTypeCalculator(srcType: NodeInfo.Kind) extends TypeCalculator(srcType, srcType) {
   override def inputTypeCalc(
     x: DataValuePrimitive,
     xType: NodeInfo.Kind,
-  ): (DataValuePrimitiveNullable, Maybe[Error]) =
-    Assert.invariantFailed(
-      "inputTypeCalc not implemented on ExpressionTypeCalculator. Call the more specialized forms directly",
-    )
+  ): Either[Error, DataValuePrimitiveNullable] = Right(x)
   override def outputTypeCalc(
     x: DataValuePrimitive,
     xType: NodeInfo.Kind,
-  ): (DataValuePrimitiveNullable, Maybe[Error]) =
-    Assert.invariantFailed(
-      "outputTypeCalc not implemented on ExpressionTypeCalculator. Call the more specialized forms directly",
-    )
-
-  override def inputTypeCalcParse(
-    state: PState,
-    context: RuntimeData,
-    x: DataValuePrimitive,
-    xType: NodeInfo.Kind,
-  ): DataValuePrimitiveNullable = {
-    val dstate = state.dState
-    val oldRepValue = dstate.repValue
-    val oldLogicalValue = dstate.logicalValue
-    dstate.repValue = x
-    dstate.logicalValue = DataValue.NoValue
-
-    val ans = Maybe(maybeInputTypeCalc.get.evaluate(state))
-
-    dstate.repValue = oldRepValue
-    dstate.logicalValue = oldLogicalValue
-    if (ans.isDefined) {
-      DataValue.unsafeFromAnyRef(ans.get)
-    } else {
-      DataValue.NoValue;
-    }
-  }
-  override def outputTypeCalcUnparse(
-    state: UState,
-    context: RuntimeData,
-    x: DataValuePrimitive,
-    xType: NodeInfo.Kind,
-  ): DataValuePrimitiveNullable = {
-    val dstate = state.dState
-    val oldRepValue = dstate.repValue
-    val oldLogicalValue = dstate.logicalValue
-    dstate.repValue = DataValue.NoValue
-    dstate.logicalValue = x
-
-    val ans = maybeOutputTypeCalc.get.evaluate(state)
-
-    dstate.repValue = oldRepValue
-    dstate.logicalValue = oldLogicalValue
-    DataValue.unsafeFromAnyRef(ans)
-  }
-
-  override def inputTypeCalcRun(
-    dstate: DState,
-    x: DataValuePrimitive,
-    xType: NodeInfo.Kind,
-  ): Unit = {
-    val oldRepValue = dstate.repValue
-    val oldLogicalValue = dstate.logicalValue
-    dstate.repValue = x
-    dstate.logicalValue = DataValue.NoValue
-
-    maybeInputTypeCalc.get.run(dstate)
-
-    dstate.repValue = oldRepValue
-    dstate.logicalValue = oldLogicalValue
-
-  }
-  override def outputTypeCalcRun(
-    dstate: DState,
-    x: DataValuePrimitive,
-    xType: NodeInfo.Kind,
-  ): Unit = {
-    val oldRepValue = dstate.repValue
-    val oldLogicalValue = dstate.logicalValue
-    dstate.repValue = DataValue.NoValue
-    dstate.logicalValue = x
-
-    maybeOutputTypeCalc.get.run(dstate)
-
-    dstate.repValue = oldRepValue
-    dstate.logicalValue = oldLogicalValue
-  }
-}
-
-class IdentifyTypeCalculator(srcType: NodeInfo.Kind) extends TypeCalculator(srcType, srcType) {
-  override def inputTypeCalc(
-    x: DataValuePrimitive,
-    xType: NodeInfo.Kind,
-  ): (DataValuePrimitiveNullable, Maybe[Error]) = (x, Maybe.Nope)
-  override def outputTypeCalc(
-    x: DataValuePrimitive,
-    xType: NodeInfo.Kind,
-  ): (DataValuePrimitiveNullable, Maybe[Error]) = (x, Maybe.Nope)
+  ): Either[Error, DataValuePrimitiveNullable] = Right(x)
 }
 
 class UnionTypeCalculator(

Review Comment:
   That said, are we sure we're not introducing yet another backward incompatibility.. We need to look at the complex schemas that were using inputTypeCalc et al, and see if they're using these union features. 



-- 
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