[GitHub] [incubator-daffodil] stevedlawrence commented on a change in pull request #468: Fix abort when viewing prefixed length elements in the debugger

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

stevedlawrence commented on a change in pull request #468:
URL: https://github.com/apache/incubator-daffodil/pull/468#discussion_r543348390



##########
File path: daffodil-runtime1-unparser/src/main/scala/org/apache/daffodil/processors/unparsers/SpecifiedLengthUnparsers.scala
##########
@@ -385,8 +388,11 @@ class SpecifiedLengthPrefixedUnparser(
   override lazy val childProcessors = Vector(prefixedLengthUnparser, eUnparser)
 
   override def unparse(state: UState): Unit = {
-    // create a "detached" element that the prefix length will be used to unparse
-    val plElem = Infoset.newElement(prefixedLengthERD).asInstanceOf[DISimple]
+    // Create a "detached" DIDocument with a single child element that the
+    // prefix length will be parsed to. This creates a completely new
+    // infoset and parses to that, so care is taken to ensure this infoset
+    // is only used for the prefix length parsing and is removed afterwards
+    val plElem = Infoset.newDetachedElement(state, prefixedLengthERD).asInstanceOf[DISimple]

Review comment:
       Perhaps an alternative would be to get the current DIDocument and add this detached element as a child of that? That avoids the additional allocation, but then cleanup becomes a bit trickier? I guess it shouldn't be hard to remove that new child? There might also be considerations with the InfosetWalker, it might have some assumptions that the DIDocument only has a single node. Probably surmountable, but does add some complexity. 




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

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