stevedlawrence commented on a change in pull request #681:
URL: https://github.com/apache/daffodil/pull/681#discussion_r750231777
##########
File path:
daffodil-runtime2/src/test/scala/org/apache/daffodil/runtime2/TestCodeGenerator.scala
##########
@@ -168,4 +174,40 @@ class TestCodeGenerator {
assert(ur.isError, "expected ur.isError to be true")
assert(ur.getDiagnostics.nonEmpty, "expected ur.getDiagnostics to be
non-empty")
}
+
+ private def updateGeneratedCodeExample(schemaFile: os.Path, rootName:
Option[String],
+ exampleCodeHeader: os.Path,
exampleCodeFile: os.Path): Unit = {
+ // Generate code from the example schema file
+ val pf = Compiler().compileFile(schemaFile.toIO, rootName)
+ assert(!pf.isError, pf.getDiagnostics.map(_.getMessage()).mkString("\n"))
+ val cg = pf.forLanguage("c")
+ val rootNS =
QName.refQNameFromExtendedSyntax(rootName.getOrElse("")).toOption
+ val codeDir = cg.generateCode(rootNS, tempDir.toString)
+ assert(!cg.isError, cg.getDiagnostics.map(_.getMessage()).mkString("\n"))
+ val generatedCodeHeader = codeDir/"libruntime"/"generated_code.h"
+ val generatedCodeFile = codeDir/"libruntime"/"generated_code.c"
+
+ // Replace the example generated code files
+ os.copy(generatedCodeHeader, exampleCodeHeader, replaceExisting = true)
+ os.copy(generatedCodeFile, exampleCodeFile, replaceExisting = true)
+ }
Review comment:
> the worst case is that those changes will appear in someone else's
unrelated pull request,
This is what I hope to avoid. I actually find the example updates really
helpful when doing code reviews--it's much easier to visualize what's changing
compared to looking only at template C-code. Tests failing is a good way to
know if the examples were updated to match the code changes and what we are
reviewing is accurate.
If the goal is friction reduction, what if we add a new sbt command that
runs and updates the examples, e.g. `sbt genExamples`. This sort of matches our
existing `genProps` and `genSchemas` sbt commands, the only difference is we
expect people to actually commit those files. And then the test to check this
could just be a github action that's something like:
```
sbt genExamples && git diff --exit-code daffodil-runtime2/
```
Note that what you have is fine, and I won't block the PR over this. And if
you do find this idea ok, it doesn't have to be part of this PR--sbt can be a
beast sometimes so this probably won't be trivial to implement.
##########
File path:
daffodil-runtime2/src/main/scala/org/apache/daffodil/runtime2/generators/CodeGeneratorState.scala
##########
@@ -36,11 +37,32 @@ import scala.collection.mutable
* Builds up the state of generated code.
*/
class CodeGeneratorState {
- private val structs = mutable.Stack[ComplexCGState]()
- private val prototypes = mutable.ArrayBuffer[String]()
- private val erds = mutable.ArrayBuffer[String]()
- private val finalStructs = mutable.ArrayBuffer[String]()
- private val finalImplementation = mutable.ArrayBuffer[String]()
+ private val elementsAlreadySeen = mutable.Map.empty[String, ElementBase]
+ private val structs = mutable.Stack.empty[ComplexCGState]
+ private val prototypes = mutable.ArrayBuffer.empty[String]
+ private val erds = mutable.ArrayBuffer.empty[String]
+ private val finalStructs = mutable.ArrayBuffer.empty[String]
+ private val finalImplementation = mutable.ArrayBuffer.empty[String]
+
+ // Returns true if the element has not been seen before (checking if
+ // a map already contains the element, otherwise adding it to the map)
+ def elementNotSeenYet(context: ElementBase): Boolean = {
+ val key = context.namedQName.toString
+ val alreadySeen = elementsAlreadySeen.contains(key)
+ if (!alreadySeen) elementsAlreadySeen += (key -> context)
+ !alreadySeen
+ }
Review comment:
Makes sense, maybe renaming something or add a comment just to make it
clear that this is only about global elements?
--
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]
