View mode: basic / threaded / horizontal-split · Log in · Help
November 21, 2009
Switch-case made less buggy, now with PATCH!
http://d.puremagic.com/issues/show_bug.cgi?id=3536

So Walter, with this you can keep your beloved fall-through.
Now can the rest of us be spared the nasty fall-through bugs, please
please please??

Also, about assert(0)... I'd be happy to change what I did if Walter and
associates feel that adding assert(0) to the list is worth its minor
complications.

(Sorry I don't have a patch for properties, but that one's harder.)

- Chad
November 21, 2009
Re: Switch-case made less buggy, now with PATCH!
Chad J wrote:
> http://d.puremagic.com/issues/show_bug.cgi?id=3536
> 
> So Walter, with this you can keep your beloved fall-through.
> Now can the rest of us be spared the nasty fall-through bugs, please
> please please??
> 
> Also, about assert(0)... I'd be happy to change what I did if Walter and
> associates feel that adding assert(0) to the list is worth its minor
> complications.
> 
> (Sorry I don't have a patch for properties, but that one's harder.)
> 
> - Chad

I like having both fall through and breaking out explicit but was the 
final syntax ever discussed here first?

Other possible options include 'fallthrough;' or just have the usual 
goto case for falling through too and let the compiler's optimization 
routines remove the unnecessary jumps when it sees the goto case as the 
next in sequence.
November 21, 2009
Re: Switch-case made less buggy, now with PATCH!
Tim Matthews, el 21 de noviembre a las 18:10 me escribiste:
> Chad J wrote:
> >http://d.puremagic.com/issues/show_bug.cgi?id=3536
> >
> >So Walter, with this you can keep your beloved fall-through.
> >Now can the rest of us be spared the nasty fall-through bugs, please
> >please please??
> >
> >Also, about assert(0)... I'd be happy to change what I did if Walter and
> >associates feel that adding assert(0) to the list is worth its minor
> >complications.
> >
> >(Sorry I don't have a patch for properties, but that one's harder.)
> >
> >- Chad
> 
> I like having both fall through and breaking out explicit but was
> the final syntax ever discussed here first?
> 
> Other possible options include 'fallthrough;' or just have the usual
> goto case for falling through too and let the compiler's
> optimization routines remove the unnecessary jumps when it sees the
> goto case as the next in sequence.

There is already goto case; (without specifying the case) for that, at
least in the specs :)

http://www.digitalmars.com/d/2.0/statement.html#GotoStatement

GotoStatement:
  goto Identifier ;
  goto default ;
  goto case ;
  goto case Expression ;

[...]

The third form, goto case;, transfers to the next CaseStatement of the
innermost enclosing SwitchStatement.

-- 
Leandro Lucarella (AKA luca)                     http://llucax.com.ar/
----------------------------------------------------------------------
GPG Key: 5F5A8D05 (F8CD F9A7 BF00 5431 4145  104C 949E BFB6 5F5A 8D05)
----------------------------------------------------------------------
Every year is getting shorter never seem to find the time.
Plans that either come to nought or half a page of scribbled lines.
Hanging on in quiet desparation is the English way.
The time is gone, the song is over, thought I'd something more to say.
November 21, 2009
Re: Switch-case made less buggy, now with PATCH!
Leandro Lucarella wrote:
> 
> There is already goto case; (without specifying the case) for that, at
> least in the specs :)
> 
> http://www.digitalmars.com/d/2.0/statement.html#GotoStatement
> 
> GotoStatement:
>    goto Identifier ;
>    goto default ;
>    goto case ;
>    goto case Expression ;
> 
> [...]
> 
> The third form, goto case;, transfers to the next CaseStatement of the
> innermost enclosing SwitchStatement.
> 

Which is a very in your face explicit fall through. So we understand the 
advantages of making fall through explicit yet for some reason we need 
to have a more discreet kind of explicit syntax?
November 21, 2009
Re: Switch-case made less buggy, now with PATCH!
Tim Matthews wrote:
> 
> Which is a very in your face explicit fall through. So we understand the
> advantages of making fall through explicit yet for some reason we need
> to have a more discreet kind of explicit syntax?

If it manages to make Walter happy enough to accept the patch, it's
totally worth it.

It makes porting the code easier too, since you don't need to know where
to put the goto.
November 21, 2009
Re: Switch-case made less buggy, now with PATCH!
Tim Matthews wrote:
> Chad J wrote:
>> http://d.puremagic.com/issues/show_bug.cgi?id=3536
>>
>> So Walter, with this you can keep your beloved fall-through.
>> Now can the rest of us be spared the nasty fall-through bugs, please
>> please please??
>>
>> Also, about assert(0)... I'd be happy to change what I did if Walter and
>> associates feel that adding assert(0) to the list is worth its minor
>> complications.
>>
>> (Sorry I don't have a patch for properties, but that one's harder.)
>>
>> - Chad
> 
> I like having both fall through and breaking out explicit but was the
> final syntax ever discussed here first?
> 

I had forgotten to add that to my test case until after I submitted the
enhancement request.  I've tested it now, and it seems to work fine.

This works under the patch:

enum Foo
{
	a,
	b,
}

void main()
{
	Foo blah = Foo.a;
	final switch(blah)
	{
		case Foo.a!:
		case Foo.b: break;
	}
}

> Other possible options include 'fallthrough;' or just have the usual
> goto case for falling through too and let the compiler's optimization
> routines remove the unnecessary jumps when it sees the goto case as the
> next in sequence.

I figured this obscure token !: that no one is ever going to use for
anything else will be much easier to swallow than a new keyword or a new
kind of switch.

I'd actually be fine with just killing fallthrough altogether, but
Walter wants it (and maybe a few others too).  As I ported phobos code I
also realized it was much easier to just add the !: syntax to the
compiler than it would have been to try and put gotos everywhere.
November 21, 2009
Re: Switch-case made less buggy, now with PATCH!
Chad J wrote:
> http://d.puremagic.com/issues/show_bug.cgi?id=3536
> 
> So Walter, with this you can keep your beloved fall-through.
> Now can the rest of us be spared the nasty fall-through bugs, please
> please please??
> 
> Also, about assert(0)... I'd be happy to change what I did if Walter and
> associates feel that adding assert(0) to the list is worth its minor
> complications.
> 
> (Sorry I don't have a patch for properties, but that one's harder.)
> 
> - Chad

Things like

case A:
case B:
      foo();
      break;

do not involve fallthrough bugs. If there's a bug in that code at all, 
it's that case A doesn't do anything. Empty case statements are not 
bug-prone.

The thing is, that with the "goto case" syntax, D already has support 
for explicit fallthrough. No new syntax is required.

BTW accidental fallthrough can be detected in the parse step, no 
semantic analysis is required, which makes it particularly easy to 
implement in a lint tool.
November 21, 2009
Re: Switch-case made less buggy, now with PATCH!
Don wrote:
> 
> Things like
> 
> case A:
> case B:
>       foo();
>       break;
> 
> do not involve fallthrough bugs. If there's a bug in that code at all,
> it's that case A doesn't do anything. Empty case statements are not
> bug-prone.
> 

My intent is not to forbid these.  I suppose I've tried to damage them
as little as possible.  Letting them slip through without notation would
do better on that count.  It's a trade-off between that and easier
(general) fallthrough syntax.

With this syntax the code required to convert C-style switch-case to
D-style becomes much easier.  You can just look for case and replace :
with !:.  Otherwise you have to figure out where to put the goto case
statement, which is usually easy enough for a human, but can be tricky
for a program that doesn't have a D parser.  Unless you are OK with
generating some garbage:

switch(foo)
{
   case 0:
       switch(bar)
       {
           case 0:
           case 1:
           // but for case 2:
           ...
       }

   case 1:
   ...
}

becomes

switch(foo)
{
   case 0:
       switch(bar)
       {
           goto case 0; case 0: <-- yuck.
           goto case 1; case 1:
           // but for goto case 2; case 2:
           ...
       }

   goto case 1; case 1:
   ...
}

You could always just follow compiler errors though, so it's a minor point.

> The thing is, that with the "goto case" syntax, D already has support
> for explicit fallthrough. No new syntax is required.
> 

I know this.  Walter knows this.  Why hasn't it been given emphatic
thumbs up?

I suspect it's because he's either busy with more important things or
really enjoys fallthrough.

Given how easy it is to remove implicit fallthrough and given that he's
expressed a strong fondness of the fallthrough behavior, I suspect it's
more the latter than the former.

At any rate, I've decided to solve both.  So here's a patch to make it
even easier to enact (especially docs + lib migration), and also a
syntax to keep fallthrough easy and compact where desired.

If the syntax is a bad idea, then it can be ditched, and I'll make a new
patch.  All Walter has to do is say the word.

> BTW accidental fallthrough can be detected in the parse step, no
> semantic analysis is required, which makes it particularly easy to
> implement in a lint tool.

Lint tools
- Don't exist for D. (AFAIK)
- Add another step to my workflow/build process.  The compiler should be
doing this stuff anyways.

So yeah, I don't really care much for lint tools.  Let's just Do It
Right and not need any lint tools.

- Chad
November 22, 2009
Re: Switch-case made less buggy, now with PATCH!
Chad J wrote:
> Don wrote:
>> Things like
>>
>> case A:
>> case B:
>>       foo();
>>       break;
>>
>> do not involve fallthrough bugs. If there's a bug in that code at all,
>> it's that case A doesn't do anything. Empty case statements are not
>> bug-prone.
>>
> 
> My intent is not to forbid these.  I suppose I've tried to damage them
> as little as possible   Letting them slip through without notation would
> do better on that count.  It's a trade-off between that and easier
> (general) fallthrough syntax.

If you think general fallthrough syntax is important, you've 
misunderstood the argument for this feature. See below.

> With this syntax the code required to convert C-style switch-case to
> D-style becomes much easier.  You can just look for case and replace :
> with !:.  Otherwise you have to figure out where to put the goto case
> statement, which is usually easy enough for a human, but can be tricky
> for a program that doesn't have a D parser.  Unless you are OK with
> generating some garbage:

I think you've just created the strongest argument AGAINST this feature: 
that it makes it too hard for machine-generated code.  Forget the !: 
hack. No chance.

>> The thing is, that with the "goto case" syntax, D already has support
>> for explicit fallthrough. No new syntax is required.
>>
> 
> I know this.  Walter knows this.  Why hasn't it been given emphatic
> thumbs up?
> 
> I suspect it's because he's either busy with more important things or
> really enjoys fallthrough.

He made it very, very clear that he thinks that legitimate fallthrough 
is common. If he's right, then this whole thing is a bad idea.

With something like this, you're really wasting your time making a patch 
for it. Patches are only worthwhile in the cases where Walter hasn't 
said anything, or in which he made a positive comment but nothing has 
happened for a long time. Once he's said something negative about a 
feature, providing a patch is not going to change his mind.

BTW, it's _really_ difficult to get an unsolicited patch into DMD. It's 
happened about twice ever.

> Given how easy it is to remove implicit fallthrough and given that he's
> expressed a strong fondness of the fallthrough behavior, I suspect it's
> more the latter than the former.
> 
> At any rate, I've decided to solve both.  So here's a patch to make it
> even easier to enact (especially docs + lib migration), and also a
> syntax to keep fallthrough easy and compact where desired.
> 
> If the syntax is a bad idea, then it can be ditched, and I'll make a new
> patch.  All Walter has to do is say the word.

The argument for it comes down to this: "fallthrough is very rare and 
therefore, most uses of fallthrough are bugs".  If many modifications to 
(say) Phobos are required, then the argument for this feature is false.
From a comment someone made previously, there were about three 
instances of it in Phobos. If you found you needed to make many changes 
than that, I'll switch sides to Walter's camp. But I think you've just 
gone about this wrong way. Although, the "auto-generated code" argument 
would now need to be addressed.
November 22, 2009
Re: Switch-case made less buggy, now with PATCH!
Don wrote:
> 
> I think you've just created the strongest argument AGAINST this feature:
> that it makes it too hard for machine-generated code.  Forget the !:
> hack. No chance.
> 

If that's the strongest argument, then this is cake.  I'll go through
Phobos and insert goto case's by hand if that's what needs to happen.

Most of the fallthrough I saw was stuff like this:

case '1': case '2': case '3':
case '4': case '5': case '6':
case '7': case '8': case '9':
...etc...
break;

Without fallthrough this is easily rewritten as

case '1': .. case '9':

or

case '1', '2', '3', '4', '5', '6', '7', '8', '9':

Not the easiest thing to make a sed expression do, but quite obvious by
hand.  No !: needed.

Anyhow, "hack"?  "No chance."?  You feel quite strongly.

> 
> He made it very, very clear that he thinks that legitimate fallthrough
> is common. If he's right, then this whole thing is a bad idea.
> 
> With something like this, you're really wasting your time making a patch
> for it. Patches are only worthwhile in the cases where Walter hasn't
> said anything, or in which he made a positive comment but nothing has
> happened for a long time. Once he's said something negative about a
> feature, providing a patch is not going to change his mind.
> 
> BTW, it's _really_ difficult to get an unsolicited patch into DMD. It's
> happened about twice ever.
> 

I feel this matters enough that I will roll the dice anyways.

> 
> The argument for it comes down to this: "fallthrough is very rare and
> therefore, most uses of fallthrough are bugs".  

Not as I remember.

"inline assembly is very rare and therefore, most uses of inline
assembly are bugs."
Such reasoning wouldn't get inline asm removed either.  Fallthrough is
not bad because it is rare.  I think that's only mentioned because the
rarity makes it easier to let go of fallthrough altogether.

If many modifications to
> (say) Phobos are required, then the argument for this feature is false.
> From a comment someone made previously, there were about three instances
> of it in Phobos. If you found you needed to make many changes than that,
> I'll switch sides to Walter's camp. 

"needed".  Heck no.  "wanted" would be more appropriate.  I could have
minimized changes much more, but that costs a little in terms of the
language being nice in the long term.

There are way more than three fallthroughs in Phobos2.  Most of it is
empty cases.  Axing fallthrough also axes empty cases, unless you
explicitly say otherwise (and I didn't).  Empty cases can be easily
rewritten into equivalent code that is also clean and pleasing.

I also minded the "default: assert(0);" pattern, quite common, which may
or may not be fallthrough depending upon how smart the compiler wants to
be.  If the compiler wants to be dumb, then this becomes "default:
assert(0); break;" or similar.  I was being conservative and letting it
be dumb.  At least in dmd's case it's easy to recognize "assert(0);" as
unconditional branching, so it need not be dumb.

But I think you've just gone about
> this wrong way. 

Perhaps.  I'll find out for myself.

Also, I'll say it again in a slightly different way:  I'd be fine with
adding no new syntax and making a special case for empty case
statements.  Really I would.

Although, the "auto-generated code" argument would now
> need to be addressed.

Is the auto-generated code thing really important enough to /need/ to
address it?  I know I mentioned it, but I also called it "a minor point".

Man, I add a small nicety and it becomes some kind of uber awesome
argument against the much more important issue.  Sorry, I don't quite
follow the logic.
« First   ‹ Prev
1 2 3 4
Top | Discussion index | About this forum | D home