Should a MultiInfo be recovered when serializing and parsing?

[ekiwi-sifive]

Currently a MultiInfo gets serialized in such a way that it is impossible to recover:

val info = MultiInfo(Seq(MultiInfo(Seq(FileInfo("a"))), FileInfo("b"), FileInfo("c")))

serializes to @[a b c] which is then parsed as FileInfo("a b c").

Is this the expected behavior?

[ekiwi-sifive]

Going one step further: If this is the desired behavior, why don't we get rid of the MultiInfo node and change the FileInfo.++ method to append to the string?

// current implementation
def ++(that: Info): Info = if (that == NoInfo) this else MultiInfo(Seq(this, that))
// suggested implementation:
def ++(that: Info): Info = that match {
    case NoInfo => this
    case FileInfo(escapedThat) => FileInfo(escaped + " " + escapedThat)
}

[ekiwi-sifive]

Going one step further: If this is the desired behavior, why don't we get rid of the MultiInfo node and change the FileInfo.++ method to append to the string?

As an alternative, if we want to keep the Strings separate until right before serialization, we could change FileInfo to have a Seq[String].

[ekiwi-sifive]

As discussed in the dev meeting, this issue is on hold until @jackkoenig has time to revamp the whole Info system.

[jackkoenig]

I think the answer to your question "Should a MultiInfo be recovered when serializing and parsing?" is "yes", it's just a question of how exactly to do it. I don't think we should concatenate into a single FileInfo because of the tendency of multiple FIRRTL nodes with the same source locator being collapsed. I'm not sure of the right way to represent Multi-Info in the serialized form but it's definitely a problem worth solving. Perhaps having the proposed FileLineCol type would work because then @[file.scala 1:23 other.scala 3:45] could be parsed into a MultiInfo(Seq(FileLineCol(...), FileLineCol(...))) and anything that doesn't match would parse into a FileInfo as an arbitrary string.

[ekiwi-sifive]

file.scala 1:23 other.scala 3:45 might be a valid file name on Linux.
If we do want to keep them separate it seems much easier to just allow for multiple @[ ... ] blocks:

@[file.scala 1:23] @[other.scala 3:45]

[seldridge]

Pedantically, file.scala 1:23] @[other.scala 3:45 is a valid filename... (The number of sharp edges here is astronomical.)

A perhaps naive approach would be to go to URIs/URLs to represent the file. This should make spaces actually work as identifiers here.

Java seems to let you go back and forth pretty easy:

@ val foo = new java.io.File("file.scala 1:23] @[other.scala 3:45") 
foo: java.io.File = file.scala 1:23] @[other.scala 3:45

@ val fooURI = foo.toURI 
fooURI: java.net.URI = file:/home/schuyler/repos/github.com/freechipsproject/chisel3/file.scala%201:23%5D%20@%5Bother.scala%203:45

@ val bar = new java.io.File(fooURI) 
bar: java.io.File = /home/schuyler/repos/github.com/freechipsproject/chisel3/file.scala 1:23] @[other.scala 3:45

[ekiwi-sifive]

Pedantically, file.scala 1:23] @[other.scala 3:45 is a valid filename... (The number of sharp edges here is astronomical.)

Yes, but then it would be encoded as file.scala 1:23\] @[other.scala 3:45 (note the escaped \]). This isn't the case in the current release but it will be fixed once my PR (#1690) is merged.