Everything

php

Home » Project » Newsgroups » php.internals » Labeled Breaks (not the G-word) »

Labeled Breaks (not the G-word)

php.internals

Sara Golemon

20 years ago

http://libssh2.org/patches/labeled_break.diff

A few notes about the implementation:

* The parser is modified slightly to catch T_BREAK T_LNUMBER ';' and T_BREAK
T_STRING ';' specifically.  This is in-line with the decision to remove the
ability to jump to variable targets.  e.g. `break $foo;` will result in an
E_PARSE.

* current break container is moved from op1.u.opline_num to extended_value
(This means updating continue; as well)

* op1 is used to store the label's name until pass_two(), at that point it's
converted to a jmp_addr.  Non-labeled breaks leave op1 unused and jmp_addr
explicitly set to NULL

* zend_brk opcode handler updated to use op1->u.jmp_addr if available
(labeled breaks), or el->brk (numbered breaks) if not.  zend_brk and
zend_cont both updated per current break container change above.

* Labeled breaks must target the same break container or higher within the
same op_array, and must only jump "forward" in execution.  These checks are
handled in pass_two() which is the earliest point at which all labeled are
declared.

* Label hash is only allocated/initialized if labels are declared.

Known Issues:

* When an error is thrown from pass_two(), the error message states it's in
Unknown on line 0.  Gotta find a better way to handle that....

* This adds an element to zend_op_array so be sure to either `make clean` or
at least `touch Zend/*.c` after applying it otherwise you'll be in segfault
country.

-Sara

Sara Golemon

20 years ago

Set of tests available too now:
http://libssh2.org/patches/labeled_break_tests.tgz

It can be untarred from the root of a PHP dist and it'll unfold into
Zend/tests/break/

Dmitry Stogov

20 years ago

Hi Sara,

I didn't like "goto" in PHP, but after looking in this patch I see that
limited "goto" can work.

1) The new syntax makes mess for me.
"break" is a statement that exits from loop or switch, now we make a "goto"
statement but name it "break".
I belive that if we really implement goto (even with some limitations) we
shuld name it "goto", but not "break".
(changed in attached patch).

2) After (1) I don't see any reason in limitation to jump only down, if we
can jump up as well. (the limitation is removed in attached patch).

2) Your patch doesn't care about freeing loop variables and we got a memory
leaks in the following example (this is fixed in attahed patch).

<?php
foreach (array(1,2,3) as $v) {
break LABEL;
}
LABEL:
?>

3) You can declare one LABEL twich. This should emit comilation error. (this
is fixed in attached patch).

4) Keeping "labels" field in op_array after compilation make not sense as it
used only during compilation and not during execution. (this is fixed in
attached patch) May be we should place this HashTable somewere outside of
op_array.

Now I don't see inconsistencies.
But I still not sure that PHP needs "goto".

Thanks. Dmitry.

Derick Rethans

20 years ago

On Thu, 24 Nov 2005, Dmitry Stogov wrote:

> 1) The new syntax makes mess for me.
> "break" is a statement that exits from loop or switch, now we make a "goto"
> statement but name it "break".
> I belive that if we really implement goto (even with some limitations) we
> shuld name it "goto", but not "break".
> (changed in attached patch).
>
> 2) After (1) I don't see any reason in limitation to jump only down, if we
> can jump up as well. (the limitation is removed in attached patch).

We discussed both those points in the meeting, and afaik agreed on the
current behavior for several reasons:
- "goto" has a bad feeling:

"The name "goto" is misleading, and often associated with BAD
THINGS(tm). Because our proposed solution is not a real GOTO
construct, we will instead reuse the "break" keyword, and extend
it with a static label."

- we wanted to prevent spagetti code which you can do by jumping up:

"Similarly restricting the construct so that you can only jump
down should satisfy people who do not want the ability to jump all over
the place."

regards,
Derick

Dmitry Stogov

20 years ago

Yes, we discussed.
But I wasn't agree with "goto" at all.
Now after looking in the patch, I see that limited "goto" can work.

I still don't like "goto", but reusing name "break" for "goto" is mach
worst.

The following code works fine with Sara's patch:

<?php
while (1) {
break L; /* why is it named "break"? This is "goto"! */
}
if ($a) {
try {
L:
} catch (Exception $e) {
}
}
?>

BTW: some languages has "break label" operatopr but it means exit from loop
marked with specified label (this makes sense).

L: while (1) {
while (1) {
break L; /* the same as "break 2" */
}
}

Thanks. Dmitry.

Edin Kadribasic

20 years ago

Excellent work Dmitry. Hope your version of the patch gets accepted.
Well just go ahead and commit it :)

