[GitHub] [daffodil] mbeckerle commented on a change in pull request #517: Fix hang caused by a zero-length first separated infix element

[GitBox <gi...@apache.org>]

mbeckerle commented on a change in pull request #517:
URL: https://github.com/apache/daffodil/pull/517#discussion_r600836769



##########
File path: daffodil-test/src/test/resources/org/apache/daffodil/section14/sequence_groups/SequenceGroupNestedArray.tdml
##########
@@ -219,6 +219,7 @@ notice blank lines are skipped
           <ex:record>
             <ex:item>notice lines of all commas are skipped</ex:item>
           </ex:record>
+          <ex:record />

Review comment:
       I think this is a different bug. 
   
   Super thorny area of the DFDL spec. A whole tutorial needs to be written on this, and lots of tests written that we don't have so as to accurately test if we're in conformance. 
   
   The emptyValueDelimiterPolicy is not applicable. So section 9.4.2.3 says that since this is an optional occurrence of record (only the first instance is required, as minOccurs defaults to 1, not 0), nothing should be added to the infoset. 
   
   The property emptyElementParsePolicy (late addition to the DFDL v1.0 spec) is also potentially relevant here. Daffodil's implementation of this is off slightly. Our enum uses "treatAsMissing", but the DFDL spec calls this "treatAsAbsent", but regardless, this newly added property defaults to 'treatAsEmpty'. I believe it wouldn't matter either way though as the record is in optional position. 
   
   The record element is complexType and empty (no children) - which is allowed because the only child element, "item", has minOccurs="0", so none of them are required. 
   
   I'm guessing that's where the bug is. The runtime isn't properly recognizing this is an empty complex element and optional and so shouldn't be added to the infoset. 
   
   I think this is tied up in the lack of implementing defaulting for complex elements. We're not bothering to properly determine if a complex element is empty or not, because we didn't bother implementing defaulting. So the assumption is that the complex element isn't empty, so we're creating an infoset instance when we shouldn't. 




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