olabusayoT commented on a change in pull request #332: Refactor isHidden
URL: https://github.com/apache/incubator-daffodil/pull/332#discussion_r393015723
 
 

 ##########
 File path: 
daffodil-core/src/main/scala/org/apache/daffodil/grammar/primitives/ChoiceCombinator.scala
 ##########
 @@ -217,27 +217,37 @@ case class ChoiceCombinator(ch: ChoiceTermBase, 
alternatives: Seq[Gram])
   }
 
   override lazy val unparser: Unparser = {
-    val optDefaultableBranchUnparser = if (ch.isHidden) {
-      // this call is necessary since it will throw an SDE if no choice branch
-      // was defaultable
-      ch.childrenThatAreRecursivelyNotDefaultableOrOVC
+    /*
+     * Since it's impossible to know the hiddenness for terms at this level 
(unless
+     * they're a hiddenGroupRef), we always attempt to find a defaultable 
unparser.
+     * If we find one, and at runtime, we're in a hidden state, we can use 
that. If we
+     * are not in a hidden state, it doesn't get used. If we don't find one, 
and we encounter
+     * a hidden group ref here, we know we'll run into that issue at runtime, 
so we SDE.
+     */
+    val optDefaultableBranchUnparser = {
+      val defaultableBranches = ch.groupMembers.find { 
_.isFullyDefaultableOrOVC }
+      if (defaultableBranches.nonEmpty) {
+        val defaultableBranchRD = defaultableBranches.get.runtimeData
 
-      val defaultableBranches = ch.groupMembers.filter { 
_.childrenThatAreRecursivelyNotDefaultableOrOVC.length == 0 }
-      Assert.invariant(defaultableBranches.length > 0)
-      if (ch.isHidden && defaultableBranches.length > 1) {
-        SDW(WarnID.ChoiceInsideHiddenGroup, "xs:choice inside a hidden group 
has unparse ambiguity: multiple branches exist with all children either 
defaulable or have the dfdl:outputValueCalc property set. The first branch will 
be chosen during unparse. Defaultable branches are:\n%s",
-          defaultableBranches.mkString("\n"))
+        // Choices inside a hidden group ref are slightly different because we
+        // will never see events for any of the branches. Instead, we will just
+        // always pick the branch in which every thing is defaultble or OVC.
+        val defaultableBranchUnparser = 
alternatives.find(_.context.runtimeData =:= defaultableBranchRD).get.unparser
+        Some(defaultableBranchUnparser)
+      } else {
+        ch match {
+          case cgr: GroupRef if cgr.isHidden => {
+            /*
+             * This is a requirement for terms inside a hidden group. If we're 
a hidden group ref, we know
+             * our descendants need this requirements, so it's a SDE if they 
don't meet it.
 
 Review comment:
   I meant "require this", it's been update to clarify

----------------------------------------------------------------
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:
us...@infra.apache.org


With regards,
Apache Git Services

Reply via email to