[FIRRTL] Add type alias parser #5449
Conversation
This commit adds minimal changes BaseTypeAliasType for base type alias. Implement storage and methods for FIRRTLBaseType and FieldIDTypeInterface. This doesn't include recursive property chagnes and `FIRRTLBaseType::getAnonnymousType` whcih requires more broader changes.
This implements `type_isa/type_cast/type_dyn_cast/type_dyn_cast_or_null`. When a given type is an alias, these casts will visit its inner type recursively. These type casts can be used anywhere and if the requested type is non-base type or FIRRTLBaseType itself, then it will optimized in the compile time,
| @@ -4317,6 +4375,12 @@ ParseResult FIRCircuitParser::parseCircuit( | |||
| emitError("unexpected token in circuit"); | |||
| return failure(); | |||
|
|
|||
| case FIRToken::kw_type: { | |||
There was a problem hiding this comment.
On the second thought, I didn't find much useful to reject type declarations based on FIRRTL version since type declaration is a completely new thing (in a similar way to property or enum), which doesn't involve any backward compatibility. So I remove the version check here.
uenoku
force-pushed
the
dev/uenoku/parse-alias
branch
from
ab872a8
to
d5f86d5
Compare
Co-authored-by: Prithayan Barua <prithayan@gmail.com>
uenoku
force-pushed
the
dev/uenoku/parse-alias
branch
from
e376fd3
to
6920364
Compare
|
Is there a requirement re:declaring type declarations "before" (in source code position) their use? Quick testing suggests modules can use type aliases defined after, but type aliases themselves cannot do this. On one hand, requiring definitions before their use seems reasonable but it does seem a little inconsistent with using other top-level definitions where order doesn't matter. Is this the desired/expected/preferred behavior? If nothing else this should be an intentional decision and aligned with spec (didn't check). |
|
Good point, will clarify in the spec. I don't have a strong opinion but I guess we want consistent behavior to other top-level definitions. However allowing uses of type alias before declarations also implies that we can define recursive types. Not sure what recurisive types in FIRRTL, but. at least currently the type storage cannot handle cycles in type references. So we will need to detect and reject cycles in types. Besides we will need to modify |
|
Good points re:recursion + needing to figure out how to resolve port types. Not sure what right answer is, other alternative is require all type aliases def-before-use (and maybe before first module)? Not sure how generator-friendly that would be. |
Yep, that sounds reasonable restriction. I agree that it's not bad to limit type declarations to be before first module. Verifying def-before-use is also tricky since we will have to associate type declarations with their parsed positions in fir files. I'm leaning toward just restricting type declarations to be before the first module (as it's simplest). |
Separated from #5167. Add parser changes for type alias. https://github.com/chipsalliance/firrtl-spec/blob/main/spec.md#type-alias
Currently type alias for non-base types are stripped immediately while parsing with warnings.
On top of #5417