Implement typed properties #3734

nikic · 2019-01-07T11:35:42Z

RFC: https://wiki.php.net/rfc/typed_properties_v2

This is a squashed version of #3313 to make final review easier. The previous thread has become unwieldy.

carusogabriel

@nikic From the tests part, just a couple of tests, for example, Zend/tests/type_declarations/typed_properties_019.phpt, that have a fell extra lines at the end of the lines. I'll run this implementation in some OSS projects, see if I can found some missing tests 👍

nikic · 2019-01-07T11:51:28Z

@carusogabriel I've added some basic property type support to https://github.com/nikic/TypeUtil for this purpose. For now I've only tested that PHP-Parser works after automatically annotating it with property types (needs some manual fixup to fix inaccurate @var annotations and different initialization semantics of typed properties). Would be great to do these kinds of tests on more projects.

RFC: https://wiki.php.net/rfc/typed_properties_v2 Co-authored-by: Bob Weinand <bobwei9@hotmail.com> Co-authored-by: Joe Watkins <krakjoe@php.net> Co-authored-by: Dmitry Stogov <dmitry@zend.com>

Add two more macros ZEND_TRY_ASSIGN_VALUE and ZEND_TRY_ASSIGN_COPY to assign zvals and replace all remaining uses of the internal API. Switch the API to not use a temporary zval anymore, as all users of the API take care to do this themselves.

I found the 1+count here much more confusing than helpful.

That's what it does...

