stevedlawrence commented on code in PR #1427:
URL: https://github.com/apache/daffodil/pull/1427#discussion_r1952762611
##########
daffodil-runtime1/src/main/scala/org/apache/daffodil/runtime1/infoset/InfosetImpl.scala:
##########
@@ -1888,7 +1896,7 @@ sealed class DIComplex(override val erd:
ElementRuntimeData)
if (fastSeq != null) {
// Daffodil does not support query expressions yet, so there should only
// be one item in the list
- noQuerySupportCheck(fastSeq, qname)
+ noQuerySupportCheck(fastSeq.toSeq, qname)
Review Comment:
Probably shouldn't be changed as part of this PR, but this `toSeq` is going
to do a copy, which isn't necessary. The `noQuerySupportCheck` function doesn't
need a copy, it just scans the ArrayBuffer to see if there are multiple
elements and errors if so.
My guess is fastSeq used to be a `Seq` at some point and so this worked fine
without a copy, but then we changed it to an `ArrayBuffer` without updating
this, and didn't realize Scala was automatically copying fastSeq to a Seq. So
it's good that Scala is not warning about this kind of thing and we should
probably fix this at some point.
After this is all done we might want to look at all the `.toSeq`'s that we
add as part of this conversion and make sure we aren't doing unnecessary copies.
--
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]
