Re: Inconsistencies in 5.3

Hello Marcus, Tuesday, August 12, 2008, 3:36:36 PM, you wrote: > Hello Etienne, > Tuesday, August 12, 2008, 3:04:48 AM, you wrote: >> Hello, >> On Tue, Aug 12, 2008 at 2:39 AM, Stanislav Malyshev <stas@zend.com> wrote: >>> Hi! >>> >>>> but if the {} syntax is introduced, it will be made to affect only the >>>> code inside it, right? If so, I fail to see your point, since the new >>>> syntax will solve that problem. >>> >>> While introducing a whole collection of new problems - such as that we will >>> have now split scope, that you can get in and out many times in the same >>> file. As we discussed on the list many times, it's not exactly what we want >>> and definitely not the use case it was designed for. I don't see any use for >>> it besides promoting bad coding practices. >> To me >> namespace A { >> code >> } >> namespace B { >> code >> } >> code >> seems equivalent to >> namespace A; >> code >> namespace B; >> code >> namespace <none>; >> code >> Only nicer. And I can hardly how it's going to cause more problems? >> But if that's so, fine. >> My point is that if we are going to allow multiple namespaces per file >> solely on the perspective of permitting packaging, we should also >> allow mixing namespaced and non-namespaced code for that same >> perspective, and the current syntax is not going to allow that. >> I'd really like the {} syntax if multiple namespaces per file is >> allowed. If it's not, then it's not much more than syntactic sugar and >> I couldn't care less. > Exact same reasoning here. Now, I want to follow up with two things: > I) personal recent encounters > II) collection of argumenmts > III) personal preference > IV) what to do next > I) personal recent encounters > Technically it is pretty easy to come up with parser/lexer changes that > allow anything you want. Starting from real parser support for only one > namespace as the first statement per file down to muliple nested > namespaces after some initial code outside any namespace as in global, > patch for that attached. It does not include zend_language_scanner.c > and is not fully functionally. it just allows getting a feeling for > namespaces with curlies and and a potential limitationt to have non > namespaces code only prior to the first namespace keyword. Also it > could be done much easier but I wanted the extended info stuff the way > it is. > At the end of the day my biggest issues her are: > 1) Namespaces without curly braces and concatenation of files lead to > difference code behavior. > I am not saying that curly braces would solve this fully per se, as you > could still use include/require statements. My point is rather that we > do not addredss this issue at all right now. > And btw, with curly braces we could at least disallow include/require > inside namespaces. Just as we don't allow include/require in classes > outside methods. > 2) When we allow multiple namespaces per file for concatenation, then > why do we not allow code outside of namespaces. Because sooner or > later we want that too. We would want that for the exact reason we > want to have multiple namespaces today - concatenation of files. > However without curly braces we can never concatenate files with code > outside namespaces. > 3) We are moving in the wrong direction here. Instead of keeping the > language easy we make it more complex to understand and deal with. And > the biggest reasons brought up so far are, circled around > - concatenation for speed > - whitespace > - which other language we relate to > I am really sorry for having to keep this running, but we are running > into somethign extremely bad. And it doesn't even work out in the long > run. > Another example. Remember when we first introduced cli? We thought of > adding a new file type that wouldn't need PIs (<?php...?>). We luckily > decided against that idea to keep the language easy. Without curlies we > will sooner or later be in a situation where there are two kinds of php > files. Those with namespaces and those without and they wouldn't be > able to cooperate. > II) collection of argumenmts > Let me try to summarize up to this point. The following options have been > outlined and discussed in more detail: > 1) Kepp all as is: multiple 'namespace' declarations per file, no {} > + For some people this means no additional whitespace (other people do > not have this issue, yet counted as +) > 0 No direct nesting support (some like it, some don't) > 0 Looks and feels more or less like Java packages > 0 Original namespaces in pre 5.0beta had curlies > 0 Is different from C++'s namespaces so it should look different, > more in: http://news.php.net/php.internals/39838 > - Indirect nesting (only): php -r 'namespace foo; namespace foo::bar;' > - Like Java but allows multiple per file and has different keyword > - Multiple namespaces per file as a hack that is not meant to be used > - Two types of scripts, those with and those without namespaces, which > also prevents concatenation as a general solution if one file has > global code > - A file included in a namespaced method is not in that same namespace, > if we fix this we end up in different behavior of include/require for > namespe/non namespace files. If we disallow that, we make it harder > to understand when it is allowed and when not. > - A file included from a namespaced function/method body ends up in > that function method but not in its namespace: > http://lxr.php.net/source/ZendEngine2/tests/ns_069.phpt > http://lxr.php.net/source/ZendEngine2/tests/ns_069.inc > 2) Change keyword 'namespace' to 'package' > 0 Besides the file concatenation this would be exactly what Java does > - Namespace has been a reserved keyword, added 15th August 2007: > > > http://cvs.php.net/viewvc.cgi/phpdoc/en/appendices/reserved.xml?r1=1.66&r2=1.67 > - 'Package' is not a reserved keyword at all > [The arguments of 1) aply] > 2b) Like 2) but no multiple namespaces per file (no file concatenation) > 0 Phar can be used to put several file in one anyway > 3) Add curly braces to 'namespace' > + Real nesting (some people don't like it but we support it anyway) > + Allows for global, non namespaced code, and thus full concatenation of > files > 0 Looks and feels exactly like C++ (besides we need to discuss whether to > allow code outside namespace). > 0 Makes it a block structure like class which uses {} > - For some people this means additional whitespace (other people do not > have this issue, yet counted as -) > Why reuse keyword 'namespace' to identify root namespace (namespace::)? > - Why not use ':::' instead (re2c allows that easily) ? > Why no non namespced code? > - Issues with compiler caches > Do we need to support concatenation of files at all costs? > - We actually ship phar which allows to pack as many namespaced files > in a single php file, no matter what namespace support we end up > allowing. > III) personal preference > My preference is 3) and the next one would be 2) without file > concatenation support, as in one namespace per file only. > IV) what to do next > Someone should collect more arguments from the various threads (I am > sure there are more) and put them on a wiki. This pretty much looks > like a job for Lukas :-) In a pretty long IRC discussion we discussed besides other stuff the following: a) Prevent include/require inside namespaced functions. Patch: php-no-include-in-namespaced-functions-6.0-20080812.diff.txt Should pretty much work. b) Allow both 'namspace foo;' as well as 'namespace foo {}'. Patch: php-namespaces-with-curlies-6.0-20080812.diff.txt Note that this patch does not come with test updates and necessary error message adjustage. Best regards, Marcus

Index: Zend/zend.c =================================================================== RCS file: /repository/ZendEngine2/zend.c,v retrieving revision 1.421 diff -u -p -d -r1.421 zend.c --- Zend/zend.c 12 Aug 2008 17:15:58 -0000 1.421 +++ Zend/zend.c 12 Aug 2008 20:11:22 -0000 @@ -874,6 +874,7 @@ static void compiler_globals_ctor(zend_c CG(interactive) = 0; CG(literal_type) = ZEND_STR_TYPE; + CG(function_nest_level) = 0; compiler_globals->auto_globals = (HashTable *) malloc(sizeof(HashTable)); zend_u_hash_init_ex(compiler_globals->auto_globals, global_auto_globals_table->nNumOfElements, NULL, NULL, 1, UG(unicode), 0); Index: Zend/zend_compile.c =================================================================== RCS file: /repository/ZendEngine2/zend_compile.c,v retrieving revision 1.837 diff -u -p -d -r1.837 zend_compile.c --- Zend/zend_compile.c 12 Aug 2008 17:15:59 -0000 1.837 +++ Zend/zend_compile.c 12 Aug 2008 20:11:23 -0000 @@ -1216,6 +1216,7 @@ void zend_do_begin_function_declaration( zstr lcname; zend_bool orig_interactive; + CG(function_nest_level)++; if (is_method) { if (CG(active_class_entry)->ce_flags & ZEND_ACC_INTERFACE) { if ((Z_LVAL(fn_flags_znode->u.constant) & ~(ZEND_ACC_STATIC|ZEND_ACC_PUBLIC))) { @@ -1498,6 +1499,7 @@ void zend_do_end_function_declaration(co zend_do_extended_info(TSRMLS_C); zend_do_return(NULL, 0 TSRMLS_CC); + CG(function_nest_level)--; pass_two(CG(active_op_array) TSRMLS_CC); @@ -4487,6 +4489,16 @@ void zend_do_include_or_eval(int type, z { zend_op *opline = get_next_op(CG(active_op_array) TSRMLS_CC); + if (type != ZEND_EVAL && CG(function_nest_level) + && CG(current_namespace) && Z_STRLEN_P(CG(current_namespace))) + { + zend_error(E_COMPILE_WARNING, + "Include or require statement inside function %v%s%v inside namespace %R", + CG(active_class_entry) ? CG(active_class_entry)->name : EMPTY_ZSTR, + CG(active_class_entry) ? "::" : "", + CG(active_op_array)->function_name.v, + Z_TYPE_P(CG(current_namespace)), Z_UNIVAL_P(CG(current_namespace))); + } opline->opcode = ZEND_INCLUDE_OR_EVAL; opline->result.op_type = IS_VAR; opline->result.u.var = get_temporary_variable(CG(active_op_array)); Index: Zend/zend_globals.h =================================================================== RCS file: /repository/ZendEngine2/zend_globals.h,v retrieving revision 1.180 diff -u -p -d -r1.180 zend_globals.h --- Zend/zend_globals.h 11 Aug 2008 17:19:01 -0000 1.180 +++ Zend/zend_globals.h 12 Aug 2008 20:11:23 -0000 @@ -88,6 +88,7 @@ struct _zend_compiler_globals { char *heredoc; int heredoc_len; + int function_nest_level; zend_op_array *active_op_array; Index: Zend/tests/ns_069.phpt =================================================================== RCS file: /repository/ZendEngine2/tests/ns_069.phpt,v retrieving revision 1.1 diff -u -p -d -r1.1 ns_069.phpt --- Zend/tests/ns_069.phpt 12 Aug 2008 11:51:34 -0000 1.1 +++ Zend/tests/ns_069.phpt 12 Aug 2008 20:11:24 -0000 @@ -3,22 +3,36 @@ --FILE-- <?php -namespace foo; +namespace foo::bar; + +function funcA() { + var_dump((binary)__NAMESPACE__); + include __DIR__ . '/ns_069.inc'; + var_dump((binary)__NAMESPACE__); +} class Test { - static function f() { + static function funcB() { var_dump((binary)__NAMESPACE__); include __DIR__ . '/ns_069.inc'; var_dump((binary)__NAMESPACE__); } } -Test::f(); +funcA(); +Test::funcB(); ?> ===DONE=== ---EXPECT-- -string(3) "foo" +--EXPECTF-- + +Warning: Include or require statement inside function foo::bar::funcA inside namespace foo::bar in %sns_069.php on line %d + +Warning: Include or require statement inside function foo::bar::Test::funcB inside namespace foo::bar in %sns_069.php on line %d +string(8) "foo::bar" string(0) "" -string(3) "foo" +string(8) "foo::bar" +string(8) "foo::bar" +string(0) "" +string(8) "foo::bar" ===DONE=== Index: Zend/zend_compile.c =================================================================== RCS file: /repository/ZendEngine2/zend_compile.c,v retrieving revision 1.837 diff -u -p -d -r1.837 zend_compile.c --- Zend/zend_compile.c 12 Aug 2008 17:15:59 -0000 1.837 +++ Zend/zend_compile.c 12 Aug 2008 20:47:30 -0000 @@ -5382,7 +5382,21 @@ void zend_do_build_namespace_name(znode } /* }}} */ -void zend_do_namespace(const znode *name TSRMLS_DC) /* {{{ */ +void zend_do_end_namespace(TSRMLS_D) /* {{{ */ +{ + if (CG(current_namespace)) { + zval_dtor(CG(current_namespace)); + CG(current_namespace) = NULL; + } + if (CG(current_import)) { + zend_hash_destroy(CG(current_import)); + efree(CG(current_import)); + CG(current_import) = NULL; + } +} +/* }}} */ + +void zend_do_begin_namespace(const znode *name TSRMLS_DC) /* {{{ */ { unsigned int lcname_len; zstr lcname; @@ -5408,16 +5446,8 @@ void zend_do_namespace(const znode *name } efree(lcname.v); - if (CG(current_namespace)) { - zval_dtor(CG(current_namespace)); - } else { - ALLOC_ZVAL(CG(current_namespace)); - } - if (CG(current_import)) { - zend_hash_destroy(CG(current_import)); - efree(CG(current_import)); - CG(current_import) = NULL; - } + zend_do_end_namespace(TSRMLS_C); + ALLOC_ZVAL(CG(current_namespace)); *CG(current_namespace) = name->u.constant; } Index: Zend/zend_compile.h =================================================================== RCS file: /repository/ZendEngine2/zend_compile.h,v retrieving revision 1.388 diff -u -p -d -r1.388 zend_compile.h --- Zend/zend_compile.h 12 Aug 2008 17:15:59 -0000 1.388 +++ Zend/zend_compile.h 12 Aug 2008 20:47:30 -0000 @@ -543,7 +543,8 @@ void zend_do_abstract_method(const znode void zend_do_declare_constant(znode *name, znode *value TSRMLS_DC); void zend_do_build_namespace_name(znode *result, znode *prefix, znode *name TSRMLS_DC); -void zend_do_namespace(const znode *name TSRMLS_DC); +void zend_do_begin_namespace(const znode *name TSRMLS_DC); +void zend_do_end_namespace(TSRMLS_D); void zend_do_use(znode *name, znode *new_name, int is_global TSRMLS_DC); void zend_do_end_compilation(TSRMLS_D); Index: Zend/zend_language_parser.y =================================================================== RCS file: /repository/ZendEngine2/zend_language_parser.y,v retrieving revision 1.210 diff -u -p -d -r1.210 zend_language_parser.y --- Zend/zend_language_parser.y 12 Aug 2008 10:22:57 -0000 1.210 +++ Zend/zend_language_parser.y 12 Aug 2008 20:47:30 -0000 @@ -153,17 +153,23 @@ %% /* Rules */ start: - top_statement_list { zend_do_end_compilation(TSRMLS_C); } + top_statement_or_namespace_list { zend_do_end_compilation(TSRMLS_C); } ; -top_statement_list: - top_statement_list { zend_do_extended_info(TSRMLS_C); } top_statement { HANDLE_INTERACTIVE(); } +top_statement_or_namespace_list: + top_statement_or_namespace_list { zend_do_extended_info(TSRMLS_C); } top_statement_or_namespace { HANDLE_INTERACTIVE(); } | /* empty */ ; -namespace_name: - T_STRING { $$ = $1; } - | namespace_name T_PAAMAYIM_NEKUDOTAYIM T_STRING { zend_do_build_namespace_name(&$$, &$1, &$3 TSRMLS_CC); } +top_statement_or_namespace: + top_statement + | T_NAMESPACE namespace_name ';' { zend_do_begin_namespace(&$2 TSRMLS_CC); } + | T_NAMESPACE namespace_name '{' { zend_do_begin_namespace(&$2 TSRMLS_CC); } top_statement_list '}' { zend_do_end_namespace(TSRMLS_C); } +; + +top_statement_list: + top_statement_list { zend_do_extended_info(TSRMLS_C); } top_statement { HANDLE_INTERACTIVE(); } + | /* empty */ ; top_statement: @@ -171,11 +177,15 @@ top_statement: | function_declaration_statement { zend_do_early_binding(TSRMLS_C); } | class_declaration_statement { zend_do_early_binding(TSRMLS_C); } | T_HALT_COMPILER '(' ')' ';' { zend_do_halt_compiler_register(TSRMLS_C); YYACCEPT; } - | T_NAMESPACE namespace_name ';' { zend_do_namespace(&$2 TSRMLS_CC); } | T_USE use_declarations ';' | constant_declaration ';' ; +namespace_name: + T_STRING { $$ = $1; } + | namespace_name T_PAAMAYIM_NEKUDOTAYIM T_STRING { zend_do_build_namespace_name(&$$, &$1, &$3 TSRMLS_CC); } +; + use_declarations: use_declarations ',' use_declaration | use_declaration