[GitHub] [daffodil] stevedlawrence commented on a change in pull request #681: Fix some code generation corner cases

[GitBox <gi...@apache.org>]

stevedlawrence commented on a change in pull request #681:
URL: https://github.com/apache/daffodil/pull/681#discussion_r749649976



##########
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:
       Rather than overwriting these files, what are your thoughts on just comparing the generate and expected files and fail the test if they aren't the same? That way if the test fails that means we either broke something or we just forgot to update the expected files.

##########
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:
       Isn't it possible to have to elements with the same named qname? Or is this about global elements, which I guess must always be unqiue? Or maybe I'm just not understanding this? Maybe an example what what went wrong would be helpful?




-- 
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