You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@daffodil.apache.org by "tuxji (via GitHub)" <gi...@apache.org> on 2023/06/08 15:23:20 UTC
[GitHub] [daffodil] tuxji opened a new pull request, #1029: Add codegen-c support for runtime length hexBinary
tuxji opened a new pull request, #1029:
URL: https://github.com/apache/daffodil/pull/1029
Extend Daffodil's C code generator to support runtime length hexBinary elements. Also make some cleanups for better consistency.
DAFFODIL-2819
dependency-scan.yml: Saw a better example somewhere (it was called dependency-graph.yml) so modify and rename to match that example.
DaffodilCCodeGenerator.scala: Change comments and names in compileCode and pickCommand to be more similar to another codegen. Extend generateCode to handle RepOrderedWithMinMaxSequenceChild.
BinaryValueCodeGenerator.scala: Remove restriction on runtime lengths.
CodeGeneratorState.scala: Add cExpression to implement a poor man's C code expression generator until Daffodil adds custom code generation functionality to compiled DFDL expressions. Remove namespace prefixes in makeLegalForC to ensure they don't appear in C code. Extend arrayMaxOccurs to handle more cases. Make cStructFieldAccess do a better job of converting a DFDL expression written as a path to a C struct field dereference.
HexBinaryCodeGenerator.scala: Allow specified length hexBinary elements to use runtime lengths as well as constant lengths.
ex_nums.tdml: Add a new test to check support for runtime length hexBinary elements.
Evaluatable.scala: Allow codegen-c to get compiled expression from EvaluatableExpression.
TestExNums.scala: Make both daffodil and daffodilC run new 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.
To unsubscribe, e-mail: commits-unsubscribe@daffodil.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [daffodil] mbeckerle commented on a diff in pull request #1029: Add codegen-c support for runtime length hexBinary
Posted by "mbeckerle (via GitHub)" <gi...@apache.org>.
mbeckerle commented on code in PR #1029:
URL: https://github.com/apache/daffodil/pull/1029#discussion_r1223324181
##########
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:
Why is this not covered? This different invocation of the code-generator for this specific kind of sequence child would seem to be a key point of this change set.
--
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
[GitHub] [daffodil] tuxji commented on a diff in pull request #1029: Add codegen-c support for runtime length hexBinary
Posted by "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
[GitHub] [daffodil] tuxji merged pull request #1029: Add codegen-c support for runtime length hexBinary
Posted by "tuxji (via GitHub)" <gi...@apache.org>.
tuxji merged PR #1029:
URL: https://github.com/apache/daffodil/pull/1029
--
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