November 22, 2009
Chad J wrote:
> 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;

You didn't read my post. That is NOT bug-prone. I don't think anyone EVER makes bugs in that form.
It's actually got very little in common with a true fall-through. Real fallthrough involves a 'goto' by stealth: the case introduces an entry point in the middle of a code block.

>> 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.

Fair enough.

> 
>> 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.

Not at all. Requiring changes to existing code is a very big negative to any proposal. If the number of changes required is small,

> 
> 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.

I doubt it.

> 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.

You've missed the point. Andrei made a proposal for eliminating accidental fallthrough bugs. It didn't involve any new syntax, and changed very little existing code. Anything which violates either of those things has very little chance of acceptance. You've done a patch which completely ignores his proposal, and which violates both.

The comment about the auto-generated code raises an aspect which hadn't been considered in the original proposal.
November 22, 2009
On Sun, Nov 22, 2009 at 1:50 AM, Chad J <chadjoan@__spam.is.bad__gmail.com> wrote:
>> 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."

Bad analogy.  It's pretty much impossible to unintentionally put some inline ASM in your code.

--bb
November 22, 2009
Bill Baxter wrote:
> On Sun, Nov 22, 2009 at 1:50 AM, Chad J <chadjoan@__spam.is.bad__gmail.com> wrote:
>>> 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."
> 
> Bad analogy.  It's pretty much impossible to unintentionally put some inline ASM in your code.
> 
> --bb

That's my point.

I was refuting the idea that the argument for removing fallthrough was based on its rarity.

Rarity has little to do with safety.  Intent has everything to do with safety.
November 22, 2009
Don wrote:
> Chad J wrote:
>> 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;
> 
> You didn't read my post. That is NOT bug-prone. I don't think anyone
> EVER makes bugs in that form.
> It's actually got very little in common with a true fall-through. Real
> fallthrough involves a 'goto' by stealth: the case introduces an entry
> point in the middle of a code block.
> 

I DID read your post.  I'm not saying that it is bug prone.  I was just demonstrating how it is not very hard to fix code that is broken by forbidding fallthrough.  I'm explaining why I don't understand how the "machine-generated" code thing is such a big issue.

>>
>>> 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.
> 
> Not at all. Requiring changes to existing code is a very big negative to any proposal. If the number of changes required is small,
> 

continue...

>>
>> But I think you've just gone about
>>> this wrong way.
>>
>> Perhaps.  I'll find out for myself.
> 
> I doubt it.
> 

Ow.

> 
> You've missed the point. Andrei made a proposal for eliminating accidental fallthrough bugs. It didn't involve any new syntax, and changed very little existing code. Anything which violates either of those things has very little chance of acceptance. You've done a patch which completely ignores his proposal, and which violates both.
> 

Sorry I didn't know about Andrei's proposal.  Buried in the NG.

Why didn't you just say that Andrei made another proposal and that you're peeved I didn't use it?

> The comment about the auto-generated code raises an aspect which hadn't been considered in the original proposal.

I also don't get this argument against requiring code changes.  D2 will break your code and is allowed to do so for the sake of progress and long term good.  Everyone knows this.
November 22, 2009
On 11/22/2009 08:22 AM, Don wrote:
>
> You've missed the point. Andrei made a proposal for eliminating
> accidental fallthrough bugs. It didn't involve any new syntax, and
> changed very little existing code. Anything which violates either of
> those things has very little chance of acceptance. You've done a patch
> which completely ignores his proposal, and which violates both.
>
> The comment about the auto-generated code raises an aspect which hadn't
> been considered in the original proposal.

What was Andrei's proposal? If my memory serves (big if), he just wanted to disallow fallthrough and require use of 'goto case' when fallthrough is desired.

I'm having a hard time following this discussion, but where is auto generated code an issue under this patch versus Andrei's proposal versus what we have now? And how does this patch change more code than Andrei's proposal would?

I'm assuming this patch is modified to allow

case 1:
case 2:
	...

November 22, 2009
On 11/20/2009 07:51 PM, 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

<patch critic>

switch(i){
 case 1:       //this should not yield an error. change it.
 case 2:
   blah;
   break;
}

switch(i){
 case 1:
   {
     break;
   }           //this should not yield an error. change it.
}

switch(i){
  default:
    blah;      //this should yield an error. change it.
  case 0:
    case0onlyblah;
}


</patch critic>
November 22, 2009
Ellery Newcomer wrote:
> On 11/20/2009 07:51 PM, 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
> 
> <patch critic>
> 
> switch(i){
>  case 1:       //this should not yield an error. change it.
>  case 2:
>    blah;
>    break;
> }
> 

Reasonable.

> switch(i){
>  case 1:
>    {
>      break;
>    }           //this should not yield an error. change it.
> }
> 

Doable.  But do we want to?  I think it requires semantic analysis. (Yeah I used SA for mine anyways, but mine could have be done in parse as Don mentioned.)

This is a good point to mention at any rate.  I forgot I had to deal with these.  It's one of those things that can break code, even though it has little to do with fallthrough.

> switch(i){
>   default:
>     blah;      //this should yield an error. change it.
>   case 0:
>     case0onlyblah;
> }
> 

It already does.

> 
> </patch critic>

Thanks
- Chad
November 22, 2009
Ellery Newcomer wrote:
> On 11/22/2009 08:22 AM, Don wrote:
>>
>> You've missed the point. Andrei made a proposal for eliminating
>> accidental fallthrough bugs. It didn't involve any new syntax, and
>> changed very little existing code. Anything which violates either of
>> those things has very little chance of acceptance. You've done a patch
>> which completely ignores his proposal, and which violates both.
>>
>> The comment about the auto-generated code raises an aspect which hadn't
>> been considered in the original proposal.
> 
> What was Andrei's proposal? If my memory serves (big if), he just wanted to disallow fallthrough and require use of 'goto case' when fallthrough is desired.
> 
> I'm having a hard time following this discussion, but where is auto generated code an issue under this patch versus Andrei's proposal versus what we have now? 

It's not.

And how does this patch change more code than Andrei's
> proposal would?

> 
> I'm assuming this patch is modified to allow
> 
> case 1:
> case 2:
>     ...

I wasn't. Introducing case xxx!: is the problem.

November 22, 2009
Chad J wrote:
> Don wrote:
>> Chad J wrote:
>>> 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;
>> You didn't read my post. That is NOT bug-prone. I don't think anyone
>> EVER makes bugs in that form.
>> It's actually got very little in common with a true fall-through. Real
>> fallthrough involves a 'goto' by stealth: the case introduces an entry
>> point in the middle of a code block.
>>
> 
> I DID read your post.  I'm not saying that it is bug prone.  I was just
> demonstrating how it is not very hard to fix code that is broken by
> forbidding fallthrough.  I'm explaining why I don't understand how the
> "machine-generated" code thing is such a big issue.
> 
>>>> 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.
>> Not at all. Requiring changes to existing code is a very big negative to
>> any proposal. If the number of changes required is small,
>>
> 
> continue...
> 
>>> But I think you've just gone about
>>>> this wrong way. 
>>> Perhaps.  I'll find out for myself.
>> I doubt it.
>>
> 
> Ow.
> 
>> You've missed the point. Andrei made a proposal for eliminating
>> accidental fallthrough bugs. It didn't involve any new syntax, and
>> changed very little existing code. Anything which violates either of
>> those things has very little chance of acceptance. You've done a patch
>> which completely ignores his proposal, and which violates both.
>>
> 
> Sorry I didn't know about Andrei's proposal.  Buried in the NG.

I'm surprised about that. It's what all the recent posts were based on!

> 
> Why didn't you just say that Andrei made another proposal and that
> you're peeved I didn't use it?

I'm not peeved.

>> The comment about the auto-generated code raises an aspect which hadn't
>> been considered in the original proposal.
> 
> I also don't get this argument against requiring code changes.  D2 will
> break your code and is allowed to do so for the sake of progress and
> long term good.  Everyone knows this.

Yes, but it still needs a good reason to break with C/C++/D1 in a way which increases language complexity.
November 23, 2009
Don wrote:
> Chad J wrote:
>>
>> Sorry I didn't know about Andrei's proposal.  Buried in the NG.
> 
> I'm surprised about that. It's what all the recent posts were based on!
> 

Now I'm confused.

http://www.digitalmars.com/webnews/newsgroups.php?art_group=digitalmars.D&article_id=101110

The recent posts were in a thread that *I* started.

Andrei was the first one in on the action:

Andrei Alexandrescu wrote:
> Chad J wrote:
>> So, switch-case statements are a frequent source of nasty bugs.  Fixing them (well) requires breaking backwards compatibility.
>>
>> Any chance this will happen for D2?
>>
>> (This is intended as more of a reminder and simple curiosity than a
>> discussion.)
>
> I wish very much that a transferring control flow statement (break, return, goto etc.) is required at the end of each case. Then, the rare case when you want to break through is easy to implement as goto case xxx; and all is good.
>
> Walter's answer to that has put me to silence for good. "But I use fall-through all the time!" I then knew the feature will never make it.
>
>
> Andrei

Agreeable words indeed, but not terribly detailed.  I have to imagine you and I are looking at different things.

>>
>> Why didn't you just say that Andrei made another proposal and that you're peeved I didn't use it?
> 
> I'm not peeved.
> 

"You've done a patch which completely ignores his proposal, and which violates both. "

"completely ignores" and "violates".  Strong words.  Ya coulda fooled me.

There's nothing wrong with being peeved ;)

>>> The comment about the auto-generated code raises an aspect which hadn't been considered in the original proposal.
>>
>> I also don't get this argument against requiring code changes.  D2 will break your code and is allowed to do so for the sake of progress and long term good.  Everyone knows this.
> 
> Yes, but it still needs a good reason to break with C/C++/D1 in a way which increases language complexity.

Of course.

I think we disagree on what constitutes a good reason.

I'm all about reaping as many very long-term benefits as possible. Breaking a lot of code for a little gain over a long time is totally justified in my mind.  (And this is a relatively small code breakage.) If D2 takes off, we might be stuck with it for many years.  I'm kinda hoping D3 doesn't happen, at least not unless we have automatic D2->D3 converters and make the migration easy.  I hear python did this very well.  But now I'm rambling, so I'll leave it at that.