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

Magic __get() does not work nested #384

Closed
Grexeo opened this Issue Aug 11, 2011 · 7 comments

Comments

Projects
None yet
7 participants
@Grexeo

Grexeo commented Aug 11, 2011

Test case:

class one {
    public static $obj;

    public static function create() {
        return self::$obj = new two();
    }

}

class two {
    public function run() {
        $something = $this->three;
    }
    public function __get($alias) {
        $thing = new $alias;
    }
}

class three {
    public function __construct() {
        $four = one::$obj->four;
    }
}

class four {
    public function __construct() {
        echo "Bug fixed! :-)";
    }
}

one::create()->run();

Expected output: "Bug fixed! :-)"

Actual output: Nothing. Error log: "HipHop Notice: Undefined property: two::$four"

@TorbenF

This comment has been minimized.

TorbenF commented Aug 17, 2011

Confirmed.

My Testcase:

class A {
        protected $m_a = array();

        public function __construct()  {
                $this->m_a['abc'] = 123;
        }

        public function __get($sName)
        {
                if (isset($this->m_a[$sName])) {
                        return $this->m_a[$sName];
                }
                return NULL;
        }

}

class B extends A
{
       public function __get($sName) {
                if ($sName === 'xyz') {
                        return $this->abc;
                }
                return parent::__get($sName);
        }
}

$a = new A();
echo 'a: "' . $a->abc . '"' . "\n";

$b = new B();
echo 'b: "' . $b->xyz . '"' . "\n";
@kmaslack

This comment has been minimized.

Contributor

kmaslack commented Dec 28, 2012

I just wanted to mention that I took a look at this. I'm not sure what to do yet.

We're trying to emulate Zend's behavior of disallowing reentry of __get, but apparently Zend only avoids reentry when doing __get on the same property name. Arguably the Zend behavior is a bit inconsistent, and we should either allow recursion or disallow it; after all, there's nothing magic about recursing with the same name that guarantees the recursion won't terminate, and at the end of the day stack depth checks will save us just like all other infinite recursions.

@dolmens

This comment has been minimized.

dolmens commented Jan 17, 2013

who will fork a project to solve this?

@sannysoft

This comment has been minimized.

sannysoft commented Mar 29, 2013

Hope to have this bug fixed soon - otherwise I can't run Yii framework with HipHop

@crgwbr

This comment has been minimized.

crgwbr commented Apr 22, 2013

+1 This should be allowed until stack depth is exceeded—just like all other instances of recursion.

@kmaslack

This comment has been minimized.

Contributor

kmaslack commented Apr 22, 2013

If somebody feels passionately about this, please feel free to get in touch. This is not as easy as it looks, though.

It's not totally trivial to emulate Zend's behavior here in a perfromance-neutral way. It's not true that __get() is just like any other recursion; __get() magically suppresses reentry with the same property name, so that saying "$this->$propName = ..." within __get() does what you expect (i.e., create a new dynamic property on $this, not enter __set()). To get this to work, we'd need to wrap each invocation of __get() with something that a) marks the fact that $this has already had __get() called for it, and b) forgets this if __get() hasn't created a dynamic property with the same name.

@ptarjan

This comment has been minimized.

Contributor

ptarjan commented May 13, 2013

We're closing out all bugs older than 2 months. http://www.hiphop-php.com/wp/?p=575

If this is still an issue, please re-open it, and in order of goodness:

  1. Give detailed repro steps
  2. Write a test case in hphp/tests/quick (run it with hphp/tests/run) and send the pull request
  3. Fix it in a pull request
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment