Skip to content

Use toVector for XML literal sequences#11065

Merged
SethTisue merged 1 commit into
scala:2.13.xfrom
lrytz:xmlVector
Aug 8, 2025
Merged

Use toVector for XML literal sequences#11065
SethTisue merged 1 commit into
scala:2.13.xfrom
lrytz:xmlVector

Conversation

@lrytz

@lrytz lrytz commented May 20, 2025

Copy link
Copy Markdown
Member

In <a><b/><c/><a>, a NodeBuffer (which extends ArrayBuffer) is used to accumulate the children. The buffer is passed to new Elem($buf: _*), which only works thanks to the implicit collection.Seq[Node] => NodeSeq declared in scala-xml.

With -Vprint:typer:

scala> <a><b/></a>
[[syntax trees at end of                     typer]] // <console>

      private[this] val res0: scala.xml.Elem = new scala.xml.Elem(null, "a", scala.xml.Null, scala.xml.TopScope, false, (xml.this.NodeSeq.seqToNodeSeq({
        val $buf: scala.xml.NodeBuffer = new scala.xml.NodeBuffer();
        $buf.&+(new scala.xml.Elem(null, "b", scala.xml.Null, scala.xml.TopScope, true));
        $buf
      }): _*));

The implicit was not inserted in 2.12 because the varargs parameter of Elem accepted a collection.Seq.

This PR changes the compiler translation to use toVector.

Note that there was a related change in scala-xml 2.4.0 to use immutable.Seq instead of collection.Seq for core parts of the scala-xml API (https://github.com/scala/scala-xml/releases/tag/v2.4.0).

@scala-jenkins scala-jenkins added this to the 2.13.17 milestone May 20, 2025
@lrytz

lrytz commented May 20, 2025

Copy link
Copy Markdown
Member Author

I went for Vector because it's more compact than List, and it's more efficient than List or ArraySeq for concatenation / prepending / appending. NodeSeq.newBuilder uses a ListBuffer though..

@lrytz

lrytz commented May 20, 2025

Copy link
Copy Markdown
Member Author

... this affects IntelliJ not anymore, updated the PR so that plain XML sequences keep type NodeBuffer.

image

In `<a><b/><c/><a>`, a `NodeBuffer` (which extends `ArrayBuffer`) is
used to accumulate the children. The buffer is passed to
`new Elem($buf: _*)`, which only works thanks to the implicit
`collection.Seq[Node] => NodeSeq` declared in scala-xml.

With `-Vprint:typer`:

```scala
scala> <a><b/></a>
[[syntax trees at end of                     typer]] // <console>

      private[this] val res0: scala.xml.Elem = new scala.xml.Elem(null, "a", scala.xml.Null, scala.xml.TopScope, false, (xml.this.NodeSeq.seqToNodeSeq({
        val $buf: scala.xml.NodeBuffer = new scala.xml.NodeBuffer();
        $buf.&+(new scala.xml.Elem(null, "b", scala.xml.Null, scala.xml.TopScope, true));
        $buf
      }): _*));
```

The implicit was not inserted in 2.12 because the varargs parameter of
Elem accepted a `collection.Seq`.
@SethTisue

Copy link
Copy Markdown
Member

eligible for forward-port to Scala 3?

@lrytz

lrytz commented Aug 8, 2025

Copy link
Copy Markdown
Member Author

That's already done: scala/scala3#23221

@SethTisue SethTisue merged commit 177b745 into scala:2.13.x Aug 8, 2025
3 checks passed
@lrytz lrytz added the release-notes worth highlighting in next release notes label Sep 19, 2025
SethTisue added a commit to scala/scala3 that referenced this pull request Jan 19, 2026
Forward-port of scala/scala#11065

In `<a><b/><c/><a>`, a `NodeBuffer` (which extends `ArrayBuffer`) is
used to accumulate the children. The buffer is passed to `new Elem($buf:
_*)`, which only works thanks to the implicit `collection.Seq[Node] =>
NodeSeq` declared in scala-xml.

With `-Vprint:typer`:

```scala
scala> <a><b/></a>
[[syntax trees at end of                     typer]] // rs$line$1

    val res0: scala.xml.Elem =
      {
        new _root_.scala.xml.Elem(null, "a", _root_.scala.xml.Null,
          scala.xml.TopScope, false,
          {
            val $buf: scala.xml.NodeBuffer = new _root_.scala.xml.NodeBuffer()
            $buf.&+(
              {
                {
                  new _root_.scala.xml.Elem(null, "b", _root_.scala.xml.Null,
                    scala.xml.TopScope, true, [ : scala.xml.Node]*)
                }
              }
            )
            scala.xml.NodeSeq.seqToNodeSeq($buf)
          }*
        )
      }
```

The implicit was not inserted in 2.12 because the varargs parameter of
Elem accepted a `collection.Seq`.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

release-notes worth highlighting in next release notes

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants