mbeckerle commented on a change in pull request #488:
URL: https://github.com/apache/incubator-daffodil/pull/488#discussion_r575292767
##########
File path: daffodil-runtime2/src/main/resources/c/libruntime/infoset.c
##########
@@ -160,10 +160,11 @@ walkInfosetNode(const VisitEventHandler *handler, const
InfosetBase *infoNode)
handler->visitNumberElem(handler, childERD, numLocation);
break;
case CHOICE:
+ // Point next ERD to choice of alternative elements' ERDs
if (!infoNode->erd->initChoice(infoNode, rootElement()))
{
error_msg =
- "node's _choice field was not initialized during walk";
+ "Walk error: no match between choice dispatch key and any
branch key";
Review comment:
This is another example of something that should be impossible. Getting
this error means "runtime 2 has a bug". We should abort here.
##########
File path: daffodil-runtime2/src/main/resources/examples/NestedUnion.c
##########
@@ -379,7 +380,8 @@ data_unparseSelf(const data *instance, UState *ustate)
bar_unparseSelf(&instance->bar, ustate);
break;
default:
- ustate->error_msg = "node's _choice field was not initialized during
unparsing";
+ ustate->error_msg =
+ "Unparse error: no match between choice dispatch key and any
branch key";
Review comment:
Another just abort() case.
##########
File path: daffodil-runtime2/src/main/resources/c/libruntime/infoset.c
##########
@@ -159,6 +159,13 @@ walkInfosetNode(const VisitEventHandler *handler, const
InfosetBase *infoNode)
error_msg =
handler->visitNumberElem(handler, childERD, numLocation);
break;
+ case CHOICE:
+ if (!infoNode->erd->initChoice(infoNode, rootElement()))
Review comment:
So, if a choice is an element, we put a typecode in the corresponding
ERD and we have everything we need to cope with choices.
But if a choice isn't an element, then.... we need a scheme that works for
having more than one choice appearing in the sequence of a complex type
element.
One approach is anonymous (but uniquely named) Quasi-elements. These are
elements with ERDs and all, but with a bit set in the ERD indicating that it's
a quasi-element, and so when traversing, it is elided, so that the user (e.g.,
a path expression refering into it) can ignore its presence. This eliding
becomes pervasive in path expressions, in traversal walks, etc.
The other approach is inlining these, in which case we have multiple kinds
of children - simple element, complex element, and choice-group.
This latter approach is, honestly, more what people expect if they are
creating C structs. Having to have a whole node for a choice,... in some sense
the whole point of a choice is to avoid having a node.
To the extent that elements of complex type as children get inlined into the
enclosing parent structure, these two approaches may not be very different
really.
All this work is on a branch, so I'm not going to hold off on approving this
change set to resolve this, so long as we recognize that it's not really
reasonable for Runtime2's subset of DFDL to require every choice to be its own
element in the long run.
What you have here is still good progress, and worth integrating.
##########
File path: daffodil-runtime2/src/main/resources/examples/NestedUnion.c
##########
@@ -361,7 +361,8 @@ data_parseSelf(data *instance, PState *pstate)
bar_parseSelf(&instance->bar, pstate);
break;
default:
- pstate->error_msg = "node's _choice field was not initialized during
parsing";
+ pstate->error_msg =
+ "Parse error: no match between choice dispatch key and any branch
key";
Review comment:
I think this should just abort(msg)
----------------------------------------------------------------
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:
[email protected]
