-
Notifications
You must be signed in to change notification settings - Fork 275
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
[OM] parsing depend on the ordering of Class #6866
Comments
Yeah I think we shouldn't reject this (actually the same error would happen for type aliases as well). This might be tricky because currently class types on ports are computed form classOp itself so it's hard to fix when there are classes that reference each other on ports. e.g.
|
Is possible to delay the classOp construction? |
This is a shortcoming of the current FIRRTL parser for classes, which is taking a single pass in lexicographic order. So for the short term, if you know you're making Foo, and you're making a Bar with a reference to Foo, you can try to re-arrange your generator to produce Foo before Bar. Another option is to type erase the instance type in Bar. Rather than a The longer term option to handle this in the parser would probably require taking multiple passes through the textual FIRRTL to collect all the class names before parsing the class bodies. This is what we do for modules, so things like this are legal:
|
Yes, cannot use type aliases or classes defined later in the file. Modules we parse their signatures in one pass and then the bodies after -- this approach doesn't work for type aliases or classes which each may contain in their signature references to other types (aliased or classes). For this reason, module signatures also must only use types (aliased, classes, so on) defined before that point in the file. We parse directly into typed IR and rely on this for diagnostics (e.g., accessing a field of a type that isn't a bundle or is but doesn't have one of that name). Def-before-use policy also avoids various sorts of cycles -- mutually recursive type alias, so on -- that would need to be checked for otherwise. I'm not sure this can be accommodated without some significant changes + overhead re:tracking/indirection and if cannot be done in a single pass (+pre-pass for top-level declarations) should be considered carefully. Mostly thinking about type aliases -- but probably class ref types too? (BTW - do we really parse class names in lexicographic order and not the order they appear in the file? (!)) |
Yeah, this would need to be thought through carefully if we want to support this. Unlike modules, which can't have other modules in their signature, type aliases and object references can show up in signatures of modules and classes.
Sorry, I think I meant lexical order. What I mean is top to bottom in the order they appear in the file. |
Chisel provides a unsafe version of get class type:
Class.unsafeGetClassTypeByName
, this is used when user need a Class Type w/o get the Scala val of which.This will cause an issue:
I guarantee a
Class Foo
will be defined after thisClass Bar
, so I use it at Foo use the unsafe API, see:However firtool complains:
Proposing checking the unknown class at the last of parser, or create a new pass for validation.
The text was updated successfully, but these errors were encountered: