On Wed, 20 Sep 2006 12:09:28 -0700, Walter Bright wrote:

> I've been interested in various ideas for static checking for common bug patterns for D. For example:
> 
> 	for (int i = 0; i < 10; i++)
> 	{	foo();
> 		break;
> 	}
> 
> would be flagged as a suspicious use of break. Are there any legitimate uses of break in this manner? Any thoughts and ideas in this direction?

I have used this type of thing as a temporary debugging aid. But I suppose I could use ...

 	for (int i = 0; i < 10; i++)
 	{	foo();
 		debug break;
 	}


-- 
Derek Parnell
Melbourne, Australia
"Down with mediocrity!"

Walter Bright wrote:
> Ivan Senji wrote:
>> But is it safe do declare the above example as a bug? Maybe that break is placed there to see what just one pass of the loop does (maybe for debugging purposes).
> 
> That certainly is the question. I don't think I'd want to see such code in released source, but:
> 
> 1) is it legitimate for debug/test code?
> 2) can it come about as the side effect of some other coding pattern?
> 3) if it is made illegal, is that going to be a bigger problem than it solves?
> 4) does it really solve a problem?
> 
> For example, it could happen with (example found on the internet):
> 
>     for (int i = 0; i < 10; i++)
>     {
>         if (condition)
>             action();
>             break;
>     }
> 
> where the user forgot to put { } around the else clause. It's sort of like how:
> 
>     for (int i = 0; i < 10; i++);
>     {
>         ...
>     }
> 
> is illegal in D (note the ; after the closing parenthesis). I've known people to spend many hours trying to track down this nearly invisible bug.

I think this a great idea. The for loop example is one of the so-called incremental improvements in D that saved me quite some time vs C++. I almost forgot how annoying it is to spend time on hunting for bugs that are just typo's.
It's very reasonable for such a tool imho to demand debug code that is otherwise a bug be written with debug version statements, thus I don't see the debug argument as a con.

Derek Parnell wrote:
> I have used this type of thing as a temporary debugging aid. But I suppose
> I could use ...
> 
>  	for (int i = 0; i < 10; i++)
>  	{	foo();
>  		debug break;
>  	}

Also you could not run the lint tool on the temporary debugging code. Like Frits said, it's nice to have a tool that detects forgotten debugging code, so if the tool trips on common debugging aids that might even be useful.

Walter Bright wrote:
> I've been interested in various ideas for static checking for common bug patterns for D. For example:
> 
>     for (int i = 0; i < 10; i++)
>     {    foo();
>         break;
>     }
> 
> would be flagged as a suspicious use of break. Are there any legitimate uses of break in this manner? Any thoughts and ideas in this direction?

Yes, there are. Once I had this problem:

	Obj *v[n], **p;

	for (p = v; *p != NULL; p++) {
	  do_something(*p);
	  if (condition1(*p))
	    continue;	/* failed */

	  do_something_else(*p);
	  if (condition2(*p))
	    continue;	/* failed */
	  ...

	  do_the_last_thing(*p);
	  if (conditionN(*p))
	    continue;	/* failed */

	  break;	/* good! */
	}

	if (*p != NULL)
	  return *p;
	else
	  throw NoSuitableIndividual;

It was an AI application. This function returns the first Obj in array 'v' which survives all 17 conditions of a natural selection process. Of course, the last condition could be rewritten as something like:

	...
	  if (!conditionN(*p))
	    return *p;	/* good! */
	}
	...

But it is just an example.

Frits van Bommel wrote:
> Walter Bright wrote:
>> xs0 wrote:
>>> Walter Bright wrote:
>>>> I've been interested in various ideas for static checking for common bug patterns for D. For example:
>>>>
>>>>     for (int i = 0; i < 10; i++)
>>>>     {    foo();
>>>>         break;
>>>>     }
>>>>
>>>> would be flagged as a suspicious use of break. Are there any legitimate uses of break in this manner? Any thoughts and ideas in this direction?
>>>
>>> Well, I occasionally write something similar:
>>>
>>> for (int i = 0; i < max_retries; i++) {
>>>     try {
>>>         foo();
>>>     } catch (Exception e) {
>>>         continue;
>>>     }
>>>     break;
>>> }
>>
>> Hmm. That does look like a quite reasonable use case.
> 
> It also looks like a quite /detectable/ use case. Just look for a continue statement somewhere in that loop (and outside any nested ones).

Couldn't this be simplified to:

for(...) {
   try {
     foo();
     break;
   } catch(...)
   }
}

Though I guess this doesn't necessarily mean the first form is lint-error-worthy.

Kevin