Skip to content

Readonly properties mutable when assigning reference to them #7942

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
ondrejmirtes opened this issue Jan 14, 2022 · 8 comments
Closed

Readonly properties mutable when assigning reference to them #7942

ondrejmirtes opened this issue Jan 14, 2022 · 8 comments

Comments

@ondrejmirtes
Copy link
Contributor

Description

Readonly properties should not be mutable with any means, but the following code surprisingly works (https://3v4l.org/qiDTk):

<?php

class Foo
{
	public readonly int $bar;
	public function set(int &$i)
	{
		$this->bar = &$i;
	}
}

$i = 1;
$foo = new Foo();
$foo->set($i);
$i = 20;
var_dump($foo->bar); // 20

Additionally, without the & in int &$i, the property value stays 1 but does not report any error: https://3v4l.org/NMDAp

PHP Version

PHP 8.1

Operating System

No response

@pthreat
Copy link

pthreat commented Jan 14, 2022

However one must note that the following example works as intended:

<?php // lint >= 8.1

namespace ReadonlyPropertyPassedByRef;

class Foo
{

	public readonly int $bar;
	
	public function set(int &$i)
	{
		$this->bar = &$i;
	}

}

$i = 1;
$z = 25;
$foo = new Foo();
$foo->set($i);
$i = &$z;
var_dump($foo->bar);

The reference of the variable has been set as read only, not it's value.

@cmb69
Copy link
Member

cmb69 commented Jan 14, 2022

Not sure whether this is a bug; it might fall in the "readonly properties do not preclude interior mutability" category.

@iluuu1994
Copy link
Member

/cc @nikic

The engine does prevent creating references from readonly properties.
https://3v4l.org/cRgNM#v8.1.1

Maybe we could do the same in the other direction.
https://3v4l.org/M1n37#v8.1.1

class Foo {
    public readonly int $bar;
    public function __construct(int &$bar) {
        // Error right here, can't assign reference
        $this->bar = &$bar;
    }
}

$i = 42;
$foo = new Foo($i);
$i++;
var_dump($foo->bar);

@ondrejmirtes
Copy link
Contributor Author

@cmb69 I’d consider it a bug because otherwise this sentence from the RFC isn’t really true:

Reading a readonly property will always return the same value, no matter what code runs in between

@nikic
Copy link
Member

nikic commented Jan 14, 2022

Yeah, this should not be allowed.

@nikic
Copy link
Member

nikic commented Jan 16, 2022

In zend_std_get_property_ptr_ptr(), we currently allow acquiring a property pointer if the current value is undef and the scope has initialization access. This then allows zend_assign_to_property_reference() to assign a reference to the property.

@nikic
Copy link
Member

nikic commented Jan 16, 2022

Always falling back to read_property for readonly props means that $obj->prop[] = 1 no longer works as an initializing assignment (it will generate a "must not be accessed before initialization" error). That's probably not a big loss, as the sane way to write that would be $obj->prop = [1] (you can only do the append once). The error message for the $obj->prop =& $ref case is also not great (same "accessed before initialization" error).

iluuu1994 added a commit to iluuu1994/php-src that referenced this issue Mar 10, 2022

Verified

This commit was signed with the committer’s verified signature.
iluuu1994 Ilija Tovilo
Closes phpGH-7942
iluuu1994 added a commit to iluuu1994/php-src that referenced this issue Mar 11, 2022

Verified

This commit was signed with the committer’s verified signature.
iluuu1994 Ilija Tovilo
Closes phpGH-7942
iluuu1994 added a commit to iluuu1994/php-src that referenced this issue Mar 11, 2022

Verified

This commit was signed with the committer’s verified signature.
iluuu1994 Ilija Tovilo
Closes phpGH-7942
iluuu1994 added a commit to iluuu1994/php-src that referenced this issue Mar 21, 2022

Verified

This commit was signed with the committer’s verified signature.
iluuu1994 Ilija Tovilo
Closes phpGH-7942
iluuu1994 added a commit to iluuu1994/php-src that referenced this issue Jun 17, 2022

Verified

This commit was signed with the committer’s verified signature.
iluuu1994 Ilija Tovilo
Closes phpGH-7942
iluuu1994 added a commit to iluuu1994/php-src that referenced this issue Jun 17, 2022

Verified

This commit was signed with the committer’s verified signature.
iluuu1994 Ilija Tovilo
Closes phpGH-7942
iluuu1994 added a commit to iluuu1994/php-src that referenced this issue Jun 24, 2022

Verified

This commit was signed with the committer’s verified signature.
iluuu1994 Ilija Tovilo
Closes phpGH-7942
iluuu1994 added a commit to iluuu1994/php-src that referenced this issue Jun 27, 2022

Verified

This commit was signed with the committer’s verified signature.
iluuu1994 Ilija Tovilo
Closes phpGH-7942
@ondrejmirtes
Copy link
Contributor Author

Thank you @iluuu1994!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants