[PATCH] allow T_INLINE_HTML before T_NAMESPACE

16 years ago by Greg Beaver — view source

unread

Hi,

This is a simple patch that allows files like this:

main.php:

<html> <head> <title>template example</title> </head> </html> <body> <?php namespace my::template; // stuff ?> </body>

to work without parse error.

Greg

P.S. this is the last outstanding namespace issue that I'm aware of
aside from the bracket wars, which is 100% philosophical, all the issues
fixed by my patches are functional problems in namespaces.

16 years ago by David Coallier — view source

unread

2008/9/12 Greg Beaver greg@chiaraquartet.net:

Hi,

This is a simple patch that allows files like this:

main.php:
<html> <head> <title>template example</title> </head> </html> <body> <?php namespace my::template; // stuff ?> </body>

Is it me or this doesn't look really clean? I have a clear idea that
namespaces are there to add an extra structural layer to the code and
not to be randomly used in some HTML/PHP/Javascript mixing.

Since the current namespace implementation doesn't want to have
multiple namespaces per file, I think we should restrict a namespace
per file to... a namespace per file, and nothing else. I can't imagine
taking over someone's code who had declared a namespace under is
footer template.

Perhaps I am missing something but that would be bloody ugly in the end :)

--
Slan,
David

16 years ago by Ryan Panning — view source

unread

David Coallier wrote:

2008/9/12 Greg Beaver greg@chiaraquartet.net:

Hi,

This is a simple patch that allows files like this:

main.php:
<html> <head> <title>template example</title> </head> </html> <body> <?php namespace my::template; // stuff ?> </body>

Is it me or this doesn't look really clean? I have a clear idea that
namespaces are there to add an extra structural layer to the code and
not to be randomly used in some HTML/PHP/Javascript mixing.

Since the current namespace implementation doesn't want to have
multiple namespaces per file, I think we should restrict a namespace
per file to... a namespace per file, and nothing else. I can't imagine
taking over someone's code who had declared a namespace under is
footer template.

Perhaps I am missing something but that would be bloody ugly in the end :)

Since you're looking for input from everyone, I'll throw my 2 cents in.

I agree that this should not be allowed. Even with just a new line or
space, PHP developers should learn that that is not a good practice.

My feelings for namespaces is that it's for frameworks and such. I would
be USING [use ...] inside a PHP/XHTML/JS type of file, not declaring a
namespace.

16 years ago by Marcus Boerger — view source

unread

Hello Greg,

please don't.

marcus

Friday, September 12, 2008, 9:11:39 PM, you wrote:

Hi,

This is a simple patch that allows files like this:

main.php:

<html> <head> <title>template example</title> </head> </html> <body> <?php namespace my::template; // stuff

?>>

</body>

to work without parse error.

Greg

P.S. this is the last outstanding namespace issue that I'm aware of
aside from the bracket wars, which is 100% philosophical, all the issues
fixed by my patches are functional problems in namespaces.

Best regards,
Marcus

16 years ago by Greg Beaver — view source

unread

Marcus Boerger wrote:

Hello Greg,

please don't

OK. Nice working with you Marcus, this is high class stuff. I'm glad
to see the work I'm doing is taken so seriously.

Bye for now,
Greg

16 years ago by Lukas Kahwe Smith — view source

unread

Marcus Boerger wrote:

Hello Greg,

please don't

OK. Nice working with you Marcus, this is high class stuff. I'm glad
to see the work I'm doing is taken so seriously.

Greg, as you can see several people think its better to ensure that
namespace/use statements are always at the top. This is all Marcus was
expressing in his "german straight to the point" kind of way.

That being said, its awesome that you are making sure that when we are
discussing namespaces, that we can talk about what behavior we want,
without having to wonder if its doable or not. This is very much
appreciated!

regards,
Lukas Kahwe Smith
mls@pooteeweet.org

16 years ago by Marcus Boerger — view source

unread

Hello Greg,

Friday, September 12, 2008, 11:35:11 PM, you wrote:

Marcus Boerger wrote:

Hello Greg,

please don't

OK. Nice working with you Marcus, this is high class stuff. I'm glad
to see the work I'm doing is taken so seriously.

Args, sorry if you got this wrong. I think you did some very nice work in
namespace and other areas. Just this specific feature is something I don't
like at all. Hence 'please' don't do it. And there is no more argument to
come... not other than I don't like it.

Best regards,
Marcus

16 years ago by johannes@php.net — view source

unread

Hi,

This is a simple patch that allows files like this:

main.php:
<html>

[...]

<?php
namespace my::template;
// stuff
?>

I'd prefer:

main.php:

<?php
namespace foo;
?>

looks cleaner imo.

johannes

16 years ago by Stan Vassilev | FM — view source

unread

Hi,

I'm in favor of this patch for simple reason that the "nothing before the
first namespace" limitation is more of a WTF than a sound design prevented
to limit an actual problem.

We have multiple namespaces per file, so having some of the file in the
global namespace (i.e. == no namespace) is perfectly natural in that setup.

In fact, I'd argue that we need a way to break out to global namespace
after a namespace too.

Something that would be naturally solved by {} but since this is
discarded... (but you see how it's not purely phylosophical issues either).

Regards,
Stan Vassilev

Hi,

This is a simple patch that allows files like this:

main.php:
<html> <head> <title>template example</title> </head> </html> <body> <?php namespace my::template; // stuff ?> </body>
to work without parse error.

Greg

P.S. this is the last outstanding namespace issue that I'm aware of
aside from the bracket wars, which is 100% philosophical, all the issues
fixed by my patches are functional problems in namespaces.

Index: Zend/zend_compile.c

RCS file: /repository/ZendEngine2/zend_compile.c,v
retrieving revision 1.647.2.27.2.41.2.85
diff -u -u -r1.647.2.27.2.41.2.85 zend_compile.c
--- Zend/zend_compile.c 29 Aug 2008 10:17:08 -0000 1.647.2.27.2.41.2.85
+++ Zend/zend_compile.c 12 Sep 2008 19:07:42 -0000
@@ -504,13 +504,17 @@
}

-void zend_do_echo(const znode *arg TSRMLS_DC)
+void zend_do_echo(const znode *arg, int is_html TSRMLS_DC)
{
zend_op *opline = get_next_op(CG(active_op_array) TSRMLS_CC);

opline->opcode = ZEND_ECHO;
opline->op1 = *arg;

SET_UNUSED(opline->op2);

if (is_html) {

opline->op2.op_type = IS_CONST; /* tell zend_do_namespace that
INLINE_HTML is OK */

} else {

SET_UNUSED(opline->op2);

}
}

void zend_do_abstract_method(const znode *function_name, znode *modifiers,
const znode *body TSRMLS_DC)
@@ -5040,10 +5044,11 @@
char *lcname;

