www.digitalmars.com         C & C++   DMDScript  

digitalmars.D - Wrong code gen / missing warning / my mistake?

reply Benjamin Thaut <code benjamin-thaut.de> writes:
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
Sep 10 2012
next sibling parent =?UTF-8?B?QWxpIMOHZWhyZWxp?= <acehreli yahoo.com> writes:
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
Sep 10 2012
prev sibling next sibling parent reply Rainer Schuetze <r.sagitario gmx.de> writes:
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.
Sep 10 2012
parent Benjamin Thaut <code benjamin-thaut.de> writes:
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
Sep 10 2012
prev sibling parent "Steven Schveighoffer" <schveiguy yahoo.com> writes:
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
Sep 10 2012