Skip to content

fix!: Fixed bugs in model CFG handling and improved CFG signatures #2334

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.

Sign up for GitHub

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

Merged
zrho merged 2 commits into from
Jun 17, 2025
Merged

fix!: Fixed bugs in model CFG handling and improved CFG signatures #2334

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
513b521
Improved model cfg typing and bugfixes.
zrho Jun 11, 2025
b78e00b
Inline `wrap_ctrl`
zrho Jun 17, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
27 changes: 14 additions & 13 deletions hugr-core/src/export.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ use crate::{
},
types::{
CustomType, EdgeKind, FuncTypeBase, MaybeRV, PolyFuncTypeBase, RowVariable, SumType,
TypeArg, TypeBase, TypeBound, TypeEnum, TypeRow,
TypeArg, TypeBase, TypeBound, TypeEnum,
type_param::{TypeArgVariable, TypeParam},
type_row::TypeRowBase,
},
Expand Down Expand Up @@ -578,7 +578,6 @@ impl<'a> Context<'a> {
pub fn export_block_signature(&mut self, block: &DataflowBlock) -> table::TermId {
let inputs = {
let inputs = self.export_type_row(&block.inputs);
let inputs = self.make_term_apply(model::CORE_CTRL, &[inputs]);
self.make_term(table::Term::List(
self.bump.alloc_slice_copy(&[table::SeqPart::Item(inputs)]),
))
Expand All @@ -590,13 +589,12 @@ impl<'a> Context<'a> {
let mut outputs = BumpVec::with_capacity_in(block.sum_rows.len(), self.bump);
for sum_row in &block.sum_rows {
let variant = self.export_type_row_with_tail(sum_row, Some(tail));
let control = self.make_term_apply(model::CORE_CTRL, &[variant]);
outputs.push(table::SeqPart::Item(control));
outputs.push(table::SeqPart::Item(variant));
}
self.make_term(table::Term::List(outputs.into_bump_slice()))
};

self.make_term_apply(model::CORE_FN, &[inputs, outputs])
self.make_term_apply(model::CORE_CTRL, &[inputs, outputs])
}

/// Creates a data flow region from the given node's children.
Expand Down Expand Up @@ -740,18 +738,21 @@ impl<'a> Context<'a> {
let signature = {
let node_signature = self.hugr.signature(node).unwrap();

let mut wrap_ctrl = |types: &TypeRow| {
let types = self.export_type_row(types);
let types_ctrl = self.make_term_apply(model::CORE_CTRL, &[types]);
let inputs = {
let types = self.export_type_row(node_signature.input());
self.make_term(table::Term::List(
self.bump
.alloc_slice_copy(&[table::SeqPart::Item(types_ctrl)]),
self.bump.alloc_slice_copy(&[table::SeqPart::Item(types)]),
))
};

let inputs = wrap_ctrl(node_signature.input());
let outputs = wrap_ctrl(node_signature.output());
Some(self.make_term_apply(model::CORE_FN, &[inputs, outputs]))
let outputs = {
let types = self.export_type_row(node_signature.output());
self.make_term(table::Term::List(
self.bump.alloc_slice_copy(&[table::SeqPart::Item(types)]),
))
};

Some(self.make_term_apply(model::CORE_CTRL, &[inputs, outputs]))
};

let scope = match closure {
Expand Down
45 changes: 18 additions & 27 deletions hugr-core/src/import.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -826,7 +826,7 @@ impl<'a> Context<'a> {
}

let region_target_types = (|| {
let [_, region_targets] = self.get_func_type(
let [_, region_targets] = self.get_ctrl_type(
region_data
.signature
.ok_or_else(|| error_uninferred!("region signature"))?,
Expand Down Expand Up @@ -860,25 +860,18 @@ impl<'a> Context<'a> {
};

// The entry node in core control flow regions is identified by being
// the first child node of the CFG node. We therefore import the entry
// node first and follow it up by every other node.
// the first child node of the CFG node. We therefore import the entry node first.
self.import_node(entry_node, node)?;

for child in region_data.children {
if *child != entry_node {
self.import_node(*child, node)?;
}
}

// Create the exit node for the control flow region.
// Create the exit node for the control flow region. This always needs
// to be second in the node list.
{
let cfg_outputs = {
let [ctrl_type] = region_target_types.as_slice() else {
let [target_types] = region_target_types.as_slice() else {
return Err(error_invalid!("cfg region expects a single target"));
};

let [types] = self.expect_symbol(*ctrl_type, model::CORE_CTRL)?;
self.import_type_row(types)?
self.import_type_row(*target_types)?
};

let exit = self
Expand All @@ -887,6 +880,13 @@ impl<'a> Context<'a> {
self.record_links(exit, Direction::Incoming, region_data.targets);
}

// Finally we import all other nodes.
for child in region_data.children {
if *child != entry_node {
self.import_node(*child, node)?;
}
}

for meta_item in region_data.meta {
self.import_node_metadata(node, *meta_item)
.map_err(|err| error_context!(err, "node metadata"))?;
Expand Down Expand Up @@ -1245,13 +1245,6 @@ impl<'a> Context<'a> {
return Err(error_unsupported!("`{}` as `TypeParam`", model::CORE_CONST));
}

if let Some([]) = self.match_symbol(term_id, model::CORE_CTRL_TYPE)? {
return Err(error_unsupported!(
"`{}` as `TypeParam`",
model::CORE_CTRL_TYPE
));
}

if let Some([item_type]) = self.match_symbol(term_id, model::CORE_LIST_TYPE)? {
// At present `hugr-model` has no way to express that the item
// type of a list must be copyable. Therefore we import it as `Any`.
Expand Down Expand Up @@ -1339,13 +1332,6 @@ impl<'a> Context<'a> {
return Err(error_unsupported!("`{}` as `TypeArg`", model::CORE_STATIC));
}

if let Some([]) = self.match_symbol(term_id, model::CORE_CTRL_TYPE)? {
return Err(error_unsupported!(
"`{}` as `TypeArg`",
model::CORE_CTRL_TYPE
));
}

if let Some([]) = self.match_symbol(term_id, model::CORE_CONST)? {
return Err(error_unsupported!("`{}` as `TypeArg`", model::CORE_CONST));
}
Expand Down Expand Up @@ -1510,6 +1496,11 @@ impl<'a> Context<'a> {
.ok_or(error_invalid!("expected a function type"))
}

fn get_ctrl_type(&mut self, term_id: table::TermId) -> Result<[table::TermId; 2], ImportError> {
self.match_symbol(term_id, model::CORE_CTRL)?
.ok_or(error_invalid!("expected a control type"))
}

fn import_func_type<RV: MaybeRV>(
&mut self,
term_id: table::TermId,
Expand Down
13 changes: 6 additions & 7 deletions hugr-core/tests/snapshots/model__roundtrip_cfg.snap
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
---
source: hugr-core/tests/model.rs
expression: "roundtrip(include_str!(\"../../hugr-model/tests/fixtures/model-cfg.edn\"))"
expression: ast
Copy link
Contributor

@croyzor croyzor Jun 17, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure what's going on here?

Copy link
Contributor Author

@zrho zrho Jun 17, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There was some refactoring in an earlier PR that changed how the snapshot test is called. The snapshot itself didn't change, and so the snapshot file was still considered valid. Now the snapshot changed, which also led to a change in the metadata that captures how the snapshot test is called.

---
(hugr 0)

Expand All @@ -22,10 +22,9 @@ expression: "roundtrip(include_str!(\"../../hugr-model/tests/fixtures/model-cfg.
(cfg [%0] [%1]
(signature (core.fn [?0] [?0]))
(cfg [%2] [%3]
(signature (core.fn [(core.ctrl [?0])] [(core.ctrl [?0])]))
(signature (core.ctrl [[?0]] [[?0]]))
(block [%2] [%3 %2]
(signature
(core.fn [(core.ctrl [?0])] [(core.ctrl [?0]) (core.ctrl [?0])]))
(signature (core.ctrl [[?0]] [[?0] [?0]]))
(dfg [%4] [%5]
(signature (core.fn [?0] [(core.adt [[?0] [?0]])]))
((core.make_adt 0) [%4] [%5]
Expand All @@ -37,15 +36,15 @@ expression: "roundtrip(include_str!(\"../../hugr-model/tests/fixtures/model-cfg.
(cfg [%0] [%1]
(signature (core.fn [?0] [?0]))
(cfg [%2] [%3]
(signature (core.fn [(core.ctrl [?0])] [(core.ctrl [?0])]))
(signature (core.ctrl [[?0]] [[?0]]))
(block [%2] [%6]
(signature (core.fn [(core.ctrl [?0])] [(core.ctrl [?0])]))
(signature (core.ctrl [[?0]] [[?0]]))
(dfg [%4] [%5]
(signature (core.fn [?0] [(core.adt [[?0]])]))
((core.make_adt 0) [%4] [%5]
(signature (core.fn [?0] [(core.adt [[?0]])])))))
(block [%6] [%3]
(signature (core.fn [(core.ctrl [?0])] [(core.ctrl [?0])]))
(signature (core.ctrl [[?0]] [[?0]]))
(dfg [%7] [%8]
(signature (core.fn [?0] [(core.adt [[?0]])]))
((core.make_adt 0) [%7] [%8]
Expand Down
6 changes: 3 additions & 3 deletions hugr-core/tests/snapshots/model__roundtrip_entrypoint.snap
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
---
source: hugr-core/tests/model.rs
expression: "roundtrip(include_str!(\"../../hugr-model/tests/fixtures/model-entrypoint.edn\"))"
expression: ast
---
(hugr 0)

Expand Down Expand Up @@ -40,10 +40,10 @@ expression: "roundtrip(include_str!(\"../../hugr-model/tests/fixtures/model-entr
(cfg
(signature (core.fn [] []))
(cfg [%0] [%1]
(signature (core.fn [(core.ctrl [])] [(core.ctrl [])]))
(signature (core.ctrl [[]] [[]]))
(meta core.entrypoint)
(block [%0] [%1]
(signature (core.fn [(core.ctrl [])] [(core.ctrl [])]))
(signature (core.ctrl [[]] [[]]))
(dfg [] [%2]
(signature (core.fn [] [(core.adt [[]])]))
((core.make_adt 0) [] [%2]
Expand Down
12 changes: 4 additions & 8 deletions hugr-model/src/v0/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -163,17 +163,13 @@ pub const CORE_BYTES_TYPE: &str = "core.bytes";
/// - **Result:** `core.static`
pub const CORE_FLOAT_TYPE: &str = "core.float";

/// Type of a control flow edge.
/// Type of control flow regions.
///
/// - **Parameter:** `?types : (core.list core.type)`
/// - **Result:** `core.ctrl_type`
/// - **Parameter:** `?inputs : (core.list (core.list core.type))`
/// - **Parameter:** `?outputs : (core.list (core.list core.type))`
/// - **Result:** `core.type`
pub const CORE_CTRL: &str = "core.ctrl";

/// The type of the types for control flow edges.
///
/// - **Result:** `?type : core.static`
pub const CORE_CTRL_TYPE: &str = "core.ctrl_type";

/// The type for runtime constants.
///
/// - **Parameter:** `?type : core.type`
Expand Down
28 changes: 14 additions & 14 deletions hugr-model/tests/fixtures/model-cfg.edn
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,17 +6,17 @@
(param ?a core.type)
(core.fn [?a] [?a])
(dfg [%0] [%1]
(signature (core.fn [?a] [?a]))
(cfg [%0] [%1]
(signature (core.fn [?a] [?a]))
(cfg [%2] [%4]
(signature (core.fn [(core.ctrl [?a])] [(core.ctrl [?a])]))
(block [%2] [%4 %2]
(signature (core.fn [(core.ctrl [?a])] [(core.ctrl [?a]) (core.ctrl [?a])]))
(dfg [%5] [%6]
(signature (core.fn [?a] [(core.adt [[?a] [?a]])]))
((core.make_adt 0) [%5] [%6]
(signature (core.fn [?a] [(core.adt [[?a] [?a]])])))))))))
(signature (core.fn [?a] [?a]))
(cfg [%0] [%1]
(signature (core.fn [?a] [?a]))
(cfg [%2] [%3]
(signature (core.ctrl [[?a]] [[?a]]))
(block [%2] [%3 %2]
(signature (core.ctrl [[?a]] [[?a] [?a]]))
(dfg [%4] [%5]
(signature (core.fn [?a] [(core.adt [[?a] [?a]])]))
((core.make_adt 0) [%4] [%5]
(signature (core.fn [?a] [(core.adt [[?a] [?a]])])))))))))

(define-func example.cfg_order
(param ?a core.type)
Expand All @@ -26,15 +26,15 @@
(cfg [%0] [%1]
(signature (core.fn [?a] [?a]))
(cfg [%2] [%4]
(signature (core.fn [(core.ctrl [?a])] [(core.ctrl [?a])]))
(signature (core.ctrl [[?a]] [[?a]]))
(block [%3] [%4]
(signature (core.fn [(core.ctrl [?a])] [(core.ctrl [?a])]))
(signature (core.ctrl [[?a]] [[?a]]))
(dfg [%5] [%6]
(signature (core.fn [?a] [(core.adt [[?a]])]))
((core.make_adt _ _ 0) [%5] [%6]
(signature (core.fn [?a] [(core.adt [[?a]])])))))
(block [%2] [%3]
(signature (core.fn [(core.ctrl [?a])] [(core.ctrl [?a])]))
(signature (core.ctrl [[?a]] [[?a]]))
(dfg [%7] [%8]
(signature (core.fn [?a] [(core.adt [[?a]])]))
((core.make_adt _ _ 0) [%7] [%8]
Expand Down
4 changes: 2 additions & 2 deletions hugr-model/tests/fixtures/model-entrypoint.edn
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,10 +25,10 @@
(cfg [] []
(signature (core.fn [] []))
(cfg [%entry] [%exit]
(signature (core.fn [(core.ctrl [])] [(core.ctrl [])]))
(signature (core.ctrl [[]] [[]]))
(meta core.entrypoint)
(block [%entry] [%exit]
(signature (core.fn [(core.ctrl [])] [(core.ctrl [])]))
(signature (core.ctrl [[]] [[]]))
(dfg [] [%value]
(signature (core.fn [] [(core.adt [[]])]))
((core.make_adt _ _ 0) [] [%value]
Expand Down
Loading
Loading