feat!: Explicit effects system, prevent calls from annotated funcs to those with fewer effects#1723
feat!: Explicit effects system, prevent calls from annotated funcs to those with fewer effects#1723acl-cqc wants to merge 61 commits into
Conversation
…...should this be in `Context`?
…n Context and check_bb/cfg/etc.
|
This PR contains breaking changes to the public Python API. Breaking changes summary |
|
Bencher Report| Branch | acl/max_effects |
| Testbed | Linux |
Click to view all benchmark results
| Benchmark | hugr_bytes | Benchmark Result bytes x 1e3 (Result Δ%) | Upper Boundary bytes x 1e3 (Limit %) | hugr_nodes | Benchmark Result nodes (Result Δ%) | Upper Boundary nodes (Limit %) |
|---|---|---|---|---|---|---|
| tests/benchmarks/test_big_array.py::test_big_array_compile | 📈 view plot 🚷 view threshold | 158.77 x 1e3(0.00%)Baseline: 158.77 x 1e3 | 160.36 x 1e3 (99.01%) | 📈 view plot 🚷 view threshold | 6,641.00(0.00%)Baseline: 6,641.00 | 6,707.41 (99.01%) |
| tests/benchmarks/test_ctrl_flow.py::test_many_ctrl_flow_compile | 📈 view plot 🚷 view threshold | 27.53 x 1e3(-0.02%)Baseline: 27.53 x 1e3 | 27.81 x 1e3 (98.99%) | 📈 view plot 🚷 view threshold | 1,074.00(0.00%)Baseline: 1,074.00 | 1,084.74 (99.01%) |
| tests/benchmarks/test_queue_push_pop.py::test_queue_push_benchmark_compile | 📈 view plot 🚷 view threshold | 10.91 x 1e3(0.00%)Baseline: 10.91 x 1e3 | 11.02 x 1e3 (99.01%) | 📈 view plot 🚷 view threshold | 308.00(0.00%)Baseline: 308.00 | 311.08 (99.01%) |
| tests/benchmarks/test_queue_push_pop.py::test_queue_push_pop_benchmark_compile | 📈 view plot 🚷 view threshold | 14.84 x 1e3(+0.01%)Baseline: 14.84 x 1e3 | 14.98 x 1e3 (99.02%) | 📈 view plot 🚷 view threshold | 435.00(0.00%)Baseline: 435.00 | 439.35 (99.01%) |
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #1723 +/- ##
==========================================
- Coverage 92.98% 92.95% -0.03%
==========================================
Files 135 137 +2
Lines 13056 13174 +118
==========================================
+ Hits 12140 12246 +106
- Misses 916 928 +12 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
acl-cqc
changed the title
| arrow = ( | ||
| "->" | ||
| if ty.declared_effects is None | ||
| else f"-{Effect.format_list(ty.declared_effects)}->" |
There was a problem hiding this comment.
This suggests we may want to work harder to separate out cases where the effects are relevant from cases where they are not, and only render them when they are. (E.g. TooManyEffectsError and OverloadNoMatchError, where the effects are not even in a FunctionType; but then TypeMismatchError does need to render effects in detail in the mismatch is between Callables differing only in effects)...this does show up quite a bit in error messages ATM
| raise GuppyError(LinearComptimeError(node.right, ty)) | ||
| case ast.Call(func=ast.Name(id="effects")) as fx: | ||
| if not isinstance(ty, FunctionType): | ||
| raise GuppyError(InvalidFlagError(node.right)) |
There was a problem hiding this comment.
This InvalidFlagError is not right, it's probably a new kind of error specific to @effects (a construct of only very limited use i.e. in connection with Callable!)...I haven't put in tests of each case: LHS not a Callable type; multiple @effects; individual effects not valid.
| if not isinstance(e, ast.Name): | ||
| raise GuppyError(InvalidFlagError(node.right)) | ||
| try: | ||
| effects.append(Effect.__from_str__(e.id)) |
There was a problem hiding this comment.
I mean, you need the relevant import ANY in scope to make python 3.12 happy, but 3.14 is happy even without that. Moreover - just like other type modifiers - the effects(ANY) doesn't even reflect what you've imported....
| for inp in sig.inputs | ||
| ) | ||
| s += ", ..." if has_var_args else "" | ||
| # TODO Not clear how to display effects in a Python-like syntax? (skip for now) |
There was a problem hiding this comment.
perhaps def foo(x : int) -> int (with effects []) ?
| from collections.abc import Callable, Sequence | ||
| from types import FrameType | ||
|
|
||
| from guppylang import Effect |
There was a problem hiding this comment.
This seems wrong - guppylang_internals is now referring to public guppylang - because here we have decorators used in stdlib etc. Maybe a good argument to just combine the enums in guppylang-internals and re-export?
| before being used.""" | ||
|
|
||
| @custom_function(compiler=ReadFutureBoolCompiler()) | ||
| # We do *not* model the pipeline as a side-effect |
There was a problem hiding this comment.
is this a good comment? Should I just drop it?
This reverts commit 41b68c68f112276a24747ccc0e76fd78aa147ae4.
3 participants
closes #1745.
However, note this is pure typechecking, and does not add any new order edges (ideally we would use the effect-system information to replace
EXTENSION_OPS_WITH_SIDE_EFFECTSbut leaving that to the PR that solves #1746.ANY.[Effect.ANY]@effectsmodifier; without which, you get[ANY]by default.Do I want more tests of the quantum stdlib? Any ideas what?
BREAKING CHANGE: Functions taking Callable will be able to accept either only Callables with effects (the default) or (if annotated) those without, rather than both; those returning Callables will need an explicit annotation if the returnee is pure.