[FIRRTL][FIRParser] Tokenize Commas

[mmaloney-sf]

Due to historical circumstances, the SFC parser for FIRRTL treated commas (,) as whitespace. This behavior was carried over into the CIRCT implementation of firtool.

This behavior is utterly bizarre.

Moreover, the FIRRTL spec has indicated comma tokens (,) for at least every version since last year.

This PR removes this quirk and properly tokenizes commas, requiring them in the places dictated by the FIRRTL spec.

The impact of this PR should be low, but it may break code (in CIRCT, in Chisel, and elsewhere) in places where FIRRTL was being emitted with a loose interpretation, or where the spec was ambiguous.

Changes:

• Update several tests which were leaning into the "commas-are-whitespace" behavior.
• Removed ',' from the list of "horizontal whitespace" and added an explicit FIRToken::comma token.
• Made changes to the various parse*() methods to consume the tokens.
• Note the use of a pattern I found useful in several places where I use a boolean first to skip parsing comma tokens on the first iteration. Please check me here, since I learned C++ before lambdas were a thing.
• Made a few judgment calls around unspeced constructs (smem).
• • I changed the FIRRTL emitter in one place where the syntax was unspeced, and there seemed to be conflicting examples.
• @seldridge For some reason, it seemed ambiguous whether layer decls need commas. I opted for commas. I can swap if I got this backwards.
• I added parseRUW() as a non-optional variant of parseOptionalRUW(), since the comma token can be used to determine optionality in one case.

[dtzSiFive]

Technically this is only mandatory as of 4.0.0, https://github.com/chipsalliance/firrtl-spec/pull/191 . However I think just requiring this regardless is fine re:compatibility.

I too am interested in impact of this.

cc https://github.com/chipsalliance/firrtl-spec/issues/188 .

[mmaloney-sf]

@dtzSiFive Thanks for pointing that out. I feel like I removed that verbiage and then forgot it.

And thank you for pointing out that I didn't feature-gate this. I'm not sure how I should proceed on that point. I had plans to add a lot of small tweaks to the syntax, but it could easily make it a bit of spaghetti to keep strict backwards compat. But then again, I don't know much people care about that compatibility.

[seldridge]

Note: no version of Chisel that I have ever seen has ever not emitted commas unconditionally. There should be no risk in changing this and it should be backwards compatible.

[mmaloney-sf]

Please add explicit tests that dangling commas in lists and before optional components are rejected.

Done.

[mmaloney-sf]

If there are no other comments, could someone merge?

[mmaloney-sf]

Sounds good to me, @uenoku