Skip to content

feat!: Add Visibility to FuncDefn/FuncDecl. #2143

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

Merged
merged 94 commits into from
Jul 9, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
94 commits
Select commit Hold shift + click to select a range
187cc8b
Cherry-pick call_graph + dead_funcs
acl-cqc Jun 11, 2025
a5a18e5
WIP Add link_name, builder methods, update some passes
acl-cqc Jun 11, 2025
cabd1bc
Add link_name to hugr-py FuncDefn, update schema
acl-cqc Jun 11, 2025
1d0ac85
FuncDecl: add link_name_mut
acl-cqc Jun 13, 2025
0543ece
FuncDefn deprecations
acl-cqc Jun 16, 2025
231c2cd
FuncDecl deprecation(?)
acl-cqc Jun 16, 2025
68fd467
Remove get_func_name, we have the defn/decl to hand
acl-cqc Jun 13, 2025
bf5ead7
model import/export via metadata
acl-cqc Jun 16, 2025
c5f4363
And update snapshots
acl-cqc Jun 16, 2025
4dcf9e1
Merge remote-tracking branch 'origin/main' into acl/linking_prelims
acl-cqc Jun 16, 2025
69b33d2
Fix monomorphize test
acl-cqc Jun 16, 2025
2ea97f4
(just) clippy
acl-cqc Jun 16, 2025
71f4106
Move IncludeExports up to root of hugr-passes, use for const_fold, +d…
acl-cqc Jun 16, 2025
b5d5e9e
docs
acl-cqc Jun 16, 2025
c7c734b
Cherry-pick #2343
acl-cqc Jun 16, 2025
45c8131
model Visibility w/temp(), update capnp, revert model metadata/link_name
acl-cqc Jun 16, 2025
c0c170b
Update snapshots again (Revert metadata, add "pub")
acl-cqc Jun 16, 2025
44a1cba
remove commented-out define_funtion_link_name, revert hugr-cli debug
acl-cqc Jun 17, 2025
c4c5188
Update hugr-core, revert monomorph; TODO core+model python/bindings
acl-cqc Jun 17, 2025
582142c
Fix parsing; update snapshots - all aliases now output as "pub"
acl-cqc Jun 17, 2025
e848eed
Update hugr-py FuncDefn + snapshot
acl-cqc Jun 17, 2025
a9acc5f
hugr-py: also FuncDecl
acl-cqc Jun 17, 2025
075b4dc
hugr-model python - no tests but pytest passing
acl-cqc Jun 17, 2025
8d285ac
update-schema
acl-cqc Jun 17, 2025
16da753
Doc fixes
acl-cqc Jun 18, 2025
5d2abf7
doc
acl-cqc Jun 18, 2025
7780a9b
reduce change vs main
acl-cqc Jun 18, 2025
8c82bdc
move priv_vis
acl-cqc Jun 18, 2025
ac363bc
model: inline print_vis, rm CORE_META_FUNCNAME
acl-cqc Jun 18, 2025
c034749
dead_code use IncludeExports, fix FuncDecl/Const
acl-cqc Jun 18, 2025
e1192ee
use deprecation for remove_dead_funcs (add new ..._vis), format
acl-cqc Jun 18, 2025
23f4786
Merge remote-tracking branch 'origin/main' into acl/linking_prelims
acl-cqc Jun 18, 2025
7245487
Avoid deprecated FuncDefn::new in hugr-persistent
acl-cqc Jun 18, 2025
239f179
doc typo
acl-cqc Jun 24, 2025
22df54a
hugr-model: add Visibility::default()==Private, make ops+aliases use …
acl-cqc Jun 25, 2025
cfbab3b
hugr-model snapshots
acl-cqc Jun 25, 2025
2acaf65
FuncDefn::new uses public for main else private...TODO define_func, F…
acl-cqc Jun 25, 2025
042d079
Same for define_func/FunctionBuilder::new; remove (_pub,_priv) except…
acl-cqc Jun 25, 2025
939fd9a
Rm default_for_name, all builder methods construct via FuncDefn::new
acl-cqc Jun 25, 2025
44408fe
FuncDecl: remove new_public/new_priv, don't deprecate new
acl-cqc Jun 25, 2025
eb26b2f
Fix doc + missing see-also
acl-cqc Jun 27, 2025
0ddd0e7
FuncDefn::new also private but logs if name==main
acl-cqc Jun 25, 2025
b0744ea
Revert "FuncDefn::new also private but logs if name==main"
acl-cqc Jun 27, 2025
ca43cf1
hugr_core Visibility is not Copy
acl-cqc Jun 27, 2025
5dbed79
hugr-model Visibility not Copyable -> allocate
acl-cqc Jun 27, 2025
40f9c66
Don't mark hugr-model Visibility as non-exhaustive
acl-cqc Jun 27, 2025
d2c56dd
Test validate_linkage, improve comment
acl-cqc Jun 27, 2025
cacae92
Merge remote-tracking branch 'origin/main' into acl/linking_prelims
acl-cqc Jun 27, 2025
0c360f4
dead_code.rs: Remove bogus allow-deprecated
acl-cqc Jun 27, 2025
723c819
remove_dead_funcs_pass2, fold_constants
acl-cqc Jun 27, 2025
06f3d54
fix rename, fmt
acl-cqc Jun 27, 2025
b1e0d9b
Revert DeadCodeElim behaviour on Consts
acl-cqc Jun 27, 2025
74664ad
DeadCodeElim only elims below entrypoint
acl-cqc Jun 27, 2025
4816064
clippy-llvm
acl-cqc Jun 27, 2025
43500f7
Remove declare_private
acl-cqc Jun 27, 2025
f80b80e
powerup'd clippy, add quotes
acl-cqc Jun 27, 2025
46e2272
Merge remote-tracking branch 'origin/main' into acl/linking_prelims
acl-cqc Jun 27, 2025
853cc1d
more clippy
acl-cqc Jun 27, 2025
288675c
imports; reduce change
acl-cqc Jun 27, 2025
e63265e
Merge remote-tracking branch 'origin/main' into acl/linking_prelims
acl-cqc Jul 1, 2025
2878a28
Merge remote-tracking branch 'origin/main' into acl/linking_prelims
acl-cqc Jul 2, 2025
02bcbe6
All FuncDefn's private
acl-cqc Jul 2, 2025
bb5d75b
More clarifications
acl-cqc Jul 2, 2025
dbbd8a9
Not quite that bad - dataflow means const-fold preserves non-module e…
acl-cqc Jul 2, 2025
4155429
Further clarify const-fold. Blimey
acl-cqc Jul 2, 2025
7cdf236
not-a-hugr test - this passes but clearly shouldn't
acl-cqc Jul 2, 2025
882c5b0
testing schemas: use TestingHugr; test now fails as desired
acl-cqc Jul 2, 2025
c0fa4f1
Replace silly test with better ones
acl-cqc Jul 4, 2025
2127d51
Fix name of NamedSchema
acl-cqc Jul 4, 2025
72ab368
Improve error printing
acl-cqc Jul 4, 2025
0c9739a
Fix prop_roundtrip_type via any_serde_type_{arg,param}...wtf ArrayOrT…
acl-cqc Jul 4, 2025
60f3698
fix roundtrip_poly_func_type
acl-cqc Jul 4, 2025
70d5bb3
Fix roundtrip_optype via any_serde_type_arg_vec
acl-cqc Jul 4, 2025
539e569
Update generate_schema.py to add (Testing|Serial)Hugr|Extension|Packa…
acl-cqc Jul 4, 2025
74bf9e6
Generalize test to cover both testing and main schema
acl-cqc Jul 4, 2025
1d2ba5a
generate_schema uses DEFAULT_REF_TEMPLATE + oneOf
acl-cqc Jul 4, 2025
5caa8d6
Pass path into test
acl-cqc Jul 5, 2025
93bcbc2
Allow ArrayOrTermSer::Term(Term::ListConcat)...but we'll never see that
acl-cqc Jul 5, 2025
db68a76
Better comments/todos
acl-cqc Jul 7, 2025
a5b65de
Merge remote-tracking branch 'origin/main' into acl/schema_test2
acl-cqc Jul 7, 2025
45b2412
More comment/todo tweaks, test use impl IntoIterator
acl-cqc Jul 7, 2025
c001d53
Merge branch 'acl/schema_test2' into acl/linking_prelims
acl-cqc Jul 7, 2025
4f07f9b
and fix test by adding visibility
acl-cqc Jul 7, 2025
3a460a4
Revert hugr-passes changes
acl-cqc Jul 7, 2025
7d5d81d
Merge remote-tracking branch 'origin/main' into acl/schema_test2
acl-cqc Jul 8, 2025
05eb7be
Merge branch 'acl/schema_test2' into acl/linking_prelims
acl-cqc Jul 8, 2025
c52ce08
make_simple_hugr docs say public; make it so
acl-cqc Jul 8, 2025
2cbe99f
hugr-core comment on Visibility should not mention optimization/analysis
acl-cqc Jul 8, 2025
61056af
Test loading Hugr w/out visibility
acl-cqc Jul 8, 2025
ed79543
Add python test - indeed fails w/ missing visibility
acl-cqc Jul 8, 2025
8592e75
Provide default visibility in pydantic; fix test Node-fiddling
acl-cqc Jul 8, 2025
c9fc1b5
Update schema
acl-cqc Jul 8, 2025
b32465f
Comment about serde defaults
acl-cqc Jul 8, 2025
5a81bc0
Skip if test file missing
acl-cqc Jul 8, 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
35 changes: 24 additions & 11 deletions hugr-core/src/builder/dataflow.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,14 +8,9 @@ use std::marker::PhantomData;

