[incubator-daffodil] branch master updated: Fix unused properties related to implied choice sequences

[sl...@apache.org]

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

slawrence 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 b988b7e  Fix unused properties related to implied choice sequences
b988b7e is described below

commit b988b7e0d1840390c36117b64ebd5d3ef653d0bc
Author: Steve Lawrence <sl...@apache.org>
AuthorDate: Tue Jun 25 11:01:38 2019 -0400

    Fix unused properties related to implied choice sequences
    
    When a ChoiceBranchImpliedSequence was created, because groupMembers was
    not overridden, it would actually create a complete duplicate of all the
    children terms because that's what the ModelGroup groupMembers that it
    inherits does. This meant that properties would be cached on different
    terms than we checked for unused properties, which lead to warnings.
    
    This overrides groupMembers for ChoiceBranchImpliedSequence so that it
    does not create new terms, but is instead just a sequence of the Term it
    wraps. This allows all the same behavior provided by the implied
    sequence, but 1) removes the duplicate Terms and 2) allows correct
    caching/checking of unused properties.
    
    DAFFODIL-2163
---
 .../org/apache/daffodil/dsom/ChoiceGroup.scala     | 20 --------------------
 .../scala/org/apache/daffodil/dsom/GroupDef.scala  |  2 +-
 .../org/apache/daffodil/dsom/SequenceGroup.scala   |  2 ++
 .../daffodil/grammar/ChoiceGrammarMixin.scala      | 22 ++++++++++++++++++++--
 4 files changed, 23 insertions(+), 23 deletions(-)

diff --git a/daffodil-core/src/main/scala/org/apache/daffodil/dsom/ChoiceGroup.scala b/daffodil-core/src/main/scala/org/apache/daffodil/dsom/ChoiceGroup.scala
index fd791d4..678bcce 100644
--- a/daffodil-core/src/main/scala/org/apache/daffodil/dsom/ChoiceGroup.scala
+++ b/daffodil-core/src/main/scala/org/apache/daffodil/dsom/ChoiceGroup.scala
@@ -344,26 +344,6 @@ abstract class ChoiceTermBase(
       maybeCheckByteAndBitOrderEv,
       maybeCheckBitOrderAndCharset)
   }
-
-  /**
-   * Examines all the children. Those that are non-scalar elements are
-   * encapsulated with a ChoiceBranchImpliedSequence, which is a kind of
-   * Sequence base.
-   *
-   * Thereafter daffodil can depend on the invariant that every recurring element is contained
-   * inside a sequence, and that sequence describes everything about how that
-   * element's occurrences are separated.
-   */
-  final lazy val groupMembersWithImpliedSequences = {
-    val rawGMs = groupMembers
-    val wrappedGMs = rawGMs.map {
-      gm =>
-        if (gm.isScalar) gm
-        else
-          new ChoiceBranchImpliedSequence(gm)
-    }
-    wrappedGMs
-  }
 }
 
 final class Choice(xmlArg: Node, lexicalParent: SchemaComponent, position: Int)
diff --git a/daffodil-core/src/main/scala/org/apache/daffodil/dsom/GroupDef.scala b/daffodil-core/src/main/scala/org/apache/daffodil/dsom/GroupDef.scala
index 962fb27..74f00bc 100644
--- a/daffodil-core/src/main/scala/org/apache/daffodil/dsom/GroupDef.scala
+++ b/daffodil-core/src/main/scala/org/apache/daffodil/dsom/GroupDef.scala
@@ -69,7 +69,7 @@ trait GroupDefLike
   private lazy val goodXmlChildren = LV('goodXMLChildren) { xmlChildren.flatMap { removeNonInteresting(_) } }.value
 
   /** Returns the group members that are elements or model groups. */
-  final lazy val groupMembers: Seq[Term] = LV('groupMembers) {
+  lazy val groupMembers: Seq[Term] = LV('groupMembers) {
     val positions = List.range(1, goodXmlChildren.length + 1) // range is exclusive on 2nd arg. So +1.
     val pairs = goodXmlChildren zip positions
     pairs.map {
diff --git a/daffodil-core/src/main/scala/org/apache/daffodil/dsom/SequenceGroup.scala b/daffodil-core/src/main/scala/org/apache/daffodil/dsom/SequenceGroup.scala
index b79b4fd..9ed2d58 100644
--- a/daffodil-core/src/main/scala/org/apache/daffodil/dsom/SequenceGroup.scala
+++ b/daffodil-core/src/main/scala/org/apache/daffodil/dsom/SequenceGroup.scala
@@ -358,6 +358,8 @@ final class ChoiceBranchImpliedSequence(rawGM: Term)
   extends SequenceTermBase(rawGM.xml, rawGM.optLexicalParent, rawGM.position)
   with SequenceDefMixin {
 
+  override final lazy val groupMembers: Seq[Term] = Seq(rawGM)
+
   override def separatorSuppressionPolicy: SeparatorSuppressionPolicy = SeparatorSuppressionPolicy.TrailingEmptyStrict
 
   override def sequenceKind: SequenceKind = SequenceKind.Ordered
diff --git a/daffodil-core/src/main/scala/org/apache/daffodil/grammar/ChoiceGrammarMixin.scala b/daffodil-core/src/main/scala/org/apache/daffodil/grammar/ChoiceGrammarMixin.scala
index 9851928..9b7b5a3 100644
--- a/daffodil-core/src/main/scala/org/apache/daffodil/grammar/ChoiceGrammarMixin.scala
+++ b/daffodil-core/src/main/scala/org/apache/daffodil/grammar/ChoiceGrammarMixin.scala
@@ -17,11 +17,12 @@
 
 package org.apache.daffodil.grammar
 
+import org.apache.daffodil.dsom.ChoiceBranchImpliedSequence
 import org.apache.daffodil.dsom.ChoiceTermBase
 import org.apache.daffodil.grammar.primitives.ChoiceCombinator
 import org.apache.daffodil.processors.parsers.NadaParser
-import org.apache.daffodil.processors.unparsers.Unparser
 import org.apache.daffodil.processors.unparsers.ChoiceUnusedUnparser
+import org.apache.daffodil.processors.unparsers.Unparser
 import org.apache.daffodil.util.Maybe
 
 trait ChoiceGrammarMixin extends GrammarMixin { self: ChoiceTermBase =>
@@ -30,5 +31,22 @@ trait ChoiceGrammarMixin extends GrammarMixin { self: ChoiceTermBase =>
     ChoiceCombinator(this, alternatives)
   }
 
-  private lazy val alternatives = groupMembersWithImpliedSequences.map{ _.termContentBody }
+  private lazy val alternatives = {
+    groupMembers.map { t =>
+      if (!t.isScalar) {
+        /**
+         * If this choice branch is a non-scalar, then we need to encapsulate
+         * it with a ChoiceBranchImpliedSequence, which is a kind of Sequence
+         * base. When compiling this this choice branch, Daffodil can then
+         * depend on the invariant that every recurring element is contained
+         * inside a sequence, and that sequence describes everything about how
+         * that elements occurrences are separated.
+         */
+        val cbis = new ChoiceBranchImpliedSequence(t)
+        cbis.termContentBody
+      } else {
+        t.termContentBody
+      }
+    }
+  }
 }