feat: Declare WASM modules in guppy #942

croyzor · 2025-05-07T10:38:04Z

TODO:

Add a parsing step to the WasmModuleDef class to check for parameters like

@wasm_module(blah)
class Foo(Generic[T]):
    ...

(this is required for generic size arrays to work)

A method for getting n contexts

codecov-commenter · 2025-05-09T15:14:21Z

Codecov Report

Attention: Patch coverage is 77.35192% with 65 lines in your changes missing coverage. Please review.

Project coverage is 92.43%. Comparing base (ba2eaab) to head (9b3bf0b).

Files with missing lines	Patch %	Lines
guppylang/std/qsystem/wasm.py	0.00%	37 Missing ⚠️
guppylang/definition/custom.py	84.78%	21 Missing ⚠️
guppylang/decorator.py	93.75%	2 Missing ⚠️
guppylang/tys/constarg.py	83.33%	2 Missing ⚠️
guppylang/tys/ty.py	90.00%	2 Missing ⚠️
guppylang/tys/printing.py	66.66%	1 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #942      +/-   ##
==========================================
- Coverage   92.85%   92.43%   -0.43%     
==========================================
  Files          85       88       +3     
  Lines        9901    10180     +279     
==========================================
+ Hits         9194     9410     +216     
- Misses        707      770      +63

☔ 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.

mark-koch

Thanks Craig! I left a bunch of nits and suggestions while looking through the code, but my main points are:

I'd prefer to use an OpaqueType instead of adding WasmModuleType to the core type system
Instead of CustomFunctionDefs , I think it would be cleaner to add a WasmFunctionDef that takes care of checking the signature instead of doing it at the call-site. You could follow the pattern in guppylang.definition.function for how to set it up and mostly copy the code you already have from the CustomCallCheckers and CustomCallCompilers.

Also, it would be good to have some tests for the new error messages you added

mark-koch · 2025-05-29T10:46:34Z

guppylang/checker/errors/wasm.py

+from guppylang.tys.ty import Type
+
+
+class WasmError(Error):


Suggested change

class WasmError(Error):

@dataclass(frozen=True)

class WasmError(Error):

mark-koch · 2025-05-29T10:47:39Z

guppylang/checker/errors/type_errors.py

+
+
+@dataclass(frozen=True)
+class WasmTypeConversionError(Error):


Should this be in errors/wasm.py?

mark-koch · 2025-05-29T10:49:59Z

guppylang/decorator.py

+    def wasm_module(
+        self, filename: str, filehash: int
+    ) -> Decorator[PyClass, GuppyDefinition]:


Note that your decorator doesn't work with explicit modules.

I think that's fine as long as we merge #983 first

mark-koch · 2025-05-29T10:55:19Z

guppylang/definition/custom.py

+                case FuncInput(ty=ty, flags=InputFlags.Inout) if isinstance(
+                    ty, WasmModuleType
+                ):


Suggested change

case FuncInput(ty=ty, flags=InputFlags.Inout) if isinstance(

ty, WasmModuleType

):

case FuncInput(ty=WasmModuleType(), flags=InputFlags.Inout):

mark-koch · 2025-05-29T10:56:09Z

guppylang/definition/custom.py

+                case FuncInput(ty=ty):
+                    raise GuppyError(FirstArgNotModule(self.node, ty))
+            for inp in self.func.ty.inputs[1:]:
+                assert isinstance(inp, FuncInput)


This assert seems unnecessary?

mark-koch · 2025-05-29T11:41:09Z

guppylang/tys/constarg.py

+class ConstStringArg(ConstArg):
+    def to_hugr(self) -> ht.TypeArg:
+        match self.const:
+            case ConstValue(value=v, ty=ty) if is_string_type(ty):
+                assert isinstance(v, str)
+                return ht.StringArg(v)
+            case _:
+                return super().to_hugr()


Why a new subclass instead of adding this logic to ConstArg.to_hugr?

Ah yes I meant to highlight this - adding the logic to ConstArg.to_hugr makes sense, but introduces a cyclic dependency guppylang.tys.arg.py <-> guppylang.tys.builtin.py

mark-koch · 2025-05-29T11:41:51Z

guppylang/tys/printing.py

@@ -150,6 +151,10 @@ def _visit_ConstArg(self, arg: ConstArg, inside_row: bool) -> str:
    def _visit_ConstValue(self, c: ConstValue, inside_row: bool) -> str:
        return str(c.value)

