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

Strict and Safe-Indexing for Hack Arrays #6454

Closed
dlreeves opened this Issue Oct 30, 2015 · 3 comments

Comments

Projects
None yet
5 participants
@dlreeves
Contributor

dlreeves commented Oct 30, 2015

This is a part of the Hack array proposal

Indexing for Hack arrays will have stricter semantics when using square brackets to match the behavior of Hack Collections.

No Coercion of Keys

When indexing or setting a key of an array, PHP sometimes coerces a value to either an int or string. Hack arrays should not do this. Indexing a Hack array with an invalid key type results in an exception. This also means int-like strings will not be converted to ints.

// These will result in exceptions for Hack arrays
$harray[0.0];
$harray[false];

// The string '0' will not be converted to the int 0
$harray['0'];

Throw on Non-existent Keys

Accessing an non-existent key for a PHP array results in raising a E_NOTICE and returning null. Hack arrays will throw an exception if you attempt to access an element that is not present.

Safe-Indexing

If you do not want to risk an exception being thrown when indexing into a Hack array then you can index using curly braces. Currently indexing with curly braces is the same as indexing with square brackets for PHP arrays. We will slightly change this behavior to return null when the key is not set, but NOT raise a E_NOTICE.

$dict = dict[];
$x = $dict{0}; // $x will be null
$y = $dict[0]; // Exception thrown since 0 is not set

This is meant to emulate the get methods that the various Collection APIs provide.

@simonwelsh

This comment has been minimized.

Show comment
Hide comment
@simonwelsh

simonwelsh Oct 31, 2015

Contributor

I don't like the differing behaviour between using [] and {} for access. Looks too much like an accidental typo and requires a different understanding of the symbols (as they're currently equivalent).

How common would using the {} syntax be? Is it common enough to need a shorthand for idx?

Contributor

simonwelsh commented Oct 31, 2015

I don't like the differing behaviour between using [] and {} for access. Looks too much like an accidental typo and requires a different understanding of the symbols (as they're currently equivalent).

How common would using the {} syntax be? Is it common enough to need a shorthand for idx?

@deadalnix

This comment has been minimized.

Show comment
Hide comment
@deadalnix

deadalnix Oct 31, 2015

Contributor

ditto the {} syntax seems unnecessary. Using idx seems like the right way to get null on missing entry.

Contributor

deadalnix commented Oct 31, 2015

ditto the {} syntax seems unnecessary. Using idx seems like the right way to get null on missing entry.

@jwatzman jwatzman added the hack label Nov 8, 2015

@dlreeves

This comment has been minimized.

Show comment
Hide comment
@dlreeves

dlreeves Feb 29, 2016

Contributor

Given the feedback we are going to forgo adding this feature for now. Especially with the ?? operator this seems like it would be a bit redundant.

Thanks for your input

Contributor

dlreeves commented Feb 29, 2016

Given the feedback we are going to forgo adding this feature for now. Especially with the ?? operator this seems like it would be a bit redundant.

Thanks for your input

hhvm-bot pushed a commit that referenced this issue Feb 29, 2016

Runtime support for DictArray behavior
Summary:Runtime support for new DictArrat behavior.  The dictionary type does
not implicitly convert int-like string keys to ints nor does it silently coerce
invalid key types. For invalid keys (bool, float, object, etc) an exception
will be thrown.

Additionally attempting to read a key not contained in the array will throw an
exception rather than resulting in the usual warning/null return.

See: http://hhvm.com/blog/10649/improving-arrays-in-hack
Related to #6452
Related to #6454

Reviewed By: swtaarrs

Differential Revision: D2933186

fb-gh-sync-id: fbbd690adcac73748a48f18333101975770c1245
shipit-source-id: fbbd690adcac73748a48f18333101975770c1245

hhvm-bot pushed a commit that referenced this issue Mar 1, 2016

JIT support for DictArray behavior
Summary:Adds support for new DictArray behavior to the JIT, see dependent diff
for details.

See: http://hhvm.com/blog/10649/improving-arrays-in-hack
Related to #6452
Related to #6454

Reviewed By: swtaarrs

Differential Revision: D2933192

fb-gh-sync-id: c8b6a04c26c77843ea30ec2927c5c5b00c102749
shipit-source-id: c8b6a04c26c77843ea30ec2927c5c5b00c102749

hhvm-bot pushed a commit that referenced this issue May 4, 2016

Add new array-data operations for Hack arrays
Summary:
Add new array-data operations NvTryGetInt, NvTryGetStr, LValIntRef, and
LValStrRef. NvTryGetInt and NvTryGetStr are similar to NvGetInt and NvGetStr
except they're allowed (but not required) to throw if the key isn't
present. LValIntRef and LValStrRef are likewise similar to LValInt and LValStr
except they indicate to the array that the retrieved value will be boxed into a
ref. This is for array types which don't allow refs. In addition, formalize
LValNewRef, which indicates to the array that the new value will be boxed into a
ref.

Try to use the DISPATCH macro in array-data.cpp consistently, using constexpr
aliases in the array implementations to re-use functions.

See: http://hhvm.com/blog/10649/improving-arrays-in-hack
Related to #6451
Related to #6454

Reviewed By: paulbiss

Differential Revision: D3162677

fb-gh-sync-id: d75e1b554d3c153511177ee2564dfe183c90b888
fbshipit-source-id: d75e1b554d3c153511177ee2564dfe183c90b888

hhvm-bot pushed a commit that referenced this issue May 4, 2016

Some misc pre-vec cleanup
Summary:
- Modify throwInvalidArrayKeyException() to take a pointer to the array in
question. This allows it to provide a more accurate message depending on the
array type. In addition, move the definition of throwInvalidArrayKeyException
and throwOOBArrayException into array-data.cpp since there's no need to inline
them.

- Modify Variant::toKey() to take a pointer to the array meant for the key. This
is needed to thread the array pointer to the throwInvalidArrayKeyException()
call. With this pointer, the boolean about whether to perform key conversion
is no longer needed (it can be retrieved from the array directly).

- Modify array_key_exists() to use the same logic regarding key conversion for
arrays without weak keys as is now used in Variant::toKey.

See: http://hhvm.com/blog/10649/improving-arrays-in-hack
Related to #6451
Related to #6454

Reviewed By: paulbiss

Differential Revision: D3162696

fb-gh-sync-id: 65e1f247c315884923ca2733999ba548c78081eb
fbshipit-source-id: 65e1f247c315884923ca2733999ba548c78081eb

hhvm-bot pushed a commit that referenced this issue May 5, 2016

Make PackedArray helpers more generic
Summary:
Make the helper functions used by PackedArray more generic by now longer
assuming the array kind is Packed. The functions will now either copy the kind
of the source parameter, or take the kind as an explicit parameter. In the later
case, the helpers are forced inline, so that the kind parameter will be constant
propagated. This is in preparation of the hack array vec kind, which uses most
of the packed array machinery, but has a different kind.

