Skip to content

fix(enums): preserve trait constants and validate method modifiers - #1674

Merged
nahime0 merged 57 commits into
mainfrom
fix/1317-enum-declaration-metadata
Oct 20, 2026
Merged

nahime0 merged 57 commits into
mainfrom
fix/1317-enum-declaration-metadata

Conversation

@nahime0

@nahime0 nahime0 commented Sep 30, 2026 •

Copy link
Copy Markdown
Member

Closes #1317.
Closes #1438.
Closes #1439.

Summary

Preserve flattened trait constants, reflection declaration order and final method flags in enum metadata. Reject abstract source enum methods and enforce concrete implementations of trait and interface requirements.

The review corrections lower declaration magic constants in their lexical scope and bind trait __CLASS__ values to each consumer, preserve abstract contracts through nested as adaptations and insteadof, allow final enum self returns for static contracts, and accept legal optional/extra-optional trait parameters. Signature validation excludes compiler-generated argument collectors. Explicit enum interface checks include transitive, static, visibility, parameter, return and by-reference contracts.

The three reproduced macOS CI timeouts are repaired by independently scheduling the unchanged post-eval string/scalar fixtures for each target and applying the existing cold-PDO budget to the exact combined PDO/mysqli test. No test assertions or global timeout budgets were removed.

Validation

  • Reproduced the P0 and P1 findings before the fixes.
  • 95 focused executable enum/magic-constant tests and 77 enum/related diagnostic tests pass.
  • Five enum EIR/assembly cases and ten unchanged post-eval ownership cases pass under the CI profile, independently scheduled across every supported target.
  • The exact PDO/mysqli CI-profile test passes without retries.
  • Warning-free cargo build, PHP example syntax and git diff --check pass.
  • The transitive-interface heap check is bounded against a bare enum case control. Native enum singleton shutdown retention remains a separately tracked question, not a claimed cleanup fix.
  • Rebased onto main 6db3e9a14e2a50883fb36e880650533151e881f0. Full exact-head CI remains the readiness gate.

@github-actions github-actions Bot added area:types Touches type checking, inference, or compatibility. size:s Small pull request. type:fix Corrects broken or incompatible behavior. labels Sep 30, 2026
@nahime0
nahime0 marked this pull request as ready for review September 30, 2026 16:00
@greptile-apps

greptile-apps Bot commented Sep 30, 2026 •

Copy link
Copy Markdown
Contributor

RetriggerView in GreptileConfidence Score: 5/5

[High impact] The reviewed changes appear safe to merge; no new blocking issue was confirmed.

Summary

The PR preserves enum trait constants, declaration order, and final method flags. It also checks enum method contracts and binds interface calls against the selected enum method.

Changes since the previous review:

  • Enums keep trait constants and declaration details in their metadata.
  • Enums must meet the contracts of their traits and interfaces.
  • Interface calls check arguments against the enum method they select.
  • Shared compiler paths keep temporary values alive through their use.

No new actionable findings were confirmed. The supplied previous threads are unnumbered, so they have no previousFindings entries. Their original fixes remain present.

nahime0 explicitly identifies native enum singleton shutdown retention as a separately tracked issue, not a cleanup fix claimed by this PR.

Review checks were static; no tests were executed during this review.

Diagram

%%{init: {'theme': 'neutral'}}%%
flowchart TD
  A[Enum interface call] --> B[Select concrete enum]
  B --> C[Bind and check arguments]
  C --> D{Reference assignment?}
  D -- No --> E[Return copied value]
  D -- Yes --> F{Method returns a reference?}
  F -- Yes --> G[Adopt returned cell]
  F -- No --> H[Create independent cell]
  G --> I[Publish replacement alias]
  H --> I
  I --> J[Release previous binding]
Loading

Reviews (16) · Last reviewed commit: "fix(enums): isolate selected reference r..." · Reviewed by Greptile

Comment thread src/types/checker/driver/mod.rs
@nahime0
nahime0 marked this pull request as draft October 1, 2026 08:05
@nahime0
nahime0 marked this pull request as ready for review October 1, 2026 09:18
@nahime0
nahime0 force-pushed the fix/1317-enum-declaration-metadata branch from e4ac2db to 0678ebb Compare October 2, 2026 13:48
Comment thread src/types/checker/driver/mod.rs
Comment thread src/types/checker/schema/enums.rs Outdated
Comment thread src/types/checker/schema/enums.rs
@nahime0

nahime0 commented Oct 5, 2026

Copy link
Copy Markdown
Member Author

Confidence Score: 2/5

High risk Enum metadata now keeps trait constants and method modifiers, but imported constant and method semantics still diverge from PHP.

Imported trait constants that use self:: and local methods that fail to match an imported abstract signature need to be fixed before merging.

Summary

This change feeds flattened trait constants into enum metadata, appends them after the enum's own cases and constants in reflection order, rejects constants that collide with case names, rejects methods that remain abstract, and records final on instance and static methods. An earlier review finding that a trait constant colliding with a case went undetected is fixed on this head: build_enum_info compares flattened constant names to case names, and the collision and case-sensitive tests cover it. Focused codegen and error tests for the happy path pass. Trait constants are still stored without the lexical self:: rewrite used for classes, and a local method with the same name as an imported abstract method is accepted without a signature check.

  • Flattened trait constants are appended after the enum's own declarations.
  • Case-name collisions and remaining abstract methods are rejected.
  • Final instance and static method flags are recorded in reflection metadata.
  • Earlier case-collision finding is fixed.
Diagram
flowchart LR
    decls["Enum and trait declarations"] --> flatten["Trait flattening"]
    flatten --> meta["Enum metadata"]
    meta --> consts["Constant values and reflection order"]
    meta --> mods["Abstract rejection and final flags"]
    consts --> selfgap["self:: still unresolved"]
    mods --> siggap["Incompatible body still accepted"]
Loading

Findings

  • [P1] src/types/checker/driver/mod.rs:353 — Imported trait constants keep unresolved self::
  • [P1] src/types/checker/schema/enums.rs:156 — Incompatible trait method bodies still compile
  • [P2] src/types/checker/schema/enums.rs:159 — Non-abstract enum methods may omit a body

Last reviewed commit: 0678ebb

@nahime0
nahime0 marked this pull request as draft October 5, 2026 15:39
@nahime0
nahime0 force-pushed the fix/1317-enum-declaration-metadata branch from 0678ebb to ae75c11 Compare October 5, 2026 16:12
@github-actions github-actions Bot added size:m Medium-sized pull request. and removed size:s Small pull request. labels Oct 5, 2026
@nahime0
nahime0 force-pushed the fix/1317-enum-declaration-metadata branch from ae75c11 to b48975c Compare October 5, 2026 16:29
Comment thread src/types/checker/schema/enums.rs
Comment thread src/types/checker/schema/enum_traits.rs Outdated
Comment thread src/types/checker/schema/enum_traits.rs Outdated
Comment thread src/types/checker/schema/enum_traits.rs Outdated
Comment thread src/types/checker/schema/enums.rs Outdated
@nahime0

nahime0 commented Oct 6, 2026

Copy link
Copy Markdown
Member Author

Confidence Score: 1/5

[High risk] Enum metadata now keeps trait constants and checks abstract trait methods.

The __CLASS__ panic and the trait-method contract mismatches need to be fixed before merging.

Summary

Flattened trait constants are copied into enum metadata, self:: receivers are rewritten, and those constants are listed after the enum's own cases and constants. Remaining abstract methods and methods without bodies are rejected, final is recorded on instance and static methods, and a later pass compares raw abstract trait methods with the enum body. Direct case-name collisions, self:: / self::class, same-name incompatible implementations, and missing method bodies from the earlier review are fixed on this head. The new contract check still ignores as adaptations, rejects self for a static return and legal optional-parameter changes, and an unimplemented interface method still compiles.

  • Flattened trait constants stay readable and follow the enum's own declarations in reflection.
  • Case-name collisions, abstract methods, and missing bodies are rejected.
  • Final instance and static method flags are stored for reflection.
  • Earlier case-collision, unresolved self::, direct incompatible body, and missing-body findings are fixed.
