feat: Add Guppy type aliases#1645
Conversation
|
| Branch | ss/push-qxzxrlrrysko |
| 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 | 154.02 x 1e3(0.00%)Baseline: 154.02 x 1e3 | 155.56 x 1e3 (99.01%) | 📈 view plot 🚷 view threshold | 6,630.00(0.00%)Baseline: 6,630.00 | 6,696.30 (99.01%) |
| tests/benchmarks/test_ctrl_flow.py::test_many_ctrl_flow_compile | 📈 view plot 🚷 view threshold | 27.71 x 1e3(0.00%)Baseline: 27.71 x 1e3 | 27.99 x 1e3 (99.01%) | 📈 view plot 🚷 view threshold | 1,051.00(0.00%)Baseline: 1,051.00 | 1,061.51 (99.01%) |
| tests/benchmarks/test_queue_push_pop.py::test_queue_push_benchmark_compile | 📈 view plot 🚷 view threshold | 10.09 x 1e3(0.00%)Baseline: 10.09 x 1e3 | 10.19 x 1e3 (99.01%) | 📈 view plot 🚷 view threshold | 301.00(0.00%)Baseline: 301.00 | 304.01 (99.01%) |
| tests/benchmarks/test_queue_push_pop.py::test_queue_push_pop_benchmark_compile | 📈 view plot 🚷 view threshold | 13.70 x 1e3(0.00%)Baseline: 13.70 x 1e3 | 13.83 x 1e3 (99.01%) | 📈 view plot 🚷 view threshold | 420.00(0.00%)Baseline: 420.00 | 424.20 (99.01%) |
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #1645 +/- ##
==========================================
+ Coverage 92.85% 92.89% +0.04%
==========================================
Files 148 149 +1
Lines 13849 13970 +121
==========================================
+ Hits 12859 12978 +119
- Misses 990 992 +2 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
71b3742 to
fa6be65
Compare
3ec6f5c to
4678592
Compare
|
|
||
|
|
||
| def test_type_alias_bad_type_syntax(): | ||
| with pytest.raises(SyntaxError, match="Not a valid Guppy type: `foo bar`"): |
There was a problem hiding this comment.
Can you add an error tests for invalid aliases that fail at a later point, i.e. via a GuppyError instead of a SyntaxError?
There was a problem hiding this comment.
yes see undefined_type.py
| Help: Type aliases must eventually resolve to a non-alias type. Break the cycle | ||
| by inlining one alias or introducing a struct or enum wrapper. |
There was a problem hiding this comment.
Inlining can never break cycles? Similarly, I don't see how structs or enums would help?
| Note: | ||
| | | ||
| 3 | | ||
| 4 | MyAlias = guppy.type_alias("MyAlias") | ||
| | ------------------------------------- Alias `MyAlias` is part of this cycle |
There was a problem hiding this comment.
This note doesn't seem useful if it's a self cycle
| ) | ||
| defn = RawTypeAliasDef( | ||
| DefId.fresh(), | ||
| ty, |
There was a problem hiding this comment.
This sets the name of the definition as the type string which seems odd.
I see that you use a separate _alias_name function to resolve the name later by inspecting the Python scope. Note that this is quite different from how we handle naming of definition in other parts of the compiler. Usually, guppy.type_var etc take the definition name as an argument.
Checking the scope seems reasonable though, but it should happen here in the decorator! Also, the fallback probably shouldn't be the type string. Maybe, instead raise an error and force users to pass it as an argument?
There was a problem hiding this comment.
I've gone with requiring the name in this PR to match type_var
| ctx = TypeParsingCtx(globals, allow_free_vars=True) | ||
| ty = type_from_ast(self.type_ast, ctx) | ||
| params = tuple(ctx.param_var_mapping.values()) |
There was a problem hiding this comment.
This looks like an attempt to support generic aliases? I think those would be useful, but we should either add tests for generic aliases or just set params = [] here
There was a problem hiding this comment.
generic support added and tested, along with 3.12 type ... syntax
| err.add_sub_diagnostic( | ||
| RecursiveTypeAliasError.AliasNote(defn.defined_at, alias_name) | ||
| ) |
There was a problem hiding this comment.
This will fail to render if the aliases are defined in separate files
There was a problem hiding this comment.
While testing, I also found some other rendering errors. For example
Alias1 = guppy.type_alias("Alias2")
Alias2 = guppy.type_alias("Alias3")
Alias3 = guppy.type_alias("Alias2")
@guppy
def main(x: Alias1) -> Alias1:
return x
main.check()fails with
Error in sys.excepthook:
Traceback (most recent call last):
File "guppylang-internals/src/guppylang_internals/error.py", line 120, in hook
renderer.render_diagnostic(err.error)
File "guppylang-internals/src/guppylang_internals/diagnostic.py", line 323, in render_diagnostic
self.render_snippet(
File "guppylang-internals/src/guppylang_internals/diagnostic.py", line 419, in render_snippet
leading_whitespace = min(len(line) - len(line.lstrip()) for line in all_lines)
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
ValueError: min() iterable argument is empty
Not sure what's going on there...
There was a problem hiding this comment.
fixed, and your example added as a test
| # cycle, so avoid attaching the same note more than once. | ||
| if any( | ||
| isinstance(child, RecursiveTypeAliasError.AliasNote) | ||
| and child.alias_name == alias_name |
There was a problem hiding this comment.
Going by name feels a bit fragile, can we store DefIds instead?
| if isinstance(err.error, RecursiveTypeAliasError): | ||
| _add_alias_note(err.error, defn, ctx.globals) |
There was a problem hiding this comment.
I don't understand why we need to add the notes while unwinding the exception. We already have the cycle, can't we just add all notes in one go?
There was a problem hiding this comment.
Can you add some tests for how the alias acyclicity checking interacts with structs and enums? I.e. add some integration and error tests that involve struct fields/enum variants whose types contain aliases.
4678592 to
adca967
Compare
e9493c5 to
f3d03fd
Compare
There was a problem hiding this comment.
Pull request overview
Adds first-class Guppy type aliases via guppy.type_alias(), including support for Python 3.12+ PEP 695 type statements, and introduces internal machinery for parsing/checking aliases (including cycle detection) plus new integration/error tests.
Changes:
- Add
guppy.type_alias()API (string form + Python 3.12+TypeAliasTypeform) and improve type-string source annotation. - Introduce
RawTypeAliasDef/ParsedTypeAliasDef/CheckedTypeAliasDefwith recursive-alias detection and cycle diagnostics. - Add integration tests (incl. py312-only tests) and snapshot-based error tests for alias failures.
Reviewed changes
Copilot reviewed 18 out of 18 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/integration/test_type_alias.py | New integration tests covering alias chaining, generics, structs/enums, and array/owned semantics. |
| tests/integration/test_type_alias_py312.py | New Python 3.12+ tests validating type X[...] = "..." → guppy.type_alias(X) behavior and Copy/Drop bounds mapping. |
| tests/error/test_alias_errors.py | New error-test harness for alias error fixtures and a direct syntax error assertion for invalid type strings. |
| tests/error/alias_errors/undefined_type.py | Error fixture: alias references an undefined type. |
| tests/error/alias_errors/undefined_type.err | Snapshot for undefined-type alias error. |
| tests/error/alias_errors/too_many_args.py | Error fixture: instantiate a 1-param alias with 2 type args. |
| tests/error/alias_errors/too_many_args.err | Snapshot for too-many-type-args error. |
| tests/error/alias_errors/struct_cycle.py | Error fixture: recursive alias via type argument into a struct. |
| tests/error/alias_errors/struct_cycle.err | Snapshot for recursive alias error. |
| tests/error/alias_errors/recursive.py | Error fixture: direct self-referential alias. |
| tests/error/alias_errors/recursive.err | Snapshot for direct recursion error. |
| tests/error/alias_errors/partial_cycle.py | Error fixture: chain that leads into a smaller alias cycle. |
| tests/error/alias_errors/partial_cycle.err | Snapshot for partial-cycle detection + note. |
| tests/error/alias_errors/mutual_recursive.py | Error fixture: mutual recursion between two aliases. |
| tests/error/alias_errors/mutual_recursive.err | Snapshot for mutual-cycle detection + note. |
| tests/error/alias_errors/init.py | Marks the alias error fixtures directory as a package. |
| guppylang/src/guppylang/decorator.py | Implements guppy.type_alias(), py312 type-stmt ingestion, and helper param extraction; improves source lookup for better diagnostics. |
| guppylang-internals/src/guppylang_internals/definition/alias.py | Adds internal alias definition types and recursion detection/diagnostics. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
mark-koch
left a comment
There was a problem hiding this comment.
Thanks, generic aliases make this feature a lot better!
The biggest question imo is whether we actually want to support the 3.12 syntax? I don't see much benefit since we have to wrap it in guppy.type_var anyways. Also, const vars are not supported so users have to fall back to the other syntax anyways...
| check_not_recursive( | ||
| self, TypeParsingCtx(globals, param_var_mapping=dict(param_var_mapping)) | ||
| ) | ||
| ctx = TypeParsingCtx(globals, param_var_mapping=param_var_mapping) |
There was a problem hiding this comment.
Why do we need two different TypeParsingCtxs here and why duplicate the param_var_mapping? The context should have allow_free_vars=False so nothing should be mutated
| import ast as _ast | ||
|
|
||
| from guppylang_internals.ast_util import get_file | ||
| from guppylang_internals.span import Span |
| return | ||
|
|
||
| # Determine the file that the main error is anchored to (may be None if unset) | ||
| err_file: str | None = _span_file(err.span) |
There was a problem hiding this comment.
| err_file: str | None = _span_file(err.span) | |
| err_file = to_span(err.span).file if err.span else None |
| seen_ids: set[DefId] = { | ||
| child.defn_id | ||
| for child in err.children | ||
| if isinstance(child, RecursiveTypeAliasError.AliasNote) | ||
| } |
There was a problem hiding this comment.
The error starts without any children so this will always be empty?
| if defn.id not in seen_ids and defn.defined_at is not None: | ||
| # Skip if the AST node lacks file annotation or is from a different file | ||
| note_file = get_file(defn.defined_at) | ||
| if note_file is None or note_file != err_file: |
There was a problem hiding this comment.
Can note_file be None if defn.defined_at is not None?
| elif bound is None: | ||
| param = TypeParam( | ||
| i, tp.__name__, must_be_copyable=False, must_be_droppable=False | ||
| ) | ||
| elif bound is Copy: | ||
| param = TypeParam( | ||
| i, tp.__name__, must_be_copyable=True, must_be_droppable=False | ||
| ) | ||
| elif bound is Drop: | ||
| param = TypeParam( | ||
| i, tp.__name__, must_be_copyable=False, must_be_droppable=True | ||
| ) |
There was a problem hiding this comment.
A lot of duplication here. Consider something like
if bound is None:
bounds = ()
elif isinstance(bound, tuple):
bounds = bound
else:
bounds = (bound,)and then construct the TypeParam based on that
| Handles ``TypeVar`` with optional ``Copy``/``Drop`` bounds. Raises | ||
| ``TypeError`` for unsupported parameter kinds (``TypeVarTuple``, | ||
| ``ParamSpec``) and for bounds that require ``globals`` to resolve (e.g. | ||
| ``nat``-const params — use ``params=[N]`` for those). |
There was a problem hiding this comment.
Mhmm the fact that this doesn't work really makes me question if we should even support this syntax?
| run_error_test(file, capsys, snapshot) | ||
|
|
||
|
|
||
| def test_type_alias_bad_type_syntax(): |
There was a problem hiding this comment.
Can you add tests for all the different errors that you raise from guppy.type_alias?
| type MyInt = "int" | ||
| MyInt = guppy.type_alias(MyInt) |
There was a problem hiding this comment.
Imo this doesn't really bring many benefits over the other syntax
| # T: (Copy, Drop) matches the default guppy.type_var("T") constraints | ||
| type Boxed[T: (Copy, Drop)] = "Box[T]" |
There was a problem hiding this comment.
Can you add an error test for what happens if the bound doesn't match?
|
Agree that the python 312 syntax doesn't add enough, reverting |
This PR adds first-class Guppy type aliases via `guppy.type_alias(...)`. It introduces: - alias definitions in the type-definition pipeline - alias resolution during type parsing and instantiation Recursive and mutually recursive aliases are rejected. The recursion check follows the same general strategy already used for recursive structs/enums: - temporarily intercept alias instantiation while parsing the alias body - detect recursive re-entry instead of recursing forever - raise a compiler error For aliases, this now produces alias-specific diagnostics with: - a dedicated `RecursiveTypeAliasError` - notes pointing at the alias definitions involved in the cycle - a help hint suggesting how to break it Authored with OpenAI Codex. Closes #1066
- Use DefId-based deduplication instead of name lookup to avoid false positives when multiple aliases share a name - Attach cycle notes in a single pass inside dummy_check_instantiate rather than re-attaching while unwinding the exception stack - Guard against cross-file or un-annotated spans so that notes are only emitted when the span belongs to the same file as the error - Correct the help message: inlining cannot break a cycle; replace the cyclic aliases with a concrete type instead - Remove the note for self-cycles (A -> A) since the error span label already says 'expands to itself' Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Mirrors guppy.type_var("T") where the definition name is always passed
as an explicit first argument. New signature:
Row = guppy.type_alias("Row", "array[int, 4]")
Pair = guppy.type_alias("Pair", "tuple[T, U]", params=[T, U])
Removes _infer_assignment_name() (bytecode inspection) and the dis
import. The linecache fallback in _parse_expr_string() is kept since
it is still used for source-location annotation of type parse errors.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
- Add explicit_params field to RawTypeAliasDef to carry user-supplied
type parameters through the parse stage
- Update ParsedTypeAliasDef.check to respect explicit params: when
provided, type vars in the alias body are resolved against them (in
the declared order); when omitted, free type vars are collected from
the body in order of first appearance (implicit mode)
- Add params= kwarg to type_alias() accepting a list of type variables
created with guppy.type_var() or guppy.nat_var()
- Add _params_from_list() helper to convert GuppyDefinition type vars
to indexed Parameter objects
Examples:
# Implicit (free-var collection)
BoxAlias = guppy.type_alias("Box[T]")
# Explicit with fixed order
Pair = guppy.type_alias("tuple[T, U]", params=[T, U])
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
- partial_cycle: alias chain A1->A2->A3->A2 where A1 is outside the cycle but leads into it; tests that only aliases inside the cycle receive notes (not A1) - too_many_args: instantiating a 1-param alias with 2 type args - undefined_type: alias body references an unknown identifier Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
- Generic aliases: explicit single param (params=[T]), two params with custom order (params=[A, B]), implicit free-var collection - name= kwarg: verifies the alias name can be overridden explicitly - Struct/enum interaction: alias of struct, alias in struct field, generic alias in struct field, alias of enum, alias in enum variant field — ensures alias acyclicity checking does not interfere with struct/enum types Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
The PEP 695 'type' statement provides a more concise syntax:
type Pair[T: (Copy, Drop), U: (Copy, Drop)] = "tuple[T, U]"
Pair = guppy.type_alias(Pair)
This mirrors the pattern already supported for generic classes
(guppy.type_var + type statement bracket list).
Bound mapping for type statement type params:
T (no bound) -> linear (must_be_copyable=False, must_be_droppable=False)
T: Copy -> copyable only
T: Drop -> droppable only
T: (Copy, Drop) -> copyable and droppable (classical, the default)
Handles both Python 3.12/3.13 (__bound__) and Python 3.14+
(__constraints__) representations of tuple-style bounds.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
The previous help message suggested replacing cyclic aliases with 'a concrete type', but Guppy does not support cyclic types at all — structs and enums are also checked for recursive definitions. There is no actionable alternative to suggest, so the help diagnostic is removed entirely. The error title and span label are already self-explanatory. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
- Skip the redundant note on the alias the error span already points to - Change label from "Alias 'X' is part of this cycle" to "'X' defined here" Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…rload error On Python <3.12 exactly one @overload was visible, which mypy rejects. Moving both stubs inside sys.version_info >= (3, 12) means mypy falls back to the implementation signature on older Pythons. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
- Use run_float_fn_approx fixture (not run_int_fn) for float alias test, and rename test to test_float_alias - In _patched_check_instantiate, delete the instance attribute on exit rather than restoring a bound method — ensures method resolution returns cleanly to the class descriptor with no lingering shadow Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
test_float_alias duplicated test_alias_chain with a different scalar type; the original reason (validating a name= kwarg fallback) no longer exists with the explicit-name API. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
The PEP 695 `type X[T] = "..."` path added little: users still had to wrap variables in `guppy.type_var`, and const vars could not be expressed through it at all. Drop it in favour of the explicit `type_alias(name, ty, params=...)` API. This removes the version-guarded overloads, the `TypeAliasType` handling, `_type_alias_from_type_stmt`, `_params_from_type_alias_params`, and the dedicated py312 test module. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
`_params_from_list` now validates that each entry is a type variable (`type_var`, `nat_var`, or `const_var`) and returns the underlying `ParamDef`s rather than eagerly converting them to `Parameter`s. This lets `const_var` params through: their type is an unparsed AST that can only be resolved once globals are available. `ParsedTypeAliasDef` now carries the `ParamDef`s in a `param_defs` field and resolves them to `Parameter`s in `check()`, parsing `RawConstVarDef`s into `ConstVarDef`s along the way. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
The recursion check and the real type parse now share one context instead of constructing two and copying the param mapping between them. The explicit-param mapping is never mutated and the implicit pass re-collects free vars by name, so sharing the context is safe and drops the redundant `dict(...)` copy. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Move the `get_file`/`to_span` imports to module top, derive the error's file with `to_span(err.span).file` instead of a bespoke helper, and drop the dead `err.children` dedup seed (the error has no children yet when notes are added). The same-file / non-None file guard is kept and documented: both `add_sub_diagnostic` and `to_span` require notes to carry a matching file. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
The linecache fallback was added for the since-removed name-inference work and is unrelated to type aliases. Restore the original `inspect.getsourcelines` based implementation; the alias error tests (including the cross-file `partial_cycle` case) still pass. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Cover the `TypeError`s raised by `guppy.type_alias` itself: a non-string name, a non-string type expression, a missing type argument, and invalid `params` entries (a plain value and a non-type-variable definition). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
If `defn.defined_at` is set, `get_file` should always return a filename — a None here would indicate a bug in how the AST was annotated, not a graceful fallback case. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
`type_alias(name: str, ty: str, ...)` statically forbids non-string arguments and a missing `ty`. The three isinstance checks and their corresponding tests just reproduced what mypy and Python's own argument machinery already enforce — remove them and keep only the genuinely runtime errors (bad type syntax, invalid params). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
2556d7b to
0a6f59e
Compare
Summary
Adds
guppy.type_alias()to create named type aliases. Closes #1066.API
Basic alias
The name is always passed explicitly as the first argument — consistent with
guppy.type_var("T").Generic aliases
Free type variables in the body are collected implicitly, in order of first appearance:
Or pass
params=to fix the order explicitly (required when order matters):Python 3.12+
typestatementOn Python 3.12+ the PEP 695
typestatement is also supported. The alias value must be a quoted string; type parameters are read from the[...]list: