stevedlawrence commented on a change in pull request #488:
URL: https://github.com/apache/incubator-daffodil/pull/488#discussion_r572977604
##########
File path: daffodil-runtime2/src/main/resources/examples/ex_nums.c
##########
@@ -660,10 +559,10 @@ littleEndian_unparseSelf(const littleEndian *instance,
UState *ustate)
static void
ex_nums_initSelf(ex_nums *instance)
{
+ instance->_base.erd = &ex_nums_ERD;
array_initSelf(&instance->array);
bigEndian_initSelf(&instance->bigEndian);
littleEndian_initSelf(&instance->littleEndian);
- instance->_base.erd = &ex_nums__ERD;
}
Review comment:
What are these src/main/resources/examples files for? Are these examples
of what the code generate will create? Are these for testing purposes? Perhaps
they should be in src/test/resources?
##########
File path:
daffodil-runtime2/src/main/scala/org/apache/daffodil/runtime2/generators/CodeGeneratorState.scala
##########
@@ -105,41 +129,178 @@ class CodeGeneratorState {
qnameInit
}
+ /**
+ * We want to convert a choiceDispatchKey expression into C struct dot
+ * notation (rootElement->[subElement.field]) which will access the C
+ * struct field containing the choiceDispatchKey's runtime value.
+ *
+ * We make some assumptions to make generating the dot notation easier:
+ * - the expression starts with '{xs:string( and ends with )}'
+ * - the expression returns the value of a previous element without
+ * changing the value in any way (except converting it to xs:string)
+ * - both the expression and the C code use only local names (for now...)
+ * - we can map the context node's path to a Unix-like slash path
+ * - all dpath operations look like Unix-like relative paths (../tag)
+ * - we can normalize the new path and convert it to C struct dot notation
+ * - we can store the accessed value in an int64_t local variable safely
+ */
+ private def choiceDispatchField(context: ElementBase): String = {
+ // We want to use SchemaComponent.scPath but it's private so duplicate it
here (for now...)
+ def scPath(sc: SchemaComponent): Seq[SchemaComponent] =
sc.optLexicalParent.map { scPath }.getOrElse(Nil) :+ sc
+ val localNames = scPath(context).map {
+ case er: AbstractElementRef => er.refQName.local
+ case e: ElementBase => e.namedQName.local
+ case ed: GlobalElementDecl => ed.namedQName.local
+ case _ => ""
+ }
+ val absoluteSlashPath = localNames.mkString("/")
+ val dispatchSlashPath = context.complexType.modelGroup match {
+ case choice: Choice if choice.isDirectDispatch =>
+ val expr = choice.choiceDispatchKeyEv.expr.toBriefXML()
+ val before = "'{xs:string("
+ val after = ")}'"
+ val relativePath = if (expr.startsWith(before) && expr.endsWith(after))
+ expr.substring(before.length, expr.length - after.length) else expr
+ val normalizedURI = new URI(absoluteSlashPath + "/" +
relativePath).normalize
+ normalizedURI.getPath.substring(1)
+ case _ => ""
+ }
+ // Strip namespace prefixes since C code uses only local names (for now...)
+ val localDispatchSlashPath = dispatchSlashPath.replaceAll("/[^:]+:", "/")
+ val res = localDispatchSlashPath.replace('/', '.')
+ res
+ }
Review comment:
As this runtime2 matures, I wonder if eventually we would want to add
code generate support to the DPath. This way you can take advantage all of the
DPath expression capabilities like type checking/conversion. But then rather
than creating our runtime1 objects, you can generate code. This likely requires
a decent amount of refactoring of the DPath, but probably somethign worth
keeping in the back of our minds. As is, this feels pretty fragile, and I'm not
even sure it will error if a dispatch expression doesn't match the supported
format.
##########
File path:
daffodil-runtime2/src/main/scala/org/apache/daffodil/runtime2/generators/CodeGeneratorState.scala
##########
@@ -162,10 +322,31 @@ class CodeGeneratorState {
def addComplexTypeStatements(child: ElementBase): Unit = {
val C = localName(child)
val e = child.name
+ val hasChoice = structs.top.initChoiceStatements.nonEmpty
Review comment:
If I'm reading this right, it seems this only supports direct dispatch
choices that are the model group of a complex type, e.g.:
```xml
<xs:element name="foo">
<xs:complexType>
<xs:choice dfdl:choiceDispatchKey="...">
...
</xs:choice>
</xs:complexType>
</xs:element>
```
So sequences with "anonymous" choices are not suppported? I guess supporting
those adds more complications since you currently use ``_choice`` to figure out
which union member is used, and anonmous choices means you could have multiple
unions and _choice's, which someone need a distint name?
Does this distint name issue also arise in sequence with multiple elements
with the same name? For example:
```xml
<xs:element name="foo">
<xs:complexType>
<xs:sequence>
<xs:element name="a" ... />
<xs:element name="b" ... />
<xs:element name="a" ... />
</xs:choice>
</xs:complexType>
</xs:element>
```
Would this currently result in to struct members with the name "a" and cause
an error?
##########
File path:
daffodil-runtime2/src/test/resources/org/apache/daffodil/runtime2/ingress_xdcc_bw.tdml
##########
@@ -0,0 +1,196 @@
+<?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
+ defaultConfig="config-runtime2"
+ defaultImplementations="daffodil daffodil-runtime2"
+ defaultRoundTrip="none"
+ description="TDML tests for ingress_xdcc_bw"
+ xmlns:daf="urn:ogf:dfdl:2013:imp:daffodil.apache.org:2018:ext"
+ xmlns:dfdl="http://www.ogf.org/dfdl/dfdl-1.0/";
+ xmlns:tdml="http://www.ibm.com/xmlns/dfdl/testData";>
+
+ <tdml:defineConfig name="config-runtime1">
+ <daf:tunables>
+ <daf:tdmlImplementation>daffodil</daf:tdmlImplementation>
+ </daf:tunables>
+ </tdml:defineConfig>
+
+ <tdml:defineConfig name="config-runtime2">
+ <daf:tunables>
+ <daf:tdmlImplementation>daffodil-runtime2</daf:tdmlImplementation>
+ </daf:tunables>
+ </tdml:defineConfig>
+
+ <tdml:parserTestCase
+ description="ingress_xdcc_bw parse test 111"
+ model="ingress_xdcc_bw.dfdl.xsd"
+ name="ingress_xdcc_bw_parse_111"
+ root="GapsPDU">
+ <tdml:document>
+ <tdml:documentPart
type="file">ingress_xdcc_bw_parse_111.dat</tdml:documentPart>
+ </tdml:document>
+ <tdml:infoset>
+ <tdml:dfdlInfoset
type="file">ingress_xdcc_bw_unparse_111.xml</tdml:dfdlInfoset>
+ </tdml:infoset>
+ </tdml:parserTestCase>
+
+ <tdml:unparserTestCase
+ description="ingress_xdcc_bw unparse test 111"
+ model="ingress_xdcc_bw.dfdl.xsd"
+ name="ingress_xdcc_bw_unparse_111"
+ root="GapsPDU">
+ <tdml:infoset>
+ <tdml:dfdlInfoset
type="file">ingress_xdcc_bw_unparse_111.xml</tdml:dfdlInfoset>
+ </tdml:infoset>
+ <tdml:document>
+ <tdml:documentPart
type="file">ingress_xdcc_bw_parse_111.dat</tdml:documentPart>
+ </tdml:document>
+ </tdml:unparserTestCase>
+
+ <tdml:parserTestCase
+ description="ingress_xdcc_bw parse test 112"
+ model="ingress_xdcc_bw.dfdl.xsd"
+ name="ingress_xdcc_bw_parse_112"
+ root="GapsPDU">
+ <tdml:document>
+ <tdml:documentPart
type="file">ingress_xdcc_bw_parse_112.dat</tdml:documentPart>
+ </tdml:document>
+ <tdml:infoset>
+ <tdml:dfdlInfoset
type="file">ingress_xdcc_bw_unparse_112.xml</tdml:dfdlInfoset>
+ </tdml:infoset>
+ </tdml:parserTestCase>
+
+ <tdml:unparserTestCase
+ description="ingress_xdcc_bw unparse test 112"
+ model="ingress_xdcc_bw.dfdl.xsd"
+ name="ingress_xdcc_bw_unparse_112"
+ root="GapsPDU">
+ <tdml:infoset>
+ <tdml:dfdlInfoset
type="file">ingress_xdcc_bw_unparse_112.xml</tdml:dfdlInfoset>
+ </tdml:infoset>
+ <tdml:document>
+ <tdml:documentPart
type="file">ingress_xdcc_bw_parse_112.dat</tdml:documentPart>
+ </tdml:document>
+ </tdml:unparserTestCase>
+
+ <tdml:parserTestCase
+ description="ingress_xdcc_bw parse test 113"
+ model="ingress_xdcc_bw.dfdl.xsd"
+ name="ingress_xdcc_bw_parse_113"
+ root="GapsPDU">
+ <tdml:document>
+ <tdml:documentPart
type="file">ingress_xdcc_bw_parse_113.dat</tdml:documentPart>
+ </tdml:document>
+ <tdml:infoset>
+ <tdml:dfdlInfoset
type="file">ingress_xdcc_bw_unparse_113.xml</tdml:dfdlInfoset>
+ </tdml:infoset>
+ </tdml:parserTestCase>
+
+ <tdml:unparserTestCase
+ description="ingress_xdcc_bw unparse test 113"
+ model="ingress_xdcc_bw.dfdl.xsd"
+ name="ingress_xdcc_bw_unparse_113"
+ root="GapsPDU">
+ <tdml:infoset>
+ <tdml:dfdlInfoset
type="file">ingress_xdcc_bw_unparse_113.xml</tdml:dfdlInfoset>
+ </tdml:infoset>
+ <tdml:document>
+ <tdml:documentPart
type="file">ingress_xdcc_bw_parse_113.dat</tdml:documentPart>
+ </tdml:document>
+ </tdml:unparserTestCase>
+
+ <tdml:parserTestCase
+ description="ingress_xdcc_bw parse test 114"
+ model="ingress_xdcc_bw.dfdl.xsd"
+ name="ingress_xdcc_bw_parse_114"
+ root="GapsPDU">
+ <tdml:document>
+ <tdml:documentPart
type="file">ingress_xdcc_bw_parse_114.dat</tdml:documentPart>
+ </tdml:document>
+ <tdml:infoset>
+ <tdml:dfdlInfoset
type="file">ingress_xdcc_bw_unparse_114.xml</tdml:dfdlInfoset>
+ </tdml:infoset>
+ </tdml:parserTestCase>
+
+ <tdml:unparserTestCase
+ description="ingress_xdcc_bw unparse test 114"
+ model="ingress_xdcc_bw.dfdl.xsd"
+ name="ingress_xdcc_bw_unparse_114"
+ root="GapsPDU">
+ <tdml:infoset>
+ <tdml:dfdlInfoset
type="file">ingress_xdcc_bw_unparse_114.xml</tdml:dfdlInfoset>
+ </tdml:infoset>
+ <tdml:document>
+ <tdml:documentPart
type="file">ingress_xdcc_bw_parse_114.dat</tdml:documentPart>
+ </tdml:document>
+ </tdml:unparserTestCase>
+
+ <tdml:parserTestCase
+ description="ingress_xdcc_bw parse test 115"
+ model="ingress_xdcc_bw.dfdl.xsd"
+ name="ingress_xdcc_bw_parse_115"
+ root="GapsPDU">
+ <tdml:document>
+ <tdml:documentPart
type="file">ingress_xdcc_bw_parse_115.dat</tdml:documentPart>
+ </tdml:document>
+ <tdml:infoset>
+ <tdml:dfdlInfoset
type="file">ingress_xdcc_bw_unparse_115.xml</tdml:dfdlInfoset>
+ </tdml:infoset>
+ </tdml:parserTestCase>
+
+ <tdml:unparserTestCase
+ description="ingress_xdcc_bw unparse test 115"
+ model="ingress_xdcc_bw.dfdl.xsd"
+ name="ingress_xdcc_bw_unparse_115"
+ root="GapsPDU">
+ <tdml:infoset>
+ <tdml:dfdlInfoset
type="file">ingress_xdcc_bw_unparse_115.xml</tdml:dfdlInfoset>
+ </tdml:infoset>
+ <tdml:document>
+ <tdml:documentPart
type="file">ingress_xdcc_bw_parse_115.dat</tdml:documentPart>
+ </tdml:document>
+ </tdml:unparserTestCase>
+
+ <tdml:parserTestCase
+ description="ingress_xdcc_bw parse test 116"
+ model="ingress_xdcc_bw.dfdl.xsd"
+ name="ingress_xdcc_bw_parse_116"
+ root="GapsPDU">
+ <tdml:document>
+ <tdml:documentPart
type="file">ingress_xdcc_bw_parse_116.dat</tdml:documentPart>
+ </tdml:document>
+ <tdml:infoset>
+ <tdml:dfdlInfoset
type="file">ingress_xdcc_bw_unparse_116.xml</tdml:dfdlInfoset>
+ </tdml:infoset>
+ </tdml:parserTestCase>
+
+ <tdml:unparserTestCase
+ description="ingress_xdcc_bw unparse test 116"
+ model="ingress_xdcc_bw.dfdl.xsd"
+ name="ingress_xdcc_bw_unparse_116"
+ root="GapsPDU">
+ <tdml:infoset>
+ <tdml:dfdlInfoset
type="file">ingress_xdcc_bw_unparse_116.xml</tdml:dfdlInfoset>
+ </tdml:infoset>
+ <tdml:document>
+ <tdml:documentPart
type="file">ingress_xdcc_bw_parse_116.dat</tdml:documentPart>
+ </tdml:document>
+ </tdml:unparserTestCase>
+
+</tdml:testSuite>
Review comment:
Does it make sense to put all these tests in daffodil-tests, or
somewhere not runtime2 specific? It makes sense that they are in here because
they're made to ensure that runtime2 works, but these schemas aren't runtime2
specific. They should run just fine in DFDL runtime, or maybe even the IBM DFDL
runtime.
I would think that maybe runtime2 should only contain things like unit tests
that are truely specific to runtime2. For example, tests that generate code and
then examing the generated code to make sure the right thing is generated. Not
sure we need that kind of test, but that's the kind of tests that should be in
runtime2 rather than daffodil-test.
----------------------------------------------------------------
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]
