refactor!: unwrap BasicBlock enum #772
Conversation
ss2165
force-pushed
the
refactor/break-block
branch
from
1747eb1
to
7a619b0
Compare
|
Ok, so the cunning followup which might make this noting that all impls (for such T) have the same |
|
My other thought is nomenclature. How about |
|
Yeah those new names seem a bit nicer - but that's definitely a broader spec-inclusive issue |
ss2165
force-pushed
the
refactor/break-block
branch
from
ca97696
to
aeff10a
Compare
| } => FunctionType::new(type_row![], type_row![]).with_extension_delta(extension_delta), | ||
| BasicBlock::Exit { .. } => FunctionType::new(type_row![], type_row![]), | ||
| }) | ||
| Some( |
There was a problem hiding this comment.
Hmmm, I highly suspect this should be None (but instead we should implement fn extension_delta which by default delegates to this). However, that's not really part of this PR
BREAKING_CHANGES: DFB and Exit are not standalone structs. A level of nesting in serialization is also removed for these operations.
ss2165
force-pushed
the
refactor/break-block
branch
from
aeff10a
to
79c7bc4
Compare
src/ops/controlflow.rs
Outdated
|
|
||
| /// Functionality shared by DFB and Exit CFG block types. | ||
| pub trait BasicBlock { | ||
| /// The input signature of the contained dataflow graph. |
There was a problem hiding this comment.
The dataflow signature of the values input to the block? (Exit blocks do not have a "contained dataflow graph")
|
Ok, so - on the naming front: the spec says DFB and Exit are the two types of Basic Block; this PR keeps DFB and Exit but largely drops the notion of "Basic Block"! So I think it would be fine to do some name changes in the code (with appropriate references to the spec - But, generally, I'm happy to fold this into #754 and then follow up with a renaming PR. (The ideal would probably be to unwrap basicblock and rename in a first PR and then do #754 after, but I'm not going to make you do that!) |
|
(I think we should not put #754 in without this, because of the rather dubious inner-signature for exit blocks) |
|
I've taken your suggestion and reordered the PRs, this one is being closed in favour of #781 |
BREAKING_CHANGES: DFB and Exit are now standalone structs. A level of nesting in serialization is also removed for these operations.