Skip to content
This repository has been archived by the owner on Aug 20, 2024. It is now read-only.

Commit

Permalink
Fix MultiInfo parser + serialization bug (#2265)
Browse files Browse the repository at this point in the history
* Restore parsed MultiInfo structure in firrtl parser

* Change erroneous expected output in InfoSpec test

FileInfo compression sorts the outputted entries alphabetically, but
this test did not reflect that fact

* Fix typo in comment

* Add unit tests for file locator parsing

* Fix syntax issues and typos

* More redundant braces removed

Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com>
  • Loading branch information
jared-barocsi and mergify[bot] authored Jun 18, 2021
1 parent ecd6d7a commit eb0841d
Show file tree
Hide file tree
Showing 2 changed files with 61 additions and 3 deletions.
49 changes: 47 additions & 2 deletions src/main/scala/firrtl/Visitor.scala
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,51 @@ class Visitor(infoMode: InfoMode) extends AbstractParseTreeVisitor[FirrtlNode] w
private def string2Int(s: String): Int = string2BigInt(s).toInt

private def visitInfo(ctx: Option[InfoContext], parentCtx: ParserRuleContext): Info = {
// Convert a compressed FileInfo string into either into a singular FileInfo or a MultiInfo
// consisting of several FileInfos
def parseCompressedInfo(escaped: String): Info = {
var out: Seq[FileInfo] = Seq()

// Regular expression to match and capture the general File.format line:col pattern.
// Also matches the remaining part of the string which doesn't match the expression;
// which will be passed directly into the output as a FileInfo.
val splitCompressedInfo = """([^\s:]+)((?: \d+:(?:\d+|\{\d+(?:,\d+)+\}))+)|(?:[^\s].*)""".r

// Regular expression to capture the line number and column numbers in the compressed file info pattern.
val splitLineDescriptors = """(\d+):((?:\d+|\{\d+(?:,\d+)+\}))""".r

// Regular expression to match against individual column numbers in each line:col or line:{col1,col2}
// descriptor.
val splitColDescriptors = """\d+""".r

val matches = splitCompressedInfo.findAllIn(escaped)

// Grab each File.format line:col token from the input string
splitCompressedInfo.findAllIn(escaped).matchData.foreach { info =>
Option(info.group(1)) match {
// If there were no subgroups, the regex matched against a non-conforming source locator
// pattern, so do not process it
case None => out = out :+ ir.FileInfo.fromEscaped(info.toString)
case Some(file) =>
val lineDescriptors = info.group(2)
// Grab each line:col values from the separated (compressed) FileInfo.
splitLineDescriptors.findAllIn(lineDescriptors).matchData.foreach { lineDescriptor =>
val line = lineDescriptor.group(1)
val cols = lineDescriptor.group(2)
splitColDescriptors.findAllIn(cols).matchData.foreach {
// Use all the necessary info to generate normal uncompressed FileInfos
col => out = out :+ ir.FileInfo.fromEscaped(s"$file $line:$col")
}
}
}
}

out.size match {
case 0 => NoInfo
case 1 => out.head
case _ => new MultiInfo(out)
}
}
def genInfo(filename: String): String =
stripPath(filename) + " " + parentCtx.getStart.getLine + ":" +
parentCtx.getStart.getCharPositionInLine
Expand All @@ -68,11 +113,11 @@ class Visitor(infoMode: InfoMode) extends AbstractParseTreeVisitor[FirrtlNode] w
infoMode match {
case UseInfo =>
if (useInfo.length == 0) NoInfo
else ir.FileInfo.fromEscaped(useInfo)
else parseCompressedInfo(useInfo)
case AppendInfo(filename) if (useInfo.length == 0) =>
ir.FileInfo.fromEscaped(genInfo(filename))
case AppendInfo(filename) =>
val useFileInfo = ir.FileInfo.fromEscaped(useInfo)
val useFileInfo = parseCompressedInfo(useInfo)
val newFileInfo = ir.FileInfo.fromEscaped(genInfo(filename))
ir.MultiInfo(useFileInfo, newFileInfo)
case GenInfo(filename) =>
Expand Down
15 changes: 14 additions & 1 deletion src/test/scala/firrtlTests/InfoSpec.scala
Original file line number Diff line number Diff line change
Expand Up @@ -288,7 +288,7 @@ class InfoSpec extends FirrtlFlatSpec with FirrtlMatchers {
" wire [31:0] a = c ? b : d; // @[A 1:{2,3,4} B 2:3 4:5]"
)
result("A 2:3", "B 1:{2,3,4}", "C 4:5") should containLine(
" wire [31:0] a = c ? b : d; // @[B 1:{2,3,4} A 2:3 C 4:5]"
" wire [31:0] a = c ? b : d; // @[A 2:3 B 1:{2,3,4} C 4:5]"
)
}

Expand Down Expand Up @@ -321,4 +321,17 @@ class InfoSpec extends FirrtlFlatSpec with FirrtlMatchers {
for (line <- check)
result should containLine(line)
}

"Source Locators" should "not lose information when going through serialization + parsing" in {
def check(info: ir.Info): Unit = {
assert(Parser.parseInfo(info.serialize) == info)
}

check(ir.NoInfo)
check(ir.FileInfo("B"))
check(ir.FileInfo("A 4:5"))
check(ir.FileInfo("A 4:6"))
check(ir.MultiInfo(ir.FileInfo("A 4:5"), ir.FileInfo("B 5:5")))
check(ir.MultiInfo(ir.FileInfo("A 4:5"), ir.FileInfo("A 5:5")))
}
}

0 comments on commit eb0841d

Please sign in to comment.