You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@daffodil.apache.org by ja...@apache.org on 2018/04/05 13:17:06 UTC

[incubator-daffodil] branch master updated: Revert "Fixes some issues with separated empty optional elements, (ie. 1:2::4:5)."

This is an automated email from the ASF dual-hosted git repository.

jadams pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/incubator-daffodil.git


The following commit(s) were added to refs/heads/master by this push:
     new f23602a  Revert "Fixes some issues with separated empty optional elements, (ie. 1:2::4:5)."
f23602a is described below

commit f23602ac894c436cb7a9eebe05b75229d96e911e
Author: Josh Adams <ja...@tresys.com>
AuthorDate: Thu Apr 5 08:37:48 2018 -0400

    Revert "Fixes some issues with separated empty optional elements, (ie. 1:2::4:5)."
    
    This reverts commit a54caa74deb895816aa274353d389a241c303917.
    Although this commit allowed some cases of empty optional elements with
    trailing suppression to work it caused regressions in some previously
    working schema projects (DAFFODIL-1922). Parts of this commit may still be used once
    changes have been made to the compiler that will allow full support for
    trailing separator supression (DAFFODIL-1919)
    
    DAFFODIL-1919 and DAFFODIL-1922
---
 .../grammar/LocalElementGrammarMixin.scala         | 16 +++----
 .../primitives/PrimitivesElementKinds.scala        | 18 ++++---
 .../unparsers/ElementKindUnparsers.scala           | 11 +++--
 .../processors/parsers/ElementKindParsers.scala    | 14 ++----
 .../apache/daffodil/section13/packed/packed.tdml   | 47 +-----------------
 .../array_optional_elem/ArrayOptionalElem.tdml     | 55 ----------------------
 .../daffodil/section13/packed/TestPacked.scala     | 38 ---------------
 .../daffodil/section13/packed/TestPacked.scala     |  2 -
 .../TestArrayOptionalElem.scala                    |  3 --
 9 files changed, 29 insertions(+), 175 deletions(-)

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 50d9e2e..ce33817 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 0368b5e..f8f2dcf 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 dce6baf..088df46 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 ba7e605..1886ee8 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 107cfbd..31a857f 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 35e217f..3bda61a 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 4cfdb8a..0000000
--- 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 1095370..cd3a33f 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 220cb8f..b2ec157 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") }
 }

-- 
To stop receiving notification emails like this one, please contact
jadams@apache.org.