Everything
php
Learn Community Releases The PHP Project
Home » Project » Newsgroups » php.internals » [PATCH] disallow arguments by ref in magic methods »

[PATCH] disallow arguments by ref in magic methods

php.internals

Antony Dovgal

19 years ago
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

Richard Quadling

19 years ago
On 30/08/2007, Antony Dovgal <antony@zend.com> wrote:
> 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!"

Dmitry Stogov

19 years ago
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.

Marcus Börger

19 years ago
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.
>> -----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 >> >> -- >> PHP Internals - PHP Runtime Development Mailing List >> To unsubscribe, visit: http://www.php.net/unsub.php >>
Best regards, Marcus

Stanislav Malyshev

19 years ago
> 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

Antony Dovgal

19 years ago
On 30.08.2007 23:27, Stanislav Malyshev wrote:
>> 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?
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

Stanislav Malyshev

19 years ago
> 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

Antony Dovgal

19 years ago
Committed. On 30.08.2007 16:55, Antony Dovgal wrote:
> 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