Skip to content

Commit 5bbe0cd

Browse files
acl-cqcacl-cqc
authored
feat!: Add Visibility to FuncDefn/FuncDecl. (#2143)
Note: modified to come after #2412 as I want those Schema checks in place to check this works! closes #2354, #1752. Includes cherry-pick of #2343. * Add `enum Visibility`. In fact *2 (hugr-model version is not serde-able so cannot be used in hugr-core). * `FuncDefn` and `FuncDecl` gain this. (A good use-case for a private FuncDecl is building cyclic functions via builder `define_declaration`.) * I've not added to `AliasDecl` or `AliasDefn` - I could, it would be consistent, but we hardly use them.... * `validate` checks that the *public* children of a Module have unique names (except multiple FuncDecls may alias). * `FuncDefn::new` defaults to private (also builder `define_function` / `FunctionBuilder::new`, and json-deserialize in both python and rust). There is a `_vis` taking an explicit Visibility parameter. * `FuncDecl::new` defaults to public (also builder `declare_function`, and json-deserialize*2). Again there is `_vis`. * hugr-model import/export + roundtrip + AST printing via new keyword `pub`. Note since `hugr-core` does not have visibility on aliases or extension operations I've defaulted these to private to avoid any change to textual output. * add field to hugr-py, using "string"-enum (`Literal["Public", "Private"]`) * I've attempted to add hugr-model hugr-py bindings, but there are no tests of these yet. Note all changes to hugr-passes delayed until a follow-up PR. BREAKING CHANGE: hugr-model: Symbol has an extra field
1 parent f557008 commit 5bbe0cd

41 files changed

Lines changed: 950 additions & 422 deletions

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

‎hugr-core/src/builder/dataflow.rs‎

Lines changed: 24 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -8,14 +8,9 @@ use std::marker::PhantomData;
88

99
use crate::hugr::internal::HugrMutInternals;
1010
use crate::hugr::{HugrView, ValidationError};
11-
use crate::ops::{self, OpParent};
12-
use crate::ops::{DataflowParent, Input, Output};
13-
use crate::{Direction, IncomingPort, OutgoingPort, Wire};
14-
11+
use crate::ops::{self, DataflowParent, FuncDefn, Input, OpParent, Output};
1512
use crate::types::{PolyFuncType, Signature, Type};
16-
17-
use crate::Node;
18-
use crate::{Hugr, hugr::HugrMut};
13+
use crate::{Direction, Hugr, IncomingPort, Node, OutgoingPort, Visibility, Wire, hugr::HugrMut};
1914

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

154149
impl FunctionBuilder<Hugr> {
155-
/// Initialize a builder for a `FuncDefn` rooted HUGR
150+
/// Initialize a builder for a [`FuncDefn`](ops::FuncDefn)-rooted HUGR;
151+
/// the function will be private. (See also [Self::new_vis].)
152+
///
156153
/// # Errors
157154
///
158155
/// Error in adding DFG child nodes.
159156
pub fn new(
160157
name: impl Into<String>,
161158
signature: impl Into<PolyFuncType>,
162159
) -> Result<Self, BuildError> {
163-
let signature: PolyFuncType = signature.into();
164-
let body = signature.body().clone();
165-
let op = ops::FuncDefn::new(name, signature);
160+
Self::new_with_op(FuncDefn::new(name, signature))
161+
}
162+
163+
/// Initialize a builder for a FuncDefn-rooted HUGR, with the specified
164+
/// [Visibility].
165+
///
166+
/// # Errors
167+
///
168+
/// Error in adding DFG child nodes.
169+
pub fn new_vis(
170+
name: impl Into<String>,
171+
signature: impl Into<PolyFuncType>,
172+
visibility: Visibility,
173+
) -> Result<Self, BuildError> {
174+
Self::new_with_op(FuncDefn::new_vis(name, signature, visibility))
175+
}
176+
177+
fn new_with_op(op: FuncDefn) -> Result<Self, BuildError> {
178+
let body = op.signature().body().clone();
166179

167180
let base = Hugr::new_with_entrypoint(op).expect("FuncDefn entrypoint should be valid");
168181
let root = base.entrypoint();

‎hugr-core/src/builder/module.rs‎

Lines changed: 73 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -4,15 +4,14 @@ use super::{
44
dataflow::{DFGBuilder, FunctionBuilder},
55
};
66

7-
use crate::hugr::ValidationError;
87
use crate::hugr::internal::HugrMutInternals;
98
use crate::hugr::views::HugrView;
109
use crate::ops;
11-
use crate::types::{PolyFuncType, Type, TypeBound};
12-
1310
use crate::ops::handle::{AliasID, FuncID, NodeHandle};
11+
use crate::types::{PolyFuncType, Type, TypeBound};
12+
use crate::{Hugr, Node, Visibility};
13+
use crate::{hugr::ValidationError, ops::FuncDefn};
1414

15-
use crate::{Hugr, Node};
1615
use smol_str::SmolStr;
1716

1817
/// Builder for a HUGR module.
@@ -69,25 +68,61 @@ impl<T: AsMut<Hugr> + AsRef<Hugr>> ModuleBuilder<T> {
6968
f_id: &FuncID<false>,
7069
) -> Result<FunctionBuilder<&mut Hugr>, BuildError> {
7170
let f_node = f_id.node();
72-
let decl =
73-
self.hugr()
74-
.get_optype(f_node)
75-
.as_func_decl()
76-
.ok_or(BuildError::UnexpectedType {
77-
node: f_node,
78-
op_desc: "crate::ops::OpType::FuncDecl",
79-
})?;
80-
let name = decl.func_name().clone();
81-
let sig = decl.signature().clone();
82-
let body = sig.body().clone();
83-
self.hugr_mut()
84-
.replace_op(f_node, ops::FuncDefn::new(name, sig));
71+
let opty = self.hugr_mut().optype_mut(f_node);
72+
let ops::OpType::FuncDecl(decl) = opty else {
73+
return Err(BuildError::UnexpectedType {
74+
node: f_node,
75+
op_desc: "crate::ops::OpType::FuncDecl",
76+
});
77+
};
78+
79+
let body = decl.signature().body().clone();
80+
*opty = ops::FuncDefn::new_vis(
81+
decl.func_name(),
82+
decl.signature().clone(),
83+
decl.visibility().clone(),
84+
)
85+
.into();
86+
87+
let db = DFGBuilder::create_with_io(self.hugr_mut(), f_node, body)?;
88+
Ok(FunctionBuilder::from_dfg_builder(db))
89+
}
90+
91+
/// Add a [`ops::FuncDefn`] node of the specified visibility.
92+
/// Returns a builder to define the function body graph.
93+
///
94+
/// # Errors
95+
///
96+
/// This function will return an error if there is an error in adding the
97+
/// [`ops::FuncDefn`] node.
98+
pub fn define_function_vis(
99+
&mut self,
100+
name: impl Into<String>,
101+
signature: impl Into<PolyFuncType>,
102+
visibility: Visibility,
103+
) -> Result<FunctionBuilder<&mut Hugr>, BuildError> {
104+
self.define_function_op(FuncDefn::new_vis(name, signature, visibility))
105+
}
106+
107+
fn define_function_op(
108+
&mut self,
109+
op: FuncDefn,
110+
) -> Result<FunctionBuilder<&mut Hugr>, BuildError> {
111+
let body = op.signature().body().clone();
112+
let f_node = self.add_child_node(op);
113+
114+
// Add the extensions used by the function types.
115+
self.use_extensions(
116+
body.used_extensions().unwrap_or_else(|e| {
117+
panic!("Build-time signatures should have valid extensions. {e}")
118+
}),
119+
);
85120

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

90-
/// Declare a function with `signature` and return a handle to the declaration.
125+
/// Declare a [Visibility::Public] function with `signature` and return a handle to the declaration.
91126
///
92127
/// # Errors
93128
///
@@ -97,10 +132,26 @@ impl<T: AsMut<Hugr> + AsRef<Hugr>> ModuleBuilder<T> {
97132
&mut self,
98133
name: impl Into<String>,
99134
signature: PolyFuncType,
135+
) -> Result<FuncID<false>, BuildError> {
136+
self.declare_vis(name, signature, Visibility::Public)
137+
}
138+
139+
/// Declare a function with the specified `signature` and [Visibility],
140+
/// and return a handle to the declaration.
141+
///
142+
/// # Errors
143+
///
144+
/// This function will return an error if there is an error in adding the
145+
/// [`crate::ops::OpType::FuncDecl`] node.
146+
pub fn declare_vis(
147+
&mut self,
148+
name: impl Into<String>,
149+
signature: PolyFuncType,
150+
visibility: Visibility,
100151
) -> Result<FuncID<false>, BuildError> {
101152
let body = signature.body().clone();
102153
// TODO add param names to metadata
103-
let declare_n = self.add_child_node(ops::FuncDecl::new(name, signature));
154+
let declare_n = self.add_child_node(ops::FuncDecl::new_vis(name, signature, visibility));
104155

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

115-
/// Add a [`ops::FuncDefn`] node and returns a builder to define the function
116-
/// body graph.
166+
/// Adds a [`ops::FuncDefn`] node and returns a builder to define the function
167+
/// body graph. The function will be private. (See [Self::define_function_vis].)
117168
///
118169
/// # Errors
119170
///
@@ -124,19 +175,7 @@ impl<T: AsMut<Hugr> + AsRef<Hugr>> ModuleBuilder<T> {
124175
name: impl Into<String>,
125176
signature: impl Into<PolyFuncType>,
126177
) -> Result<FunctionBuilder<&mut Hugr>, BuildError> {
127-
let signature: PolyFuncType = signature.into();
128-
let body = signature.body().clone();
129-
let f_node = self.add_child_node(ops::FuncDefn::new(name, signature));
130-
131-
// Add the extensions used by the function types.
132-
self.use_extensions(
133-
body.used_extensions().unwrap_or_else(|e| {
134-
panic!("Build-time signatures should have valid extensions. {e}")
135-
}),
136-
);
137-
138-
let db = DFGBuilder::create_with_io(self.hugr_mut(), f_node, body)?;
139-
Ok(FunctionBuilder::from_dfg_builder(db))
178+
self.define_function_op(FuncDefn::new(name, signature))
140179
}
141180

142181
/// Add a [`crate::ops::OpType::AliasDefn`] node and return a handle to the Alias.

‎hugr-core/src/core.rs‎

Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -276,6 +276,46 @@ impl<N: HugrNode> std::fmt::Display for Wire<N> {
276276
}
277277
}
278278

279+
/// Marks [FuncDefn](crate::ops::FuncDefn)s and [FuncDecl](crate::ops::FuncDecl)s as
280+
/// to whether they should be considered for linking.
281+
#[derive(
282+
Clone,
283+
Debug,
284+
derive_more::Display,
285+
PartialEq,
286+
Eq,
287+
PartialOrd,
288+
Ord,
289+
serde::Serialize,
290+
serde::Deserialize,
291+
)]
292+
#[cfg_attr(test, derive(proptest_derive::Arbitrary))]
293+
#[non_exhaustive]
294+
pub enum Visibility {
295+
/// Function is visible or exported
296+
Public,
297+
/// Function is hidden, for use within the hugr only
298+
Private,
299+
}
300+
301+
impl From<hugr_model::v0::Visibility> for Visibility {
302+
fn from(value: hugr_model::v0::Visibility) -> Self {
303+
match value {
304+
hugr_model::v0::Visibility::Private => Self::Private,
305+
hugr_model::v0::Visibility::Public => Self::Public,
306+
}
307+
}
308+
}
309+
310+
impl From<Visibility> for hugr_model::v0::Visibility {
311+
fn from(value: Visibility) -> Self {
312+
match value {
313+
Visibility::Public => hugr_model::v0::Visibility::Public,
314+
Visibility::Private => hugr_model::v0::Visibility::Private,
315+
}
316+
}
317+
}
318+
279319
/// Enum for uniquely identifying the origin of linear wires in a circuit-like
280320
/// dataflow region.
281321
///

‎hugr-core/src/export.rs‎

Lines changed: 22 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@ use crate::{
2020
};
2121

2222
use fxhash::{FxBuildHasher, FxHashMap};
23+
use hugr_model::v0::Visibility;
2324
use hugr_model::v0::{
2425
self as model,
2526
bumpalo::{Bump, collections::String as BumpString, collections::Vec as BumpVec},
@@ -230,16 +231,6 @@ impl<'a> Context<'a> {
230231
}
231232
}
232233

233-
/// Get the name of a function definition or declaration node. Returns `None` if not
234-
/// one of those operations.
235-
fn get_func_name(&self, func_node: Node) -> Option<&'a str> {
236-
match self.hugr.get_optype(func_node) {
237-
OpType::FuncDecl(func_decl) => Some(func_decl.func_name()),
238-
OpType::FuncDefn(func_defn) => Some(func_defn.func_name()),
239-
_ => None,
240-
}
241-
}
242-
243234
fn with_local_scope<T>(&mut self, node: table::NodeId, f: impl FnOnce(&mut Self) -> T) -> T {
244235
let prev_local_scope = self.local_scope.replace(node);
245236
let prev_local_constraints = std::mem::take(&mut self.local_constraints);
@@ -338,8 +329,11 @@ impl<'a> Context<'a> {
338329
}
339330

340331
OpType::FuncDefn(func) => self.with_local_scope(node_id, |this| {
341-
let name = this.get_func_name(node).unwrap();
342-
let symbol = this.export_poly_func_type(name, func.signature());
332+
let symbol = this.export_poly_func_type(
333+
func.func_name(),
334+
func.visibility().clone().into(),
335+
func.signature(),
336+
);
343337
regions = this.bump.alloc_slice_copy(&[this.export_dfg(
344338
node,
345339
model::ScopeClosure::Closed,
@@ -349,15 +343,21 @@ impl<'a> Context<'a> {
349343
}),
350344

351345
OpType::FuncDecl(func) => self.with_local_scope(node_id, |this| {
352-
let name = this.get_func_name(node).unwrap();
353-
let symbol = this.export_poly_func_type(name, func.signature());
346+
let symbol = this.export_poly_func_type(
347+
func.func_name(),
348+
func.visibility().clone().into(),
349+
func.signature(),
350+
);
354351
table::Operation::DeclareFunc(symbol)
355352
}),
356353

357354
OpType::AliasDecl(alias) => self.with_local_scope(node_id, |this| {
358355
// TODO: We should support aliases with different types and with parameters
359356
let signature = this.make_term_apply(model::CORE_TYPE, &[]);
357+
// Visibility is not spec'd in hugr-core
358+
let visibility = this.bump.alloc(Visibility::default()); // good to common up!?
360359
let symbol = this.bump.alloc(table::Symbol {
360+
visibility,
361361
name: &alias.name,
362362
params: &[],
363363
constraints: &[],
@@ -370,7 +370,10 @@ impl<'a> Context<'a> {
370370
let value = this.export_type(&alias.definition);
371371
// TODO: We should support aliases with different types and with parameters
372372
let signature = this.make_term_apply(model::CORE_TYPE, &[]);
373+
// Visibility is not spec'd in hugr-core
374+
let visibility = this.bump.alloc(Visibility::default()); // good to common up!?
373375
let symbol = this.bump.alloc(table::Symbol {
376+
visibility,
374377
name: &alias.name,
375378
params: &[],
376379
constraints: &[],
@@ -545,7 +548,8 @@ impl<'a> Context<'a> {
545548

546549
let symbol = self.with_local_scope(node, |this| {
547550
let name = this.make_qualified_name(opdef.extension_id(), opdef.name());
548-
this.export_poly_func_type(name, poly_func_type)
551+
// Visibility of OpDef's has no effect
552+
this.export_poly_func_type(name, Visibility::default(), poly_func_type)
549553
});
550554

551555
let meta = {
@@ -796,13 +800,14 @@ impl<'a> Context<'a> {
796800
pub fn export_poly_func_type<RV: MaybeRV>(
797801
&mut self,
798802
name: &'a str,
803+
visibility: Visibility,
799804
t: &PolyFuncTypeBase<RV>,
800805
) -> &'a table::Symbol<'a> {
801806
let mut params = BumpVec::with_capacity_in(t.params().len(), self.bump);
802807
let scope = self
803808
.local_scope
804809
.expect("exporting poly func type outside of local scope");
805-
810+
let visibility = self.bump.alloc(visibility);
806811
for (i, param) in t.params().iter().enumerate() {
807812
let name = self.bump.alloc_str(&i.to_string());
808813
let r#type = self.export_term(param, Some((scope, i as _)));
@@ -814,6 +819,7 @@ impl<'a> Context<'a> {
814819
let body = self.export_func_type(t.body());
815820

816821
self.bump.alloc(table::Symbol {
822+
visibility,
817823
name,
818824
params: params.into_bump_slice(),
819825
constraints,

0 commit comments

Comments
 (0)