FAQ
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

Search Discussions

  • Peter Cowburn at Apr 27, 2010 at 8:51 am

    On 27 April 2010 09:17, Etienne Kneuss wrote:
    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,000
    c) 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

    --
    PHP Internals - PHP Runtime Development Mailing List
    To unsubscribe, visit: http://www.php.net/unsub.php
  • Daniel Egeberg at Apr 27, 2010 at 9:14 am

    On Tue, Apr 27, 2010 at 10:51, Peter Cowburn wrote:
    On 27 April 2010 09:17, Etienne Kneuss wrote:
    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,000
    c) 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
  • Johannes Schlüter at Apr 27, 2010 at 4:45 pm
    Hi,
    On Tue, 2010-04-27 at 10:17 +0200, Etienne Kneuss wrote:
    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
  • Ralph Schindler at Aug 4, 2010 at 4:58 pm
    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,
    On Tue, 2010-04-27 at 10:17 +0200, Etienne Kneuss wrote:
    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

  • Stas Malyshev at Aug 5, 2010 at 10:28 pm
    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
  • Ralph Schindler at Aug 6, 2010 at 2:22 pm
    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?
  • Ralph Schindler at Aug 6, 2010 at 2:50 pm
    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 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?
  • Gustavo Lopes at Aug 6, 2010 at 3:24 pm

    On Fri, 06 Aug 2010 15:50:33 +0100, Ralph Schindler 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
  • Stas Malyshev at Aug 6, 2010 at 5:41 pm
    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
  • Gustavo Lopes at Aug 7, 2010 at 3:25 am

    On Sat, 07 Aug 2010 03:55:25 +0100, Etienne Kneuss wrote:


    Returning a ref is an additional constraint, meaning that it would
    actually be covariance, and not contravariance. But yes, it makes
    sense.
    Yes, you are right. To put it another way: a subclass should be able to
    * Relax the preconditions
    * Strengthen the post-conditions

    I actually noticed this shortly after sending the e-mail, but hoped no one
    else would :p
    Actually, after rereading what you said and the proposed diff, I'm not
    sure it makes sense. It should _not_ be consistent with arguments.

    A method subtype may relax the parent prototype by not requiring a
    ref for an argument. This is fine and it's indeed contra-variant.
    Again, my mistake. The if before was indeed about arguments...
    However, the return value of a method subtype should be able to return a
    ref even if the parent prototype doesnt. However, if the parent requires
    a ref, the child should too. Which is covariant.
    I'm confused, this was what exactly what I suggested.

    The condition I suggested is this (proto is the parent, fe the child):

    if (proto->common.return_reference && !fe->common.return_reference) {
    return 0;
    }

    So it returns an error condition if the parent returns a reference and the
    child doesn't.

    It's the check on the arguments that's wrong in this sense. However, I
    think invariance should be required here. The thing is, passing an
    argument by reference is actually also a post-condition -- if we pass an
    argument by reference, we're going to do something with it.

    In fact, invariance is required for the non-"rest" part of the arguments:

    if (fe->common.arg_info[i].pass_by_reference !=
    proto->common.arg_info[i].pass_by_reference) {

    Again, sorry for the confusion.

    --
    Gustavo Lopes
  • Derick Rethans at Aug 6, 2010 at 11:48 am

    On Tue, 27 Apr 2010, Etienne Kneuss wrote:

    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

Related Discussions

Discussion Navigation
viewthread | post
Discussion Overview
groupphp-internals @
categoriesphp
postedApr 27, '10 at 8:17a
activeAug 7, '10 at 3:25a
posts12
users8
websitephp.net

People

Translate

site design / logo © 2022 Grokbase