Edin

Dmitry Stogov wrote:

Ilia A.

20 years ago

Derick Rethans wrote:

> - we wanted to prevent spagetti code which you can do by jumping up:
>
> 	"Similarly restricting the construct so that you can only jump
> 	down should satisfy people who do not want the ability to jump all over
> 	the place."

IMO making this limitation puts a serious cramp of the functionality and
the limiter should be removed.

Ilia

Edin Kadribasic

20 years ago

Ilia Alshanetsky wrote:

> Derick Rethans wrote:
>
>
>>- we wanted to prevent spagetti code which you can do by jumping up:
>>
>>	"Similarly restricting the construct so that you can only jump
>>	down should satisfy people who do not want the ability to jump all over
>>	the place."
>
>
> IMO making this limitation puts a serious cramp of the functionality and
> the limiter should be removed.

I agree, I see no need for this limit.

Edin

Wez Furlong

20 years ago

Agreed; I see no reason to call it break (that makes it even more
confusing), and limiting jumps to one direction only is dumb; there is
no technical reason to do that.

Perl has a "goto" keyword, but it's pronounced "magic goto".  We can
have a "goto" keyword and pronounce it "limited goto".

Bad feelings and anticipated misuse are not good enough reasons to
make this feature obscure.

So, I'm +1 on the patch with sane names and removed restrictions;
Thanks Sara and Dmitry :)

--Wez.

On 11/24/05, Edin Kadribasic <edink@emini.dk> wrote:

Ilia A.

20 years ago

Wez Furlong wrote:

> Perl has a "goto" keyword, but it's pronounced "magic goto".  We can
> have a "goto" keyword and pronounce it "limited goto".

If goto has bad connotations for many people and break seems confusing,
how about calling it "jmp" or "jump" which precisely what the code does.

Ilia

Sascha Schumann

20 years ago

> So, I'm +1 on the patch with sane names and removed restrictions;

Agreed.

- Sascha

Gareth Ardron

20 years ago

Wez Furlong wrote:

>Agreed; I see no reason to call it break (that makes it even more
>confusing), and limiting jumps to one direction only is dumb; there is
>no technical reason to do that.
>
>Perl has a "goto" keyword, but it's pronounced "magic goto".  We can
>have a "goto" keyword and pronounce it "limited goto".
>
>Bad feelings and anticipated misuse are not good enough reasons to
>make this feature obscure.
>
>So, I'm +1 on the patch with sane names and removed restrictions;
>Thanks Sara and Dmitry :)
>

If goto's not right, and break is confusing - what about breakto or
something else that won't cause the confusion of break but also won't
cause a load of wah's from people expecting goto to behave in a
different way ?

Marcus Börger

20 years ago

Hello Wez,

i already see the whole world laughing at us :-)

marcus

Thursday, November 24, 2005, 5:43:48 PM, you wrote:

> Agreed; I see no reason to call it break (that makes it even more
> confusing), and limiting jumps to one direction only is dumb; there is
> no technical reason to do that.

> Perl has a "goto" keyword, but it's pronounced "magic goto".  We can
> have a "goto" keyword and pronounce it "limited goto".

> Bad feelings and anticipated misuse are not good enough reasons to
> make this feature obscure.

> So, I'm +1 on the patch with sane names and removed restrictions;
> Thanks Sara and Dmitry :)

> --Wez.

> On 11/24/05, Edin Kadribasic <edink@emini.dk> wrote:
>> Ilia Alshanetsky wrote:
>> > Derick Rethans wrote:
>> >
>> >
>> >>- we wanted to prevent spagetti code which you can do by jumping up:
>> >>
>> >>      "Similarly restricting the construct so that you can only jump
>> >>      down should satisfy people who do not want the ability to jump all over
>> >>      the place."
>> >
>> >
>> > IMO making this limitation puts a serious cramp of the functionality and
>> > the limiter should be removed.
>>
>> I agree, I see no need for this limit.
>>
>> Edin
>>

Best regards,
Marcus

Zeev Suraski

20 years ago

Guys,

Laughter has nothing to do with it.

However, as discussed between those who attended the meeting in
Paris, goto earned its bad connotations for a reason.  It was agreed
that providing a general-purpose C-goto equivalent is not a good idea
because it *will* very quickly lead to spaghetti coding styles.  We
also came to the conclusion that the main use case that is not
covered by PHP today is that of 'centralized' error recovery, which
requires forward jumping only (if it wasn't for that use case, I
think we would have had consensus not to add any type of goto at all).

So, the logical conclusion appeared to be:

1.  A forward-jumping construct only, to avoid giving users too much
ammo to shoot themselves in the foot with spaghetti coding.
2.  A construct which is not named 'goto', not because of the bad
connotations (that's actually a good reason to keep it named 'goto'),
but in order to make it clear that it's not goto, so that nobody
expects it to behave like goto in other languages.

I'm not sold on reusing the break construct for that, since we're not
really breaking, and I kind of like Ilia's "jump" idea.  Another
possibility would be reusing 'continue', which makes a bit more
English sense, even though it's not very similar to continue's semantics.

Zeev

At 21:42 24/11/2005, Marcus Boerger wrote:

Jani Taskinen

20 years ago

On Sun, 27 Nov 2005, Zeev Suraski wrote:

> I'm not sold on reusing the break construct for that, since we're not really
> breaking, and I kind of like Ilia's "jump" idea.  Another possibility would be
> reusing 'continue', which makes a bit more English sense, even though it's not
> very similar to continue's semantics.

+1 for "jump", which I did suggest in the meeting. :)

--Jani

Ilia A.

20 years ago

Zeev Suraski wrote:

> 1.  A forward-jumping construct only, to avoid giving users too much
> ammo to shoot themselves in the foot with spaghetti coding.

One of the major uses of "jump" is the ability to "retry on error" by
jumping backwards inside the code. Given that the limit of the
functionality is completely superficial, there are no engine reasons for
it, I'd much prefer we did not impose it.

Ilia

Jared Williams

20 years ago

> Guys,
>
> Laughter has nothing to do with it.
>
> However, as discussed between those who attended the meeting
> in Paris, goto earned its bad connotations for a reason.  It
> was agreed that providing a general-purpose C-goto equivalent
> is not a good idea because it *will* very quickly lead to
> spaghetti coding styles.  We also came to the conclusion that
> the main use case that is not covered by PHP today is that of
> 'centralized' error recovery, which requires forward jumping
> only (if it wasn't for that use case, I think we would have
> had consensus not to add any type of goto at all).
>
> So, the logical conclusion appeared to be:
>
> 1.  A forward-jumping construct only, to avoid giving users
> too much ammo to shoot themselves in the foot with spaghetti coding.
> 2.  A construct which is not named 'goto', not because of the
> bad connotations (that's actually a good reason to keep it
> named 'goto'), but in order to make it clear that it's not
> goto, so that nobody expects it to behave like goto in other
> languages.
>
> I'm not sold on reusing the break construct for that, since
> we're not really breaking, and I kind of like Ilia's "jump"
> idea.  Another possibility would be reusing 'continue', which
> makes a bit more English sense, even though it's not very
> similar to continue's semantics.
>

I got a horrid thought that people will end up trying to get around the limitation, abusing the forward only goto and other language
constructs to get it todo backward jumps, and you'll have even worse code than with a backward jumping goto.

Jared

Robert Cummings

20 years ago

On Sun, 2005-11-27 at 07:54, Zeev Suraski wrote:

> Guys,
>
> Laughter has nothing to do with it.
>
> However, as discussed between those who attended the meeting in
> Paris, goto earned its bad connotations for a reason.

Goto got it's bad name from when it was used to jump to line numbers. I
can't think of a case where jumping to a label within current cope is in
any way unclear.

> It was agreed
> that providing a general-purpose C-goto equivalent is not a good idea
> because it *will* very quickly lead to spaghetti coding styles.  We
> also came to the conclusion that the main use case that is not
> covered by PHP today is that of 'centralized' error recovery, which
> requires forward jumping only (if it wasn't for that use case, I
> think we would have had consensus not to add any type of goto at all).
>
> So, the logical conclusion appeared to be:
>
> 1.  A forward-jumping construct only, to avoid giving users too much
> ammo to shoot themselves in the foot with spaghetti coding.

I think it should be called "leap" given your arguments. Because I think
your having a "leap of faith" that you will EVER prevent bad developers
from shooting themselves in the foot.

IMHO you're marginalizing a useful operator to satisfy your perception
of the reputation of goto. I don't think that reputation is well
deserved, and I've never heard of spaghetti goto code in C... which as
we know does allow backward and forward jumping within scope (not to be
confused with none actually existing --- see previous comment about
prevention of foot shooting).

Cheers,
Rob.

--
.------------------------------------------------------------.
| InterJinn Application Framework - http://www.interjinn.com |
:------------------------------------------------------------:
| An application and templating framework for PHP. Boasting  |
| a powerful, scalable system for accessing system services  |
| such as forms, properties, sessions, and caches. InterJinn |
| also provides an extremely flexible architecture for       |
| creating re-usable components quickly and easily.          |
`------------------------------------------------------------'

Dmitry Stogov

20 years ago

One more thing.

The patch has problems with "interactve mode" (php -a).

Thanks. Dmitry.

Sara Golemon

20 years ago

> 1) The new syntax makes mess for me.
> 2) After (1) I don't see any reason in limitation to jump only down,
>

Both of these items were based on the PDM meeting notes which required this
specific behavior.

> 2) Your patch doesn't care about freeing loop variables
> and we got a memory leaks in the following example
>

I was thinking zend_brk_cont() took care of all the containers, forgot about
the outtermost block.

> 3) You can declare one LABEL twice
>

Doi..  Good point.

> 4) Keeping "labels" field in op_array after compilation make no sense
>

Agreed, but CG(labels) wouldn't necessarily work as you could be parsing a
function while in the middle of parsing the main scope and they'd clobber
over each other.  Freeing it at the end of pass_two() is a step in the right
direction of course.

> But I still not sure that PHP needs "goto".
>

TBH, I was surprised to see this on the table in the first place myself...

-Sara

Christian Schneider

20 years ago

Sara Golemon wrote:

> TBH, I was surprised to see this on the table in the first place myself...

To me the summary of events looks like:
1) People disagree whether goto is A Good Thing
2) PDM discusses it and settles for a compromise solution
3) internals removes the restrictions of the compromise
...
?) It is added even though no clear majority wanted a full-blown goto

This view might be a sarcastic one but the decision making process seems
a bit messed up,
- Chris

Ilia A.

20 years ago

Christian Schneider wrote:

> 3) internals removes the restrictions of the compromise

Only one restriction is being removed, it being the ability to jump back
in the code.

> ?) It is added even though no clear majority wanted a full-blown goto

Last time goto was discussed it was a very close decision and discussion
was on a basis of a full blown GOTO support, that even supported dynamic
identifier.

Ilia

Rasmus Lerdorf

20 years ago

Christian Schneider wrote:

> Sara Golemon wrote:
>> TBH, I was surprised to see this on the table in the first place
>> myself...
>
> To me the summary of events looks like:
> 1) People disagree whether goto is A Good Thing
> 2) PDM discusses it and settles for a compromise solution

No, we settled on an initial approach and based on the proposed
patch/implementation we would go from there.  There was no final
decision at the PDM.  In general, things from the PDM is simply 8 people
agreeing, nothing more.  Final decisions are made here on internals.

-Rasmus

Marcus Börger

20 years ago

Hello Sara,

sorry to jump in, i didn'T read the thread but i have more ideas.
From my point of view view should have the whole thing support any
target. So idefinitively want to call it goto and if it cannot be
goto i want to favor log_jump or long_goto or hey what about 'bad'.
Also can't we do goto as an operator? I would suggest one of '==>'
or '>>>' or '=>>'.

Just kidding. But the only way to drop this feature down seems to
be the bulshitting tactics that was used for other features like
the thing called ifsetor. Further more goto would only be for the
advantage of very skillfulled people. To everyone else it simply
would make the code bad n the sense of not understandabele,
unmaintainable and error prone.

best regards
marcus

p.s.: '=>>' would be a conflict.


Wednesday, November 23, 2005, 11:39:30 PM, you wrote:

> http://libssh2.org/patches/labeled_break.diff

> A few notes about the implementation:

> * The parser is modified slightly to catch T_BREAK T_LNUMBER ';' and T_BREAK
> T_STRING ';' specifically.  This is in-line with the decision to remove the
> ability to jump to variable targets.  e.g. `break $foo;` will result in an
> E_PARSE.

> * current break container is moved from op1.u.opline_num to extended_value
> (This means updating continue; as well)

> * op1 is used to store the label's name until pass_two(), at that point it's
> converted to a jmp_addr.  Non-labeled breaks leave op1 unused and jmp_addr
> explicitly set to NULL

> * zend_brk opcode handler updated to use op1->u.jmp_addr if available
> (labeled breaks), or el->brk (numbered breaks) if not.  zend_brk and
> zend_cont both updated per current break container change above.

> * Labeled breaks must target the same break container or higher within the
> same op_array, and must only jump "forward" in execution.  These checks are
> handled in pass_two() which is the earliest point at which all labeled are
> declared.

> * Label hash is only allocated/initialized if labels are declared.

> Known Issues:

> * When an error is thrown from pass_two(), the error message states it's in
> Unknown on line 0.  Gotta find a better way to handle that....

> * This adds an element to zend_op_array so be sure to either `make clean` or
> at least `touch Zend/*.c` after applying it otherwise you'll be in segfault
> country.

> -Sara

Best regards,
Marcus