jadams-tresys closed pull request #60: Revert "Fixes some issues with separated 
empty optional elements, (ie…
URL: https://github.com/apache/incubator-daffodil/pull/60
 
 
   

This is a PR merged from a forked repository.
As GitHub hides the original diff on merge, it is displayed below for
the sake of provenance:

As this is a foreign pull request (from a fork), the diff is supplied
below (as it won't show otherwise due to GitHub magic):

diff --git 
a/daffodil-core/src/main/scala/org/apache/daffodil/grammar/LocalElementGrammarMixin.scala
 
b/daffodil-core/src/main/scala/org/apache/daffodil/grammar/LocalElementGrammarMixin.scala
index 50d9e2ea9..ce33817c4 100644
--- 
a/daffodil-core/src/main/scala/org/apache/daffodil/grammar/LocalElementGrammarMixin.scala
+++ 
b/daffodil-core/src/main/scala/org/apache/daffodil/grammar/LocalElementGrammarMixin.scala
@@ -108,14 +108,12 @@ trait LocalElementGrammarMixin extends GrammarMixin { 
self: ElementBase =>
   // thinking about how occursCountKind='stopValue' works.)
 
   private lazy val separatedContentAtMostN = prod("separatedContentAtMostN") {
-    (separatedContentAtMostNWithoutTrailingEmpties
-    // There may be ambiguity here if the enclosing sequence and this sequence
+    (separatedContentAtMostNWithoutTrailingEmpties // FIXME: We don't know 
whether we can absorb trailing separators or not here.
+    // We don't know if this repeating thing is in trailing position, or in 
the middle
+    // of a sequence. There is also ambiguity if the enclosing sequence and 
this sequence
     // have the same separator.
-    ~
-    (if (couldBeLastElementInModelGroup)
-      RepAtMostTotalN(self, maxOccurs, separatedEmpty) // absorb extra 
separators, if found.
-    else
-      EmptyGram)
+    //      ~
+    //      RepAtMostTotalN(self, maxOccurs, separatedEmpty) // absorb extra 
separators, if found.
     )
   }
 
@@ -208,12 +206,12 @@ trait LocalElementGrammarMixin extends GrammarMixin { 
self: ElementBase =>
       case (Trailing___, Implicit__, UNB, ___) if 
(!isLastDeclaredRequiredElementOfSequence) => SDE("occursCountKind='implicit' 
with unbounded maxOccurs only allowed for last element of a sequence")
       case (Trailing___, Implicit__, UNB, min) => 
separatedContentWithMinUnbounded
       case (Trailing___, Implicit__, max, min) if min > 0 => 
separatedContentWithMinAndMax
-      case (Trailing___, Implicit__, max, ___) => separatedContentAtMostN
+      case (Trailing___, Implicit__, max, ___) => separatedContentAtMostN // 
FIXME: have to have all of them - not trailing position
       case (TrailingStr, Implicit__, UNB, ___) if 
(!isLastDeclaredRequiredElementOfSequence) => SDE("occursCountKind='implicit' 
with unbounded maxOccurs only allowed for last element of a sequence")
       case (TrailingStr, Implicit__, UNB, ___) => 
separatedContentWithMinUnboundedWithoutTrailingEmpties // we're depending on 
optionalEmptyPart failing on empty content.
       case (TrailingStr, Implicit__, max, ___) => 
separatedContentAtMostNWithoutTrailingEmpties
       case (Always_____, Implicit__, UNB, ___) => 
separatedContentWithMinUnbounded
-      case (Always_____, Implicit__, max, ___) => 
separatedContentAtMostNWithoutTrailingEmpties
+      case (Always_____, Implicit__, max, ___) => separatedContentAtMostN
       case (Always_____, Parsed____, ___, __2) => 
separatedContentZeroToUnbounded
       case (Always_____, StopValue_, ___, __2) => 
separatedContentZeroToUnbounded
       case (policy /**/ , ock /****/ , max, __2) => 
SDE("separatorSuppressionPolicy='" + policy + "' not allowed with 
occursCountKind='" + ock + "'.")
diff --git 
a/daffodil-core/src/main/scala/org/apache/daffodil/grammar/primitives/PrimitivesElementKinds.scala
 
b/daffodil-core/src/main/scala/org/apache/daffodil/grammar/primitives/PrimitivesElementKinds.scala
index 0368b5e72..f8f2dcf74 100644
--- 
a/daffodil-core/src/main/scala/org/apache/daffodil/grammar/primitives/PrimitivesElementKinds.scala
+++ 
b/daffodil-core/src/main/scala/org/apache/daffodil/grammar/primitives/PrimitivesElementKinds.scala
@@ -112,19 +112,23 @@ case class ComplexTypeCombinator(ct: ComplexTypeBase, 
body: Gram) extends Termin
 case class SequenceCombinator(sq: SequenceTermBase, rawTerms: Seq[Gram])
   extends Terminal(sq, true) {
 
-  lazy val terms = rawTerms.filterNot{ _.isEmpty }
+  override lazy val isEmpty = {
+    val rt = rawTerms
+    val frt = rt.filterNot { _.isEmpty }
+    val res = frt.isEmpty
+    res
+  }
 
-  override lazy val isEmpty = terms.isEmpty
+  private val mt: Gram = EmptyGram
+  lazy val body = rawTerms.foldRight(mt) { _ ~ _ }
 
-  lazy val parsers = terms.map { term =>
-    term.parser
-  }.toArray
+  lazy val terms = rawTerms.filterNot { _.isEmpty }
 
   lazy val unparsers = terms.map { term =>
     term.unparser
-  }.toArray
+  }.toVector
 
-  lazy val parser: DaffodilParser = new 
SequenceCombinatorParser(sq.termRuntimeData, parsers)
+  lazy val parser: DaffodilParser = new 
SequenceCombinatorParser(sq.termRuntimeData, body.parser)
 
   override lazy val unparser: DaffodilUnparser = {
     sq.checkHiddenSequenceIsDefaultableOrOVC
diff --git 
a/daffodil-runtime1-unparser/src/main/scala/org/apache/daffodil/processors/unparsers/ElementKindUnparsers.scala
 
b/daffodil-runtime1-unparser/src/main/scala/org/apache/daffodil/processors/unparsers/ElementKindUnparsers.scala
index dce6baf80..088df469a 100644
--- 
a/daffodil-runtime1-unparser/src/main/scala/org/apache/daffodil/processors/unparsers/ElementKindUnparsers.scala
+++ 
b/daffodil-runtime1-unparser/src/main/scala/org/apache/daffodil/processors/unparsers/ElementKindUnparsers.scala
@@ -46,7 +46,7 @@ class ComplexTypeUnparser(rd: RuntimeData, bodyUnparser: 
Unparser)
   }
 }
 
-class SequenceCombinatorUnparser(ctxt: ModelGroupRuntimeData, childUnparsers: 
Array[Unparser])
+class SequenceCombinatorUnparser(ctxt: ModelGroupRuntimeData, childUnparsers: 
Vector[Unparser])
   extends CombinatorUnparser(ctxt)
   with ToBriefXMLImpl {
 
@@ -57,19 +57,22 @@ class SequenceCombinatorUnparser(ctxt: 
ModelGroupRuntimeData, childUnparsers: Ar
   // Sequences of nothing (no initiator, no terminator, nothing at all) should
   // have been optimized away
   Assert.invariant(childUnparsers.length > 0)
-  Assert.invariant(ctxt.groupMembers.length == childUnparsers.length)
+
+  // Since some of the grammar terms might have folded away to EmptyGram,
+  // the number of unparsers here may be different from the number of
+  // children of the sequence group.
+  Assert.invariant(ctxt.groupMembers.length >= childUnparsers.length)
 
   override lazy val childProcessors: Seq[Processor] = childUnparsers
 
   def unparse(start: UState): Unit = {
 
+    start.groupIndexStack.push(1L) // one-based indexing
 
     var index = 0
     var doUnparser = false
     val limit = childUnparsers.length
 
-    start.groupIndexStack.push(1L) // one-based indexing
-
     while (index < limit) {
       doUnparser = false
       val childUnparser = childUnparsers(index)
diff --git 
a/daffodil-runtime1/src/main/scala/org/apache/daffodil/processors/parsers/ElementKindParsers.scala
 
b/daffodil-runtime1/src/main/scala/org/apache/daffodil/processors/parsers/ElementKindParsers.scala
index ba7e60547..1886ee82d 100644
--- 
a/daffodil-runtime1/src/main/scala/org/apache/daffodil/processors/parsers/ElementKindParsers.scala
+++ 
b/daffodil-runtime1/src/main/scala/org/apache/daffodil/processors/parsers/ElementKindParsers.scala
@@ -119,26 +119,18 @@ class DynamicEscapeSchemeParser(escapeScheme: 
EscapeSchemeParseEv,
   }
 }
 
-class SequenceCombinatorParser(rd: TermRuntimeData, childParsers: 
Array[Parser])
+class SequenceCombinatorParser(rd: TermRuntimeData, bodyParser: Parser)
   extends CombinatorParser(rd) {
   override def nom = "Sequence"
 
   override lazy val runtimeDependencies = Nil
 
-  override lazy val childProcessors = childParsers.toSeq
-
-  val numChildParsers = childParsers.size
+  override lazy val childProcessors = Seq(bodyParser)
 
   def parse(start: PState): Unit = {
-    var i: Int = 0
-
     start.mpstate.groupIndexStack.push(1L) // one-based indexing
 
-    while ((i < numChildParsers) && (start.processorStatus eq Success)) {
-      val parser = childParsers(i)
-      parser.parse1(start)
-      i += 1
-    }
+    bodyParser.parse1(start)
 
     start.mpstate.groupIndexStack.pop()
     start.mpstate.moveOverOneGroupIndexOnly()
diff --git 
a/daffodil-test/src/test/resources/org/apache/daffodil/section13/packed/packed.tdml
 
b/daffodil-test/src/test/resources/org/apache/daffodil/section13/packed/packed.tdml
index 107cfbd89..31a857f6c 100644
--- 
a/daffodil-test/src/test/resources/org/apache/daffodil/section13/packed/packed.tdml
+++ 
b/daffodil-test/src/test/resources/org/apache/daffodil/section13/packed/packed.tdml
@@ -175,7 +175,7 @@
     <dfdl:format ref="ex:GeneralFormat" lengthKind="delimited" 
encoding="ISO-8859-1" occursCountKind="implicit"
     textNumberCheckPolicy="strict" textNumberPadCharacter="0" 
textNumberJustification="right"
     lengthUnits="bytes" binaryPackedSignCodes="C D F C" 
binaryNumberCheckPolicy="strict"
-    binaryDecimalVirtualPoint="2" separatorSuppressionPolicy="trailingEmpty"/>
+    binaryDecimalVirtualPoint="2"/>
 
   <xs:element name="packedIntSeq" dfdl:lengthKind="delimited">
     <xs:complexType>
@@ -231,20 +231,6 @@
     </xs:complexType>
   </xs:element>
 
-  <xs:element name="ibmOptIntSeq" dfdl:lengthKind="delimited" >
-    <xs:complexType>
-      <xs:sequence dfdl:initiatedContent="no" dfdl:separatorPosition="infix"
-        dfdl:sequenceKind="ordered" dfdl:separator=":">
-        <xs:element name="a" type="xs:int" dfdl:representation="binary" 
dfdl:binaryNumberRep="ibm4690Packed" minOccurs="0"/>
-        <xs:element name="b" type="xs:int" dfdl:representation="binary" 
dfdl:binaryNumberRep="ibm4690Packed" minOccurs="0"/>
-        <xs:element name="c" type="xs:int" dfdl:representation="binary" 
dfdl:binaryNumberRep="ibm4690Packed" minOccurs="0"/>
-        <xs:element name="d" type="xs:int" dfdl:representation="binary" 
dfdl:binaryNumberRep="ibm4690Packed" minOccurs="0"/>
-        <xs:element name="e" type="xs:int" dfdl:representation="binary" 
dfdl:binaryNumberRep="ibm4690Packed" minOccurs="0"/>
-      </xs:sequence>
-    </xs:complexType>
-  </xs:element>
-
-
   </tdml:defineSchema>
 
 
@@ -859,37 +845,6 @@
     </tdml:infoset>
   </tdml:parserTestCase>
 
-  <tdml:parserTestCase name="DelimitedIBM4690IntOptSeq01" root="ibmOptIntSeq" 
model="s14"
-    description="Parsing a delimited separated sequence with optional 
elements" roundTrip="false">
-
-    <tdml:document>
-      <tdml:documentPart type="byte">FFF1 3A 3A 3A 3A FFF5</tdml:documentPart>
-    </tdml:document>
-    <tdml:infoset>
-      <tdml:dfdlInfoset>
-        <ibmOptIntSeq>
-          <a>1</a>
-          <e>5</e>
-        </ibmOptIntSeq>
-      </tdml:dfdlInfoset>
-    </tdml:infoset>
-  </tdml:parserTestCase>
-
-  <tdml:parserTestCase name="DelimitedIBM4690IntOptSeq02" root="ibmOptIntSeq" 
model="s14"
-    description="Parsing a delimited separated sequence with optional 
elements" roundTrip="false">
-
-    <tdml:document>
-      <tdml:documentPart type="byte">3A 3A 3A 3A FFF5</tdml:documentPart>
-    </tdml:document>
-    <tdml:infoset>
-      <tdml:dfdlInfoset>
-        <ibmOptIntSeq>
-          <e>5</e>
-        </ibmOptIntSeq>
-      </tdml:dfdlInfoset>
-    </tdml:infoset>
-  </tdml:parserTestCase>
-
   <tdml:parserTestCase name="DelimitedIBM4690DecSeq" root="ibmDecSeq" 
model="s14"
     description="Parsing a delimited sequence of the various ibm formats">
 
diff --git 
a/daffodil-test/src/test/resources/org/apache/daffodil/section16/array_optional_elem/ArrayOptionalElem.tdml
 
b/daffodil-test/src/test/resources/org/apache/daffodil/section16/array_optional_elem/ArrayOptionalElem.tdml
index 35e217fa4..3bda61aa7 100644
--- 
a/daffodil-test/src/test/resources/org/apache/daffodil/section16/array_optional_elem/ArrayOptionalElem.tdml
+++ 
b/daffodil-test/src/test/resources/org/apache/daffodil/section16/array_optional_elem/ArrayOptionalElem.tdml
@@ -632,59 +632,4 @@
     </tdml:infoset>
   </tdml:parserTestCase>
 
-  <tdml:defineSchema name="array-ockImplicitSeparators.dfdl.xsd" 
elementFormDefault="unqualified">
-
-    <dfdl:format lengthKind="delimited" lengthUnits="bytes"
-      ref="ex:GeneralFormat" encoding="UTF-8" separator="" initiator="" 
terminator=""
-      occursCountKind="implicit" ignoreCase="no" textNumberRep="standard"
-      representation="text" />
-
-    <xs:element name="root">
-      <xs:complexType>
-        <xs:sequence dfdl:separator=":"
-          dfdl:separatorPosition="infix" 
dfdl:separatorSuppressionPolicy="trailingEmpty"
-          dfdl:sequenceKind="ordered">
-          <xs:element name="a" type="xs:int"/>
-          <xs:element name="b" type="xs:int" minOccurs="0" />
-          <xs:element name="c" type="xs:int" minOccurs="0" />
-          <xs:element name="d" type="xs:int" minOccurs="0" />
-          <xs:element name="e" type="xs:int" minOccurs="0" />
-        </xs:sequence>
-      </xs:complexType>
-    </xs:element>
-  </tdml:defineSchema>
-
-  <tdml:parserTestCase name="occursCountKindImplicitSeparators" root="root"
-    model="array-ockImplicitSeparators.dfdl.xsd"
-    roundTrip="false">
-
-    <tdml:document><![CDATA[1::3::5]]></tdml:document>
-    <tdml:infoset>
-      <tdml:dfdlInfoset>
-        <ex:root>
-          <a>1</a>
-          <c>3</c>
-          <e>5</e>
-        </ex:root>
-      </tdml:dfdlInfoset>
-    </tdml:infoset>
-  </tdml:parserTestCase>
-
-  <tdml:unparserTestCase name="occursCountKindImplicitSeparatorsUnparser" 
root="root"
-    model="array-ockImplicitSeparators.dfdl.xsd"
-    roundTrip="false">
-
-    <tdml:document><![CDATA[1:3:5]]></tdml:document>
-    <tdml:infoset>
-      <tdml:dfdlInfoset>
-        <ex:root>
-          <a>1</a>
-          <c>3</c>
-          <e>5</e>
-        </ex:root>
-      </tdml:dfdlInfoset>
-    </tdml:infoset>
-  </tdml:unparserTestCase>
-
-
 </tdml:testSuite>
diff --git 
a/daffodil-test/src/test/scala-debug/org/apache/daffodil/section13/packed/TestPacked.scala
 
b/daffodil-test/src/test/scala-debug/org/apache/daffodil/section13/packed/TestPacked.scala
deleted file mode 100644
index 4cfdb8aa8..000000000
--- 
a/daffodil-test/src/test/scala-debug/org/apache/daffodil/section13/packed/TestPacked.scala
+++ /dev/null
@@ -1,38 +0,0 @@
-/*
- * Licensed to the Apache Software Foundation (ASF) under one or more
- * contributor license agreements.  See the NOTICE file distributed with
- * this work for additional information regarding copyright ownership.
- * The ASF licenses this file to You under the Apache License, Version 2.0
- * (the "License"); you may not use this file except in compliance with
- * the License.  You may obtain a copy of the License at
- *
- *     http://www.apache.org/licenses/LICENSE-2.0
- *
- * Unless required by applicable law or agreed to in writing, software
- * distributed under the License is distributed on an "AS IS" BASIS,
- * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
- * See the License for the specific language governing permissions and
- * limitations under the License.
- */
-
-package org.apache.daffodil.section13.packed
-
-import org.apache.daffodil.tdml.Runner
-import org.junit.AfterClass
-import org.junit.Test
-
-object TestPacked {
-  val testDir = "/org/apache/daffodil/section13/packed/"
-  lazy val runner = Runner(testDir, "packed.tdml")
-
-  @AfterClass def shutdown(): Unit = {
-    runner.reset
-  }
-
-}
-
-class TestPacked {
-  import TestPacked._
-
-  @Test def testDelimitedIBM4690IntOptSeq02(): Unit = { 
runner.runOneTest("DelimitedIBM4690IntOptSeq02") } // Needs proper trailing 
suppression - DAFFODIL-1919
-}
diff --git 
a/daffodil-test/src/test/scala/org/apache/daffodil/section13/packed/TestPacked.scala
 
b/daffodil-test/src/test/scala/org/apache/daffodil/section13/packed/TestPacked.scala
index 1095370c2..cd3a33f93 100644
--- 
a/daffodil-test/src/test/scala/org/apache/daffodil/section13/packed/TestPacked.scala
+++ 
b/daffodil-test/src/test/scala/org/apache/daffodil/section13/packed/TestPacked.scala
@@ -84,8 +84,6 @@ class TestPacked {
   @Test def testDelimitedBCDIntSeqUnparser(): Unit = { 
runner.runOneTest("DelimitedBCDIntSeqUnparser") }
   @Test def testDelimitedBCDDecSeqUnparser(): Unit = { 
runner.runOneTest("DelimitedBCDDecSeqUnparser") }
   @Test def testDelimitedIBM4690IntSeq(): Unit = { 
runner.runOneTest("DelimitedIBM4690IntSeq") }
-  @Test def testDelimitedIBM4690IntOptSeq01(): Unit = { 
runner.runOneTest("DelimitedIBM4690IntOptSeq01") }
-  //@Test def testDelimitedIBM4690IntOptSeq02(): Unit = { 
runner.runOneTest("DelimitedIBM4690IntOptSeq02") } // Needs proper trailing 
suppression - DAFFODIL-1919
   @Test def testDelimitedIBM4690DecSeq(): Unit = { 
runner.runOneTest("DelimitedIBM4690DecSeq") }
   @Test def testDelimitedIBM4690IntSeqUnparser(): Unit = { 
runner.runOneTest("DelimitedIBM4690IntSeqUnparser") }
   @Test def testDelimitedIBM4690DecSeqUnparser(): Unit = { 
runner.runOneTest("DelimitedIBM4690DecSeqUnparser") }
diff --git 
a/daffodil-test/src/test/scala/org/apache/daffodil/section16/array_optional_elem/TestArrayOptionalElem.scala
 
b/daffodil-test/src/test/scala/org/apache/daffodil/section16/array_optional_elem/TestArrayOptionalElem.scala
index 220cb8f21..b2ec15725 100644
--- 
a/daffodil-test/src/test/scala/org/apache/daffodil/section16/array_optional_elem/TestArrayOptionalElem.scala
+++ 
b/daffodil-test/src/test/scala/org/apache/daffodil/section16/array_optional_elem/TestArrayOptionalElem.scala
@@ -70,7 +70,4 @@ class TestArrayOptionalElem {
   @Test def test_arrays_16_01() { runner1.runOneTest("arrays_16_01") }
 
   @Test def test_backtrack1Text() = { rBack.runOneTest("backtrack1Text") }
-
-  @Test def test_occursCountKindImplicitSeparators() { 
runner.runOneTest("occursCountKindImplicitSeparators") }
-  @Test def test_occursCountKindImplicitSeparatorsUnparser() { 
runner.runOneTest("occursCountKindImplicitSeparatorsUnparser") }
 }


 

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
[email protected]


With regards,
Apache Git Services

Reply via email to