if (CG(active_op_array)->last > 0) {

/* ignore ZEND_EXT_STMT and ZEND_TICKS */
/* ignore ZEND_EXT_STMT and ZEND_TICKS and T_INLINE_HTML */
int num = CG(active_op_array)->last;
while (num > 0 &&
(CG(active_op_array)->opcodes[num-1].opcode == ZEND_EXT_STMT ||
    (CG(active_op_array)->opcodes[num-1].opcode == ZEND_ECHO && 
CG(active_op_array)->opcodes[num-1].op2.op_type == IS_CONST) ||
CG(active_op_array)->opcodes[num-1].opcode == ZEND_TICKS)) {
--num;
}
Index: Zend/zend_compile.h

RCS file: /repository/ZendEngine2/zend_compile.h,v
retrieving revision 1.316.2.8.2.12.2.33
diff -u -u -r1.316.2.8.2.12.2.33 zend_compile.h
--- Zend/zend_compile.h 29 Aug 2008 18:12:47 -0000 1.316.2.8.2.12.2.33
+++ Zend/zend_compile.h 12 Sep 2008 19:07:42 -0000
@@ -385,7 +385,7 @@
void fetch_string_offset(znode *result, const znode *parent, const znode
*offset TSRMLS_DC);
void zend_do_fetch_static_member(znode *result, znode *class_znode
TSRMLS_DC);
void zend_do_print(znode *result, const znode *arg TSRMLS_DC);
-void zend_do_echo(const znode *arg TSRMLS_DC);
+void zend_do_echo(const znode *arg, int is_html TSRMLS_DC);
typedef int (*unary_op_type)(zval *, zval * TSRMLS_DC);
typedef int (*binary_op_type)(zval *, zval *, zval * TSRMLS_DC);
ZEND_API unary_op_type get_unary_op(int opcode);
Index: Zend/zend_language_parser.y

RCS file: /repository/ZendEngine2/zend_language_parser.y,v
retrieving revision 1.160.2.4.2.8.2.26
diff -u -u -r1.160.2.4.2.8.2.26 zend_language_parser.y
--- Zend/zend_language_parser.y 29 Aug 2008 17:54:29 -0000
1.160.2.4.2.8.2.26
+++ Zend/zend_language_parser.y 12 Sep 2008 19:07:42 -0000
@@ -236,7 +236,7 @@
| T_GLOBAL global_var_list ';'
| T_STATIC static_var_list ';'
| T_ECHO echo_expr_list ';'

| T_INLINE_HTML { zend_do_echo(&$1 TSRMLS_CC); }

| T_INLINE_HTML { zend_do_echo(&$1, 1 TSRMLS_CC); }
| expr ';' { zend_do_free(&$1 TSRMLS_CC); }
| T_UNSET '(' unset_variables ')' ';'
| T_FOREACH '(' variable T_AS
@@ -556,8 +556,8 @@
;

echo_expr_list:

echo_expr_list ',' expr { zend_do_echo(&$3 TSRMLS_CC); }

| expr { zend_do_echo(&$1 TSRMLS_CC); }

echo_expr_list ',' expr { zend_do_echo(&$3, 0 TSRMLS_CC); }

| expr { zend_do_echo(&$1, 0 TSRMLS_CC); }
;

Index: Zend/tests/ns_076.phpt

RCS file: Zend/tests/ns_076.phpt
diff -N Zend/tests/ns_076.phpt
--- /dev/null 1 Jan 1970 00:00:00 -0000
+++ Zend/tests/ns_076.phpt 12 Sep 2008 19:07:42 -0000
@@ -0,0 +1,26 @@
+--TEST--
+076: T_INLINE_HTML prior to namespace declaration
+--FILE--
+hi there
+<?php

+namespace foo;

+class Test {

static function f() {

var_dump((binary)NAMESPACE);

include DIR . '/ns_069.inc';

var_dump((binary)NAMESPACE);

}
+}

+Test::f();

+?>
+===DONE===
+--EXPECT--
+hi there
+string(3) "foo"
+string(0) ""
+string(3) "foo"
+===DONE===
Index: Zend/tests/ns_077.phpt

RCS file: Zend/tests/ns_077.phpt
diff -N Zend/tests/ns_077.phpt
--- /dev/null 1 Jan 1970 00:00:00 -0000
+++ Zend/tests/ns_077.phpt 12 Sep 2008 19:07:42 -0000
@@ -0,0 +1,21 @@
+--TEST--
+076: T_ECHO prior to namespace declaration
+--FILE--
+<?php
+echo 'oops';
+namespace foo;

+class Test {

static function f() {

var_dump((binary)NAMESPACE);

include DIR . '/ns_069.inc';

var_dump((binary)NAMESPACE);

}
+}

+Test::f();

+?>
+===DONE===
+--EXPECTF--
+Fatal error: Namespace declaration statement has to be the very first
statement in the script in %sns_077.php on line %d
\ No newline at end of file

16 years ago by Stan Vassilev | FM — view source

unread

Hi,

I just figured a problem related to this.

We have multiple namespaces per file so we can merge multiple files
together. We have no way to break out to global space after a namespace, and
even with this patch, no code can exist before a namespace.

So this means we can only merge files with namespaces in them.

Regards,
Stan Vassilev

Hi,

This is a simple patch that allows files like this:

main.php:
<html> <head> <title>template example</title> </head> </html> <body> <?php namespace my::template; // stuff ?> </body>
to work without parse error.

Greg

P.S. this is the last outstanding namespace issue that I'm aware of
aside from the bracket wars, which is 100% philosophical, all the issues
fixed by my patches are functional problems in namespaces.

Index: Zend/zend_compile.c

RCS file: /repository/ZendEngine2/zend_compile.c,v
retrieving revision 1.647.2.27.2.41.2.85
diff -u -u -r1.647.2.27.2.41.2.85 zend_compile.c
--- Zend/zend_compile.c 29 Aug 2008 10:17:08 -0000 1.647.2.27.2.41.2.85
+++ Zend/zend_compile.c 12 Sep 2008 19:07:42 -0000
@@ -504,13 +504,17 @@
}

-void zend_do_echo(const znode *arg TSRMLS_DC)
+void zend_do_echo(const znode *arg, int is_html TSRMLS_DC)
{
zend_op *opline = get_next_op(CG(active_op_array) TSRMLS_CC);

opline->opcode = ZEND_ECHO;
opline->op1 = *arg;

SET_UNUSED(opline->op2);

if (is_html) {

opline->op2.op_type = IS_CONST; /* tell zend_do_namespace that
INLINE_HTML is OK */

} else {

SET_UNUSED(opline->op2);

}
}

void zend_do_abstract_method(const znode *function_name, znode *modifiers,
const znode *body TSRMLS_DC)
@@ -5040,10 +5044,11 @@
char *lcname;

if (CG(active_op_array)->last > 0) {

/* ignore ZEND_EXT_STMT and ZEND_TICKS */
/* ignore ZEND_EXT_STMT and ZEND_TICKS and T_INLINE_HTML */
int num = CG(active_op_array)->last;
while (num > 0 &&
(CG(active_op_array)->opcodes[num-1].opcode == ZEND_EXT_STMT ||
    (CG(active_op_array)->opcodes[num-1].opcode == ZEND_ECHO &&
CG(active_op_array)->opcodes[num-1].op2.op_type == IS_CONST) ||
CG(active_op_array)->opcodes[num-1].opcode == ZEND_TICKS)) {
--num;
}
Index: Zend/zend_compile.h

RCS file: /repository/ZendEngine2/zend_compile.h,v
retrieving revision 1.316.2.8.2.12.2.33
diff -u -u -r1.316.2.8.2.12.2.33 zend_compile.h
--- Zend/zend_compile.h 29 Aug 2008 18:12:47 -0000 1.316.2.8.2.12.2.33
+++ Zend/zend_compile.h 12 Sep 2008 19:07:42 -0000
@@ -385,7 +385,7 @@
void fetch_string_offset(znode *result, const znode *parent, const znode
*offset TSRMLS_DC);
void zend_do_fetch_static_member(znode *result, znode *class_znode
TSRMLS_DC);
void zend_do_print(znode *result, const znode *arg TSRMLS_DC);
-void zend_do_echo(const znode *arg TSRMLS_DC);
+void zend_do_echo(const znode *arg, int is_html TSRMLS_DC);
typedef int (*unary_op_type)(zval *, zval * TSRMLS_DC);
typedef int (*binary_op_type)(zval *, zval *, zval * TSRMLS_DC);
ZEND_API unary_op_type get_unary_op(int opcode);
Index: Zend/zend_language_parser.y

RCS file: /repository/ZendEngine2/zend_language_parser.y,v
retrieving revision 1.160.2.4.2.8.2.26
diff -u -u -r1.160.2.4.2.8.2.26 zend_language_parser.y
--- Zend/zend_language_parser.y 29 Aug 2008 17:54:29 -0000
1.160.2.4.2.8.2.26
+++ Zend/zend_language_parser.y 12 Sep 2008 19:07:42 -0000
@@ -236,7 +236,7 @@
| T_GLOBAL global_var_list ';'
| T_STATIC static_var_list ';'
| T_ECHO echo_expr_list ';'

| T_INLINE_HTML { zend_do_echo(&$1 TSRMLS_CC); }

| T_INLINE_HTML { zend_do_echo(&$1, 1 TSRMLS_CC); }
| expr ';' { zend_do_free(&$1 TSRMLS_CC); }
| T_UNSET '(' unset_variables ')' ';'
| T_FOREACH '(' variable T_AS
@@ -556,8 +556,8 @@
;

echo_expr_list:

echo_expr_list ',' expr { zend_do_echo(&$3 TSRMLS_CC); }

| expr { zend_do_echo(&$1 TSRMLS_CC); }

echo_expr_list ',' expr { zend_do_echo(&$3, 0 TSRMLS_CC); }

| expr { zend_do_echo(&$1, 0 TSRMLS_CC); }
;

Index: Zend/tests/ns_076.phpt

RCS file: Zend/tests/ns_076.phpt
diff -N Zend/tests/ns_076.phpt
--- /dev/null 1 Jan 1970 00:00:00 -0000
+++ Zend/tests/ns_076.phpt 12 Sep 2008 19:07:42 -0000
@@ -0,0 +1,26 @@
+--TEST--
+076: T_INLINE_HTML prior to namespace declaration
+--FILE--
+hi there
+<?php

+namespace foo;

+class Test {

static function f() {

var_dump((binary)NAMESPACE);

include DIR . '/ns_069.inc';

var_dump((binary)NAMESPACE);

}
+}

+Test::f();

+?>
+===DONE===
+--EXPECT--
+hi there
+string(3) "foo"
+string(0) ""
+string(3) "foo"
+===DONE===
Index: Zend/tests/ns_077.phpt

RCS file: Zend/tests/ns_077.phpt
diff -N Zend/tests/ns_077.phpt
--- /dev/null 1 Jan 1970 00:00:00 -0000
+++ Zend/tests/ns_077.phpt 12 Sep 2008 19:07:42 -0000
@@ -0,0 +1,21 @@
+--TEST--
+076: T_ECHO prior to namespace declaration
+--FILE--
+<?php
+echo 'oops';
+namespace foo;

+class Test {

static function f() {

var_dump((binary)NAMESPACE);

include DIR . '/ns_069.inc';

var_dump((binary)NAMESPACE);

}
+}

+Test::f();

+?>
+===DONE===
+--EXPECTF--
+Fatal error: Namespace declaration statement has to be the very first
statement in the script in %sns_077.php on line %d
\ No newline at end of file

16 years ago by Lucas Stephanou — view source

unread

My first msg in this list

And I agree with you David.

Mix html and namespace is very very ugly.

2008/9/12 David Coallier davidc@php.net:

2008/9/12 Greg Beaver greg@chiaraquartet.net:

Hi,

This is a simple patch that allows files like this:

main.php:
<html> <head> <title>template example</title> </head> </html> <body> <?php namespace my::template; // stuff ?> </body>

Is it me or this doesn't look really clean? I have a clear idea that
namespaces are there to add an extra structural layer to the code and
not to be randomly used in some HTML/PHP/Javascript mixing.

Since the current namespace implementation doesn't want to have
multiple namespaces per file, I think we should restrict a namespace
per file to... a namespace per file, and nothing else. I can't imagine
taking over someone's code who had declared a namespace under is
footer template.

Perhaps I am missing something but that would be bloody ugly in the end :)

--
Slan,
David

--

--
Lucas Stephanou

16 years ago by Elizabeth M Smith — view source

unread

Lucas Stephanou wrote:

My first msg in this list

And I agree with you David.

Mix html and namespace is very very ugly.

2008/9/12 David Coallier davidc@php.net:

2008/9/12 Greg Beaver greg@chiaraquartet.net:

Hi,

This is a simple patch that allows files like this:

main.php:
<html> <head> <title>template example</title> </head> </html> <body> <?php namespace my::template; // stuff ?> </body>

Is it me or this doesn't look really clean? I have a clear idea that
namespaces are there to add an extra structural layer to the code and
not to be randomly used in some HTML/PHP/Javascript mixing.

Since the current namespace implementation doesn't want to have
multiple namespaces per file, I think we should restrict a namespace
per file to... a namespace per file, and nothing else. I can't imagine
taking over someone's code who had declared a namespace under is
footer template.

Perhaps I am missing something but that would be bloody ugly in the end :)

--
Slan,
David

--

--
Lucas Stephanou

Ugh, don't top-post please

the issue is without the patch, even an empty space or a newline at the
start of a file will cause a fatal error

Thanks,
Elizabeth

16 years ago by Stanislav Malyshev — view source

unread

Hi!

the issue is without the patch, even an empty space or a newline at the
start of a file will cause a fatal error

