[7.2] Add polyfill for spl_object_id() #97
Conversation
|
|
||
| public static function spl_object_id($object) | ||
| { | ||
| if (null === self::$hashMask) { |
There was a problem hiding this comment.
You can use an early return here.
There was a problem hiding this comment.
not sure it would help, L108 and L111 are not the same, so that would require duplicating L111, is that what you mean?
There was a problem hiding this comment.
actually hashMask is calculated without usage of input param here$object. maybe extract it to separated function and then
public static function spl_object_id($object)
{
return self::getHashMask() ^ hexdec(substr(spl_object_hash($object), self::$hashOffset, PHP_INT_SIZE));
}?
There was a problem hiding this comment.
I prefer raw perf, thus prevent an extra call here
There was a problem hiding this comment.
here we are, init logic split in separate function, called only once
| public function testSplObjectId() | ||
| { | ||
| $obj = new \stdClass(); | ||
| $id = spl_object_id($obj); |
There was a problem hiding this comment.
please test that calling spl_object_id for non-object works the same as native function
|
@nicolas-grekas I suppose that VarDumper can then be migrated to use the new |
|
note: merging this PR must wait until the PR is merged in PHP itself. We don't want to merge the polyfill if they reject the PR. |
src/Php72/Php72.php
Outdated
| private static function initHashMask() | ||
| { | ||
| $obj = (object) array(); | ||
| self::$hashOffset = 16 - PHP_INT_SIZE; |
There was a problem hiding this comment.
if you want to improve perf even further, you could even fully qualify these constants (as done for \PHP_VERSION_ID). Not sure it is worth it though (this code is called only once)
There was a problem hiding this comment.
done by removing $hashOffset
|
@stof VarDumper can use this polyfill only in master, because of HHVM compat... |
|
The |
|
Thank you @nicolas-grekas. |
This PR was merged into the 1.5-dev branch. Discussion ---------- [7.2] Add polyfill for spl_object_id() As [discussed on php-internals](https://externals.io/message/99751) and implemented in php/php-src#2611. Implementation [borrowed from VarDumper](https://github.com/symfony/symfony/blob/2.7/src/Symfony/Component/VarDumper/Cloner/VarCloner.php#L300). Commits ------- 06995f5 [7.2] Add polyfill for spl_object_id()
|
@fabpot Sorry to comment on a closed PR, but I don't think this is worth a new issue: it would be great to get a release tagged that includes this PR (I hate having to include |
|
@robfrawley Done, 1.5 is out now. |
As discussed on php-internals and implemented in php/php-src#2611.
Implementation borrowed from VarDumper.