use crate::hugr::internal::HugrMutInternals;
use crate::hugr::{HugrView, ValidationError};
use crate::ops::{self, OpParent};
use crate::ops::{DataflowParent, Input, Output};
use crate::{Direction, IncomingPort, OutgoingPort, Wire};

use crate::ops::{self, DataflowParent, FuncDefn, Input, OpParent, Output};
use crate::types::{PolyFuncType, Signature, Type};

use crate::Node;
use crate::{Hugr, hugr::HugrMut};
use crate::{Direction, Hugr, IncomingPort, Node, OutgoingPort, Visibility, Wire, hugr::HugrMut};

/// Builder for a [`ops::DFG`] node.
#[derive(Debug, Clone, PartialEq)]
Expand Down Expand Up @@ -152,17 +147,35 @@ impl<B, T> DFGWrapper<B, T> {
pub type FunctionBuilder<B> = DFGWrapper<B, BuildHandle<FuncID<true>>>;

impl FunctionBuilder<Hugr> {
/// Initialize a builder for a `FuncDefn` rooted HUGR
/// Initialize a builder for a [`FuncDefn`](ops::FuncDefn)-rooted HUGR;
/// the function will be private. (See also [Self::new_vis].)
///
/// # Errors
///
/// Error in adding DFG child nodes.
pub fn new(
name: impl Into<String>,
signature: impl Into<PolyFuncType>,
) -> Result<Self, BuildError> {
let signature: PolyFuncType = signature.into();
let body = signature.body().clone();
let op = ops::FuncDefn::new(name, signature);
Self::new_with_op(FuncDefn::new(name, signature))
}

/// Initialize a builder for a FuncDefn-rooted HUGR, with the specified
/// [Visibility].
///
/// # Errors
///
/// Error in adding DFG child nodes.
pub fn new_vis(
name: impl Into<String>,
signature: impl Into<PolyFuncType>,
visibility: Visibility,
) -> Result<Self, BuildError> {
Self::new_with_op(FuncDefn::new_vis(name, signature, visibility))
}

fn new_with_op(op: FuncDefn) -> Result<Self, BuildError> {
let body = op.signature().body().clone();

let base = Hugr::new_with_entrypoint(op).expect("FuncDefn entrypoint should be valid");
let root = base.entrypoint();
Expand Down
107 changes: 73 additions & 34 deletions hugr-core/src/builder/module.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,15 +4,14 @@ use super::{
dataflow::{DFGBuilder, FunctionBuilder},
};

use crate::hugr::ValidationError;
use crate::hugr::internal::HugrMutInternals;
use crate::hugr::views::HugrView;
use crate::ops;
use crate::types::{PolyFuncType, Type, TypeBound};

use crate::ops::handle::{AliasID, FuncID, NodeHandle};
use crate::types::{PolyFuncType, Type, TypeBound};
use crate::{Hugr, Node, Visibility};
use crate::{hugr::ValidationError, ops::FuncDefn};

use crate::{Hugr, Node};
use smol_str::SmolStr;

/// Builder for a HUGR module.
Expand Down Expand Up @@ -69,25 +68,61 @@ impl<T: AsMut<Hugr> + AsRef<Hugr>> ModuleBuilder<T> {
f_id: &FuncID<false>,
) -> Result<FunctionBuilder<&mut Hugr>, BuildError> {
let f_node = f_id.node();
let decl =
self.hugr()
.get_optype(f_node)
.as_func_decl()
.ok_or(BuildError::UnexpectedType {
node: f_node,
op_desc: "crate::ops::OpType::FuncDecl",
})?;
let name = decl.func_name().clone();
let sig = decl.signature().clone();
let body = sig.body().clone();
self.hugr_mut()
.replace_op(f_node, ops::FuncDefn::new(name, sig));
let opty = self.hugr_mut().optype_mut(f_node);
let ops::OpType::FuncDecl(decl) = opty else {
return Err(BuildError::UnexpectedType {
node: f_node,
op_desc: "crate::ops::OpType::FuncDecl",
});
};

let body = decl.signature().body().clone();
*opty = ops::FuncDefn::new_vis(
decl.func_name(),
decl.signature().clone(),
decl.visibility().clone(),
)
.into();

let db = DFGBuilder::create_with_io(self.hugr_mut(), f_node, body)?;
Ok(FunctionBuilder::from_dfg_builder(db))
}

/// Add a [`ops::FuncDefn`] node of the specified visibility.
/// Returns a builder to define the function body graph.
///
/// # Errors
///
/// This function will return an error if there is an error in adding the
/// [`ops::FuncDefn`] node.
pub fn define_function_vis(
&mut self,
name: impl Into<String>,
signature: impl Into<PolyFuncType>,
visibility: Visibility,
) -> Result<FunctionBuilder<&mut Hugr>, BuildError> {
self.define_function_op(FuncDefn::new_vis(name, signature, visibility))
}

fn define_function_op(
&mut self,
op: FuncDefn,
) -> Result<FunctionBuilder<&mut Hugr>, BuildError> {
let body = op.signature().body().clone();
let f_node = self.add_child_node(op);

// Add the extensions used by the function types.
self.use_extensions(
body.used_extensions().unwrap_or_else(|e| {
panic!("Build-time signatures should have valid extensions. {e}")
}),
);

let db = DFGBuilder::create_with_io(self.hugr_mut(), f_node, body)?;
Ok(FunctionBuilder::from_dfg_builder(db))
}

/// Declare a function with `signature` and return a handle to the declaration.
/// Declare a [Visibility::Public] function with `signature` and return a handle to the declaration.
///
/// # Errors
///
Expand All @@ -97,10 +132,26 @@ impl<T: AsMut<Hugr> + AsRef<Hugr>> ModuleBuilder<T> {
&mut self,
name: impl Into<String>,
signature: PolyFuncType,
) -> Result<FuncID<false>, BuildError> {
self.declare_vis(name, signature, Visibility::Public)
}

/// Declare a function with the specified `signature` and [Visibility],
/// and return a handle to the declaration.
///
/// # Errors
///
/// This function will return an error if there is an error in adding the
/// [`crate::ops::OpType::FuncDecl`] node.
pub fn declare_vis(
&mut self,
name: impl Into<String>,
signature: PolyFuncType,
visibility: Visibility,
) -> Result<FuncID<false>, BuildError> {
let body = signature.body().clone();
// TODO add param names to metadata
let declare_n = self.add_child_node(ops::FuncDecl::new(name, signature));
let declare_n = self.add_child_node(ops::FuncDecl::new_vis(name, signature, visibility));

// Add the extensions used by the function types.
self.use_extensions(
Expand All @@ -112,8 +163,8 @@ impl<T: AsMut<Hugr> + AsRef<Hugr>> ModuleBuilder<T> {
Ok(declare_n.into())
}

/// Add a [`ops::FuncDefn`] node and returns a builder to define the function
/// body graph.
/// Adds a [`ops::FuncDefn`] node and returns a builder to define the function
/// body graph. The function will be private. (See [Self::define_function_vis].)
///
/// # Errors
///
Expand All @@ -124,19 +175,7 @@ impl<T: AsMut<Hugr> + AsRef<Hugr>> ModuleBuilder<T> {
name: impl Into<String>,
signature: impl Into<PolyFuncType>,
) -> Result<FunctionBuilder<&mut Hugr>, BuildError> {
let signature: PolyFuncType = signature.into();
let body = signature.body().clone();
let f_node = self.add_child_node(ops::FuncDefn::new(name, signature));

// Add the extensions used by the function types.
self.use_extensions(
body.used_extensions().unwrap_or_else(|e| {
panic!("Build-time signatures should have valid extensions. {e}")
}),
);

let db = DFGBuilder::create_with_io(self.hugr_mut(), f_node, body)?;
Ok(FunctionBuilder::from_dfg_builder(db))
self.define_function_op(FuncDefn::new(name, signature))
}

/// Add a [`crate::ops::OpType::AliasDefn`] node and return a handle to the Alias.
Expand Down
40 changes: 40 additions & 0 deletions hugr-core/src/core.rs
Original file line number Diff line number Diff line change
Expand Up @@ -276,6 +276,46 @@ impl<N: HugrNode> std::fmt::Display for Wire<N> {
}
}

/// Marks [FuncDefn](crate::ops::FuncDefn)s and [FuncDecl](crate::ops::FuncDecl)s as
/// to whether they should be considered for linking.
#[derive(
Clone,
Debug,
derive_more::Display,
PartialEq,
Eq,
PartialOrd,
Ord,
serde::Serialize,
serde::Deserialize,
)]
#[cfg_attr(test, derive(proptest_derive::Arbitrary))]
#[non_exhaustive]
pub enum Visibility {
/// Function is visible or exported
Public,
/// Function is hidden, for use within the hugr only
Private,
}

impl From<hugr_model::v0::Visibility> for Visibility {
fn from(value: hugr_model::v0::Visibility) -> Self {
match value {
hugr_model::v0::Visibility::Private => Self::Private,
hugr_model::v0::Visibility::Public => Self::Public,
}
}
}

impl From<Visibility> for hugr_model::v0::Visibility {
fn from(value: Visibility) -> Self {
match value {
Visibility::Public => hugr_model::v0::Visibility::Public,
Visibility::Private => hugr_model::v0::Visibility::Private,
}
}
}

/// Enum for uniquely identifying the origin of linear wires in a circuit-like
/// dataflow region.
///
Expand Down
38 changes: 22 additions & 16 deletions hugr-core/src/export.rs
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ use crate::{
};

use fxhash::{FxBuildHasher, FxHashMap};
use hugr_model::v0::Visibility;
use hugr_model::v0::{
self as model,
bumpalo::{Bump, collections::String as BumpString, collections::Vec as BumpVec},
Expand Down Expand Up @@ -230,16 +231,6 @@ impl<'a> Context<'a> {
}
}

/// Get the name of a function definition or declaration node. Returns `None` if not
/// one of those operations.
fn get_func_name(&self, func_node: Node) -> Option<&'a str> {
match self.hugr.get_optype(func_node) {
OpType::FuncDecl(func_decl) => Some(func_decl.func_name()),
OpType::FuncDefn(func_defn) => Some(func_defn.func_name()),
_ => None,
}
}

fn with_local_scope<T>(&mut self, node: table::NodeId, f: impl FnOnce(&mut Self) -> T) -> T {
let prev_local_scope = self.local_scope.replace(node);
let prev_local_constraints = std::mem::take(&mut self.local_constraints);
Expand Down Expand Up @@ -338,8 +329,11 @@ impl<'a> Context<'a> {
}

OpType::FuncDefn(func) => self.with_local_scope(node_id, |this| {
let name = this.get_func_name(node).unwrap();
let symbol = this.export_poly_func_type(name, func.signature());
let symbol = this.export_poly_func_type(
func.func_name(),
func.visibility().clone().into(),
func.signature(),
);
regions = this.bump.alloc_slice_copy(&[this.export_dfg(
node,
model::ScopeClosure::Closed,
Expand All @@ -349,15 +343,21 @@ impl<'a> Context<'a> {
}),

OpType::FuncDecl(func) => self.with_local_scope(node_id, |this| {
let name = this.get_func_name(node).unwrap();
let symbol = this.export_poly_func_type(name, func.signature());
let symbol = this.export_poly_func_type(
func.func_name(),
func.visibility().clone().into(),
func.signature(),
);
table::Operation::DeclareFunc(symbol)
}),

OpType::AliasDecl(alias) => self.with_local_scope(node_id, |this| {
// TODO: We should support aliases with different types and with parameters
let signature = this.make_term_apply(model::CORE_TYPE, &[]);
// Visibility is not spec'd in hugr-core
let visibility = this.bump.alloc(Visibility::default()); // good to common up!?
let symbol = this.bump.alloc(table::Symbol {
visibility,
name: &alias.name,
params: &[],
constraints: &[],
Expand All @@ -370,7 +370,10 @@ impl<'a> Context<'a> {
let value = this.export_type(&alias.definition);
// TODO: We should support aliases with different types and with parameters
let signature = this.make_term_apply(model::CORE_TYPE, &[]);
// Visibility is not spec'd in hugr-core
let visibility = this.bump.alloc(Visibility::default()); // good to common up!?
let symbol = this.bump.alloc(table::Symbol {
visibility,
name: &alias.name,
params: &[],
constraints: &[],
Expand Down Expand Up @@ -545,7 +548,8 @@ impl<'a> Context<'a> {

let symbol = self.with_local_scope(node, |this| {
let name = this.make_qualified_name(opdef.extension_id(), opdef.name());
this.export_poly_func_type(name, poly_func_type)
// Visibility of OpDef's has no effect
this.export_poly_func_type(name, Visibility::default(), poly_func_type)
});

let meta = {
Expand Down Expand Up @@ -796,13 +800,14 @@ impl<'a> Context<'a> {
pub fn export_poly_func_type<RV: MaybeRV>(
&mut self,
name: &'a str,
visibility: Visibility,
Copy link
Collaborator

Choose a reason for hiding this comment

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

Is this really right? I would rather not have visibility in the type system

Copy link
Contributor Author

Choose a reason for hiding this comment

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

IMHO this is a bit misnamed - it's exporting a thing with a poly func type, in fact a specific thing, a Symbol. (I mean, it already has a name parameter, right....)

Yes, adding Visibility to Symbol is perhaps a bit of a change (I did check with @zrho first) - I'd argue (1) hugr-model goes for uniformity over tailoring everything to specific cases, (2) adding visibility elsewhere is a big PITA, (3) There's no reason not to apply Visibility to e.g. aliases too, just that we aren't bothered to do this in hugr-core atm.

t: &PolyFuncTypeBase<RV>,
) -> &'a table::Symbol<'a> {
let mut params = BumpVec::with_capacity_in(t.params().len(), self.bump);
let scope = self
.local_scope
.expect("exporting poly func type outside of local scope");

let visibility = self.bump.alloc(visibility);
for (i, param) in t.params().iter().enumerate() {
let name = self.bump.alloc_str(&i.to_string());
let r#type = self.export_term(param, Some((scope, i as _)));
Expand All @@ -814,6 +819,7 @@ impl<'a> Context<'a> {
let body = self.export_func_type(t.body());

self.bump.alloc(table::Symbol {
visibility,
name,
params: params.into_bump_slice(),
constraints,
Expand Down
Loading
Loading