And add tests for typed properties + references + undefined classes. I think the behavior here is not correct and we should treat this the same way we do elsewhere (if the class can't be loaded, conclude that it can't be an instance of the class, but don't issue an error). Going back to this behavior for now to avoid an assertion failure...

This is probably going to need changes in a few more places that currently implicitly assume only CE types can occur.

If the value_type is not known at compile-time, don't perform checks for whether the copy is needed or not, always do it. Also make sure we free the copy in the error case...

If we're in a class that has type hints, actually seeing a type is not unexpected.

This makes it consistent with the function for ref types and avoids having to handle these differently.

And remove some code handling future possibilities where a property could be both int and double. I don't like having untestable code around. Let's deal with this fully once we have union types.

And remove some redundant checks.

Made a mistake here when refactoring the code, causing a failure in serialization test.

Based on the descision that we do not want to enforce __get() return types for inaccessible propreties.

It's not relevant in any other case and might be wrong wrt cache state I think.

We have the proprerty offset here and know it's valid, so we can use our property helper table to fetch it, rather than doing a full property info lookup.

By switching to the slot API, which doesn't need unmangled keys and is faster anyway... Also moving those APIs to a place that has a complete prop_info def.

I originall thought this wasn't possible, but now that it turned out that object_handlers also always has slots available, we can do this.

Otherwise we can probably run into the same issues here as we could with destruction.

Used in conjunction with try_array_init, can't deref beforehand.

I'm explicitly passing the object to process_nested_data now. When I tried to use Z_OBJ_P(rval) things started failing in odd ways and I have no idea why. The variable doesn't seem to be overwritten...

It's rather unlikely that these will be available as CEs at this point, we need to look them up.

I don't think it can happen right now, but will once there is full preloading support for this.

RFC: https://wiki.php.net/rfc/typed_properties_v2 This is a squash of PR #3734, which is a squash of PR #3313. Co-authored-by: Bob Weinand <bobwei9@hotmail.com> Co-authored-by: Joe Watkins <krakjoe@php.net> Co-authored-by: Dmitry Stogov <dmitry@zend.com>

nikic · 2019-01-11T16:19:28Z

Merged via e219ec1.

moliata · 2020-03-24T16:32:46Z

@nikic, is there any chance for typed callable properties in PHP 8? Personally, this is the only thing holding me back from fully loving PHP. Even though the context problems outlined in the consistent callables RFC are fair and square, language consistency is also important and much appreciated. Typed parameters and return types basically ignore the fact whether the callable is actually callable from this scope/context. This isn't the case with properties. Here are my reasons for supporting typed callable properties:

Language consistency. Given that PHP is becoming a much more consistent and powerful language with every release, adding support for typed callable properties would improve the overall consistency as both typed callable parameters and return types are supported. Futhermore, thanks to the upcoming union types, you would be able to even completely type hint your entire project. Though that isn't possible yet due to the lack of support for callable properties.
Strict typing. Strictly limiting a property to only callables helps to avoid bugs, makes debugging process easier and in some use cases (such as frameworks) ensures that the application is not going to have side effects and will be reliable.
Closure::fromCallable() adds additional overhead. Assuming that thousands of callables need to be converted (which is often the case in frameworks when registering hundreds of routes to the router on large web applications).

Best regards,
Ben.

nikic · 2020-03-24T16:51:02Z

@moliata I doubt we will ever support callable for properties. The general consensus is that we should make it easier to create Closures from generic callables instead. (That is, add first-class syntax to reference a function/method, instead of using magic strings/arrays).

nikic added the RFC label Jan 7, 2019

carusogabriel reviewed Jan 7, 2019

View reviewed changes

nikic mentioned this pull request Jan 7, 2019

Typed Properties #3313

Closed

This comment has been minimized.

Sign in to view

nikic and others added 22 commits January 10, 2019 12:43

Implement typed properties

46fbe12

RFC: https://wiki.php.net/rfc/typed_properties_v2 Co-authored-by: Bob Weinand <bobwei9@hotmail.com> Co-authored-by: Joe Watkins <krakjoe@php.net> Co-authored-by: Dmitry Stogov <dmitry@zend.com>

Remove redundant trailing newlines from tests

973f8c5

Coding style tweaks

784aebe

Add test for typed properties on internal class

ac6b431

Code cleanup

d95f131

Avoid direct uses of zend_try_assign API

0395fe5

Add two more macros ZEND_TRY_ASSIGN_VALUE and ZEND_TRY_ASSIGN_COPY to assign zvals and replace all remaining uses of the internal API. Switch the API to not use a temporary zval anymore, as all users of the API take care to do this themselves.

Remove alloc_polymorphic_cache_slots API

37b3de6

I found the 1+count here much more confusing than helpful.

Rename get_mangled_property_name to get_unmangled_property_name

5e009e7

That's what it does...

Cleanup code and add more trait + self tests

9e82c5a

Remove requirement that reference sources must have resolved types

9c55d73

This is probably going to need changes in a few more places that currently implicitly assume only CE types can occur.

Tweak assign_to_variable

23bb53f

If the value_type is not known at compile-time, don't perform checks for whether the copy is needed or not, always do it. Also make sure we free the copy in the error case...

Remove dubious expectations

b4f5e11

If we're in a class that has type hints, actually seeing a type is not unexpected.

Move error generation into verify_property_type

f1d9f22

This makes it consistent with the function for ref types and avoids having to handle these differently.

Code cleanup

117d1da

Check for incdec overflow if no refs are involved

3923574

And remove some code handling future possibilities where a property could be both int and double. I don't like having untestable code around. Let's deal with this fully once we have union types.

Prefer showing a property type error over a reference error

be63cc1

Extract error code into cold function

1282365

And remove some redundant checks.

Code cleanup

ee84e01

Fix handling of null vs undef in auto-vivification

bc27695

Fix hardcoded paths in tests

ba09a1d

Cleanup

4e34d90

nikic added 8 commits January 10, 2019 12:43

Fix ref assignable with strict types

1327829

Made a mistake here when refactoring the code, causing a failure in serialization test.

Only use information from visible properties in opcache

22dfc44

Based on the descision that we do not want to enforce __get() return types for inaccessible propreties.

Make sure fetch_property_type_info is only used with valid offset

90cfd7a

It's not relevant in any other case and might be wrong wrt cache state I think.

write_property value may not be REF

dcc8ea8

Use faster API too look up property info in object handlers

cfd76b0

We have the proprerty offset here and know it's valid, so we can use our property helper table to fetch it, rather than doing a full property info lookup.

Fix dumping of private uninitialized props

efbcbd6

By switching to the slot API, which doesn't need unmangled keys and is faster anyway... Also moving those APIs to a place that has a complete prop_info def.

Move everything over to the slot API

bb3e82b

I originall thought this wasn't possible, but now that it turned out that object_handlers also always has slots available, we can do this.

Use slot API during cloning

1005f92

Otherwise we can probably run into the same issues here as we could with destruction.

nikic force-pushed the typed_props branch from 7ae0a3e to 1005f92 Compare January 10, 2019 11:44

nikic added 15 commits January 10, 2019 13:14

Remove bogus DEREF

88d966b

Remove unnecessary SEPARATE

8b1a6a3

Prefer checking type prior to modification

d1ac734

Fix ref post incdec without type sources

dc68354

Remove manual ref sources initialization

d7dbf11

Property dump DIM_WRITE/OBJ_WRITE flags

223cd77

Remove bogus PARAM_ZVAL_DEREF

5b7a5b0

Used in conjunction with try_array_init, can't deref beforehand.

Convert checks to assertions

df34631

Use slot API in unserialization

fa59a56

I'm explicitly passing the object to process_nested_data now. When I tried to use Z_OBJ_P(rval) things started failing in odd ways and I have no idea why. The variable doesn't seem to be overwritten...

Fetch class types in property hints in opcache

26139a4

It's rather unlikely that these will be available as CEs at this point, we need to look them up.

Move ASSIGN_OBJ_REF to the right place

4eec659

Support typed properties in opcache file cache

e6510a5

Also handle CE types in persistence

8264d27

I don't think it can happen right now, but will once there is full preloading support for this.

Add UPGRADING entry

6150ddd

Add UPGRADING.INTERNALS notes

7e7b446

nikic closed this Jan 11, 2019

TysonAndre mentioned this pull request Dec 10, 2019

Skip over uninitialized properties when serializing igbinary/igbinary#250

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Implement typed properties #3734

Implement typed properties #3734

Uh oh!

nikic commented Jan 7, 2019

Uh oh!

carusogabriel left a comment

Uh oh!

nikic commented Jan 7, 2019

Uh oh!

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

nikic commented Jan 11, 2019

Uh oh!

moliata commented Mar 24, 2020

Uh oh!

nikic commented Mar 24, 2020

Uh oh!

Uh oh!

Implement typed properties #3734

Implement typed properties #3734

Uh oh!

Conversation

nikic commented Jan 7, 2019

Uh oh!

carusogabriel left a comment

Choose a reason for hiding this comment

Uh oh!

nikic commented Jan 7, 2019

Uh oh!

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

nikic commented Jan 11, 2019

Uh oh!

moliata commented Mar 24, 2020

Uh oh!

nikic commented Mar 24, 2020

Uh oh!

Uh oh!