Skip to content
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

Initial ReflectionReference implementation #3550

Closed
wants to merge 1 commit into from

Conversation

@nikic
Copy link
Member

nikic commented Sep 25, 2018

Implementation of ReflectionReference based on the discussion in https://externals.io/message/102638. Currently the class definition is

final class ReflectionReference {
    /* Returns ReflectionReference if array element is a reference, null otherwise. */
    public static function fromArrayElement(array $array, int|string $key): ?ReflectionReference;
    /* Returns unique identifier for the reference. The return value format is unspecified. */
    public function getId(): int|string;

    private function __construct(); // Always throws
    private function __clone(); // Always throws
}

The return value of getId() uniquely identifies the reference for the duration of its lifetime. An ID may be reused if the reference is destroyed. The format of the return value is unspecified any may change. Only the fact that it will be an integer or string is guaranteed.

In the current implementation the address of the reference is directly returned as the ID. We should be hiding the address here, possibly by hashing it with a secret key. the ID is a raw SHA1 hash of the reference address and a per-process key. Raw meaning 20 character binary data, no hex encoding.

@nikic

This comment has been minimized.

Copy link
Member Author

nikic commented Sep 25, 2018

@nicolas-grekas

This comment has been minimized.

Copy link
Contributor

nicolas-grekas commented Sep 25, 2018

Thanks, I'll definitely check this PR!

@nikic

This comment has been minimized.

Copy link
Member Author

nikic commented Sep 26, 2018

@sarciszewski Any tips on how to return scrambled memory addresses, for the getId() function? I could run the address through sha1 together with a per-process secret key, but I'd prefer something more lightweight. A fast 64-bit cipher would probably be ideal here.

@php-pulls

This comment has been minimized.

Copy link

php-pulls commented Oct 12, 2018

Comment on behalf of petk at php.net:

Labelling

@nikic nikic force-pushed the nikic:reflection-reference branch 2 times, most recently from f3ea6ef to d96bef5 Jan 14, 2019
@nikic nikic added RFC and removed Enhancement labels Jan 15, 2019
@nicolas-grekas

This comment has been minimized.

Copy link
Contributor

nicolas-grekas commented Feb 6, 2019

Should the class implements Reflector for consistency with other reflection-related classes?
Should it throw on serialize?
(maybe that's already the case btw, in which case sorry for asking)

@nikic

This comment has been minimized.

Copy link
Member Author

nikic commented Feb 6, 2019

Should the class implements Reflector for consistency with other reflection-related classes?

Not if we can avoid it :) Reflector is quite the abomination and should really be removed entirely... I'd prefer not to have implicit stringification on this class just to satisfy this interface. Thankfully there is precedent in not implementing Reflector in the form of ReflectionGenerator.

Should it throw on serialize?

Yes it should! I've made serialize throw for all other reflection classes a while ago, but forgot it here...

@nikic nikic force-pushed the nikic:reflection-reference branch from cbead86 to d712326 Feb 7, 2019
@nikic

This comment has been minimized.

Copy link
Member Author

nikic commented Feb 7, 2019

Serialization is now properly forbidden.

@nikic

This comment has been minimized.

Copy link
Member Author

nikic commented Feb 13, 2019

RFC accepted, merged as 6347f0b.

@nikic nikic closed this Feb 13, 2019
jrfnl added a commit to PHPCompatibility/PHPCompatibility that referenced this pull request Jun 14, 2019
> - Reflection:
>    A new ReflectionReference class has been added, which allows detecting
>    references and comparing them for identity.

Refs:
* https://wiki.php.net/rfc/reference_reflection
* https://github.com/php/php-src/blob/42cc58ff7b2fee1c17a00dc77a4873552ffb577f/UPGRADING#L388-L391
* php/php-src#3550
* php/php-src@6347f0b
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
3 participants
You can’t perform that action at this time.