That's good. There should be no unaccounted newlines or empty spaces in
PHP code. I had not once things break because somebody left whitespace
floating around in some script and it broke HTML (or yet more fun, ended
up in some image or PDF).

Stanislav Malyshev, Zend Software Architect
stas@zend.com http://www.zend.com/
(408)253-8829 MSN: stas@zend.com

[PATCH] allow T_INLINE_HTML before T_NAMESPACE

Index: Zend/zend_compile.c

CG(active_op_array)->opcodes[num-1].op2.op_type == IS_CONST) ||
CG(active_op_array)->opcodes[num-1].opcode == ZEND_TICKS)) {
--num;
}
Index: Zend/zend_compile.h

Index: Zend/tests/ns_076.phpt

+?>
+===DONE===
+--EXPECT--
+hi there
+string(3) "foo"
+string(0) ""
+string(3) "foo"
+===DONE===
Index: Zend/tests/ns_077.phpt

Index: Zend/zend_compile.c

CG(active_op_array)->opcodes[num-1].op2.op_type == IS_CONST) ||
CG(active_op_array)->opcodes[num-1].opcode == ZEND_TICKS)) {
--num;
}
Index: Zend/zend_compile.h

Index: Zend/tests/ns_076.phpt

+?>
+===DONE===
+--EXPECT--
+hi there
+string(3) "foo"
+string(0) ""
+string(3) "foo"
+===DONE===
Index: Zend/tests/ns_077.phpt

That's good. There should be no unaccounted newlines or empty spaces in
PHP code. I had not once things break because somebody left whitespace
floating around in some script and it broke HTML (or yet more fun, ended
up in some image or PDF).

[PATCH] allow T_INLINE_HTML before T_NAMESPACE

Index: Zend/zend_compile.c

CG(active_op_array)->opcodes[num-1].op2.op_type == IS_CONST) || CG(active_op_array)->opcodes[num-1].opcode == ZEND_TICKS)) { --num; } Index: Zend/zend_compile.h

Index: Zend/tests/ns_076.phpt

+?> +===DONE=== +--EXPECT-- +hi there +string(3) "foo" +string(0) "" +string(3) "foo" +===DONE=== Index: Zend/tests/ns_077.phpt

Index: Zend/zend_compile.c

CG(active_op_array)->opcodes[num-1].op2.op_type == IS_CONST) || CG(active_op_array)->opcodes[num-1].opcode == ZEND_TICKS)) { --num; } Index: Zend/zend_compile.h

Index: Zend/tests/ns_076.phpt

+?> +===DONE=== +--EXPECT-- +hi there +string(3) "foo" +string(0) "" +string(3) "foo" +===DONE=== Index: Zend/tests/ns_077.phpt

That's good. There should be no unaccounted newlines or empty spaces in PHP code. I had not once things break because somebody left whitespace floating around in some script and it broke HTML (or yet more fun, ended up in some image or PDF).

CG(active_op_array)->opcodes[num-1].op2.op_type == IS_CONST) ||
CG(active_op_array)->opcodes[num-1].opcode == ZEND_TICKS)) {
--num;
}
Index: Zend/zend_compile.h

+?>
+===DONE===
+--EXPECT--
+hi there
+string(3) "foo"
+string(0) ""
+string(3) "foo"
+===DONE===
Index: Zend/tests/ns_077.phpt

CG(active_op_array)->opcodes[num-1].op2.op_type == IS_CONST) ||
CG(active_op_array)->opcodes[num-1].opcode == ZEND_TICKS)) {
--num;
}
Index: Zend/zend_compile.h

+?>
+===DONE===
+--EXPECT--
+hi there
+string(3) "foo"
+string(0) ""
+string(3) "foo"
+===DONE===
Index: Zend/tests/ns_077.phpt

That's good. There should be no unaccounted newlines or empty spaces in
PHP code. I had not once things break because somebody left whitespace
floating around in some script and it broke HTML (or yet more fun, ended
up in some image or PDF).