Skip to content

fix(compiler): fall back to Zend lookup for expression-valued class constants - #172

Open
yuan-dian wants to merge 1 commit into
swoole:masterfrom
yuan-dian:fix/class-const-expr-inline-tmpvars
Open

yuan-dian wants to merge 1 commit into
swoole:masterfrom
yuan-dian:fix/class-const-expr-inline-tmpvars

Conversation

@yuan-dian

Copy link
Copy Markdown
Contributor

Summary

Expression-valued class constants (e.g. const ALL = A | B) are inlined by
findNativeClassConst as raw value strings into the expression that fetches
them. The lowered form of those expressions references materialize temporaries
(tmp_var_N) whose declarations only exist in the arrayExpr prelude emitted
at the constant's definition site — so any fetching function compiled from the
inlined string references locals it never declares. This makes affected
projects uncompilable with C2065 / C2593-style errors, while --dry passes
silently because diagnostics only surface during code generation.

This PR makes findNativeClassConst return false for such constants so the
fetching sites fall back to the existing Zend class-constant lookup, in both
the class-owned and interface-owned branches.

Reproduction

const ALL = Flags::F_A | Flags::F_B;   // or: interface const with expression value

class Consumer {
    public function mask(): int {
        return self::ALL;               // any fetch inside a function body
    }
}

Generated before the fix (self-referencing + undeclared temporary):

tmp_var_0 = (((php::Var(tmp_var_0)) | (php::Var(tmp_var_1))));
// error: 'tmp_var_1' was not declared in this scope   (C2065)

After the fix:

tmp_var_0 = (php::constant(get_persistent_class(PersistentClassId{N}, get_str(X)), get_str(Y)));

Root cause

Stage Behavior
parse expression initializer lowered to tmp_var_N materialize temporaries; declarations live only in ConstantDef::$arrayExpr prelude
const definition site arrayExpr prelude is emitted — definition is self-contained ✅
fetch site findNativeClassConst returns ConstantDef::$value (the lowered string) — temporaries inlined without their declarations ❌
interface-owned const via implementing class identical inline path (:3363-3380) — same defect

Changes

  • src/CompilerBase.php — in findNativeClassConst, guard both the class-owned
    scalar branch and the interface-owned branch: when arrayExpr !== '' or the
    value string carries a tmp_var_, return false before setting
    nativeConst. Callers (parseClassConstFetch, parseDynamicClassConstFetch,
    isHoistSafeClassConstFetch) already handle false by emitting the Zend
    lookup php::constant(ce, name) / php::classConstant(...).
  • phpunit/code/class-constant-expression-inline.php — fixture with a literal
    constant (still inlined) and an expression constant in the same class, plus
    an interface constant fetched on an implementing class.
  • phpunit/src/ClassConstantExpressionInlineTest.php — asserts the leaf
    literal stays inlined while expression constants go through php::constant(
    with no inlined php::Var(tmp_var_N).

Behavior preserved

  • Literal / fully foldable constants keep being inlined (no codegen cost).
  • Compile-time consumers of constants (getClassConstValue →
    evaluateClassConstValue) re-evaluate the AST and are unaffected.
  • The guard is deliberately conservative: a string literal that merely
    contains tmp_var_ loses inlining but still resolves correctly.

Testing

  • New test is RED on the parent commit (failure text is exactly the
    broken generated body) and GREEN with the guard (2 tests, 11 assertions).
  • Full PHPUnit suite keeps master's pre-existing failures with an identical
    failing test list
    on Windows (37 errors / 106 failures / 111 warnings) and
    Linux (3205 tests, 6 errors / 21 failures — same as branch base).
  • AOT build of the interface fixture compiles, links and prints the expected
    result (previously failed at g++ with tmp_var_1 was not declared).

…onstants

findNativeClassConst inlines a constant's value string directly into the
fetching expression. When the initializer is an expression, parse lowers it
into materialize temporaries (tmp_var_N) whose declarations only exist in
arrayExpr's prelude, which is emitted together with array constants. The
inlined value therefore references locals the fetching function does not
have: `const ALL = A | B` fetched inside a method body compiles to
`((php::Var(tmp_var_0)) | (php::Var(tmp_var_1)))`, a self-referencing
temporary plus an undeclared one (C2065 / C2593).

Return false when the value string carries a materialize temporary or the
constant has an array expression, so the existing Zend class-constant
lookup is used instead. That lookup reads the already evaluated literal
registered for the constant and is cached in the convert phase, the same
shape as the existing forward-reference fallback. Literal constants keep
being inlined. Interface-owned constants resolved through an implementing
class take the same guard, since they flow through the identical inline
path.

Add a PHPUnit codegen fixture with a literal constant and an expression
constant in the same class, asserting the leaf constant is still inlined
while the expression constant goes through the Zend lookup and carries no
inlined temporary. An interface constant fetched on an implementing class
is covered by the same assertions.

Validation: the new test fails on the parent commit and passes with the
guard; the full suite keeps the pre-existing failures of master with an
identical failing test list on both Windows and Linux, and an AOT build
of the interface fixture links and prints the expected result.
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