stevedlawrence commented on code in PR #901:
URL: https://github.com/apache/daffodil/pull/901#discussion_r1067161050
##########
daffodil-runtime2/src/main/scala/org/apache/daffodil/runtime2/generators/BinaryValueCodeGenerator.scala:
##########
@@ -21,81 +21,65 @@ import org.apache.daffodil.dsom.ElementBase
import org.apache.daffodil.schema.annotation.props.gen.BitOrder
import org.apache.daffodil.schema.annotation.props.gen.ByteOrder
import org.apache.daffodil.schema.annotation.props.gen.LengthKind
-import org.apache.daffodil.schema.annotation.props.gen.OccursCountKind
import org.apache.daffodil.util.Maybe.Nope
// Base trait which provides common code to generate C code for primitive
value elements
trait BinaryValueCodeGenerator {
- // Generate C code for a primitive value element by calculating how many
times the element occurs
- // and applying the given partially applied function values the necessary
number of times. Intended
- // to be called by other traits which extend this trait, not directly by
Runtime2CodeGenerator.
- def binaryValueGenerateCode(e: ElementBase, addField: String => Unit,
validateFixed: String => Unit): Unit = {
+ // Generate C code for a primitive value element differently depending on
how many times the element occurs.
+ // Intended to be called by other traits which extend this trait, not
directly by Runtime2CodeGenerator.
+ def binaryValueGenerateCode(e: ElementBase, addField: String => Unit,
validateFixed: String => Unit, cgState: CodeGeneratorState): Unit = {
// For the time being this is a very limited back end.
// So there are some restrictions to enforce.
e.schemaDefinitionUnless(e.bitOrder eq BitOrder.MostSignificantBitFirst,
"Only dfdl:bitOrder 'mostSignificantBitFirst' is supported.")
e.schemaDefinitionUnless(e.maybeByteOrderEv == Nope ||
e.byteOrderEv.isConstant, "Runtime dfdl:byteOrder expressions not supported.")
e.schemaDefinitionUnless(e.lengthKind == LengthKind.Prefixed ||
e.elementLengthInBitsEv.isConstant, "Runtime dfdl:length expressions not
supported.")
- // Calculate how many times the element occurs in the data.
- val arraySize = e.occursCountKind match {
- case OccursCountKind.Fixed if e.maxOccurs > 1 => e.maxOccurs
- case OccursCountKind.Fixed if e.maxOccurs == 1 => 0
- case OccursCountKind.Implicit if e.minOccurs == 1 && e.maxOccurs == 1 => 0
- case _ => e.SDE("occursCountKind %s minOccurs %d maxOccurs %d is not
supported in C code generator",
- e.occursCountKind.toString, e.minOccurs, e.maxOccurs)
+ // Call the given partially applied function values with their remaining
unbound argument (deref)
+ val hasArray = cgState.arrayMaxOccurs(e) > 0
Review Comment:
> arrayMaxOccurs which is actually trying to do several things at the same
time
I think this is maybe what's causing me confusiong. What if instead we add a
new function that just validates if occursCountKind and maxOccurs values are
supported for runtime2 and that throws an SDE if not supported? That function
is called prior to this, and then this logic just becomes:
```scala
val deref = if (e.isArray) "[i]" else ""
```
And places where the size of the array is needed become:
```scala
val arrayDef = if (e.isArray) s"[$e.maxOccurs]" else ""
```
This way, we've already validated that if it's an array then both the occurs
count of the array and maxOccurs values are supported. This allow us to split
the validation from the actual values, and we can reuse the existing `isArray`
and `maxOccurs` values which are standardized and easy to make sense of.
--
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]