+    @_visit.register
+    def _visit_WasmModuleType(self, w: WasmModuleType, inside_row: bool) -> str:
+        return f"WasmModuleType(defn={w.defn!s})"


This is the type that is printed to users. It should be the name that the user gave to the class defining the module

mark-koch · 2025-05-29T11:43:00Z

guppylang/tys/ty.py

+        ty = wasm().get_type("context")
+        return ty.instantiate([])
+
+    # TODO: I don't know what to write here


What you wrote is valid since WasmModuleType contains no nested types 👍

mark-koch · 2025-05-29T11:45:33Z

guppylang/tys/ty.py

@@ -663,6 +664,30 @@ def transform(self, transformer: Transformer) -> "Type":
        )


+@dataclass(frozen=True)
+class WasmModuleType(TypeBase):


I really don't like the idea of adding wasm to the core type system.

Especially since you're just wrapping a definition and no other logic, why not use an OpaqueType? Everywhere you check for a WasmModuleType, you could just check that it's opaque and the definition matches?

mark-koch · 2025-05-29T11:49:32Z

guppylang/module.py

@@ -82,6 +82,9 @@ class GuppyModule:
    # Storage for source code that has been read by the compiler
    _sources: SourceMap

+    # Live WASM contexts
+    _next_wasm_context: int


You could also do

_wasm_ids = itertools.count()

Then, you can just do next(self._wasm_ids) and the counter goes up automatically

Or even better, move the counter to WasmModuleDef instead of sticking it to the module

qartik mentioned this pull request May 7, 2025

feat: add wasm module #920

Closed

qartik linked an issue May 9, 2025 that may be closed by this pull request

Add support for wasm #755

Open

qartik and others added 16 commits May 20, 2025 15:59

feat: add wasm module (WIP)

426a5e6

Bits and pieces (WIP)

2da0811

Maintain uniqueness of WASM contexts for a module

219e293

Add discard; make progress

7361fe2

Unification of WasmModuleTypes

d5c570f

Redundant comment

af7f7d8

Oups - add missing file

95b1cb2

Associate WASM functions with modules

cc8aef1

Let more types be wasmable

caf37f4

Add dummy WASM decorators for sphinx

c8a7b71

Add ConstStringArg

c6d66d8

checkpoint

fe23a6c

yeee

55805ee

Remove array from test file

8de275a

cleanup

c0f70b9

Merge remote-tracking branch 'origin/main' into 755-add-support-for-wasm

07e1f62

croyzor force-pushed the cr/wasm-wip branch from d5855ee to 07e1f62 Compare May 28, 2025 08:22

cleanup

9b3bf0b

croyzor changed the title ~~🚧 Wasm WIP 🚧~~ feat: Declare WASM modules in guppy May 28, 2025

croyzor requested a review from mark-koch May 28, 2025 13:15

mark-koch reviewed May 29, 2025

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

feat: Declare WASM modules in guppy #942

feat: Declare WASM modules in guppy #942

Uh oh!

croyzor commented May 7, 2025 •

edited

Loading

Uh oh!

codecov-commenter commented May 9, 2025 •

edited

Loading

Uh oh!

mark-koch left a comment

Uh oh!

mark-koch May 29, 2025

Uh oh!

mark-koch May 29, 2025

Uh oh!

mark-koch May 29, 2025

Uh oh!

mark-koch May 29, 2025

Uh oh!

mark-koch May 29, 2025

Uh oh!

mark-koch May 29, 2025

Uh oh!

croyzor May 29, 2025

Uh oh!

mark-koch May 29, 2025

Uh oh!

mark-koch May 29, 2025

Uh oh!

mark-koch May 29, 2025

Uh oh!

mark-koch May 29, 2025

Uh oh!

mark-koch May 29, 2025

Uh oh!

Uh oh!

	class WasmError(Error):
	@dataclass(frozen=True)
	class WasmError(Error):



		@dataclass(frozen=True)
		class WasmTypeConversionError(Error):

feat: Declare WASM modules in guppy #942

Are you sure you want to change the base?

feat: Declare WASM modules in guppy #942

Uh oh!

Conversation

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

Uh oh!

Uh oh!

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

Uh oh!

Codecov Report

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!

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!

Uh oh!

croyzor commented May 7, 2025 •

edited

Loading

codecov-commenter commented May 9, 2025 •

edited

Loading