Hi,
it looks like the fact that ArrayAccess::offsetGet is not returning a
reference is a recurrent problem, I see basically 4 options:
a) Ignore the issue, change nothing
b) Rewrite offsetGet to return a ref, breaking BC
c) Create a new ArrayAccess interface where it does return a ref
d) Relax prototype checks so that both are allowed.
Personally, I believe that a fatal on such prototypes error is not
warranted, so I'd prefer (d).
What do you think would be the best option? Can you think of another?
Best,
--
Etienne Kneuss
Hi,
it looks like the fact that ArrayAccess::offsetGet is not returning a
reference is a recurrent problem, I see basically 4 options:a) Ignore the issue, change nothing
b) Rewrite offsetGet to return a ref, breaking BC
-1,000,000c) Create a new ArrayAccess interface where it does return a ref
d) Relax prototype checks so that both are allowed.
Of the options presented, I think d) would be the best of the bunch.
Folks seem to expect the ability to get references so any solution
that gives them that would be better IMO than keeping the interface
as-is.
Personally, I believe that a fatal on such prototypes error is not
warranted, so I'd prefer (d).What do you think would be the best option? Can you think of another?
Best,
--
Etienne Kneuss
Hi,
it looks like the fact that ArrayAccess::offsetGet is not returning a
reference is a recurrent problem, I see basically 4 options:a) Ignore the issue, change nothing
b) Rewrite offsetGet to return a ref, breaking BC
-1,000,000c) Create a new ArrayAccess interface where it does return a ref
d) Relax prototype checks so that both are allowed.
Of the options presented, I think d) would be the best of the bunch.
Folks seem to expect the ability to get references so any solution
that gives them that would be better IMO than keeping the interface
as-is.
Without having the technical insight in how it works internally, I
would also say that from an end-user perspective, option (d) makes
most sense.
Personally, I believe that a fatal on such prototypes error is not
warranted, so I'd prefer (d).What do you think would be the best option? Can you think of another?
--
Daniel Egeberg
Hi,
it looks like the fact that ArrayAccess::offsetGet is not returning a
reference is a recurrent problem, I see basically 4 options:
The main use case is some nested structure like
$o = new ArrayObject();
/.../
$o[23][42] = "foobar";
right?
a) Ignore the issue, change nothing
b) Rewrite offsetGet to return a ref, breaking BC
c) Create a new ArrayAccess interface where it does return a ref
d) Relax prototype checks so that both are allowed.
If the above case is correct and due to me not liking references I
wonder whether there is a way to for an option e) which adds support for
this in some way to the engine.
johannes
Hey all,
Was there any more discussion on this? I've been bit by this as well,
several times over the past 3 years.
I'd opt for option (d) for all prototype/signature checking. Here's why:
I don't think the return "type" should be considered part of the
signature when enforcing LSP. PHP (currently), does not care what the
type of the return value is, much less if there is even one to be
returned. Whether or not it is a reference or a value should not impact
the conditions presented to the caller, in other words "the
preconditions are not strengthened in a subtype." Since PHP does no
enforcement of return values, postconditions don't seem to apply.
Also, relaxing the checks would be backwards compatible. Since no code
should currently exist that does this (its an E_FATAL currently if the
signature originates in an interface, E_STRICT notice when the signature
originates in an abstract/base class overridden in a subclass).
Thoughts?
Ralph Schindler
Johannes Schlüter wrote:
Hi,
it looks like the fact that ArrayAccess::offsetGet is not returning a
reference is a recurrent problem, I see basically 4 options:The main use case is some nested structure like
$o = new ArrayObject();
/.../
$o[23][42] = "foobar";right?
a) Ignore the issue, change nothing
b) Rewrite offsetGet to return a ref, breaking BC
c) Create a new ArrayAccess interface where it does return a ref
d) Relax prototype checks so that both are allowed.
If the above case is correct and due to me not liking references I
wonder whether there is a way to for an option e) which adds support for
this in some way to the engine.johannes
Hi!
I'd opt for option (d) for all prototype/signature checking. Here's why:
I think relaxing the check may make sense. Do you have some code example
that doesn't work and you want it to work?
Stanislav Malyshev, Software Architect
SugarCRM: http://www.sugarcrm.com/
(408)454-6900 ext. 227
I can give 2 examples, one that triggers the problem, the other that is
a real world issue:
Simple:
<?php
interface I {
public function foo($name);
}
class C implements I {
public function & foo($name) {}
}
$c = new Bar();
running this produces:
$ php -d error_reporting=32767 test-reference-in-signature.php
PHP Fatal error: Declaration of C::foo() must be compatible with that
of I::foo() in path/to/test-reference-in-signature.php on line 8
Real world issue with ArrayAccess:
<?php
class SomeContainer implements ArrayAccess {
protected $_data = array('foo' => array(1,2,3));
public function & offsetGet($name) {
$r = & $this->_data['foo'];
return $r;
}
public function offsetSet($name, $value) {}
public function offsetExists($name) {}
public function offsetUnset($name) {}
}
$b = new Bar();
$b['foo'][3] = 4; // implies Bar::offsetGet() happens before assign
running this produces:
$ php -d error_reporting=32767 test-reference-in-arrayaccess.php
PHP Fatal error: Declaration of SomeContainer::offsetGet() must be
compatible with that of ArrayAccess::offsetGet() in
path/to/test-reference-in-arrayaccess.php on line 3
-ralph
Stas Malyshev wrote:
Hi!
I'd opt for option (d) for all prototype/signature checking. Here's why:
I think relaxing the check may make sense. Do you have some code example
that doesn't work and you want it to work?
The attached patch is the suggested fix. I made this against master on
github.
-ralph
Ralph Schindler wrote:
I can give 2 examples, one that triggers the problem, the other that is
a real world issue:
Simple:
<?php
interface I {
public function foo($name);
}class C implements I {
public function & foo($name) {}
}$c = new Bar();
running this produces:
$ php -d error_reporting=32767 test-reference-in-signature.php
PHP Fatal error: Declaration of C::foo() must be compatible with that
of I::foo() in path/to/test-reference-in-signature.php on line 8
Real world issue with ArrayAccess:
<?php
class SomeContainer implements ArrayAccess {
protected $_data = array('foo' => array(1,2,3));
public function & offsetGet($name) {
$r = & $this->_data['foo'];
return $r;
}
public function offsetSet($name, $value) {}
public function offsetExists($name) {}
public function offsetUnset($name) {}
}$b = new Bar();
$b['foo'][3] = 4; // implies Bar::offsetGet() happens before assignrunning this produces:
$ php -d error_reporting=32767 test-reference-in-arrayaccess.php
PHP Fatal error: Declaration of SomeContainer::offsetGet() must be
compatible with that of ArrayAccess::offsetGet() in
path/to/test-reference-in-arrayaccess.php on line 3-ralph
Stas Malyshev wrote:
Hi!
I'd opt for option (d) for all prototype/signature checking. Here's
why:I think relaxing the check may make sense. Do you have some code
example that doesn't work and you want it to work?
On Fri, 06 Aug 2010 15:50:33 +0100, Ralph Schindler ralph@smashlabs.com
wrote:
The attached patch is the suggested fix. I made this against master on
github.
In my opinion, it would make more sense, as was already suggested before,
to require the return to be passed by reference only if the prototype
specifies it should be passed by reference. This could be argued to be a
form of return contravariance.
As a bonus, it would be consistent with the "pass_rest_by_reference" flag
check:
if (proto->common.pass_rest_by_reference
&& !fe->common.pass_rest_by_reference) {
return 0;
}
//if (fe->common.return_reference != proto->common.return_reference) {
if (proto->common.return_reference && !fe->common.return_reference) {
return 0;
}
--
Gustavo Lopes
Hi!
In my opinion, it would make more sense, as was already suggested before,
to require the return to be passed by reference only if the prototype
specifies it should be passed by reference. This could be argued to be a
form of return contravariance.
Yes, this makes sense.
--
Stanislav Malyshev, Software Architect
SugarCRM: http://www.sugarcrm.com/
(408)454-6900 ext. 227
Hi,
it looks like the fact that ArrayAccess::offsetGet is not returning a
reference is a recurrent problem, I see basically 4 options:a) Ignore the issue, change nothing
b) Rewrite offsetGet to return a ref, breaking BC
c) Create a new ArrayAccess interface where it does return a ref
d) Relax prototype checks so that both are allowed.
Personally, I believe that a fatal on such prototypes error is not
warranted, so I'd prefer (d).
Can you give an example of this one?
Derick
--
http://derickrethans.nl | http://xdebug.org
Like Xdebug? Consider a donation: http://xdebug.org/donate.php
twitter: @derickr and @xdebug