Newsgroups: php.internals
Path: news.php.net
Xref: news.php.net php.internals:33889
Mailing-List: contact internals-help@lists.php.net; run by ezmlm
Received-SPF: error (pb1.pair.com: domain chiaraquartet.net from 38.99.98.18 cause and error)
Message-ID: <475CC20C.1020003@chiaraquartet.net>
Date: Sun, 09 Dec 2007 22:35:24 -0600
User-Agent: Thunderbird 2.0.0.6 (X11/20071022)
MIME-Version: 1.0
To: Sam Barrow <sam@sambarrow.com>
CC: Jessie Hernandez <jrhernandez05@gmail.com>, internals@lists.php.net
References: <93ED589E60BA254F97435FE6C97F2C6702B9222A@leedsmet-exch1.leedsmet.ac.uk>	 <475B4F1D.5060601@gmail.com> <475B90BF.2030801@chiaraquartet.net>	 <475C5CF5.4040008@gmail.com> <1197240730.9893.20.camel@sams-room>
In-Reply-To: <1197240730.9893.20.camel@sams-room>
Content-Type: multipart/mixed;
 boundary="------------040701080308060602000905"
Subject: Re: AW: [PHP-DEV] A rebuttal to Re: RFC: Dropping Namespace
From: greg@chiaraquartet.net (Gregory Beaver)

--------------040701080308060602000905
Content-Type: text/plain; charset=ISO-8859-1
Content-Transfer-Encoding: 7bit

Sam Barrow wrote:
> I agree with this 100%, is this something that could be changed? I don't
> see a reason behind it.
>
> On Sun, 2007-12-09 at 16:24 -0500, Jessie Hernandez wrote:
>   
>> internal class/function with the same name.
>>
>> Dmitry, what's the reason this lookup logic wasn't used in your patch?

Hi,

Actually, there several good reasons.  For one, every single unknown
class declaration inside a namespace would require a double check of
class existence, hitting performance.

Here's another example showing the "gotcha" that is inherent:

file1.php:
<?php
namespace Foo;
class IwantThis {}
?>

file2.php:
<?php
class IwantThis {}
?>

file3.php:
<?php
namespace Foo;
include 'file2.php';
function __autoload($class)
{
include 'file1.php'; // stupid but illustrates the point
}
$a = new IwantThis;
?>

Currently, executing file3.php would result in this logic chain:

1) does Foo::IwantThis exist? no => step 2
2) does internal class IwantThis exist? no => step 3
3) try autoloading Foo::IwantThis, loads file1.php, now Foo::IwantThis
exists

If this logic chain were changed to instead check for any globally
defined class of that name, the logic chain would be:

1) does Foo::IwantThis exist?  no => step 2
2) does ::IwantThis class exist? yes => use ::IwantThis

In other words, it would completely break autoloading for namespaced
classes.  Of course, this is the same problem as conflicting with
internal classnames, but the scope of the problem is far larger because
there are so few classes internal to PHP, and even fewer that are likely
to conflict with unqualified names.

So, it could be changed (patch attached for 5.3 to prove the concept,
it's not the best way to do this but shows the way it could be done),
but the price would be an absolute requirement that all classes be
explicitly "use"d in some manner inside a namespace.

Greg

--------------040701080308060602000905
Content-Type: text/plain;
 name="all_classes.patch.txt"
Content-Transfer-Encoding: 7bit
Content-Disposition: inline;
 filename="all_classes.patch.txt"

Index: Zend/zend_execute_API.c
===================================================================
RCS file: /repository/ZendEngine2/zend_execute_API.c,v
retrieving revision 1.331.2.20.2.24.2.14
diff -u -r1.331.2.20.2.24.2.14 zend_execute_API.c
--- Zend/zend_execute_API.c	7 Dec 2007 17:11:23 -0000	1.331.2.20.2.24.2.14
+++ Zend/zend_execute_API.c	10 Dec 2007 04:31:42 -0000
@@ -1601,8 +1601,7 @@
 				php_name++;
 				php_name_len = class_name_len - (php_name - class_name);
 				php_name = zend_str_tolower_dup(php_name, php_name_len);
-				if (zend_hash_find(EG(class_table), php_name, php_name_len + 1, (void **) &pce) == SUCCESS &&
-					(*pce)->type == ZEND_INTERNAL_CLASS
+				if (zend_hash_find(EG(class_table), php_name, php_name_len + 1, (void **) &pce) == SUCCESS
 				) {
 					efree(php_name);
 					return *pce;
Index: Zend/zend_compile.c
===================================================================
RCS file: /repository/ZendEngine2/zend_compile.c,v
retrieving revision 1.647.2.27.2.41.2.29
diff -u -r1.647.2.27.2.41.2.29 zend_compile.c
--- Zend/zend_compile.c	7 Dec 2007 17:11:23 -0000	1.647.2.27.2.41.2.29
+++ Zend/zend_compile.c	10 Dec 2007 04:31:45 -0000
@@ -1599,6 +1599,7 @@
 			       internal one. */
 				*fetch_type |= ZEND_FETCH_CLASS_RT_NS_CHECK;
 			}
+			*fetch_type |= ZEND_FETCH_CLASS_RT_NS_CHECK;
 			tmp.op_type = IS_CONST;
 			tmp.u.constant = *CG(current_namespace);
 			zval_copy_ctor(&tmp.u.constant);

--------------040701080308060602000905--