"Sara Golemon" pollita@php.net writes:

Here my reservation would be in throwing errors where there previously
wasn't one (this would apply to both Objects and Arrays as offsets).

Prior to release of PHP5 is most certainly the right time to make this sort of
change. There are already other PHP code changes required when porting to
PHP5, so adding warnings/errors where they didn't exist previously, to
indicate an undocumented "feature" that was (likely) being misused, is fine
IMHO. Fixing that sort of thing will become part of the porting process to
PHP5, and will possibly avert a hidden bug.

Derrell

But since there's no BC need to allow resources in array initialization,
then sure, leave it to the scripter to cast it to an int if that's REALLY
what they want. (Again "why?" comes to mind).

PEAR::DB uses this method - I'm seeing a million errors on sites that have
updated to latest CVS version.

You're forced to i use this method if you want to link a resource back to
it's creation information.

If you have an array of IPs, and you create an array of resources from each
IP, a third array linking the IP back to the casted interger value of the
resource is required if you want to get the IP back. (This is very confusing
to write).

Here's an example:

<?php
$servers[] = "139.134.253.11:27025";
$servers[] = "203.114.130.7:27015";

foreach ($servers as $server)
{
list($ip, $port) = explode(':', $server);
$sock = @fsockopen("udp://" . $ip, $port, $errno, $errstr, 1);
if ($sock !== false)
{
$sockets[] = $sock;
// Create an array so we can link the socket back to the server
$server_keys[$sock] = array_search($server, $servers);

}

}

while (stream_select($r, $w = null, $e = null, 0, calctimeout($maxtime,
$starttime)) !== 0)
{
++$i;
foreach ($r as $socket)
{
$response = stream_get_contents($socket);
// Put the response into an array with the original IP as the key
$results[$server_keys[$socket]][] = $response;
}
}
?>

I hope that demonstrates the purpose properly.

Ofcourse this method can still be used, it just requires an extra cast
(which is a cleaner solution anyway).

Regards,
Aidan

Maybe we should move this to an E_STRICT? E_WARNING is more consistent with
existing warnings but I wouldn't want to be over strict, especially when
making this change a couple of weeks before release.
We could escalate the warning for 5.1 if we feel we need to do so.

Andi

At 11:33 AM 6/24/2004 +1000, Aidan Lister wrote:

But since there's no BC need to allow resources in array initialization,
then sure, leave it to the scripter to cast it to an int if that's REALLY
what they want. (Again "why?" comes to mind).

PEAR::DB uses this method - I'm seeing a million errors on sites that have
updated to latest CVS version.

You're forced to i use this method if you want to link a resource back to
it's creation information.

If you have an array of IPs, and you create an array of resources from each
IP, a third array linking the IP back to the casted interger value of the
resource is required if you want to get the IP back. (This is very confusing
to write).

Here's an example:

<?php
$servers[] = "139.134.253.11:27025";
$servers[] = "203.114.130.7:27015";

foreach ($servers as $server)
{
list($ip, $port) = explode(':', $server);
$sock = @fsockopen("udp://" . $ip, $port, $errno, $errstr, 1);
if ($sock !== false)
{
$sockets[] = $sock;
// Create an array so we can link the socket back to the server
$server_keys[$sock] = array_search($server, $servers);

}

}

while (stream_select($r, $w = null, $e = null, 0, calctimeout($maxtime,
$starttime)) !== 0)
{
++$i;
foreach ($r as $socket)
{
$response = stream_get_contents($socket);
// Put the response into an array with the original IP as the key
$results[$server_keys[$socket]][] = $response;
}
}
?>

I hope that demonstrates the purpose properly.

Ofcourse this method can still be used, it just requires an extra cast
(which is a cleaner solution anyway).

Regards,
Aidan

For myself, I agree with you. Using resources as array offsets just
feels
wrong, and I'm more than happy to leave that be. My only concern there
was
consistency with $arr[$res] = $val; behavior which does pickup the lval
and has since ages long past.

But since there's no BC need to allow resources in array initialization,
then sure, leave it to the scripter to cast it to an int if that's REALLY
what they want. (Again "why?" comes to mind).

