[GitHub] [daffodil] tuxji commented on a diff in pull request #1029: Add codegen-c support for runtime length hexBinary

["tuxji (via GitHub)" <gi...@apache.org>]

tuxji commented on code in PR #1029:
URL: https://github.com/apache/daffodil/pull/1029#discussion_r1223492198


##########
daffodil-codegen-c/src/main/scala/org/apache/daffodil/codegen/c/generators/CodeGeneratorState.scala:
##########
@@ -387,30 +388,68 @@ class CodeGeneratorState(private val root: ElementBase) {
     code.replace("\r\n", "\n").replace("\n", System.lineSeparator)
   }
 
-  // Returns a name for the given element's C struct identifier
+  // Returns a C name for the given element's C struct identifier
   private def cStructName(context: ElementBase): String = {
     val sb = buildName(context, new StringBuilder)
     makeLegalForC(sb)
     val name = sb.toString
     name
   }
 
-  // Returns a name for the given element's element runtime data
+  // Returns a C name for the given element's element runtime data
   private def erdName(context: ElementBase): String = {
     val sb = buildName(context, new StringBuilder) ++= "ERD"
     makeLegalForC(sb)
     val name = sb.toString
     name
   }
 
-  // Returns a name for the given element's local name
+  // Returns a C name for the given element's local name.
   def cName(context: ElementBase): String = {
     val sb = new StringBuilder(context.namedQName.local)
     makeLegalForC(sb)
     val name = sb.toString
     name
   }
 
+  // Converts a dfdl:length expression to a C expression.  We make some
+  // simplifying assumptions to make converting the expression easier:
+  // - all field names start with ../ or /rootName
+  // - all field names end at the first non-XML-identifier character
+  // - all field names can be converted to C identifiers using cStructFieldAccess
+  // - the expression performs only arithmetic (with no casts) and computes a length
+  // - the expression is almost C-ready but may contain some non-C named operators
+  // - may need to replace any div, idiv, mod with / and % instead
+  // Eventually we should make the compiled expression generate the C code itself
+  // instead of using this superficial replaceAllIn approach.
+  def cExpression(expr: String): String = {
+    // Match each field name and replace it with a C struct field dereference
+    val field = """(((\.\./)+|/)[\p{L}_][\p{L}:_\-.0-9/]*)""".r
+    val exprWithFields = field.replaceAllIn(
+      expr,
+      m => {
+        val fieldName = m.group(1).stripPrefix("../")
+        val cFieldName = cStructFieldAccess(fieldName)
+        cFieldName
+      },
+    )
+    // Match each named operator and replace it with a C operator
+    val operator = """(\bidiv\b|\bdiv\b|\bmod\b)""".r
+    val exprWithOperators = operator.replaceAllIn(
+      exprWithFields,
+      m => {
+        val operatorName = m.group(1)

Review Comment:
   This line and the lines below it are not covered because the runtime length expression I used in the unit test didn't use those idiv / div / mod operators, while my local schemas did use expressions like `dfdl:length="{ ((../dataoffset idiv 16) - 5) * 4 }"/>`.



##########
daffodil-codegen-c/src/main/scala/org/apache/daffodil/codegen/c/generators/CodeGeneratorState.scala:
##########
@@ -880,6 +924,7 @@ class CodeGeneratorState(private val root: ElementBase) {
       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 OccursCountKind.Implicit if e.maxOccurs > 0 => e.maxOccurs

Review Comment:
   This change is also related to the RepOrderedWithMinMaxSequenceChild changes and is not covered for the same reason (further work would be required to finish the changes).



##########
daffodil-codegen-c/src/main/scala/org/apache/daffodil/codegen/c/DaffodilCCodeGenerator.scala:
##########
@@ -278,6 +280,7 @@ object DaffodilCCodeGenerator
       case g: RepOrderedExactlyNSequenceChild => repOrderedExactlyNSequenceChild(g, cgState)
       case g: RepOrderedExpressionOccursCountSequenceChild =>
         repOrderedExpressionOccursCountSequenceChild(g, cgState)
+      case g: RepOrderedWithMinMaxSequenceChild => repOrderedWithMinMaxSequenceChild(g, cgState)

Review Comment:
   Line 283 is not covered because this particular set of RepOrderedWithMinMaxSequenceChild changes was not for runtime length hexBinary.  I think it was for an implicit length integer type element with minOccurs=1 and maxOccurs=16 attributes, but I'm not certain.  I just tested that element and further work would be needed to finish supporting such an element (it can only go at the end of the schema and codegen-c doesn't generate runnable code for it yet).



-- 
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: commits-unsubscribe@daffodil.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org