[GitHub] [daffodil] mbeckerle commented on a diff in pull request #966: Better error message when contentLength/valueLength argument is an array

["mbeckerle (via GitHub)" <gi...@apache.org>]

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


##########
daffodil-core/src/main/scala/org/apache/daffodil/core/dpath/Expression.scala:
##########
@@ -2829,13 +2829,19 @@ sealed abstract class LengthExprBase(
   ) {
 
   protected final def leafReferencedElements = {
-    val arg = args(0).asInstanceOf[PathExpression]
+    val arg = args(0) match {
+      case arg: PathExpression => arg
+      case _ => Assert.invariantFailed("NodeInfo.Exists, but arg was not a PathExpression.")

Review Comment:
   It's annoying, but invariantFailed always wants the COVERAGE-ON/COVERAGE-OFF stuff around it. 
   I wish we could somehow build that in so it is implied for these assertions, but I don't know how. 
   
   Maybe there's a way to configure coverty so that it has a file that declares these sort of things so that one does not need to surround them?



##########
daffodil-core/src/main/scala/org/apache/daffodil/core/dpath/Expression.scala:
##########
@@ -2829,13 +2829,19 @@ sealed abstract class LengthExprBase(
   ) {
 
   protected final def leafReferencedElements = {
-    val arg = args(0).asInstanceOf[PathExpression]
+    val arg = args(0) match {
+      case arg: PathExpression => arg
+      case _ => Assert.invariantFailed("NodeInfo.Exists, but arg was not a PathExpression.")
+    }
+    // test_lengthPatternBinaryPatternLimit in section 12 shows that the length can
+    // be evaluated when it is an array of hex binary values
+    if (arg.inherentType != NodeInfo.HexBinary && arg.isPathToOneWholeArray)

Review Comment:
   This indicates a more significant issue.
   
   The expresion "." when it corresponds to an element declaration that is an array, it does NOT denote the array, it denotes the one infoset node only. 
   
   So if the array element is name 'a', then "." is not equivalent to "../a" which references the whole array. 
   
   Furthermore, suppose you write ".[1]", and '.' corresonds to an element that is  declared to be an array. 
   
   This was a subject of recent DFDL workgroup discussion, and the conclusion is that "." is never an array. So ".[1]" is always equivalent to just "." and refers to a single node only. 
   
   So I think the path expression "." (A PathExpression containing one step: Self) is going to need special treatment. 
   



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