Diagram
flowchart LR
    decls["Enum and trait declarations"] --> flatten["Trait flattening"]
    flatten --> meta["Enum metadata"]
    meta --> consts["Constant values and reflection order"]
    meta --> checks["Abstract trait requirement check"]
    consts --> magic["__CLASS__ still a MagicConstant"]
    checks --> adapt["Raw trait method, not the adaptation"]
Loading

Findings

  • [P0] src/types/checker/schema/enums.rs:511 — Reading a trait __CLASS__ constant panics
  • [P1] src/types/checker/schema/enum_traits.rs:48 — Trait as adaptations are not part of the contract
  • [P1] src/types/checker/schema/enum_traits.rs:84 — static abstract returns reject self
  • [P1] src/types/checker/schema/enum_traits.rs:82 — Legal parameter-list changes are rejected
  • [P1] src/types/checker/schema/enums.rs:162 — Implemented interfaces are not checked

Earlier findings on this head:

  • [P1] Enum case collision goes undetected — fixed: build_enum_info rejects a flattened constant whose name matches a case, and const a / case A still compiles, matching PHP 8.5.
  • [P1] Imported trait constants keep unresolved self:: — fixed: resolve_lexical_class_value rewrites self::A and self::class, and the lexical regression prints 3|LexicalEnum|4.
  • [P1] Incompatible trait method bodies still compile — fixed for a same-name body: f(string) against abstract f(int) is now "Cannot narrow parameter".
  • [P2] Non-abstract enum methods may omit a body — fixed: validate_method_shape reports "Non-abstract method must have a body".

Last reviewed commit: b48975c

@nahime0
nahime0 force-pushed the fix/1317-enum-declaration-metadata branch from b48975c to db4f92a Compare October 6, 2026 11:34
@github-actions github-actions Bot added area:eir Touches EIR definitions, lowering, validation, or passes. area:tooling-ci Touches CI, development tooling, Docker, or repository scripts. labels Oct 6, 2026
@nahime0
nahime0 force-pushed the fix/1317-enum-declaration-metadata branch from db4f92a to 559c245 Compare October 6, 2026 12:30

nahime0 commented Oct 6, 2026

Copy link
Copy Markdown
Member Author

Rebased the reviewed fixes onto main 9e7cad3 and verified publication at 559c245. The new main already carries the first-post-eval string checks and cold PDO timeout, so those were kept without duplication. The remaining scalar ownership checks retain one test per supported target.

Focused post-rebase validation: all five enum trait/interface target checks, all five post-eval scalar target checks, and the three native enum-review regressions pass; cargo build and diff hygiene pass. Earlier validation of the substantive fix passed 95 native tests, 77 diagnostics, and the broader target controls, as reported above. Full CI has restarted on the exact new head, and this remains draft until it completes successfully.

@github-actions github-actions Bot removed the area:tooling-ci Touches CI, development tooling, Docker, or repository scripts. label Oct 6, 2026
Comment thread src/types/checker/driver/mod.rs Outdated
Comment thread src/types/checker/schema/classes/enum_interfaces.rs
@nahime0

nahime0 commented Oct 7, 2026

Copy link
Copy Markdown
Member Author

Confidence Score: 2/5

[High risk] Enum metadata now keeps trait constants and checks trait and interface method contracts.

The unordered transitive-interface expansion and the interface parameter-shape check need to be fixed before merging.

Summary

Flattened trait constants are copied into enum metadata, lexical self:: receivers and trait __CLASS__ placeholders are bound to the consuming enum, and reflection lists those constants after the enum's own cases and constants. Abstract trait requirements are rebuilt through nested as / insteadof adaptations and compared with source-visible parameter shapes, while completed enum metadata now goes through the class interface checker. The nine earlier findings are fixed on this head. Two new gaps remain: parent interfaces are published one enum at a time in HashMap order, and the interface checker still requires an identical parameter list.

  • Trait constants, case collisions, final flags, and magic-constant binding are covered by codegen and diagnostic tests.
  • Adapted trait signatures accept optional widening, self for static, and protected aliases.
  • Interface presence, visibility, return types, and by-reference returns are checked after enum schemas exist.
  • Post-eval ownership fixtures are split per target without dropping assertions.
