feat: Protocol checking by croyzor · Pull Request #1765 · Quantinuum/guppylang

croyzor · 2026-05-22T15:45:37Z

Closes #1396, closes #1210, closes #1653

github-actions · 2026-05-22T15:47:00Z

Bencher Report

Branch	cr/proto-check
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.00%) Baseline: 27.53 x 1e3	27.81 x 1e3 (99.01%)	📈 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.00%) Baseline: 14.84 x 1e3	14.98 x 1e3 (99.01%)	📈 view plot 🚷 view threshold	435.00 (0.00%) Baseline: 435.00	439.35 (99.01%)

🐰 View full continuous benchmarking report in Bencher

codecov-commenter · 2026-05-22T15:50:08Z

Codecov Report

❌ Patch coverage is 85.53719% with 35 lines in your changes missing coverage. Please review.
✅ Project coverage is 93.30%. Comparing base (2dc589a) to head (2083883).
⚠️ Report is 2 commits behind head on main.

Files with missing lines	Patch %	Lines
...rc/guppylang_internals/checker/protocol_checker.py	83.14%	15 Missing ⚠️
...ls/src/guppylang_internals/checker/func_checker.py	42.85%	8 Missing ⚠️
...pylang-internals/src/guppylang_internals/tys/ty.py	46.66%	8 Missing ⚠️
...ls/src/guppylang_internals/checker/expr_checker.py	91.30%	2 Missing ⚠️
...rnals/src/guppylang_internals/definition/struct.py	92.30%	2 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #1765      +/-   ##
==========================================
+ Coverage   92.98%   93.30%   +0.32%     
==========================================
  Files         135      136       +1     
  Lines       13053    13278     +225     
==========================================
+ Hits        12137    12389     +252     
+ Misses        916      889      -27

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

codspeed-hq · 2026-05-22T16:04:19Z

Merging this PR will not alter performance

✅ 9 untouched benchmarks

_{Comparing cr/proto-check (2083883) with main (2dc589a)}

mark-koch

🔥 🔥 🔥

Sorry I didn't have the time to look through everything in detail yet, but here are some comments from a first read through. Will have a closer look next week

mark-koch · 2026-05-28T12:12:47Z

            if isinstance(defn, ParsedProtocolDef):
                assert isinstance(func_ty, FunctionType)
-                raise GuppyError(
-                    UnsupportedError(
-                        node.func, "Checking protocol method calls", singular=True
-                    )
-                )
+                raise RequiresMonomorphizationError


Why would defn ever be a ParsedProtocolDef?

Ah I see because attribute accesses are turned into Global names with the protocol id.

I think it would be better to add a new node type that holds a protocol id and the name of the method that is used. Then we wouldn't need to cop out here

mark-koch · 2026-05-28T12:21:22Z

+                            member_ty,
+                            with_loc(
+                                node,
+                                GlobalName(id=member_name, def_id=proto.def_id),


Returning a global name with the id of the full protocol feels a bit weird. Don't we want some sort of identifier for wich method of the protocol is used?

mark-koch · 2026-05-28T12:23:58Z

-        "Self", copyable=self_ty_head.copyable, droppable=self_ty_head.droppable
+        "Self", copyable=True, droppable=True


Why this change?

mark-koch · 2026-05-28T12:24:43Z

        )


+def handle_implicit_proto_self_arg(


Could you add a docstring explaining what this function does

mark-koch · 2026-05-28T12:25:38Z

+
+
+@dataclass(frozen=True)
+class ImplProofBase(ABC):


Also docstrings for the things in this file

mark-koch · 2026-05-28T12:40:33Z

+6 |     def foo(self) -> None: ...
+  |             ^^^^ Inference of type for `self` is not supported in protocol
+  |                  methods


Imo better to use the regular MissingArgAnnotationError error (maybe with a note that self inference on protcols is not supported)

mark-koch · 2026-05-28T12:41:53Z

+5 | class Foo:
+6 |     @guppy.require
+7 |     def foo(n: nat) -> None: ...
+  |                ^^^ `n` must be of type `self`


Self (captitalised) or Foo?

mark-koch · 2026-05-28T12:43:20Z

+23 | @guppy
+24 | def baz(f: FooInt) -> None:
+25 |     return eat_foostr(f)
+   |                       ^ Type `FooInt` does not implement protocol `Foo`


Shouldnt it be "does not implement protocol Foo[str]"?

mark-koch · 2026-05-28T12:45:22Z

+5 | class Proto[T]:
+6 |     @guppy.require
+7 |     def foo[T](t: T) -> T: ...
+  |                ^^^^ Type `T` does not implement protocol `Proto`


A nice future improvement would be to add notes explaining why the protocol is not implemented

True, created #1779

mark-koch · 2026-05-28T12:46:26Z

+## TODO: See if this can work in light of the monomorphisation changes assuming
+## check and compile are called on entrypoints


Are these TODOs resolved?

croyzor force-pushed the cr/proto-check branch from 7195271 to addfbf1 Compare May 22, 2026 15:56

Protocol checking

934e72c

croyzor force-pushed the cr/proto-check branch from addfbf1 to 934e72c Compare May 22, 2026 16:05

croyzor added 4 commits May 26, 2026 16:32

Add missing unification logic for type var's implements

a91ad16

Update substitute logic for existential

ed63c32

Delete ProtocolCall, Monomorphization should resolve everything

7a2895c

Delete comments

a6d6687

croyzor changed the title ~~feat: Protocol checking~~ feat!: Protocol checking May 26, 2026

croyzor mentioned this pull request May 26, 2026

refactor!: Delete redundant error message #1767

Merged

Separate out py312 protocol tests

d880fd6

croyzor requested a review from mark-koch May 26, 2026 15:57

croyzor added 2 commits May 26, 2026 17:14

Delete old comments

9193bd8

revert redundant stuff

1f743ae

croyzor marked this pull request as ready for review May 26, 2026 16:33

croyzor requested a review from a team as a code owner May 26, 2026 16:33

croyzor added 4 commits May 27, 2026 11:38

Delete some nonsense

c1994ec

Merge remote-tracking branch 'origin/main' into cr/proto-check

90d2f85

Delete duplicate function

b76c67f

Catch protocol methods with @guppy annotation

7a85906

croyzor changed the title ~~feat!: Protocol checking~~ feat: Protocol checking May 27, 2026

croyzor added 2 commits May 27, 2026 15:12

Add test

d5b8219

Move test to py312 zone

2083883

mark-koch reviewed May 28, 2026

View reviewed changes

		"Self", copyable=self_ty_head.copyable, droppable=self_ty_head.droppable
		"Self", copyable=True, droppable=True

		## TODO: See if this can work in light of the monomorphisation changes assuming
		## check and compile are called on entrypoints

Conversation

croyzor commented May 22, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

github-actions Bot commented May 22, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

codecov-commenter commented May 22, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

codspeed-hq Bot commented May 22, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Merging this PR will not alter performance

Uh oh!

mark-koch left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mark-koch May 28, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

croyzor commented May 22, 2026 •

edited

Loading

github-actions Bot commented May 22, 2026 •

edited

Loading

codecov-commenter commented May 22, 2026 •

edited

Loading

codspeed-hq Bot commented May 22, 2026 •

edited

Loading

mark-koch May 28, 2026 •

edited

Loading