The following code


bool endsWith(string str, string end)
{
	size_t to = str.length - end.length;
	for(sizediff_t i = str.length - 1; i >= to; --i)
	{
		if(str[i] != end[i-to])
			return false;
	}
	return true;
}

int main(string[] args)
{
	return cast(int)endsWith("blub", "blub");
}

compiled with dmd 2.060 gives me a range violation. (with i = -1) although it shouldn't. If I change to from size_t "to" sizediff_t everything is fine. The comparison between the unsigned "to" and the signed "i" is not done correctly.

Is this a code gen bug? Or is it missing a compiler warning / error? Or is this entierly my fault?

Kind Regards
Benjamin Thaut

On 09/10/2012 10:08 AM, Benjamin Thaut wrote:
> The following code
>
>
> bool endsWith(string str, string end)
> {
> size_t to = str.length - end.length;

size_t is an unsigned type. That expression above is very dangerous.

> for(sizediff_t i = str.length - 1; i >= to; --i)

Yes, sizediff_t is signed, but...

> {
> if(str[i] != end[i-to])

to is unsigned so 'i-to' is unsigned.

> return false;
> }
> return true;
> }
>
> int main(string[] args)
> {
> return cast(int)endsWith("blub", "blub");
> }
>
> compiled with dmd 2.060 gives me a range violation. (with i = -1)
> although it shouldn't. If I change to from size_t "to" sizediff_t
> everything is fine. The comparison between the unsigned "to" and the
> signed "i" is not done correctly.
>
> Is this a code gen bug? Or is it missing a compiler warning / error? Or
> is this entierly my fault?
>
> Kind Regards
> Benjamin Thaut

Ali

On 10.09.2012 19:08, Benjamin Thaut wrote:
> The following code
>
>
> bool endsWith(string str, string end)
> {
>      size_t to = str.length - end.length;
>      for(sizediff_t i = str.length - 1; i >= to; --i)

Unfortunately, unsigned takes precedence over signed in any calculation, so "i >= to" is an unsigned comparison, where i is converted to unsigned first.

A lot of calculation that involve size_t and implicite conversions or substractions are pretty error-prone.

On Mon, 10 Sep 2012 13:08:29 -0400, Benjamin Thaut <code@benjamin-thaut.de> wrote:

> The following code
>
>
> bool endsWith(string str, string end)
> {
> 	size_t to = str.length - end.length;
> 	for(sizediff_t i = str.length - 1; i >= to; --i)
> 	{
> 		if(str[i] != end[i-to])
> 			return false;
> 	}
> 	return true;
> }
>
> int main(string[] args)
> {
> 	return cast(int)endsWith("blub", "blub");
> }
>
> compiled with dmd 2.060 gives me a range violation. (with i = -1) although it shouldn't. If I change to from size_t "to" sizediff_t everything is fine. The comparison between the unsigned "to" and the signed "i" is not done correctly.
>
> Is this a code gen bug? Or is it missing a compiler warning / error? Or is this entierly my fault?

It is behaving as expected.

through integer promotion rules, i >= to is converted to unsigned comparison.  So if i becomes -1, then it really becomes a comparison between 2^32 - 1 and 0.

In other words, if to is 0, then the loop will never terminate (because unsigned can never be less than 0).

The solution is to re-design your loop.  I try to avoid conditions that could be done with negative values for the above reason.

sizediff_t offset = str.length - end.length;
for(sizediff_t i = 0; i < end.length; ++i)
{
   if(str[offset + i] != end[i])
       return false;
}
return true;

Or if you want to skip all this, you can just use the wonderful D slice syntax :)

if(str.length > end.length)
   return str[$-end.length..$] == end;
else
   return false;

-Steve

Am 10.09.2012 19:58, schrieb Rainer Schuetze:
> On 10.09.2012 19:08, Benjamin Thaut wrote:
>  > The following code
>  >
>  >
>  > bool endsWith(string str, string end)
>  > {
>  >      size_t to = str.length - end.length;
>  >      for(sizediff_t i = str.length - 1; i >= to; --i)
>
> Unfortunately, unsigned takes precedence over signed in any calculation,
> so "i >= to" is an unsigned comparison, where i is converted to unsigned
> first.
>
> A lot of calculation that involve size_t and implicite conversions or
> substractions are pretty error-prone.

If they are that error prone, shouldn't there be some kind of compiler warning?

Kind Regards
Benjamin Thaut