Skip to content

Initial ReflectionReference implementation#3550

Closed
nikic wants to merge 1 commit into
php:masterfrom
nikic:reflection-reference
Closed

Initial ReflectionReference implementation#3550
nikic wants to merge 1 commit into
php:masterfrom
nikic:reflection-reference

Conversation

@nikic

@nikic nikic commented Sep 25, 2018 •
edited
Loading

Copy link
Copy Markdown
Member

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

nikic commented Sep 25, 2018

Copy link
Copy Markdown
Member Author

/cc @nicolas-grekas

@nicolas-grekas

nicolas-grekas commented Sep 25, 2018

Copy link
Copy Markdown
Contributor

Thanks, I'll definitely check this PR!

@nikic

nikic commented Sep 26, 2018

Copy link
Copy Markdown
Member Author

@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

php-pulls commented Oct 12, 2018

Copy link
Copy Markdown

Comment on behalf of petk at php.net:

Labelling

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

nicolas-grekas commented Feb 6, 2019

Copy link
Copy Markdown
Contributor

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

nikic commented Feb 6, 2019

Copy link
Copy Markdown
Member Author

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 reflection-reference branch from cbead86 to d712326 Compare February 7, 2019 14:58
@nikic

nikic commented Feb 7, 2019

Copy link
Copy Markdown
Member Author

Serialization is now properly forbidden.

@nikic

nikic commented Feb 13, 2019

Copy link
Copy Markdown
Member Author

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
@jrfnl
PHP 7.4: NewClasses: add ReflectionReference class
b2a45b1 
> - 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
@jrfnl jrfnl mentioned this pull request Jun 14, 2019
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

RFC

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@nikic @nicolas-grekas @php-pulls