You are right, which is a sucky inconsistency. Instead of "fixing" array
initialization, maybe the right thing to do, is to add an E_WARNING to
$arr[$res] instead? Or at least an E_STRICT.

I can get behind that approach. That only leaves the severity level and
error text to be decided. Given the behavior gap between these two, I vote
for E_WARNING. How does this sound:

Index: Zend/zend_execute.c

RCS file: /repository/ZendEngine2/zend_execute.c,v
retrieving revision 1.649
diff -u -r1.649 zend_execute.c
--- Zend/zend_execute.c 18 Jun 2004 18:33:46 -0000 1.649
+++ Zend/zend_execute.c 23 Jun 2004 16:30:51 -0000
@@ -839,8 +839,10 @@
}
}
break;

• case IS_DOUBLE:
  case IS_RESOURCE:

• zend_error(E_WARNING, "Resource ID#%d used as offset, casting to integer
  (%d)", dim->value.lval, dim->value.lval);
• /* Fall Through */
• case IS_DOUBLE:
  case IS_BOOL:
  case IS_LONG: {
  long index;
  @@ -3402,6 +3404,7 @@
  zend_hash_update(array_ptr->value.ht, "", sizeof(""), &expr_ptr,
  sizeof(zval *), NULL);
  break;
  default:
• zend_error(E_WARNING, "Illegal offset type");
  zval_ptr_dtor(&expr_ptr);
  /* do nothing */
  break;

The $arr[$obj] = $val; style raises E_WARNING, so it makes sense (to me)
to
do the same with the $arr = array($obj => $val); syntax.

Yep, I agree. We should issue an E_WARNING in this case similar to
$arr[$obj]. We should need to copy the illegal offset error msg.

See second block of above diff...

-Sara

As far as I'm concerned you can go ahead and commit this.

At 09:34 AM 6/23/2004 -0700, Sara Golemon wrote:

For myself, I agree with you. Using resources as array offsets just
feels
wrong, and I'm more than happy to leave that be. My only concern there
was
consistency with $arr[$res] = $val; behavior which does pickup the lval
and has since ages long past.

But since there's no BC need to allow resources in array initialization,
then sure, leave it to the scripter to cast it to an int if that's REALLY
what they want. (Again "why?" comes to mind).

You are right, which is a sucky inconsistency. Instead of "fixing" array
initialization, maybe the right thing to do, is to add an E_WARNING to
$arr[$res] instead? Or at least an E_STRICT.

I can get behind that approach. That only leaves the severity level and
error text to be decided. Given the behavior gap between these two, I vote
for E_WARNING. How does this sound:

Index: Zend/zend_execute.c

RCS file: /repository/ZendEngine2/zend_execute.c,v
retrieving revision 1.649
diff -u -r1.649 zend_execute.c
--- Zend/zend_execute.c 18 Jun 2004 18:33:46 -0000 1.649
+++ Zend/zend_execute.c 23 Jun 2004 16:30:51 -0000
@@ -839,8 +839,10 @@
}
}
break;

• case IS_DOUBLE:
  case IS_RESOURCE:

• zend_error(E_WARNING, "Resource ID#%d used as offset, casting to integer
  (%d)", dim->value.lval, dim->value.lval);
• /* Fall Through */
• case IS_DOUBLE:
  case IS_BOOL:
  case IS_LONG: {
  long index;
  @@ -3402,6 +3404,7 @@
  zend_hash_update(array_ptr->value.ht, "", sizeof(""), &expr_ptr,
  sizeof(zval *), NULL);
  break;
  default:
• zend_error(E_WARNING, "Illegal offset type");
  zval_ptr_dtor(&expr_ptr);
  /* do nothing */
  break;

The $arr[$obj] = $val; style raises E_WARNING, so it makes sense (to me)
to
do the same with the $arr = array($obj => $val); syntax.

Yep, I agree. We should issue an E_WARNING in this case similar to
$arr[$obj]. We should need to copy the illegal offset error msg.

See second block of above diff...

-Sara