[FIRRTL] Add list concatenation parser support. #7512
Conversation
This parses `list_concat(exp+)` into the ListConcatOp. The type of the list is inferred during parsing, and all expressions must be the same. It is a parser error to not concatenate at least one list.
| return failure(); | ||
|
|
||
| if (operands.empty()) | ||
| return emitError(loc, "need at least one List to concatenate"); |
There was a problem hiding this comment.
This is legal syntactically according to the spec, but for this specific operation, the IR requires at least one operand. It makes the parser simpler to implement by requiring at least one operand and inferring the type from the types of the operands, but if it would be preferable to not require this during parsing (and instead let the op's verifier enforce it), I can change that.
|
Thanks for the review. Good question about inserting casts... I thought about that, and I can't think of a reason we'd want that today. The automatic casts during list create ops might still make sense in some cases, but really I think we should move away from that. By the time we have list concat ops, we should already be dealing with lists of the same types (and if they're lists of AnyRefs, the casts would be inserted if necessary when the lists were created). I can't think of a use case for concatenating lists of different object types and wanting that to automatically insert the necessary casts to lists of AnyRef type; the frontend should really just handle this. |
Great! That sounds solid to me (and ty for explanation not quoted above for brevity)! |
This parses
list_concat(exp+)into the ListConcatOp. The type of the list is inferred during parsing, and all expressions must be the same. It is a parser error to not concatenate at least one list.