mbeckerle commented on code in PR #1579:
URL: https://github.com/apache/daffodil/pull/1579#discussion_r2461103086
##########
daffodil-core/src/main/scala/org/apache/daffodil/core/runtime1/ChoiceTermRuntime1Mixin.scala:
##########
@@ -80,35 +80,25 @@ trait ChoiceTermRuntime1Mixin { self: ChoiceTermBase =>
// element event to be in the infoset (for round-tripping from parse).
It's value will
// get recomputed, but it can appear with a stale value (left-over from
parse)
// in the arriving infoset for unparse.
- //
- val optDefaultBranch = {
- val optEmpty: Option[Term] = {
- val emptyBranches = groupMembers.filter { gm =>
- val ies = gm.identifyingEventsForChoiceBranch
- ies.pnes.isEmpty // empty event list makes it the default, not
simply isOpen
- }
- if (emptyBranches.length > 1)
- SDW(
- WarnID.MultipleChoiceBranches,
- "Multiple choice branches with no required elements detected and
the infoset does not specify a branch, selecting the first branch for unparsing"
- )
- emptyBranches.headOption
- }
- val optOpen: Option[Term] =
- optEmpty.orElse {
- groupMembers.find { gm =>
- val ies = gm.identifyingEventsForChoiceBranch
- ies.isOpen // first open one is used if there is no branch that
has empty event list. (test ptg_1u)
- }
- }
- val optDefault: Option[Term] =
- optOpen.orElse {
- groupMembers.find {
- _.canUnparseIfHidden
- } // optional, defaultable, OVC, or IVC
- }
- optDefault
+ // Rather than defaulting to the first empty branch, we instead default to
the first
+ // empty, open or defaultable branch.
+ val emptyOpenOrDefault = groupMembers.filter {
+ case gm if gm.identifyingEventsForChoiceBranch.pnes.isEmpty => true
+ case gm if gm.identifyingEventsForChoiceBranch.isOpen => true
+ case gm if gm.canUnparseIfHidden => true
+ case _ => false
+ }
+ if (
+ emptyOpenOrDefault.length > 1 && emptyOpenOrDefault.forall(
+ _.identifyingEventsForChoiceBranch.pnes.isEmpty
+ )
+ ) {
+ SDW(
+ WarnID.MultipleChoiceBranches,
+ "Multiple choice branches with no required elements detected and the
infoset does not specify a branch, selecting the first branch for unparsing"
+ )
Review Comment:
One can certainly have multiple open branches and this warning would be
spurious. One infoset when unparsed could contain an optional part of one open
branch, a different infoset when unparsed could contain an optional part of a
different open branch.
So this warning should not be issued if all the branches are open.
If you have multiple empty branches then the 2nd and beyond empty branches
could never be unparsed, and produce no infoset element when parsing. But...
it's not illegal. It just is a bit weird, but someone could do it to explicitly
absorb various optional syntax when parsing that is not to be regenerated when
unparsing.
So I guess I agree this warning should be removed.
--
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.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
