You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@daffodil.apache.org by GitBox <gi...@apache.org> on 2020/07/10 23:21:45 UTC

[GitHub] [incubator-daffodil] mbeckerle opened a new pull request #396: Add tests for nested choice branch cases that seemed problematic.

mbeckerle opened a new pull request #396:
URL: https://github.com/apache/incubator-daffodil/pull/396


   These tests convinced me this is all working properly.
   
   They should be added to the suite even though they did not
   find a new bug.
   
   See DAFFODIL-2367


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



[GitHub] [incubator-daffodil] mbeckerle commented on a change in pull request #396: Add tests for nested choice branch cases that seemed problematic.

Posted by GitBox <gi...@apache.org>.
mbeckerle commented on a change in pull request #396:
URL: https://github.com/apache/incubator-daffodil/pull/396#discussion_r453666254



##########
File path: daffodil-test/src/test/resources/org/apache/daffodil/section07/discriminators/nestedChoiceDiscriminator.tdml
##########
@@ -0,0 +1,306 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<!--
+  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.
+-->
+
+<tdml:testSuite
+  xmlns:ex="http://example.com"
+  xmlns:tdml="http://www.ibm.com/xmlns/dfdl/testData"
+  xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
+  xmlns:dfdl="http://www.ogf.org/dfdl/dfdl-1.0/"
+  xmlns:daf="urn:ogf:dfdl:2013:imp:daffodil.apache.org:2018:ext"
+  xmlns:xs="http://www.w3.org/2001/XMLSchema"
+  xmlns:fn="http://www.w3.org/2005/xpath-functions"
+  defaultRoundTrip="true"
+  defaultValidation="on">
+
+  <tdml:defineSchema name="s0" elementFormDefault="unqualified">
+    <xs:include schemaLocation="org/apache/daffodil/xsd/DFDLGeneralFormat.dfdl.xsd"/>
+    <dfdl:format ref="ex:GeneralFormat" lengthKind="delimited"/>
+
+    <!-- <![CDATA[
+
+    This idiom where we capture unrecognized messages under control of a variable is common and
+    there are issues with discriminators that need testing relative to it.
+
+    ]]> -->
+
+    <dfdl:defineVariable name="capture" type="xs:boolean" external="true" defaultValue="false"/>
+
+    <xs:element name="checkVar" type="xs:string"
+                dfdl:inputValueCalc='{ xs:string($ex:capture) }'/>
+
+    <xs:group name="discriminate">
+      <xs:sequence>
+        <xs:annotation>
+          <xs:appinfo source="http://www.ogf.org/dfdl/">
+            <dfdl:discriminator>{ fn:true() }</dfdl:discriminator>
+          </xs:appinfo>
+        </xs:annotation>
+      </xs:sequence>
+    </xs:group>
+
+    <xs:complexType name="messageType">
+      <xs:sequence dfdl:separator="|">
+        <xs:element name="num" type="xs:int"/>
+        <xs:element name="text" type="xs:string"/>
+      </xs:sequence>
+    </xs:complexType>
+
+    <xs:element name="root">
+      <xs:complexType>
+        <xs:sequence>
+          <xs:element ref="ex:checkVar"/>
+          <xs:element name="msg" minOccurs="1" maxOccurs="unbounded">
+            <xs:complexType>
+              <xs:sequence dfdl:initiator="%WSP*;[" dfdl:terminator="]%WSP*;">
+                <xs:element name="messageID" type="xs:string" dfdl:lengthKind="explicit" dfdl:length="1"/>
+                <xs:sequence dfdl:initiator="|">
+                  <xs:choice>
+                    <xs:choice dfdl:choiceDispatchKey="{ ./messageID }">
+                      <xs:sequence dfdl:choiceBranchKey="1">
+                        <!--
+                        This techique, where we discriminate the outer choice if the choice dispatch
+                        is successful, is better during DFDL schema development because we won't backtrack
+                        and create unrecognized messages that suppress errors during the parsing of a recognized
+                        message type.
+                        -->
+                        <xs:group ref="ex:discriminate"/>

Review comment:
       That point in the spec will need an editorial correction. I'll bring this up on the DFDL workgroup email list and see if there is consensus on this, but I thought this was resolved long ago.
   
   My understanding is that there is no PoU for a choice with direct dispatch. In the same way there is no PoU for an array with a computed number of occurrences. 
   
   That's equivalent to saying there is a PoU but it is resolved by the direct dispatch; hence, thinking of discriminators as a stack, the discriminator of the inner choice would be set true as soon as the dispatch expression resolves and matches a branch tag. 
   
   Hence, a discriminator within a branch of a direct-dispatch-choice applies to the discriminator of the enclosing outer choice, not the inner either way. 
   
   There really should be identifiers on PoUs so that choices can be labeled accordingly to catch mistakes. 




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



[GitHub] [incubator-daffodil] mbeckerle commented on a change in pull request #396: Add tests for nested choice branch cases that seemed problematic.

Posted by GitBox <gi...@apache.org>.
mbeckerle commented on a change in pull request #396:
URL: https://github.com/apache/incubator-daffodil/pull/396#discussion_r453118846



##########
File path: daffodil-test/src/test/resources/org/apache/daffodil/section07/discriminators/nestedChoiceDiscriminator.tdml
##########
@@ -0,0 +1,306 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<!--
+  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.
+-->
+
+<tdml:testSuite
+  xmlns:ex="http://example.com"
+  xmlns:tdml="http://www.ibm.com/xmlns/dfdl/testData"
+  xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
+  xmlns:dfdl="http://www.ogf.org/dfdl/dfdl-1.0/"
+  xmlns:daf="urn:ogf:dfdl:2013:imp:daffodil.apache.org:2018:ext"
+  xmlns:xs="http://www.w3.org/2001/XMLSchema"
+  xmlns:fn="http://www.w3.org/2005/xpath-functions"
+  defaultRoundTrip="true"
+  defaultValidation="on">
+
+  <tdml:defineSchema name="s0" elementFormDefault="unqualified">
+    <xs:include schemaLocation="org/apache/daffodil/xsd/DFDLGeneralFormat.dfdl.xsd"/>
+    <dfdl:format ref="ex:GeneralFormat" lengthKind="delimited"/>
+
+    <!-- <![CDATA[
+
+    This idiom where we capture unrecognized messages under control of a variable is common and
+    there are issues with discriminators that need testing relative to it.
+
+    ]]> -->
+
+    <dfdl:defineVariable name="capture" type="xs:boolean" external="true" defaultValue="false"/>
+
+    <xs:element name="checkVar" type="xs:string"
+                dfdl:inputValueCalc='{ xs:string($ex:capture) }'/>
+
+    <xs:group name="discriminate">
+      <xs:sequence>
+        <xs:annotation>
+          <xs:appinfo source="http://www.ogf.org/dfdl/">
+            <dfdl:discriminator>{ fn:true() }</dfdl:discriminator>
+          </xs:appinfo>
+        </xs:annotation>
+      </xs:sequence>
+    </xs:group>
+
+    <xs:complexType name="messageType">
+      <xs:sequence dfdl:separator="|">
+        <xs:element name="num" type="xs:int"/>
+        <xs:element name="text" type="xs:string"/>
+      </xs:sequence>
+    </xs:complexType>
+
+    <xs:element name="root">
+      <xs:complexType>
+        <xs:sequence>
+          <xs:element ref="ex:checkVar"/>
+          <xs:element name="msg" minOccurs="1" maxOccurs="unbounded">
+            <xs:complexType>
+              <xs:sequence dfdl:initiator="%WSP*;[" dfdl:terminator="]%WSP*;">
+                <xs:element name="messageID" type="xs:string" dfdl:lengthKind="explicit" dfdl:length="1"/>
+                <xs:sequence dfdl:initiator="|">
+                  <xs:choice>
+                    <xs:choice dfdl:choiceDispatchKey="{ ./messageID }">
+                      <xs:sequence dfdl:choiceBranchKey="1">
+                        <!--
+                        This techique, where we discriminate the outer choice if the choice dispatch
+                        is successful, is better during DFDL schema development because we won't backtrack
+                        and create unrecognized messages that suppress errors during the parsing of a recognized
+                        message type.
+                        -->
+                        <xs:group ref="ex:discriminate"/>
+                        <xs:element name="message1" type="ex:messageType"/>
+                      </xs:sequence>
+                      <xs:sequence dfdl:choiceBranchKey="2">
+                        <xs:group ref="ex:discriminate"/>
+                        <!--
+                        This shows the technique for coping with parse errors of recognized message types if we
+                        want to capture them anyway.
+
+                        This is not necessarily desirable. The reason why the message failed is masked by this.
+                        -->
+                        <xs:choice>
+                          <xs:element name="message2" type="ex:messageType"/>
+                          <xs:group ref="ex:messageParseError"/>
+                        </xs:choice>
+                      </xs:sequence>
+                    </xs:choice>
+                    <xs:sequence>
+                      <!--
+                      default case for completely unrecognized message
+                      -->
+                      <xs:sequence>
+                        <xs:annotation>
+                          <xs:appinfo source="http://www.ogf.org/dfdl/">
+                            <!--
+                            If capture variable not set, then just fail here, and report an error.
+                            -->
+                            <dfdl:discriminator message='{ fn:concat("Unrecognized messageID: ", ./messageID) }'
+                                                test='{ $ex:capture }'/>
+                          </xs:appinfo>
+                        </xs:annotation>
+                      </xs:sequence>
+                      <!--
+                       Otherwise, create this unrecognized capture element.
+
+                       Notice that maxOccurs="0" so valid data never contains these.
+                       Validation checking will reject data that contains these.
+
+                       This tells us that the same DFDL schema, with these capture elements expressed in it
+                       can be used for both parse/unparse and for separate validation of the data.
+                       -->
+                      <xs:element name="unrecognizedMessage" minOccurs="0" maxOccurs="0"
+                                  dfdl:occursCountKind="expression"
+                                  dfdl:occursCount='{ if ($ex:capture) then 1 else 0 }'
+                                  type="xs:string"/>
+                    </xs:sequence>
+                  </xs:choice>
+                </xs:sequence>
+              </xs:sequence>
+            </xs:complexType>
+          </xs:element>
+        </xs:sequence>
+      </xs:complexType>
+    </xs:element>
+
+    <xs:group name="messageParseError">
+      <xs:sequence>
+        <!-- default case for failure to parse a message -->
+        <xs:sequence>
+          <xs:annotation>
+            <xs:appinfo source="http://www.ogf.org/dfdl/">
+              <!--
+              If capture variable not set, then just fail here, and report an error.
+              -->
+              <dfdl:discriminator message='{ fn:concat("Unable to parse message with messageID: ", ./messageID) }'
+                                  test='{ $ex:capture }'/>
+            </xs:appinfo>
+          </xs:annotation>
+        </xs:sequence>
+        <!--
+         Otherwise, create this unrecognized capture element

