Newsgroups: php.internals
Path: news.php.net
Xref: news.php.net php.internals:31860
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: <46CCC9D6.8060106@chiaraquartet.net>
Date: Wed, 22 Aug 2007 18:42:14 -0500
User-Agent: Thunderbird 1.5.0.12 (X11/20070604)
MIME-Version: 1.0
To: Stanislav Malyshev <stas@zend.com>
CC: internals Mailing List <internals@lists.php.net>, 
 Dmitry Stogov <dmitry@zend.com>
References: <46CBA2B6.2010600@chiaraquartet.net> <46CC637B.9070500@zend.com>
In-Reply-To: <46CC637B.9070500@zend.com>
Content-Type: multipart/mixed;
 boundary="------------070605060807090101040605"
Subject: Re: [PHP-DEV] [PATCH] bracketed namespace, unset import, removal
 of namespace blah;
From: greg@chiaraquartet.net (Gregory Beaver)

--------------070605060807090101040605
Content-Type: text/plain; charset=ISO-8859-1
Content-Transfer-Encoding: 7bit

Stanislav Malyshev wrote:
>> 1) adds "unset import" syntax for declaring a namespace to have local
>> import scope (it does NOT affect variable scope or the global
>> class/function table)
>
> I don't like it. What's "unset import"? Seems to be very artificial
> concept.
>
>> be as intuitive as possible.  Having a separate scope by default for
>> import that does not inherit from the global scope is not very
>> intuitive.
>
> Yep, it isn't. But it's the only concept that we saw so far that is
> consistent and logical. 
Hi,

The purpose of all my patches is to make it possible to combine multiple
namespaces into a single file.  It looks like namespace {} syntax
introduces much more trouble than it is worth.

As such, since my only goal is to be able to combine multiple files into
a single file, the attached patch basically allows you to take these two
files:

file1.php:
<?php
namespace one;
class whatever {
}
other_php_stuff();
?>

file2.php:
<?php
include 'file1.php';
namespace two;
function thingo() {}
?>

and literally cut/paste them together to get:

<?php
namespace one;
class whatever {
}
other_php_stuff();

namespace two;
function thingo() {}
?>

The patch acts as if each namespace were a separate file with respect to
import statements as well as class/function declarations.

The only limitation is that a situation like so:

file1.php
<?php
include 'file2.php';
function thing() {}
one::thing();
?>

file2.php:
<?php
namespace one;
function thing() {echo 'hi';}
?>

combine erroneously into the "one" namespace

<?php
namespace one;
function thing() {echo 'hi';}

function thing() {}
one::thing();
?>

The solution, which is a simple one, is upon combining to insert a bogus
namespace declaration for the contents of file1.php, and is a simple one
for a build tool or a developer to accomplish.  Developers who would
combine files together would need to scan for include statements if
autoload was not used, and scanning for missing namespace declarations
is trivial in this situation:

<?php
namespace one;
function thing() {echo 'hi';}

namespace __::__main;
function thing() {}
one::thing();
?>

Can we agree on this patch?  It preserves the original simple syntax
that I like so much (no brackets), solves the import scoping problem,
and allows combining files without removing the structure in
namespacing.  It also discourages using multiple namespaces per file, as
the ease of using import across namespace declarations is gone. 
Finally, it preserves the need of the first namespace declaration being
at the top of the file, another appealing feature of the original syntax.

The attached patch is also at
http://pear.php.net/~greg/multiple_namespaces.patch.txt

Thanks for your patience,
Greg

--------------070605060807090101040605
Content-Type: text/plain;
 name="multiple_namespace.patch.txt"
Content-Transfer-Encoding: 7bit
Content-Disposition: inline;
 filename="multiple_namespace.patch.txt"

? multiple_namespace.patch.txt
? namespace.patch.txt
? namespace_brackets_unsetimport.patch.txt
? namespace_smartimport.patch.txt
Index: Zend/zend_compile.c
===================================================================
RCS file: /repository/ZendEngine2/zend_compile.c,v
retrieving revision 1.764
diff -u -r1.764 zend_compile.c
--- Zend/zend_compile.c	22 Aug 2007 07:39:37 -0000	1.764
+++ Zend/zend_compile.c	22 Aug 2007 23:15:02 -0000
@@ -4966,11 +4966,8 @@
 	unsigned int lcname_len;
 	zstr lcname;
 
-	if (CG(active_op_array)->last > 0) {
-		zend_error(E_COMPILE_ERROR, "Namespace declaration statement has to be the very first statement in the script");
-	}
-	if (CG(current_namespace)) {
-		zend_error(E_COMPILE_ERROR, "Namespace cannot be declared twice");
+	if (!CG(current_namespace) && CG(active_op_array)->last > 0) {
+		zend_error(E_COMPILE_ERROR, "First namespace declaration statement has to be the first statement in the script");
 	}
 	lcname = zend_u_str_case_fold(Z_TYPE(name->u.constant), Z_UNIVAL(name->u.constant), Z_UNILEN(name->u.constant), 0, &lcname_len);
 	if (((lcname_len == sizeof("self")-1) &&
@@ -4981,6 +4978,16 @@
 	}
 	efree(lcname.v);
 
+	if (CG(current_namespace)) {
+		zval_dtor(CG(current_namespace));
+		efree(CG(current_namespace));
+	}
+	if (CG(current_import)) {
+		zend_hash_destroy(CG(current_import));
+		efree(CG(current_import));
+	}
+	CG(current_import) = NULL;
+
 	ALLOC_ZVAL(CG(current_namespace));
 	*CG(current_namespace) = name->u.constant;
 }
Index: Zend/tests/ns_039.phpt
===================================================================
RCS file: Zend/tests/ns_039.phpt
diff -N Zend/tests/ns_039.phpt
--- /dev/null	1 Jan 1970 00:00:00 -0000
+++ Zend/tests/ns_039.phpt	22 Aug 2007 23:15:03 -0000
@@ -0,0 +1,35 @@
+--TEST--
+039: two namespace declarations
+--FILE--
+<?php
+namespace Exception;
+class test {
+static function foo() {
+  echo "Exception test::foo\n";
+}
+}
+function foo() {
+  echo "Exception foo\n";
+}
+test::foo();
+foo();
+
+namespace Second;
+import Exception as wow;
+class test {
+static function foo() {
+  echo "Second test::foo\n";
+}
+}
+function foo() {
+  echo "Second foo\n";
+}
+wow::foo();
+test::foo();
+foo();
+--EXPECTF--
+Exception test::foo
+Exception foo
+Exception foo
+Second test::foo
+Second foo
Index: Zend/tests/ns_040.phpt
===================================================================
RCS file: Zend/tests/ns_040.phpt
diff -N Zend/tests/ns_040.phpt
--- /dev/null	1 Jan 1970 00:00:00 -0000
+++ Zend/tests/ns_040.phpt	22 Aug 2007 23:15:03 -0000
@@ -0,0 +1,12 @@
+--TEST--
+040: namespace not first declaration
+--FILE--
+<?php
+$a = 'oops';
+namespace Exception;
+function foo() {
+  echo "ok\n";
+}
+Exception::foo();
+--EXPECTF--
+Fatal error: First namespace declaration statement has to be the first statement in the script in %sns_040.php on line 3
Index: Zend/tests/ns_041.phpt
===================================================================
RCS file: Zend/tests/ns_041.phpt
diff -N Zend/tests/ns_041.phpt
--- /dev/null	1 Jan 1970 00:00:00 -0000
+++ Zend/tests/ns_041.phpt	22 Aug 2007 23:15:03 -0000
@@ -0,0 +1,18 @@
+--TEST--
+041: class in first namespace extends class in second namespace
+--FILE--
+<?php
+namespace Exception;
+import Second as First;
+class test extends First::test {
+}
+new test;
+namespace Second;
+class test {
+function __construct()
+{
+echo 'ok';
+}
+}
+--EXPECT--
+ok
\ No newline at end of file

--------------070605060807090101040605--