In addition, move PackedArray::MakePacked (where it isn't used) from
mixed-array.cpp to packed-array.cpp.

There was an implicit invariant that packed arrays all have a non-zero capacity
(thus Grow won't be called on them when empty), which isn't necessarily true for
hack vec arrays, so avoid a memcpy when growing an empty array.

See: http://hhvm.com/blog/10649/improving-arrays-in-hack
Related to #6451
Related to #6454

Reviewed By: paulbiss

Differential Revision: D3163072

fb-gh-sync-id: a20a2b4a136dcf42754587e836f79b4cd0941e9e
fbshipit-source-id: a20a2b4a136dcf42754587e836f79b4cd0941e9e

hhvm-bot pushed a commit that referenced this issue May 5, 2016

Add basic support for Hack vec array
Summary:
Add basic support for Hack vec arrays. Vec arrays are similar to packed arrays
except they cannot contain references, they throw on trying to access
non-present keys, their keys don't undergo automatic conversions, and they never
change to non-packed representation (under most conditions). This commit only
adds basic support in the runtime, no support for creating them, nor any support
for builtins, nor any JIT support.

See: http://hhvm.com/blog/10649/improving-arrays-in-hack
Related to #6451
Related to #6454

Reviewed By: paulbiss

Differential Revision: D3163304

fb-gh-sync-id: 21928f4091d5cd129b7f7991a80c151316cf2673
fbshipit-source-id: 21928f4091d5cd129b7f7991a80c151316cf2673

hhvm-bot pushed a commit that referenced this issue May 5, 2016

Add VecArrayInit
Summary:
Add VecArrayInit, a class used to create Hack vec arrays in the runtime. Since
you can only add new elements to a vec array by appending, appending is the only
operation supported.

See: http://hhvm.com/blog/10649/improving-arrays-in-hack
Related to #6451
Related to #6454

Reviewed By: paulbiss

Differential Revision: D3163367

fb-gh-sync-id: 5491448dfa2e2e43e37f5269d3abcabd7dcf8b25
fbshipit-source-id: 5491448dfa2e2e43e37f5269d3abcabd7dcf8b25

hhvm-bot pushed a commit that referenced this issue May 5, 2016

Add ToVec array-data function
Summary:
Add a new function to array-data, ToVec. This converts the array to an
equivalent Hack vec array. The conversion is done by discarding the keys and
inserting the values into the new vec array in iteration order. If the array is
already a vec array, itself is returned unchanged. For all array types except
PackedArray, the implementation of the conversion is the same, so defer to a
common helper function to perform it. For a non-vec PackedArray, the conversion
can be done more efficiently since it shares a representation with vec
arrays. The conversion can throw if the source array contains a non-collapsable
reference.

See: http://hhvm.com/blog/10649/improving-arrays-in-hack
Related to #6451
Related to #6454

Reviewed By: paulbiss

Differential Revision: D3163710

fb-gh-sync-id: 230644c2c932e5849015c36bf15609872f8c1573
fbshipit-source-id: 230644c2c932e5849015c36bf15609872f8c1573

hhvm-bot pushed a commit that referenced this issue May 5, 2016

Add serializer/unserializer support for Hack vec arrays
Summary:
Add serializer/unserializer support for Hack vec arrays. Unlike normal PHP
arrays, no keys are serialized (because they're implicit by the ordering), and
the code 'v' is used to mark them. An additional check is needed during
deserializer to check for references.

See: http://hhvm.com/blog/10649/improving-arrays-in-hack
Related to #6451
Related to #6454

Reviewed By: paulbiss

Differential Revision: D3163779

fb-gh-sync-id: bfbfa145b05ae32d72101c0e74adbfae399c3cb2
fbshipit-source-id: bfbfa145b05ae32d72101c0e74adbfae399c3cb2

hhvm-bot pushed a commit that referenced this issue May 6, 2016

Add HHBC and HHIR support for Hack vec arrays
Summary:
Add a new HHBC instruction to create a Hack vec array. Also add new HHIR
instructions to allocate a vec array and to convert a PHP array to a vec array.

See: http://hhvm.com/blog/10649/improving-arrays-in-hack
Related to #6451
Related to #6454

Reviewed By: paulbiss

Differential Revision: D3163925

fb-gh-sync-id: 8054b19e501a5cc5c5df9679de5401360d4b89dd
fbshipit-source-id: 8054b19e501a5cc5c5df9679de5401360d4b89dd

hhvm-bot pushed a commit that referenced this issue May 6, 2016

Add parser support for Hack vec arrays
Summary:
Add parser support for Hack vec arrays. This includes literal vecs like "vec[1,
2, 3]" and the converting constructor named "vec()".

See: http://hhvm.com/blog/10649/improving-arrays-in-hack
Related to #6451
Related to #6454

Reviewed By: paulbiss

Differential Revision: D3164001

fb-gh-sync-id: 76988820f6cb41b822adae833253fdeacd8c046a
fbshipit-source-id: 76988820f6cb41b822adae833253fdeacd8c046a

hhvm-bot pushed a commit that referenced this issue May 6, 2016

Add test cases for Hack vec arrays
Summary:
Add test cases for Hack vec arrays. This was separated out into the final diff
to allow for easier to review.

See: http://hhvm.com/blog/10649/improving-arrays-in-hack
Related to #6451
Related to #6454

Reviewed By: paulbiss

Differential Revision: D3164026

fb-gh-sync-id: e46412c2ef4685f55b7857bb254b6fead799a82c
fbshipit-source-id: e46412c2ef4685f55b7857bb254b6fead799a82c

racardoso pushed a commit to PPC64/hhvm that referenced this issue May 17, 2016

Add new array-data operations for Hack arrays
Summary:
Add new array-data operations NvTryGetInt, NvTryGetStr, LValIntRef, and
LValStrRef. NvTryGetInt and NvTryGetStr are similar to NvGetInt and NvGetStr
except they're allowed (but not required) to throw if the key isn't
present. LValIntRef and LValStrRef are likewise similar to LValInt and LValStr
except they indicate to the array that the retrieved value will be boxed into a
ref. This is for array types which don't allow refs. In addition, formalize
LValNewRef, which indicates to the array that the new value will be boxed into a
ref.

Try to use the DISPATCH macro in array-data.cpp consistently, using constexpr
aliases in the array implementations to re-use functions.

See: http://hhvm.com/blog/10649/improving-arrays-in-hack
Related to facebook#6451
Related to facebook#6454

Reviewed By: paulbiss

Differential Revision: D3162677

fb-gh-sync-id: d75e1b554d3c153511177ee2564dfe183c90b888
fbshipit-source-id: d75e1b554d3c153511177ee2564dfe183c90b888

racardoso pushed a commit to PPC64/hhvm that referenced this issue May 17, 2016

Some misc pre-vec cleanup
Summary:
- Modify throwInvalidArrayKeyException() to take a pointer to the array in
question. This allows it to provide a more accurate message depending on the
array type. In addition, move the definition of throwInvalidArrayKeyException
and throwOOBArrayException into array-data.cpp since there's no need to inline
them.

- Modify Variant::toKey() to take a pointer to the array meant for the key. This
is needed to thread the array pointer to the throwInvalidArrayKeyException()
call. With this pointer, the boolean about whether to perform key conversion
is no longer needed (it can be retrieved from the array directly).

- Modify array_key_exists() to use the same logic regarding key conversion for
arrays without weak keys as is now used in Variant::toKey.

See: http://hhvm.com/blog/10649/improving-arrays-in-hack
Related to facebook#6451
Related to facebook#6454

Reviewed By: paulbiss

Differential Revision: D3162696

fb-gh-sync-id: 65e1f247c315884923ca2733999ba548c78081eb
fbshipit-source-id: 65e1f247c315884923ca2733999ba548c78081eb

racardoso pushed a commit to PPC64/hhvm that referenced this issue May 17, 2016

Make PackedArray helpers more generic
Summary:
Make the helper functions used by PackedArray more generic by now longer
assuming the array kind is Packed. The functions will now either copy the kind
of the source parameter, or take the kind as an explicit parameter. In the later
case, the helpers are forced inline, so that the kind parameter will be constant
propagated. This is in preparation of the hack array vec kind, which uses most
of the packed array machinery, but has a different kind.

In addition, move PackedArray::MakePacked (where it isn't used) from
mixed-array.cpp to packed-array.cpp.

There was an implicit invariant that packed arrays all have a non-zero capacity
(thus Grow won't be called on them when empty), which isn't necessarily true for
hack vec arrays, so avoid a memcpy when growing an empty array.

See: http://hhvm.com/blog/10649/improving-arrays-in-hack
Related to facebook#6451
Related to facebook#6454

Reviewed By: paulbiss

Differential Revision: D3163072

fb-gh-sync-id: a20a2b4a136dcf42754587e836f79b4cd0941e9e
fbshipit-source-id: a20a2b4a136dcf42754587e836f79b4cd0941e9e

racardoso pushed a commit to PPC64/hhvm that referenced this issue May 17, 2016

Add basic support for Hack vec array
Summary:
Add basic support for Hack vec arrays. Vec arrays are similar to packed arrays
except they cannot contain references, they throw on trying to access
non-present keys, their keys don't undergo automatic conversions, and they never
change to non-packed representation (under most conditions). This commit only
adds basic support in the runtime, no support for creating them, nor any support
for builtins, nor any JIT support.

See: http://hhvm.com/blog/10649/improving-arrays-in-hack
Related to facebook#6451
Related to facebook#6454

Reviewed By: paulbiss

Differential Revision: D3163304

fb-gh-sync-id: 21928f4091d5cd129b7f7991a80c151316cf2673
fbshipit-source-id: 21928f4091d5cd129b7f7991a80c151316cf2673

racardoso pushed a commit to PPC64/hhvm that referenced this issue May 17, 2016

Add VecArrayInit
Summary:
Add VecArrayInit, a class used to create Hack vec arrays in the runtime. Since
you can only add new elements to a vec array by appending, appending is the only
operation supported.

See: http://hhvm.com/blog/10649/improving-arrays-in-hack
Related to facebook#6451
Related to facebook#6454

Reviewed By: paulbiss

Differential Revision: D3163367

fb-gh-sync-id: 5491448dfa2e2e43e37f5269d3abcabd7dcf8b25
fbshipit-source-id: 5491448dfa2e2e43e37f5269d3abcabd7dcf8b25

racardoso pushed a commit to PPC64/hhvm that referenced this issue May 17, 2016

Add ToVec array-data function
Summary:
Add a new function to array-data, ToVec. This converts the array to an
equivalent Hack vec array. The conversion is done by discarding the keys and
inserting the values into the new vec array in iteration order. If the array is
already a vec array, itself is returned unchanged. For all array types except
PackedArray, the implementation of the conversion is the same, so defer to a
common helper function to perform it. For a non-vec PackedArray, the conversion
can be done more efficiently since it shares a representation with vec
arrays. The conversion can throw if the source array contains a non-collapsable
reference.

See: http://hhvm.com/blog/10649/improving-arrays-in-hack
Related to facebook#6451
Related to facebook#6454

Reviewed By: paulbiss

Differential Revision: D3163710

fb-gh-sync-id: 230644c2c932e5849015c36bf15609872f8c1573
fbshipit-source-id: 230644c2c932e5849015c36bf15609872f8c1573

racardoso pushed a commit to PPC64/hhvm that referenced this issue May 17, 2016

Add serializer/unserializer support for Hack vec arrays
Summary:
Add serializer/unserializer support for Hack vec arrays. Unlike normal PHP
arrays, no keys are serialized (because they're implicit by the ordering), and
the code 'v' is used to mark them. An additional check is needed during
deserializer to check for references.

See: http://hhvm.com/blog/10649/improving-arrays-in-hack
Related to facebook#6451
Related to facebook#6454

Reviewed By: paulbiss

Differential Revision: D3163779

fb-gh-sync-id: bfbfa145b05ae32d72101c0e74adbfae399c3cb2
fbshipit-source-id: bfbfa145b05ae32d72101c0e74adbfae399c3cb2

racardoso pushed a commit to PPC64/hhvm that referenced this issue May 17, 2016

Add HHBC and HHIR support for Hack vec arrays
Summary:
Add a new HHBC instruction to create a Hack vec array. Also add new HHIR
instructions to allocate a vec array and to convert a PHP array to a vec array.

See: http://hhvm.com/blog/10649/improving-arrays-in-hack
Related to facebook#6451
Related to facebook#6454

Reviewed By: paulbiss

Differential Revision: D3163925

fb-gh-sync-id: 8054b19e501a5cc5c5df9679de5401360d4b89dd
fbshipit-source-id: 8054b19e501a5cc5c5df9679de5401360d4b89dd

racardoso pushed a commit to PPC64/hhvm that referenced this issue May 17, 2016

Add parser support for Hack vec arrays
Summary:
Add parser support for Hack vec arrays. This includes literal vecs like "vec[1,
2, 3]" and the converting constructor named "vec()".

See: http://hhvm.com/blog/10649/improving-arrays-in-hack
Related to facebook#6451
Related to facebook#6454

Reviewed By: paulbiss

Differential Revision: D3164001

fb-gh-sync-id: 76988820f6cb41b822adae833253fdeacd8c046a
fbshipit-source-id: 76988820f6cb41b822adae833253fdeacd8c046a

racardoso pushed a commit to PPC64/hhvm that referenced this issue May 17, 2016

Add test cases for Hack vec arrays
Summary:
Add test cases for Hack vec arrays. This was separated out into the final diff
to allow for easier to review.

See: http://hhvm.com/blog/10649/improving-arrays-in-hack
Related to facebook#6451
Related to facebook#6454

Reviewed By: paulbiss

Differential Revision: D3164026

fb-gh-sync-id: e46412c2ef4685f55b7857bb254b6fead799a82c
fbshipit-source-id: e46412c2ef4685f55b7857bb254b6fead799a82c

@Orvid Orvid closed this Sep 28, 2016

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment