Fix GH-10497: Allow const obj->prop = value

[khaledalam]

Description

RFC: https://wiki.php.net/rfc/const_object_property_write
Discussion thread: https://news-web.php.net/php.internals/129757
Voting thread: https://news-web.php.net/php.internals/132102

Implementation commit: b16cab7

---

Fixes #10497 - Allows direct modification of object properties stored in constants, for both global constants and class constants.

Problem

Previously, this code would fail with a fatal error:

const a = new stdClass;
a->b = 42;  // Fatal error: Cannot use temporary expression in write context

The constant binding is immutable, but the object it holds is mutable by design — so the write should be allowed.

Solution

zend_delayed_compile_prop() marks the constant AST node via zend_mark_const_object_fetch(), which walks down through ZEND_AST_DIM nodes and sets a ZEND_CONST_OBJECT_FETCH attribute on a ZEND_AST_CONST / ZEND_AST_CLASS_CONST base. zend_delayed_compile_var() handles those AST kinds when the attribute is set, so the constant is fetched at runtime and the write targets the real object.

The fetch result is IS_TMP_VAR in read context (BP_VAR_R, BP_VAR_IS) as before, and IS_VAR in write context. Constants that are still substituted at compile time (true, null, persistent constants, same-file class constants) are wrapped in ZEND_QM_ASSIGN to materialise them into an IS_VAR — the property opcodes have no handler for an IS_CONST container.

Two supporting changes: zend_optimizer_update_op1_const() no longer substitutes a constant into op1 of the object-write opcodes, which would otherwise reintroduce the invalid operand during optimisation; and zend_can_write_to_variable() accepts a constant base when the PROP/DIM chain contains at least one property fetch.

Scope

Supported:

• CONST->prop = value; // global constant
• Cls::CONST->prop = value; // class constant (via class name)
• $obj::CONST->prop = value; // class constant (via instance)
• …including compound assignment, ++/--, .=, isset()/unset(), by-reference passing, and nested chains.
• destructuring assignment — [CONST->p, CONST->q] = [1, 2];
• by-reference foreach — foreach (CONST->arr as &$v) { ... }

Still rejected (unchanged):

• Rebinding a constant (CONST = ...) — parse error
• Dimension/array writes on constants (CONST[0] = ..., CONST["x"] = ...) , these write to a temporary, so they remain a compile error

Backward incompatible change

Enum cases are class constants, so they're affected. Writing to or unsetting a readonly case property, e.g. unset(Enum::Case->value) or Enum::Case->value = x , now raises the standard runtime readonly-property Error instead of the previous compile-time "Cannot use temporary expression in write context" fatal. This matches plain variable access ($x = Enum::Case; unset($x->value);). Enum case properties remain immutable.

[iluuu1994]

Thanks @khaledalam! Looks reasonable overall.

[khaledalam]

Thanks @khaledalam! Looks reasonable overall.

Thanks @iluuu1994 for the feedback, all comments addressed.

[iluuu1994]

Cool, thanks @khaledalam! This is a language change and should be discussed on the internals mailing list at least. I think this will impact GH-13800, but that's not a major issue.

[iluuu1994]

Btw, one thing in particular people might object to is:

const arrayTest = [1, 2, 3];
arrayTest[] = 4;
var_dump(arrayTest);

This becomes valid, but prints:

array(3) {
  [0]=>
  int(1)
  [1]=>
  int(2)
  [2]=>
  int(3)
}

I.e. the assignment has no effect, because it happens on a temporary value. Make sure to mention this in your e-mail to internals.

[khaledalam]

I've opened an internals thread and sent an email for discussion: https://news-web.php.net/php.internals/129757

[iluuu1994]

It sounds like @TimWolla objected, so this would need an RFC, or at least the mitigation from https://news-web.php.net/php.internals/129839.

[TimWolla]

Yes, my replies on the list were an objection against the PR as-is. Selecting request changes for visibility.

[khaledalam]

Noted, I'll look into it

RFC was created.

[khaledalam]

Voting closed. The RFC was accepted with 17 (Yes) to 2 (No) votes and 6 abstentions, which is 89.5% acceptance.

[khaledalam]

@iluuu1994 @TimWolla code review please, RFC voting closed.

[iluuu1994]

Didn't get to it today, tomorrow will have to do, hopefully before the beta 1 cutoff.

[iluuu1994]

The implementation looks nothing like the one I reviewed. What's the reason this was changed?

[iluuu1994]

I'm assuming the changes were made mainly to avoid making OBJ[0] = 1; legal. But it's a bit short-sighted:

OBJ[0]->prop = 42; // disallowed
OBJ->arr[0] = 42; // allowed
OBJ->arr[] = 42; // allowed

There's not much logic as to why the former is disallowed, but the latter is. What we really want to check is whether -> is contained anywhere in the chain.

[iluuu1994]

Should be correct now. Feel free to check. I'll merge tomorrow.

[khaledalam]

Should be correct now. Feel free to check. I'll merge tomorrow.

@iluuu1994 Heads up before you merge, I pushed a fix commit on top of your rewrite. It introduced a regression that I hit while testing the new implementation. Can you please re-review?

Compile-time evaluated constants in a write context produce an Invalid opcode fatal:

TRUE->prop = 1;                              // Fatal error: Invalid opcode 24/1/1.
NULL->prop = 1;                              // Fatal error: Invalid opcode 24/1/1.
PHP_EOL->prop = 1;                           // Fatal error: Invalid opcode 24/1/1.
class C { const N = 5; } C::N->prop = 1;     // Fatal error: Invalid opcode 24/1/1.
unset(C::N->prop);                           // Fatal error: Invalid opcode 76/1/1.
C::N->prop++;                                // Fatal error: Invalid opcode 132/1/1.
function f(&$v) {} f(C::N->prop);            // Fatal error: Invalid opcode 85/1/1.

On 8.5 these are all a clean Cannot use temporary expression in write context. Reproduces with and without opcache/JIT.

[iluuu1994]

Thanks for the hint. I believe this was a pre-existing issue but didn't check. I tweaked the implementation a bit. Please check again.

[khaledalam]

@iluuu1994 Thank you, It looks ok for me. I just rebased it to fix conflict.

[iluuu1994]

Thanks @khaledalam. Please don't forget to update the RFC status and overview page.

[khaledalam]

Thank you @iluuu1994 , I notice that the commit message links rfc/override_constants instead of rfc/const_object_property_write, just fyi.

[iluuu1994]

Not sure how that happened. Oh well, can't fix it now, we don't force-push master unless absolutely necessary.