Jump to page: 1 24  
Page
Thread overview
Switch-case made less buggy, now with PATCH!
Nov 21, 2009
Chad J
Nov 21, 2009
Tim Matthews
Nov 21, 2009
Leandro Lucarella
Nov 21, 2009
Tim Matthews
Nov 21, 2009
Chad J
Nov 21, 2009
Chad J
Nov 21, 2009
Don
Nov 21, 2009
Chad J
Nov 22, 2009
Don
Nov 22, 2009
Chad J
Nov 22, 2009
Don
Nov 22, 2009
Chad J
Nov 22, 2009
Don
Nov 23, 2009
Chad J
Nov 23, 2009
Don
Nov 23, 2009
Chad J
Nov 23, 2009
Leandro Lucarella
Nov 23, 2009
retard
Nov 23, 2009
Leandro Lucarella
Nov 23, 2009
Bill Baxter
Nov 23, 2009
Leandro Lucarella
Nov 23, 2009
Bill Baxter
Nov 24, 2009
Don
Nov 24, 2009
Jason House
Nov 22, 2009
Ellery Newcomer
Nov 22, 2009
Don
Nov 23, 2009
Ellery Newcomer
Nov 22, 2009
Bill Baxter
Nov 22, 2009
Chad J
Nov 25, 2009
BCS
Nov 22, 2009
Ellery Newcomer
Nov 22, 2009
Chad J
November 21, 2009
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
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
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
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
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
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
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
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
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
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