Diagram
flowchart LR
    decls["Enum, trait, and interface declarations"] --> flatten["Trait flattening"]
    flatten --> meta["Enum metadata with direct interfaces"]
    meta --> walk["HashMap contract walk"]
    walk --> expand["Expand one enum's interfaces"]
    expand --> check["Return and signature checks"]
Loading

Findings

  • [P1] src/types/checker/driver/mod.rs:384 — Transitive enum interfaces are applied in hash order
  • [P1] src/types/checker/schema/classes/enum_interfaces.rs:42 — Interface implementations must copy the parameter list

Earlier findings

  • [P1] Enum case collision goes undetected — fixed: const A on a trait used beside case A is "Enum constant E::A conflicts with enum case", and const a / case A prints 1A.
  • [P1] Imported trait constants keep unresolved self:: — fixed: self::A, self::class, and a direct enum self:: constant print 3|E|4.
  • [P1] Incompatible trait method bodies still compile — fixed: f(string) against abstract f(int) is "Cannot narrow parameter".
  • [P2] Non-abstract enum methods may omit a body — fixed: public function f(); is "Non-abstract method must have a body: E::f".
  • [P0] Reading a trait __CLASS__ constant panics — fixed: imported __CLASS__ and __TRAIT__ print E|T.
  • [P1] Trait as adaptations are not part of the contract — fixed: f as g with g(string) is "Cannot narrow parameter $x when implementing trait method: E::g", and f as protected with a protected body prints 1.
  • [P1] static abstract returns reject self — fixed: f(): static implemented as f(): self prints A.
  • [P1] Legal parameter-list changes are rejected — fixed for traits: f(int $x = 1, int $y = 2) against abstract f(int $x) prints 3.
  • [P1] Implemented interfaces are not checked — fixed: an enum that omits I::f is "Class E must implement interface method I::f".

Last reviewed commit: 559c245

@nahime0

nahime0 commented Oct 18, 2026

Copy link
Copy Markdown
Member Author

@greptile review

@nahime0

nahime0 commented Oct 18, 2026

Copy link
Copy Markdown
Member Author

@greptile review

@nahime0

nahime0 commented Oct 19, 2026

Copy link
Copy Markdown
Member Author

@greptile review

@nahime0

nahime0 commented Oct 19, 2026

Copy link
Copy Markdown
Member Author

@greptile-apps please review the current signed head 0a9f62d. It integrates current main and fixes the independent Grok P1: a heap-result owner is now explicitly zero-initialized before publication, including store-free rejected bindings. The five-target EIR regressions fail before the fix and pass after it; focused native controls pass with IR optimization on and off. The follow-up Grok review is clean on this exact commit.

Comment thread src/ir_lower/expr/enum_method_args.rs
@nahime0

nahime0 commented Oct 19, 2026

Copy link
Copy Markdown
Member Author

@greptile-apps Please review the current head 6c82f40, including the trailing named-omission fix. The new regressions pass in both IR modes, all five target fixtures pass, and independent Grok review is clean.

Comment thread src/ir_lower/expr/enum_method_args.rs Outdated
@nahime0
nahime0 merged commit a8f730c into main Oct 20, 2026
372 of 374 checks passed
@nahime0
nahime0 deleted the fix/1317-enum-declaration-metadata branch October 20, 2026 20:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area:codegen Touches target-aware assembly or backend lowering. area:eir Touches EIR definitions, lowering, validation, or passes. area:types Touches type checking, inference, or compatibility. scope:multi-area Touches more compiler areas than the automatic area-label cap. size:l Large pull request. type:fix Corrects broken or incompatible behavior.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Keep enum final method metadata in ClassInfo Reject abstract methods on AOT enums An enum cannot read a constant it imports from a trait

1 participant