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 2020/09/04 15:20:37 UTC

[GitHub] [incubator-daffodil] stevedlawrence commented on a change in pull request #414: Throw SDE in the case of inputValueCalc containing checkConstraints(.)

stevedlawrence commented on a change in pull request #414:
URL: https://github.com/apache/incubator-daffodil/pull/414#discussion_r483685311



##########
File path: daffodil-core/src/main/scala/org/apache/daffodil/dsom/ElementBase.scala
##########
@@ -1114,4 +1115,10 @@ trait ElementBase
     case (_, _) => this.subset((floating eq YesNo.No), "Property value floating='yes' is not supported.")
   }
 
+  private lazy val checkInputValueCalcExpression: Boolean = {
+    if (this.inputValueCalcOption.toString.contains("checkConstraints(.)")) {
+      SDE("For inputValueCalc, you cannot use checkConstraints(.) because the inputValueCalc expression is computing that value.")
+    }
+    true
+  }

Review comment:
       This feels like too specific of a check, and it's going to fail in some edgecases. For example, what if the expression was ``checkConstraints(     .)`` with a bunch of spaces. That wouldn't cause an error.
   
   Really, we don't care about checkConstraints, we just want to error if a inputValueCalc references itself. I thought we already had checks for if a property references it self. This issue isn't specific to inputValueCalc. I can also imagine cases where it references itself not via just dot, for example:
   
   ``dfdl:inputValueCalc="{ ../../foo/bar/myself }``
   
   And this isn't a trival check. It's not always possible to statically check if an element is referencing itself. For example, it could reference itself but in a previous array element, which is allowed.
   
   All that to say, I'm not sure how we improve this. Maybe the existing error is sufficient? Or perhaps we do a runtime check that if an expression tries to get the value of the context element then we do a different error message?




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

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