[PUSHED] gdb: Use std::abs instead of abs on LONGEST types

Message ID 20200227164651.13723-1-andrew.burgess@embecosm.com
State New
Headers show
Series
  • [PUSHED] gdb: Use std::abs instead of abs on LONGEST types
Related show

Commit Message

Andrew Burgess Feb. 27, 2020, 4:46 p.m.
Use std::abs so that we get the C++ overloaded version that matches
the argument type instead of the C abs function which is only for int
arguments.

There should be no user visible change after this commit.

gdb/ChangeLog:

	* gdbtypes.c (create_array_type_with_stride): Use std::abs not
	abs.
---
 gdb/ChangeLog  | 5 +++++
 gdb/gdbtypes.c | 4 ++--
 2 files changed, 7 insertions(+), 2 deletions(-)

-- 
2.14.5

Comments

Pedro Alves Feb. 27, 2020, 7:06 p.m. | #1
On 2/27/20 4:46 PM, Andrew Burgess wrote:
> Use std::abs so that we get the C++ overloaded version that matches

> the argument type instead of the C abs function which is only for int

> arguments.


Note that stdlib.h/stdmath.h are supposed to provide the overloads in
the global namespace as well; the standard requires it.  Older
GCCs got that wrong (e.g. 4.8), but more modern GCCs get it right.

Just a FYI, the patch is fine.

> 

> There should be no user visible change after this commit.

> 

> gdb/ChangeLog:

> 

> 	* gdbtypes.c (create_array_type_with_stride): Use std::abs not

> 	abs.


Thanks,
Pedro Alves
Pedro Alves via Gdb-patches Feb. 27, 2020, 7:09 p.m. | #2
On Thu, Feb 27, 2020 at 1:07 PM Pedro Alves <palves@redhat.com> wrote:
>

> On 2/27/20 4:46 PM, Andrew Burgess wrote:

> > Use std::abs so that we get the C++ overloaded version that matches

> > the argument type instead of the C abs function which is only for int

> > arguments.

>

> Note that stdlib.h/stdmath.h are supposed to provide the overloads in

> the global namespace as well; the standard requires it.  Older

> GCCs got that wrong (e.g. 4.8), but more modern GCCs get it right.

>

> Just a FYI, the patch is fine.


Hm... I saw a build error from this on arm-netbsd with clang 9, I
wonder what happened there. Anyway, the patch does fix it.

Christian

>

> >

> > There should be no user visible change after this commit.

> >

> > gdb/ChangeLog:

> >

> >       * gdbtypes.c (create_array_type_with_stride): Use std::abs not

> >       abs.

>

> Thanks,

> Pedro Alves

>
Pedro Alves Feb. 27, 2020, 7:26 p.m. | #3
On 2/27/20 7:09 PM, Christian Biesinger via gdb-patches wrote:
> On Thu, Feb 27, 2020 at 1:07 PM Pedro Alves <palves@redhat.com> wrote:

>>

>> On 2/27/20 4:46 PM, Andrew Burgess wrote:

>>> Use std::abs so that we get the C++ overloaded version that matches

>>> the argument type instead of the C abs function which is only for int

>>> arguments.

>>

>> Note that stdlib.h/stdmath.h are supposed to provide the overloads in

>> the global namespace as well; the standard requires it.  Older

>> GCCs got that wrong (e.g. 4.8), but more modern GCCs get it right.

>>

>> Just a FYI, the patch is fine.

> 

> Hm... I saw a build error from this on arm-netbsd with clang 9, I

> wonder what happened there. Anyway, the patch does fix it.

> 


( See:
 https://developers.redhat.com/blog/2016/02/29/why-cstdlib-is-more-complicated-than-you-might-think/ )

Odd, clang 5, which is what I have handy, gets it right:

$ cat abs.cc 
#include <stdlib.h>
#include <stdio.h>

void
foo (long i)
{
  printf ("long\n");
}

void
foo (int i)
{
  printf ("int\n");
}

int
main ()
{
  foo (abs ((long)1));
  foo (abs ((int)1));
}

$ clang++ abs.cc -o abs && ./abs
long
int

I wonder whether you were seeing a gnulib override issue, but I
can't find an abs override in our import.

Thanks,
Pedro Alves
Andrew Burgess Feb. 27, 2020, 9:10 p.m. | #4
* Pedro Alves <palves@redhat.com> [2020-02-27 19:06:47 +0000]:

> On 2/27/20 4:46 PM, Andrew Burgess wrote:

> > Use std::abs so that we get the C++ overloaded version that matches

> > the argument type instead of the C abs function which is only for int

> > arguments.

> 

> Note that stdlib.h/stdmath.h are supposed to provide the overloads in

> the global namespace as well; the standard requires it.  Older

> GCCs got that wrong (e.g. 4.8), but more modern GCCs get it right.

> 

> Just a FYI, the patch is fine.



Thanks for looking at this.

Thanks,
Andrew




> 

> > 

> > There should be no user visible change after this commit.

> > 

> > gdb/ChangeLog:

> > 

> > 	* gdbtypes.c (create_array_type_with_stride): Use std::abs not

> > 	abs.

> 

> Thanks,

> Pedro Alves

>

Patch

diff --git a/gdb/gdbtypes.c b/gdb/gdbtypes.c
index ef110b30445..d89df9f7409 100644
--- a/gdb/gdbtypes.c
+++ b/gdb/gdbtypes.c
@@ -1249,9 +1249,9 @@  create_array_type_with_stride (struct type *result_type,
 	     negative stride in Fortran (this doesn't mean anything
 	     special, it's still just a single element array) so do
 	     consider that case when touching this code.  */
-	  LONGEST element_count = abs (high_bound - low_bound + 1);
+	  LONGEST element_count = std::abs (high_bound - low_bound + 1);
 	  TYPE_LENGTH (result_type)
-	    = ((abs (stride) * element_count) + 7) / 8;
+	    = ((std::abs (stride) * element_count) + 7) / 8;
 	}
       else
 	TYPE_LENGTH (result_type) =