# [PATCH] disallow arguments by ref in magic methods

- Thread: https://externals.io/message/31983
- Original archive: https://news-web.php.net/php.internals/31983
- Started by Antony Dovgal on 2007-08-30T12:55:59+00:00
- Messages: 8
- Reply by email to internals@lists.php.net
- More for agents: search with https://externals.io/search.md?q={query} · JSON API at https://externals.io/api · documentation at https://externals.io/llms.txt

---

## #31983 — Antony Dovgal — 2007-08-30T12:55:59+00:00

URL: https://externals.io/message/31983#31983

Hello.

I'd like to commit these two patches (for HEAD and 5_2 appropriately).
The patches disallow declaring any magic methods as accepting arguments by ref (which makes no sense anyway).

Example:
<?php
class test {
    function __set(&$name, $val) { }
}

$t = new test;
$name = "prop";
$t->$name = 1;
?>

Expected result of this code is:
Fatal error: Method test::__set() cannot take arguments by reference in %s on line %d


The diffs:
http://dev.daylessday.org/diff/magic_by_ref_5_2.diff
http://dev.daylessday.org/diff/magic_by_ref_HEAD.diff

If there are no objections, I'm going to commit them later in the evening.

-- 
Wbr, 
Antony Dovgal

---

## #31984 — Richard Quadling — 2007-08-30T13:18:58+00:00

URL: https://externals.io/message/31983#31984 · In reply to: #31983

> Hello.
>
> I'd like to commit these two patches (for HEAD and 5_2 appropriately).
> The patches disallow declaring any magic methods as accepting arguments by ref (which makes no sense anyway).

Why not. Sure, on __set()/__get()/__unset() it could be considered
pointless, but for __call()?

> If there are no objections, I'm going to commit them later in the evening.



-- 
-----
Richard Quadling
Zend Certified Engineer : http://zend.com/zce.php?c=ZEND002498&r=213474731
"Standing on the shoulders of some very clever giants!"

---

## #31985 — Dmitry Stogov — 2007-08-30T13:46:50+00:00

URL: https://externals.io/message/31983#31985 · In reply to: #31983

I like this patch. It prevents stupid errors.
ZE always passes arguments to magic methods by value so they never may be
modified.

Thanks. Dmitry.


> -----Original Message-----
> From: Antony Dovgal [mailto:antony@zend.com] 
> Sent: Thursday, August 30, 2007 4:56 PM
> To: php-dev
> Subject: [PHP-DEV] [PATCH] disallow arguments by ref in magic methods
> 
> 
> Hello.
> 
> I'd like to commit these two patches (for HEAD and 5_2 
> appropriately). The patches disallow declaring any magic 
> methods as accepting arguments by ref (which makes no sense anyway).
> 
> Example:
> <?php
> class test {
>     function __set(&$name, $val) { }
> }
> 
> $t = new test;
> $name = "prop";
> $t->$name = 1;
> ?>
> 
> Expected result of this code is:
> Fatal error: Method test::__set() cannot take arguments by 
> reference in %s on line %d
> 
> 
> The diffs:
> http://dev.daylessday.org/diff/magic_by_ref_5_2.diff
> http://dev.daylessday.org/diff/magic_by_ref_HEAD.diff
> 
> If there are no objections, I'm going to commit them later in 
> the evening.
> 
> -- 
> Wbr, 
> Antony Dovgal

---

## #31989 — Stanislav Malyshev — 2007-08-30T19:27:50+00:00

URL: https://externals.io/message/31983#31989 · In reply to: #31983

> Expected result of this code is:
> Fatal error: Method test::__set() cannot take arguments by reference in %s on line %d

I'd make it a warning. As far as I can see, declaring __set with 
references doesn't do anything bad, just reference thing doesn't work 
(correct me here if I'm wrong). So why should we produce fatal error?

-- 
Stanislav Malyshev, Zend Software Architect
stas@zend.com   http://www.zend.com/
(408)253-8829   MSN: stas@zend.com

---

## #31990 — Antony Dovgal — 2007-08-30T19:32:37+00:00

URL: https://externals.io/message/31983#31990 · In reply to: #31989

> > […]
> 
> I'd make it a warning. As far as I can see, declaring __set with 
> references doesn't do anything bad, just reference thing doesn't work 
> (correct me here if I'm wrong). So why should we produce fatal error?

The method call doesn't work in this case, it fails with fatal error, 
just try the example I posted.
Everything else is just fine, yeah..

-- 
Wbr, 
Antony Dovgal

---

## #31991 — Stanislav Malyshev — 2007-08-30T19:50:05+00:00

URL: https://externals.io/message/31983#31991 · In reply to: #31990

> The method call doesn't work in this case, it fails with fatal error, 
> just try the example I posted.

Ah, missed that part.
-- 
Stanislav Malyshev, Zend Software Architect
stas@zend.com   http://www.zend.com/
(408)253-8829   MSN: stas@zend.com

---

## #31994 — Marcus Boerger — 2007-08-31T08:23:25+00:00

URL: https://externals.io/message/31983#31994 · In reply to: #31985

Hello Dmitry,

  same here. And why not help by forcing to do it in the only correct way?
Letting something slip while knowing it will fail anyway just because it
doesn kill php is not a good idea - well at least to me.

marcus

Thursday, August 30, 2007, 3:46:50 PM, you wrote:

> I like this patch. It prevents stupid errors.
> ZE always passes arguments to magic methods by value so they never may be
> modified.

> Thanks. Dmitry.


> > […]




Best regards,
 Marcus

---

## #31997 — Antony Dovgal — 2007-08-31T12:36:17+00:00

URL: https://externals.io/message/31983#31997 · In reply to: #31983

Committed.


> Hello.
> 
> I'd like to commit these two patches (for HEAD and 5_2 appropriately).
> The patches disallow declaring any magic methods as accepting arguments by ref (which makes no sense anyway).
> 
> Example:
> <?php
> class test {
>     function __set(&$name, $val) { }
> }
> 
> $t = new test;
> $name = "prop";
> $t->$name = 1;
> ?>
> 
> Expected result of this code is:
> Fatal error: Method test::__set() cannot take arguments by reference in %s on line %d
> 
> 
> The diffs:
> http://dev.daylessday.org/diff/magic_by_ref_5_2.diff
> http://dev.daylessday.org/diff/magic_by_ref_HEAD.diff
> 
> If there are no objections, I'm going to commit them later in the evening.
> 


-- 
Wbr, 
Antony Dovgal