Review comment:
       Not unrecognized capture element. This is a message parse error capture element. 




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



[GitHub] [incubator-daffodil] stevedlawrence commented on a change in pull request #396: Add tests for nested choice branch cases that seemed problematic.

Posted by GitBox <gi...@apache.org>.
stevedlawrence commented on a change in pull request #396:
URL: https://github.com/apache/incubator-daffodil/pull/396#discussion_r453611084



##########
File path: daffodil-test/src/test/resources/org/apache/daffodil/section07/discriminators/nestedChoiceDiscriminator.tdml
##########
@@ -0,0 +1,306 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<!--
+  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.
+-->
+
+<tdml:testSuite
+  xmlns:ex="http://example.com"
+  xmlns:tdml="http://www.ibm.com/xmlns/dfdl/testData"
+  xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
+  xmlns:dfdl="http://www.ogf.org/dfdl/dfdl-1.0/"
+  xmlns:daf="urn:ogf:dfdl:2013:imp:daffodil.apache.org:2018:ext"
+  xmlns:xs="http://www.w3.org/2001/XMLSchema"
+  xmlns:fn="http://www.w3.org/2005/xpath-functions"
+  defaultRoundTrip="true"
+  defaultValidation="on">
+
+  <tdml:defineSchema name="s0" elementFormDefault="unqualified">
+    <xs:include schemaLocation="org/apache/daffodil/xsd/DFDLGeneralFormat.dfdl.xsd"/>
+    <dfdl:format ref="ex:GeneralFormat" lengthKind="delimited"/>
+
+    <!-- <![CDATA[
+
+    This idiom where we capture unrecognized messages under control of a variable is common and
+    there are issues with discriminators that need testing relative to it.
+
+    ]]> -->
+
+    <dfdl:defineVariable name="capture" type="xs:boolean" external="true" defaultValue="false"/>
+
+    <xs:element name="checkVar" type="xs:string"
+                dfdl:inputValueCalc='{ xs:string($ex:capture) }'/>
+
+    <xs:group name="discriminate">
+      <xs:sequence>
+        <xs:annotation>
+          <xs:appinfo source="http://www.ogf.org/dfdl/">
+            <dfdl:discriminator>{ fn:true() }</dfdl:discriminator>
+          </xs:appinfo>
+        </xs:annotation>
+      </xs:sequence>
+    </xs:group>
+
+    <xs:complexType name="messageType">
+      <xs:sequence dfdl:separator="|">
+        <xs:element name="num" type="xs:int"/>
+        <xs:element name="text" type="xs:string"/>
+      </xs:sequence>
+    </xs:complexType>
+
+    <xs:element name="root">
+      <xs:complexType>
+        <xs:sequence>
+          <xs:element ref="ex:checkVar"/>
+          <xs:element name="msg" minOccurs="1" maxOccurs="unbounded">
+            <xs:complexType>
+              <xs:sequence dfdl:initiator="%WSP*;[" dfdl:terminator="]%WSP*;">
+                <xs:element name="messageID" type="xs:string" dfdl:lengthKind="explicit" dfdl:length="1"/>
+                <xs:sequence dfdl:initiator="|">
+                  <xs:choice>
+                    <xs:choice dfdl:choiceDispatchKey="{ ./messageID }">
+                      <xs:sequence dfdl:choiceBranchKey="1">
+                        <!--
+                        This techique, where we discriminate the outer choice if the choice dispatch
+                        is successful, is better during DFDL schema development because we won't backtrack
+                        and create unrecognized messages that suppress errors during the parsing of a recognized
+                        message type.
+                        -->
+                        <xs:group ref="ex:discriminate"/>

Review comment:
       This doesn't behave the way I would expect it to by my reading of the spec. From the spec, both the inner and outer choices create a point of uncertainty:
   
   > An xs:choice is always a point of uncertainty. It is resolved sequentially, or by direct dispatch ... Direct-dispatch
   > choice resolution occurs by matching the value of the dfdl:choiceDispatchKey property to the value of the
   > dfdl:choiceChoiceBranchKey [sic] property of one of the choice branches.
   
   So the direct dispatch resolves the inner PoU, but isn't that inner PoU still in scope when this discriminator is evaluated? I would expect this discriminator to resolve the inner choice PoU to true, which is essentially a no-op since it's already been resolved to true by direct dispatch. So if something fails inside the inner choice branch, I would expect we backtrack to the outer choice PoU and speculatively parse to the second branch of the outer choice. So I would expect the nestedChoice2 test to result in the unrecognizedMessage element since nothing resolves the outer choice. Feels like a bug to me, unless I'm misreading the spec.
   
   The way I see it, the problem is there isn't a good way to differentiate between "a direct dispatch branch was found but something inside it failed" vs "a direct dispatch branch was not found". The outer PoU only ever sees that something about the inner choice failed, and it can't know why. The inner discrminator seems to be able to break outside the inner PoU and provide that information, but that seems wrong to me. Perhaps what is needed is some sort of ``dfdl:choiceBranchDefault="yes"`` property, which I think would no longer require the outer choice or inner discriminators and would work as expected?




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



[GitHub] [incubator-daffodil] mbeckerle commented on a change in pull request #396: Add tests for nested choice branch cases that seemed problematic.

Posted by GitBox <gi...@apache.org>.
mbeckerle commented on a change in pull request #396:
URL: https://github.com/apache/incubator-daffodil/pull/396#discussion_r453118578



##########
File path: daffodil-test/src/test/resources/org/apache/daffodil/section05/facets/NulChars.tdml
##########
@@ -36,7 +36,7 @@
           <xs:element name="foo" dfdl:lengthKind="pattern" dfdl:lengthPattern="\x{0000}{3,3}">
             <xs:simpleType>
               <xs:restriction base="xs:string">
-                <xs:pattern value="&#xE000;*"/>
+                <xs:pattern value="[&#xE000;]*"/>

Review comment:
       Revert. This got carried over from another experiment. 




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



[GitHub] [incubator-daffodil] mbeckerle merged pull request #396: Add tests for nested choice branch cases that seemed problematic.

Posted by GitBox <gi...@apache.org>.
mbeckerle merged pull request #396:
URL: https://github.com/apache/incubator-daffodil/pull/396


   


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



[GitHub] [incubator-daffodil] stevedlawrence commented on a change in pull request #396: Add tests for nested choice branch cases that seemed problematic.

Posted by GitBox <gi...@apache.org>.
stevedlawrence commented on a change in pull request #396:
URL: https://github.com/apache/incubator-daffodil/pull/396#discussion_r453611084



##########
File path: daffodil-test/src/test/resources/org/apache/daffodil/section07/discriminators/nestedChoiceDiscriminator.tdml
##########
@@ -0,0 +1,306 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<!--
+  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.
+-->
+
+<tdml:testSuite
+  xmlns:ex="http://example.com"
+  xmlns:tdml="http://www.ibm.com/xmlns/dfdl/testData"
+  xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
+  xmlns:dfdl="http://www.ogf.org/dfdl/dfdl-1.0/"
+  xmlns:daf="urn:ogf:dfdl:2013:imp:daffodil.apache.org:2018:ext"
+  xmlns:xs="http://www.w3.org/2001/XMLSchema"
+  xmlns:fn="http://www.w3.org/2005/xpath-functions"
+  defaultRoundTrip="true"
+  defaultValidation="on">
+
+  <tdml:defineSchema name="s0" elementFormDefault="unqualified">
+    <xs:include schemaLocation="org/apache/daffodil/xsd/DFDLGeneralFormat.dfdl.xsd"/>
+    <dfdl:format ref="ex:GeneralFormat" lengthKind="delimited"/>
+
+    <!-- <![CDATA[
+
+    This idiom where we capture unrecognized messages under control of a variable is common and
+    there are issues with discriminators that need testing relative to it.
+
+    ]]> -->
+
+    <dfdl:defineVariable name="capture" type="xs:boolean" external="true" defaultValue="false"/>
+
+    <xs:element name="checkVar" type="xs:string"
+                dfdl:inputValueCalc='{ xs:string($ex:capture) }'/>
+
+    <xs:group name="discriminate">
+      <xs:sequence>
+        <xs:annotation>
+          <xs:appinfo source="http://www.ogf.org/dfdl/">
+            <dfdl:discriminator>{ fn:true() }</dfdl:discriminator>
+          </xs:appinfo>
+        </xs:annotation>
+      </xs:sequence>
+    </xs:group>
+
+    <xs:complexType name="messageType">
+      <xs:sequence dfdl:separator="|">
+        <xs:element name="num" type="xs:int"/>
+        <xs:element name="text" type="xs:string"/>
+      </xs:sequence>
+    </xs:complexType>
+
+    <xs:element name="root">
+      <xs:complexType>
+        <xs:sequence>
+          <xs:element ref="ex:checkVar"/>
+          <xs:element name="msg" minOccurs="1" maxOccurs="unbounded">
+            <xs:complexType>
+              <xs:sequence dfdl:initiator="%WSP*;[" dfdl:terminator="]%WSP*;">
+                <xs:element name="messageID" type="xs:string" dfdl:lengthKind="explicit" dfdl:length="1"/>
+                <xs:sequence dfdl:initiator="|">
+                  <xs:choice>
+                    <xs:choice dfdl:choiceDispatchKey="{ ./messageID }">
+                      <xs:sequence dfdl:choiceBranchKey="1">
+                        <!--
+                        This techique, where we discriminate the outer choice if the choice dispatch
+                        is successful, is better during DFDL schema development because we won't backtrack
+                        and create unrecognized messages that suppress errors during the parsing of a recognized
+                        message type.
+                        -->
+                        <xs:group ref="ex:discriminate"/>

Review comment:
       This doesn't behave the way I would expect it to by my reading of the spec. From the spec, both the inner and outer choices create a point of uncertainty:
   {code}
   An xs:choice is always a point of uncertainty. It is resolved sequentially, or by direct dispatch ... Direct-dispatch choice resolution occurs by matching the value of the dfdl:choiceDispatchKey property to the value of the dfdl:choiceChoiceBranchKey [sic] property of one of the choice branches.
   {code}
   
   So the direct dispatch resolves the inner PoU, but isn't that inner PoU still in scope when this discriminator is evaluated? I would expect this discriminator to resolve the inner choice PoU to true, which is essentially a no-op since it's already been resolved to true by direct dispatch. So if something fails inside the inner choice branch, I would expect we backtrack to the outer choice PoU and speculatively parse to the second branch of the outer choice. So I would expect the nestedChoice2 test to result in the unrecognizedMessage element since nothing resolves the outer choice. Feels like a bug to me, unless I'm misreading the spec.
   
   The way I see it, the problem is there isn't a good way to differentiate between "a direct dispatch branch was found but something inside it failed" vs "a direct dispatch branch was not found". The outer PoU only ever sees that something about the inner choice failed, and it can't know why. The inner discrminator seems to be able to break outside the inner PoU and provide that information, but that seems wrong to me. Perhaps what is needed is some sort of ``dfdl:choiceBranchDefault="yes"`` property, which I think would no longer require the outer choice or inner discriminators and would work as expected?

##########
File path: daffodil-test/src/test/scala/org/apache/daffodil/section07/discriminators/TestNestedChoices.scala
##########
@@ -0,0 +1,45 @@
+/*
+ * 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.section07.discriminators
+
+/*import org.junit.Test */

Review comment:
       Remove import comment.




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