Skip to content

feat: Custom modifiers as struct and enum methods - #2277

Open
nicolaassolini-qntm wants to merge 264 commits into
mainfrom
na/custom-in-struct
Open

nicolaassolini-qntm wants to merge 264 commits into
mainfrom
na/custom-in-struct

Conversation

@nicolaassolini-qntm

@nicolaassolini-qntm nicolaassolini-qntm commented Sep 4, 2026

Copy link
Copy Markdown
Contributor

follows #2274
follows #2310
close #2262
close #2276

As well as introducing the use of @guppy.unitary within structs and enums, I have updated the span in some error messages to ensure consistency

nicolaassolini-qntm and others added 30 commits June 15, 2026 17:44
@nicolaassolini-qntm nicolaassolini-qntm added the B-backport-nominated A PR (or an issue pending a PR) that is nominated for backporting to the previous minor series. label Sep 9, 2026
@nicolaassolini-qntm
nicolaassolini-qntm removed this pull request from stack #2226 September 9, 2026 14:17
@nicolaassolini-qntm
nicolaassolini-qntm changed the base branch from main to na/forbid-assigment-in-custom September 9, 2026 14:20
Base automatically changed from na/forbid-assigment-in-custom to main September 9, 2026 14:46

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔵 Needs a closer look

Retaining the inner class frame breaks explicit parent-type annotations and enclosing-scope name resolution.

Review details

Suppressed comments (2)

guppylang-internals/src/guppylang_internals/engine.py:212

  • This keeps the frame of the inner unitary class, but Globals only reads that frame's locals and globals. The containing struct/enum name is not bound there (or in the outer class frame), so the supported form from #2262, such as def __call__(self: "S"), cannot resolve S; locally defined helpers and aliases are hidden for the same reason. Since unitary-class statements are rejected and its type parameters are already copied onto each raw function, advance these methods out of both class scopes and add an explicit-self regression test.
        is_unitary_call = (
            isinstance(member, RawFunctionDef) and member.unitary_class_at is not None
        )
        self._register_type_member_parent(
            ty_id, member_id, adjust_frame=not is_unitary_call
        )

guppylang-internals/src/guppylang_internals/definition/struct.py:62

  • The updated help text is missing punctuation/conjunction between the field and method forms, producing name: Type` `@guppy`, annotated methods in every affected diagnostic. Reword it to “fields of the form name: Type, @guppy annotated methods, or @guppy.unitary classes” and update the snapshots.
    message: ClassVar[str] = (
        "Struct can contain only fields of the form `name: Type` "
        "`@guppy`, annotated methods or `@guppy.unitary` classes"
  • Files reviewed: 31/31 changed files
  • Comments generated: 0 new
  • Review effort level: Balanced

@qartik qartik left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Some feedback from Gemini 3.8 Flash:

Medium Severity

Incorrect scope frame for unitary methods in structs and enums

  • File: guppylang-internals/src/guppylang_internals/engine.py:211-226
  • Issue: Passing adjust_frame=False retains the inner @guppy.unitary class frame for __call__ and custom modifier implementations (daggered, controlled, etc.). When a struct/enum is defined inside a local scope (e.g. factory functions or test functions), Globals cannot resolve outer symbols such as explicit type annotations (e.g. self: "S" from #2262). Furthermore, PR #2310 already rejects non-method statements in @guppy.unitary classes, so the rationale in the comment is obsolete.
  • Fix: Advance the frame past both the unitary class and the struct/enum class scopes so it reaches the enclosing definition scope.

Low Severity / Test Coverage

Missing test coverage for self in enum unitary methods

  • Files: tests/integration/test_enum.py, tests/integration/test_modifier.py
  • Issue: In the added enum tests, self is ignored in every custom modifier implementation (pass or x(q)). None test accessing or pattern matching on self (e.g. match self: case CustomGates.Enabled(): ...).
  • Fix: Add an integration test where an enum unitary method matches on self.

Comment thread guppylang-internals/src/guppylang_internals/definition/struct.py Outdated
Comment thread guppylang-internals/src/guppylang_internals/definition/struct.py Outdated
Comment thread guppylang-internals/src/guppylang_internals/engine.py Outdated
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

B-backport-nominated A PR (or an issue pending a PR) that is nominated for backporting to the previous minor series.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Feature]: Use custom modifier as enum methods [Feature]: Use custom modifier as structure methods

7 participants