operands to min and max must have the same type...

operands to min and max must have the same type...

[Alexandre Oliva]

[-- Attachment #1: Type: text/plain, Size: 374 bytes --]

If we do arithmetic operations with size or ptrdiff types and they
happen to be built-in types narrower than int, they get promoted to
int, and then we can't find a match for min and max unless we do it on
both operands, which we don't.  Here's a patch that cures this.  Ok to
install if it completes a cross build to a target that has 16-bit
pointers and 32-bit integers?


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: lscxx-unpromote-size-type.patch --]
[-- Type: text/x-patch, Size: 4195 bytes --]

Index: libstdc++-v3/ChangeLog
from  Alexandre Oliva  <aoliva@redhat.com>

	* src/strstream.cc (strstreambuf::overflow): Make sure operands of
	min and max have the same type.
	* include/bits/basic_string.tcc (append, rfind, compare): Likewise.

Index: libstdc++-v3/src/strstream.cc
===================================================================
RCS file: /cvs/gcc/gcc/libstdc++-v3/src/strstream.cc,v
retrieving revision 1.15
diff -u -p -r1.15 strstream.cc
--- libstdc++-v3/src/strstream.cc 16 Dec 2002 18:23:00 -0000 1.15
+++ libstdc++-v3/src/strstream.cc 28 Feb 2003 22:36:50 -0000
@@ -1,6 +1,6 @@
 // strstream definitions -*- C++ -*-
 
-// Copyright (C) 2001, 2002 Free Software Foundation
+// Copyright (C) 2001, 2002, 2003 Free Software Foundation
 //
 // This file is part of the GNU ISO C++ Library.  This library is free
 // software; you can redistribute it and/or modify it under the
