Skip to content

fix(enum): fold enum case property fetches in constant expressions - #171

Open
yuan-dian wants to merge 2 commits into
swoole:masterfrom
yuan-dian:fix/enum-value-property-default
Open

yuan-dian wants to merge 2 commits into
swoole:masterfrom
yuan-dian:fix/enum-value-property-default

Conversation

@yuan-dian

Copy link
Copy Markdown
Contributor

Summary

Enum::Case->value / ->name are valid constant expressions since PHP 8.2
(php/php-src#8625) and native PHP
accepts them as property defaults. TypePHP's declaration validator already
allowed them, but the stub generator's constant-expression evaluator rejected
Expr_PropertyFetch, aborting compilation. This PR makes the stub-side
evaluator fold enum case ->value/->name, bringing the two evaluators back
in sync.

Repro (before)

enum TextRole: int
{
    case Unset = -1;
    case User = 1;
}

class Session
{
    public int $role = TextRole::Unset->value;   // line 11
}

function main(): void {
    var_dump((new Session())->role);
}

Before:

Fatal error: Uncaught RuntimeException: In program.php: Expression at line 11
must be a global, non-magic constant, Expr_PropertyFetch given
  thrown in src/gen_stub.php on line 204     (exit 255)

The class-constant indirection (private const R = TextRole::Unset->value;
then = self::R) fails identically, since the initializer itself is a
PropertyFetch.

After: compiles and the AOT binary prints -1, matching native PHP 8.4.

Root cause

Two constant-expression evaluators were out of sync:

Path Handles Enum::Case->value?
Declaration validator (src/Transform/ConstantExpressionValidator.php, Expr\PropertyFetch whitelisted) ✅
Declaration evaluator (src/Resolver/ClassConstantValueTrait.php, resolveCompileTimeEnumCaseBackingValue) ✅
Stub/arginfo generator (src/gen_stub.php, EvaluatedValue::createFromExpression fallback) ❌ threw for anything other than ConstFetch/ClassConstFetch

There was zero test coverage of int $x = Enum::Case->value as a property
default anywhere in the repo, which is why the gap survived.

Changes

  • src/gen_stub.php (+25) — in the ConstExprEvaluator fallback closure,
    handle Expr\PropertyFetch / Expr\NullsafePropertyFetch before the
    existing "must be a global, non-magic constant" diagnostic:
    • evaluate the receiver; on an EnumCaseRef, fold ->name to the case name
      and ->value to the backing value;
    • nullsafe receiver null → null (mirrors the declaration evaluator);
    • any other enum-case property throws a deliberate
      Undefined enum case property ... error;
    • non-enum property fetches fall through to the unchanged existing
      diagnostic (single source of truth, no message duplication).
  • src/Resolver/ClassConstantValueTrait.php (1 word) —
    resolveCompileTimeEnumCaseBackingValue private → public, so stub
    generation reuses the declaration evaluator's exact resolution rules
    (internal enums via constant(), unit enums rejected with
    "has no backing value") instead of duplicating them.
  • phpunit/src/EnumCaseValuePropertyDefaultTest.php (new) — unit coverage:
    int-backed ->value, string-backed ->value, ->name, and a negative case
    asserting non-enum property fetches keep the existing diagnostic.
  • tests/compiler/object_property/enum-case-value-property-default.phpt
    (new)
    — end-to-end runtime coverage (int/string ->value, unit-enum
    ->name, enum-typed bare-case control). The --EXPECT-- section was
    validated against native PHP 8.4.

Behavior preserved

  • int $x = SomeClass::NON_ENUM->prop in a constant expression → same
    diagnostic as before.
  • Bare enum case → scalar-typed property remains a compile error
    (EnumDeclarationRulesTest).
  • Enum-typed property + bare case (TextRole $role = TextRole::Unset;)
    unchanged.

Testing

  • RED → GREEN: the new unit tests fail on the parent commit with the
    original Expr_PropertyFetch error (3 errors), and pass with the fix
    — OK (4 tests, 8 assertions).
  • Enum regression suites (EnumCaseValuePropertyDefaultTest +
    EnumDeclarationRulesTest): OK (30 tests, 57 assertions).
  • Full suite: Tests: 3127 — the 14 failures are pre-existing environment
    issues, reproduced identically with only these production edits reverted
    (bash completion / build stats / linker — no enum overlap).
  • Repro: --dry exits 0 (was 255); full AOT build succeeds and the binary
    prints -1.
  • phpstan: clean (src/gen_stub.php is excluded by phpstan.neon as
    upstream-derived). php-cs-fixer: no new issues.

Note for reviewers

The new PHPT was not executed via run-tests.php --compiler ./tpc because the
checked-in workflow expects a self-hosted ./tpc, and the locally available
one predates this fix; e2e behavior was verified through a full
bin/tpc.php build of the repro instead. Please re-run the PHPT with a fresh
self-hosted compiler.

Commits

  • 3368b5b4 fix(enum): fold enum case property fetches in constant expressions
  • 5c18ea64 test(compiler): add phpt coverage for enum case property defaults

Property defaults such as TextRole::Unset->value are valid constant
expressions since PHP 8.2 (php/php-src#8625). The declaration validator
already accepted them, but the stub generator's constant-expression
evaluator rejected Expr_PropertyFetch and aborted compilation with an
uncaught exception.

Handle enum case ->value/->name fetches in EvaluatedValue's fallback
closure: evaluate the receiver to an EnumCaseRef and fold through the
shared resolveCompileTimeEnumCaseBackingValue (relaxed private to
public so stub generation reuses the declaration evaluator rules,
including internal enums and unit-enum rejection). Every other
property fetch keeps the existing diagnostic.
End-to-end runtime coverage for folding enum case ->value/->name into
property defaults: int-backed and string-backed ->value, unit-enum
->name, plus the enum-typed bare-case control. The EXPECT section was
validated against native PHP 8.4.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant