-
Notifications
You must be signed in to change notification settings - Fork 8
feat!: Allow making (and sharing) new FuncDecls in linearization #2108
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
base: main
Are you sure you want to change the base?
Conversation
`insert_hugr`, `insert_from_view`, and `insert_subgraph` were written before we made `Node` a type generic, and incorrectly assumed the return type maps were always `hugr::Node`s. The methods were either unusable or incorrect when using generic `HugrView`s source/targets with non-base node types. This PR fixes that, and additionally allows us us to have `SiblingSubgraph::extract_subgraph` work for generic `HugrViews`. BREAKING CHANGE: Added Node type parameters to extraction operations in `HugrMut`.
`HugrMutInternals` is part of the semi-private traits defined in `hugr-core`. While most things get re-exported in `hugr`, we `*Internal` traits require you to explicitly declare a dependency on the `-core` package (as we don't want most users to have to interact with them). For some reason there was a public re-export of the trait in a re-exported module, so it ended up appearing in `hugr` anyways. BREAKING CHANGE: Removed public re-export of `HugrMutInternals` from `hugr`.
#2027 ended up being breaking due to adding a new variant to an error enum missing the `non_exhaustive` marker. This (breaking) PR makes sure all error enums have the flag. BREAKING CHANGE: Marked all Error enums as `non_exhaustive`
* PartialValue now has a LoadedFunction variant, created by LoadFunction nodes (only, although other analyses are able to create PartialValues if they want) * This requires adding a type parameter to PartialValue for the type of Node, which gets everywhere :-(. * Use this to handle CallIndirects *with known targets* (it'll be a single known target or none at all) just like other Calls to the same function * deprecate (and ignore) `value_from_function` * Add a new trait `AsConcrete` for the result type of `PartialValue::try_into_concrete` and `PartialSum::try_into_sum` Note almost no change to constant folding (only to drop impl of `value_from_function`) BREAKING CHANGE: in dataflow framework, PartialValue now has additional variant; `try_into_concrete` requires the target type to implement AsConcrete.
Adds a generic node type to the `NodeHandle` type. This is a required change for #2029. drive-by: Implement the "Link the NodeHandles to the OpType" TODO
Closes #1595 BREAKING CHANGE: `values` field in `Extension` and `ExtensionValue` struct/class removed in rust and python. Use 0-input ops that return constant values.
Currently We have several "passes": monomorphization, dead function removal, constant folding. Each has its own code to allow setting a validation level (before and after that pass). This PR adds the ability chain (sequence) passes;, and to add validation before+after any pass or sequence; and commons up validation code. The top-level `constant_fold_pass` (etc.) functions are left as wrappers that do a single pass with validation only in test. I've left ConstFoldPass as always including DCE, but an alternative could be to return a sequence of the two - ATM that means a tuple `(ConstFoldPass, DeadCodeElimPass)`. I also wondered about including a method `add_entry_point` in ComposablePass (e.g. for ConstFoldPass, that means `with_inputs` but no inputs, i.e. all Top). I feel this is not applicable to *all* passes, but near enough. This could be done in a later PR but `add_entry_point` would need a no-op default for that to be a non-breaking change. So if we wouldn't be happy with the no-op default then I could just add it here... Finally...docs are extremely minimal ATM (this is hugr-passes), I am hoping that most of this is reasonably obvious (it doesn't really do a lot!), but please flag anything you think is particularly in need of a doc comment! BREAKING CHANGE: quite a lot of calls to current pass routines will break, specific cases include (a) `with_validation_level` should be done by wrapping a ValidatingPass around the receiver; (b) XXXPass::run() requires `use ...ComposablePass` (however, such calls will cease to do any validation). closes #1832
…eady in the Hugr (#2094) There are two issues: * Errors. The previous NodeTemplates still always work, but the Call one can fail if the Hugr doesn't contain the target function node. ATM there is no channel for reporting that error so I've had to panic. Otherwise it's an even-more-breaking change to add an error type to `NodeTemplate::add()` and `NodeTemplate::add_hugr()`. Should we? (I note `HugrMut::connect` panics if the node isn't there, but could make the `NodeTemplate::add` builder method return a BuildError...and propagate that everywhere of course) * There's a big limitation in `linearize_array` that it'll break if the *element* says it should be copied/discarded via a NodeTemplate::Call, as `linearize_array` puts the elementwise copy/discard function into a *nested Hugr* (`Value::Function`) that won't contain the function. This could be fixed via lifting those to toplevel FuncDefns with name-mangling, but I'd rather leave that for #2086 .... BREAKING CHANGE: Add new variant NodeTemplate::Call; LinearizeError no longer derives Eq.
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #2108 +/- ##
==========================================
+ Coverage 83.05% 83.35% +0.29%
==========================================
Files 218 219 +1
Lines 41783 42254 +471
Branches 37961 38356 +395
==========================================
+ Hits 34703 35219 +516
+ Misses 5275 5218 -57
- Partials 1805 1817 +12
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
This PR contains breaking changes to the public Rust API. cargo-semver-checks summary
|
56c110c
to
9cd24ff
Compare
This PR contains a breaking change and is targeting the Breaking changes are currently not allowed to be merged into Please update your PR to target the appropriate branch. |
Linearizer
. TheCallbackHandler
implements those same methods, plus alsomake_function
(with caching/de-duping via a typeFuncId
). However DelegatingLinearizer does not - you need to construct aCallbackHandler
by passing a HugrMut.CallbackHandler
also passed to ReplaceTypes callbacks so they can create functions tooThe thing that is missing here is that for array handlers we need to add a NodeTemplate::CompoundOp (i.e. a subtree in a
Hugr
) that has incoming static edges from FuncDefns in the containing Hugr. (Thus, is not valid in itself.) Or else, we lower the handlers toNodeTemplate::Call
s, to a function (created bymake_function
i.e. from a subtree in aHugr
) that itself has incoming static edges; so either way we need toinsert_hugr
a Hugr with incoming static edges. I see two ways:Hugr
will have to containFuncDecl
stand-ins for theFuncDefn
s in the outer Hugr, withNodeTemplate::CompoundOp
andmake_function
removing said FuncDecls and re-sourcing their outgoing static edges to come from corresponding FuncDefns in the outer Hugr. The question is then how the "corresponding FuncDefns" are identified, and there are a few variations:Map<Node,Node>
, from FuncDecls in the new subtree Hugr to FuncDefns in the host/outer Hugr, as an element ofNodeTemplate::CompoundOp
and an extra parameter tomake_function
,FuncId
is likely no longer required; if metadata, we might want to either remove FuncId (andmake_function
requires the FuncDefn it's given to have the metadata already) or make FuncId match the metadata (i.e.make_function
adds the metadata to the subtree itself)TODO:
FuncDefn::name
BREAKING CHANGE: trait Linearizer removed; cannot call those methods on DelegatingLinearizer directly, must call handler() with a HugrMut first. Changes to types of arguments to callbacks.