@@ -148,7 +148,7 @@ namespace std
     if (pptr() == epptr() && _M_dynamic && !_M_frozen && !_M_constant) 
       {
 	ptrdiff_t old_size = epptr() - pbase();
-	ptrdiff_t new_size = std::max(2 * old_size, ptrdiff_t(1));
+	ptrdiff_t new_size = std::max(ptrdiff_t (2 * old_size), ptrdiff_t (1));
 	
 	char* buf = _M_alloc(new_size);
 	if (buf) 
Index: libstdc++-v3/include/bits/basic_string.tcc
===================================================================
RCS file: /cvs/gcc/gcc/libstdc++-v3/include/bits/basic_string.tcc,v
retrieving revision 1.31
diff -u -p -r1.31 basic_string.tcc
--- libstdc++-v3/include/bits/basic_string.tcc 13 Feb 2003 21:39:02 -0000 1.31
+++ libstdc++-v3/include/bits/basic_string.tcc 28 Feb 2003 22:36:51 -0000
@@ -578,7 +578,8 @@ namespace std
       // Iff appending itself, string needs to pre-reserve the
       // correct size so that _M_mutate does not clobber the
       // iterators formed here.
-      size_type __len = std::min(__str.size() - __pos, __n) + this->size();
+      size_type __len = std::min(size_type (__str.size() - __pos),
+				 __n) + this->size();
       if (__len > this->capacity())
 	this->reserve(__len);
       return _M_replace_safe(_M_iend(), _M_iend(), __str._M_check(__pos),
@@ -709,7 +710,7 @@ namespace std
       size_type __size = this->size();
       if (__n <= __size)
 	{
-	  __pos = std::min(__size - __n, __pos);
+	  __pos = std::min(size_type (__size - __n), __pos);
 	  const _CharT* __data = _M_data();
 	  do 
 	    {
@@ -848,7 +849,7 @@ namespace std
       if (__pos > __size)
 	__throw_out_of_range("basic_string::compare");
       
-      size_type __rsize= std::min(__size - __pos, __n);
+      size_type __rsize= std::min(size_type (__size - __pos), __n);
       size_type __len = std::min(__rsize, __osize);
       int __r = traits_type::compare(_M_data() + __pos, __str.data(), __len);
       if (!__r)
@@ -867,8 +868,8 @@ namespace std
       if (__pos1 > __size || __pos2 > __osize)
 	__throw_out_of_range("basic_string::compare");
       
-      size_type __rsize = std::min(__size - __pos1, __n1);
-      size_type __rosize = std::min(__osize - __pos2, __n2);
+      size_type __rsize = std::min(size_type (__size - __pos1), __n1);
+      size_type __rosize = std::min(size_type (__osize - __pos2), __n2);
       size_type __len = std::min(__rsize, __rosize);
       int __r = traits_type::compare(_M_data() + __pos1, 
 				     __str.data() + __pos2, __len);
@@ -903,7 +904,7 @@ namespace std
 	__throw_out_of_range("basic_string::compare");
       
       size_type __osize = traits_type::length(__s);
-      size_type __rsize = std::min(__size - __pos, __n1);
+      size_type __rsize = std::min(size_type (__size - __pos), __n1);
       size_type __len = std::min(__rsize, __osize);
       int __r = traits_type::compare(_M_data() + __pos, __s, __len);
       if (!__r)
@@ -922,7 +923,7 @@ namespace std
 	__throw_out_of_range("basic_string::compare");
       
       size_type __osize = std::min(traits_type::length(__s), __n2);
-      size_type __rsize = std::min(__size - __pos, __n1);
+      size_type __rsize = std::min(size_type (__size - __pos), __n1);
       size_type __len = std::min(__rsize, __osize);
       int __r = traits_type::compare(_M_data() + __pos, __s, __len);
       if (!__r)

[-- Attachment #3: Type: text/plain, Size: 289 bytes --]


-- 
Alexandre Oliva   Enjoy Guarana', see http://www.ic.unicamp.br/~oliva/
Red Hat GCC Developer                 aoliva@{redhat.com, gcc.gnu.org}
CS PhD student at IC-Unicamp        oliva@{lsd.ic.unicamp.br, gnu.org}
Free Software Evangelist                Professional serial bug killer

Re: operands to min and max must have the same type...

[Phil Edwards]

On Fri, Feb 28, 2003 at 07:37:04PM -0300, Alexandre Oliva wrote:
> If we do arithmetic operations with size or ptrdiff types and they
> happen to be built-in types narrower than int, they get promoted to
> int, and then we can't find a match for min and max unless we do it on
> both operands, which we don't.  Here's a patch that cures this.  Ok to
> install if it completes a cross build to a target that has 16-bit
> pointers and 32-bit integers?


\begin{rth}
Ok.
\end{rth}

-- 
If ye love wealth greater than liberty, the tranquility of servitude greater
than the animating contest for freedom, go home and leave us in peace.  We seek
not your counsel, nor your arms.  Crouch down and lick the hand that feeds you;
and may posterity forget that ye were our countrymen.            - Samuel Adams

Re: operands to min and max must have the same type...

[Benjamin Kosnik]

-	ptrdiff_t new_size = std::max(2 * old_size, ptrdiff_t(1));
+	ptrdiff_t new_size = std::max(ptrdiff_t (2 * old_size), ptrdiff_t (1));


Should be:

ptrdiff_t new_size = std::max(static_cast<ptrdiff_t>(2 * old_size), ptrdiff_t (1));

Re: operands to min and max must have the same type...

[Alexandre Oliva]

On Mar  4, 2003, Benjamin Kosnik <bkoz@redhat.com> wrote:

> -	ptrdiff_t new_size = std::max(2 * old_size, ptrdiff_t(1));
> +	ptrdiff_t new_size = std::max(ptrdiff_t (2 * old_size), ptrdiff_t (1));


> Should be:

> ptrdiff_t new_size = std::max(static_cast<ptrdiff_t>(2 * old_size), ptrdiff_t (1));

Err...  I don't understand.  Why must one of them be a static_cast
while the other is ok as a functional cast?  Are conventions like this
documented anywhere?

-- 
Alexandre Oliva   Enjoy Guarana', see http://www.ic.unicamp.br/~oliva/
Red Hat GCC Developer                 aoliva@{redhat.com, gcc.gnu.org}
CS PhD student at IC-Unicamp        oliva@{lsd.ic.unicamp.br, gnu.org}
Free Software Evangelist                Professional serial bug killer

Re: operands to min and max must have the same type...

[Benjamin Kosnik]

>> ptrdiff_t new_size = std::max(static_cast<ptrdiff_t>(2 * old_size), ptrdiff_t (1));
>
>Err...  I don't understand.  Why must one of them be a static_cast
>while the other is ok as a functional cast?  Are conventions like this
>documented anywhere?

Sorry, in my haste I didn't notice the second. I suppose you could just
do 

ptrdiff_t(1 * old_size), ptrdiff_t(1)

(note the lack of space). 

See C++STYLE.

-benjamin

Re: operands to min and max must have the same type...

[Alexandre Oliva]

[-- Attachment #1: Type: text/plain, Size: 208 bytes --]

On Mar  5, 2003, Benjamin Kosnik <bkoz@redhat.com> wrote:

> (note the lack of space). 

> See C++STYLE.

Thanks, I didn't know about these guidelines.  Fixed with the
following patch, that I'm checking in.


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: lscxx-ws.patch --]
[-- Type: text/x-patch, Size: 3812 bytes --]

Index: libstdc++-v3/ChangeLog
from  Alexandre Oliva  <aoliva@redhat.com>

	* src/strstream.cc, include/bits/basic_string.tcc: Remove
	incorrect whitespace added in my previous change.

Index: libstdc++-v3/src/strstream.cc
===================================================================
RCS file: /cvs/gcc/gcc/libstdc++-v3/src/strstream.cc,v
retrieving revision 1.16
diff -u -p -r1.16 strstream.cc
--- libstdc++-v3/src/strstream.cc 4 Mar 2003 21:21:44 -0000 1.16
+++ libstdc++-v3/src/strstream.cc 5 Mar 2003 22:23:33 -0000
@@ -148,7 +148,7 @@ namespace std
     if (pptr() == epptr() && _M_dynamic && !_M_frozen && !_M_constant) 
       {
 	ptrdiff_t old_size = epptr() - pbase();
-	ptrdiff_t new_size = std::max(ptrdiff_t (2 * old_size), ptrdiff_t (1));
+	ptrdiff_t new_size = std::max(ptrdiff_t(2 * old_size), ptrdiff_t(1));
 	
 	char* buf = _M_alloc(new_size);
 	if (buf) 
Index: libstdc++-v3/include/bits/basic_string.tcc
===================================================================
RCS file: /cvs/gcc/gcc/libstdc++-v3/include/bits/basic_string.tcc,v
retrieving revision 1.32
diff -u -p -r1.32 basic_string.tcc
--- libstdc++-v3/include/bits/basic_string.tcc 4 Mar 2003 21:21:44 -0000 1.32
+++ libstdc++-v3/include/bits/basic_string.tcc 5 Mar 2003 22:23:33 -0000
@@ -578,7 +578,7 @@ namespace std
       // Iff appending itself, string needs to pre-reserve the
       // correct size so that _M_mutate does not clobber the
       // iterators formed here.
-      size_type __len = std::min(size_type (__str.size() - __pos),
+      size_type __len = std::min(size_type(__str.size() - __pos),
 				 __n) + this->size();
       if (__len > this->capacity())
 	this->reserve(__len);
@@ -710,7 +710,7 @@ namespace std
       size_type __size = this->size();
       if (__n <= __size)
 	{
-	  __pos = std::min(size_type (__size - __n), __pos);
+	  __pos = std::min(size_type(__size - __n), __pos);
 	  const _CharT* __data = _M_data();
 	  do 
 	    {
@@ -849,7 +849,7 @@ namespace std
       if (__pos > __size)
 	__throw_out_of_range("basic_string::compare");
       
-      size_type __rsize= std::min(size_type (__size - __pos), __n);
+      size_type __rsize= std::min(size_type(__size - __pos), __n);
       size_type __len = std::min(__rsize, __osize);
       int __r = traits_type::compare(_M_data() + __pos, __str.data(), __len);
       if (!__r)
@@ -868,8 +868,8 @@ namespace std
       if (__pos1 > __size || __pos2 > __osize)
 	__throw_out_of_range("basic_string::compare");
       
-      size_type __rsize = std::min(size_type (__size - __pos1), __n1);
-      size_type __rosize = std::min(size_type (__osize - __pos2), __n2);
+      size_type __rsize = std::min(size_type(__size - __pos1), __n1);
+      size_type __rosize = std::min(size_type(__osize - __pos2), __n2);
       size_type __len = std::min(__rsize, __rosize);
       int __r = traits_type::compare(_M_data() + __pos1, 
 				     __str.data() + __pos2, __len);
@@ -904,7 +904,7 @@ namespace std
 	__throw_out_of_range("basic_string::compare");
       
       size_type __osize = traits_type::length(__s);
-      size_type __rsize = std::min(size_type (__size - __pos), __n1);
+      size_type __rsize = std::min(size_type(__size - __pos), __n1);
       size_type __len = std::min(__rsize, __osize);
       int __r = traits_type::compare(_M_data() + __pos, __s, __len);
       if (!__r)
@@ -923,7 +923,7 @@ namespace std
 	__throw_out_of_range("basic_string::compare");
       
       size_type __osize = std::min(traits_type::length(__s), __n2);
-      size_type __rsize = std::min(size_type (__size - __pos), __n1);
+      size_type __rsize = std::min(size_type(__size - __pos), __n1);
       size_type __len = std::min(__rsize, __osize);
       int __r = traits_type::compare(_M_data() + __pos, __s, __len);
       if (!__r)

[-- Attachment #3: Type: text/plain, Size: 289 bytes --]


-- 
Alexandre Oliva   Enjoy Guarana', see http://www.ic.unicamp.br/~oliva/
Red Hat GCC Developer                 aoliva@{redhat.com, gcc.gnu.org}
CS PhD student at IC-Unicamp        oliva@{lsd.ic.unicamp.br, gnu.org}
Free Software Evangelist                Professional serial bug killer

Re: operands to min and max must have the same type...

[Benjamin Kosnik]

>Thanks, I didn't know about these guidelines.  Fixed with the
>following patch, that I'm checking in.

Thanks Alexandre!