char[] annoyance...

[Regan Heath]

On Mon, 10 Apr 2006 17:42:33 +0300, Georg Wrede <georg.wrede at nospam.org>  
wrote:
> Regan Heath wrote:
>> On Mon, 10 Apr 2006 11:58:09 +1000, Derek Parnell <derek at psych.ward>  
>> wrote:
>>
>>> On Mon, 10 Apr 2006 12:23:06 +1200, Regan Heath wrote:
>>>
>>>> Take this code:
>>>>
>>>> void main()
>>>> {
>>>>     //..open a file, read line for line, on each line:
>>>>
>>>>     for(int i = 0; i < line.length-2; i++) {
>>>>         if (line[i..i+2] != "||") continue;
>>>>         //..etc..
>>>>     }
>>>> }
>>>>
>>>> There is a subtle bug. On all lines with a length of 0 or 1 it will  
>>>> give
>>>> the following error:
>>>>
>>>> Error: ArrayBoundsError line_length.d(6)
>>>>
>>>> The problem is of course the statement "i < line.length-2".  
>>>> line.length  is
>>>> unsigned, and when you - 2 from an unsigned value.. well lets just say
>>>> that it's bigger than the actual length of the line - 2.
>>>>
>>>> Of course there are plently of other ways to code this, perhaps using
>>>> foreach, but that's not the point. The point is that this code  _can_  
>>>> be
>>>> written and on the surface looks fine. Not even -w (warnings) spots  
>>>> the
>>>> signed/unsigned problem. At the very least can we get a warning for   
>>>> this?
>>>
>>>
>>> I too have tripped up on this 'bug' and it is very annoying and   
>>> surprising.
>>>
>>> However your approach to the problem might need to change...as you  
>>> state  -
>>> "On all lines with a length of 0 or 1 it will give the following   
>>> error..."
>>> - and this is because the test that you are performing is only  
>>> applicable
>>> to lines with two or more characters in them ... so make that  
>>> condition a
>>> part of the algorithm ...
>>>
>>>   if (line.length >= 2) {
>>>     for(int i = 0; i < line.length-2; i++) {
>>>       if (line[i..i+2] != "||") continue;
>>>       //..etc..
>>>     }
>>>   }
>>>   else {
>>>      // do something for short lines...
>>>   }
>>   Thanks Derek, that's exactly what I did.
>>  The point of this post isn't to get help with one specific problem  
>> but  rather to ask whether there is a solution to the underlying  
>> problem  behaviour. I suspect this behaviour will be the source of many  
>> bugs to  come and I wonder if there is a way to avoid them.
>
> First, how come you don't use this code?
>
> 	for (int i=0; i<line.length-2; i++)
> 	{
> 		if (line[i..i] == "|")
> 			if (line[i+1..i+1] == "|")
> 				continue;
> 	}
>
> It should be more than twice as fast.

Because speed wasn't important in this case (once-off console app)

> Second, isn't your code just an excellent example of why we have bounds  
> checking in D in the first place?

Bounds checking certainly helps.

> I think the obvious and clear-for-the-reader solution is to wrap that  
> code in an if:
>
> 	if (line.length > 1)
> 	{
> 		// the above code here
> 	}
>
> I can't see what would be wrong with that.

See my reply to Derek for why I think it's 'wrong'.

Regan