Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Does the firrtl spec support splitting long lines? #4

Open
madebr opened this issue Jul 9, 2020 · 1 comment
Open

Does the firrtl spec support splitting long lines? #4

madebr opened this issue Jul 9, 2020 · 1 comment

Comments

@madebr
Copy link

madebr commented Jul 9, 2020

This is not a bug report per se, but more of a question.
See listing 15 from the specs:

{real: {word:UInt<32>, valid:UInt<1>, flip ready:UInt<1>} imag: {word:UInt<32>, valid:UInt<1>, flip ready:UInt<1>}}

The bundle is split among multiple lines with indentation.
The same goes for listing 54, where the type of the mem block is shown.

Does the firrtl spec support splitting long code among multiple lines,
or is this a result of formatting?

This is a bug report after all:
I think there should be a comma after between the real and imag bundle.

@seldridge seldridge transferred this issue from chipsalliance/firrtl Mar 4, 2022
@seldridge
Copy link
Member

seldridge commented Mar 7, 2022

I fixed the missing comma issue in d169da3. I believe this existed in the spec only to work around long-line issues. However, it makes sense to me to allow this as long as it's unambiguous. E.g., for types, once you're opened a scope with : or {, newlines don't matter and you can add them however you want.

This is slightly complicated because the Scala-based FIRRTL Compiler (SFC) will reject these while the MLIR-based FIRRTL Compiler (MFC) will accept these. Specifically, the following shows this delineation:

circuit Foo:
  module Foo:
    input a: {a: UInt<1>,
              b: UInt<1>}
    output b: {a: UInt<1>,
               b: UInt<1>}

    b <= a

The SFC antlr grammar is complaining about unexpected newlines or indents while MFC doesn't care. After it sees a { it is in "bundle parsing mode" and will parse anything it sees ignoring newlines or whitespace.

My reasoning for why this should be allowed is that other indentation-sensitive languages (e.g., Python) allow this. Specifically, Python lets you do things like the following:

if True:
    print("hello" +
"world")

if True:
    print("hello" +
          "world")

if True:
    print("hello" +



                   "world")

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants