[RFA/commit] Remove use of stdbool.h in GDB sources.

[RFA/commit] Remove use of stdbool.h in GDB sources.

[Joel Brobecker]

Hello,

Using type bool from stdbool unfortunately causes problems trying
to build GDB on AiX and Solaris:

    In file included from ../../src/gdb/utils.h:24:0,
                     from ../../src/gdb/defs.h:707,
                     from ../../src/gdb/utils.c:20:
    /[...]/curses.h:96:14: error: two or more data types in declaration
    specifiers
     typedef char bool;
                  ^
    make[2]: *** [utils.o] Error 1

In theory, the problem is in the system's curses.h which, in both cases,
do something similar. On Solaris:

    #if !defined(__cplusplus) && !defined(_BOOL)
    typedef char bool;
    #endif /* !defined(__cplusplus) && !defined(_BOOL) */

On AiX:

    #if !defined(__cplusplus) || (defined(__IBMCPP__) &&(__IBMCPP__<400))
    #ifndef _BOOL
    #define _BOOL
    typedef int bool;
    #endif
    #endif

You can reproduce the same problem by trying to compile:

    % cat toto.c
    #include <stdbool.h>
    #include <curses.h>
    % gcc -c toto.c
    In file included from toto.c:1:0:
    /[...]/curses.h:159:13: error: two or more data types in declaration
    specifiers
     typedef int bool;
             ^

This specific issue wouldn't occur if we included curses.h before
including stdbool.h, and I looked at that just to be complete.
Here is a small schematic representation of the include logic:

  * utils.c:
      -> defs.h -> utils.h -> stdbool.h
      -> gdb_curses.h -> curses.h

Because defs.h should always be first on the list, it means that
stdbool.h will always necessarily be included ahead of curses.h.

But, thinking beyond this very specific issue, it shows that using
stdbool.h is going to cause problems on these systems until either
GCC fixes those includes in a way that makes them work (not really
an option in the short term); or we switch to C++.

In the meantime, I think the path of least resistance is to revert
the use of stdbool.h, and use integers, the way we've done up until
now. The benefits of using type "bool" are modest, IMO, so not
a great loss, and a temporary one.

gdb/ChangeLog:

        * utils.h: Remove <stdbool.h> #include.
        (producer_is_gcc): Change return type to "int".
        * utils.c (producer_is_gcc): Change return type to int.
        Return 1 instead of true, and 0 instead of false.
        Adjust function documentation accordingly.

I will push this patch in the next few days, unless there are
objections.

Thanks,
-- 
Joel

---
 gdb/utils.c | 10 +++++-----
 gdb/utils.h |  4 +---
 2 files changed, 6 insertions(+), 8 deletions(-)

diff --git a/gdb/utils.c b/gdb/utils.c
index 3ce5814..4f9f4f0 100644
--- a/gdb/utils.c
+++ b/gdb/utils.c
@@ -3269,11 +3269,11 @@ producer_is_gcc_ge_4 (const char *producer)
   return minor;
 }
 
-/* Returns true if the given PRODUCER string is GCC and sets the MAJOR
-   and MINOR versions when not NULL.  Returns false if the given PRODUCER
+/* Returns nonzero if the given PRODUCER string is GCC and sets the MAJOR
+   and MINOR versions when not NULL.  Returns zero if the given PRODUCER
    is NULL or it isn't GCC.  */
 
-bool
+int
 producer_is_gcc (const char *producer, int *major, int *minor)
 {
   const char *cs;
@@ -3299,11 +3299,11 @@ producer_is_gcc (const char *producer, int *major, int *minor)
       if (*cs && isspace (*cs))
         cs++;
       if (sscanf (cs, "%d.%d", major, minor) == 2)
-	return true;
+	return 1;
     }
 
   /* Not recognized as GCC.  */
-  return false;
+  return 0;
 }
 
 /* Helper for make_cleanup_free_char_ptr_vec.  */
diff --git a/gdb/utils.h b/gdb/utils.h
index d8afa79..b8e1aff 100644
--- a/gdb/utils.h
+++ b/gdb/utils.h
@@ -21,8 +21,6 @@
 #ifndef UTILS_H
 #define UTILS_H
 
-#include <stdbool.h>
-
 #include "exceptions.h"
 
 extern void initialize_utils (void);
@@ -304,7 +302,7 @@ extern pid_t wait_to_die_with_timeout (pid_t pid, int *status, int timeout);
 #endif
 
 extern int producer_is_gcc_ge_4 (const char *producer);
-extern bool producer_is_gcc (const char *producer, int *major, int *minor);
+extern int producer_is_gcc (const char *producer, int *major, int *minor);
 
 extern int myread (int, char *, int);
 
-- 
1.9.1

Re: [RFA/commit] Remove use of stdbool.h in GDB sources.

[Pedro Alves]

On 02/27/2015 08:53 AM, Joel Brobecker wrote:
> Hello,
> 
> Using type bool from stdbool unfortunately causes problems trying
> to build GDB on AiX and Solaris:

Darn...


> But, thinking beyond this very specific issue, it shows that using
> stdbool.h is going to cause problems on these systems until either
> GCC fixes those includes in a way that makes them work (not really
> an option in the short term); or we switch to C++.

> 
> In the meantime, I think the path of least resistance is to revert
> the use of stdbool.h, and use integers, the way we've done up until
> now. The benefits of using type "bool" are modest, IMO, so not
> a great loss, and a temporary one.

I agree.

> 
> gdb/ChangeLog:
> 
>         * utils.h: Remove <stdbool.h> #include.
>         (producer_is_gcc): Change return type to "int".
>         * utils.c (producer_is_gcc): Change return type to int.
>         Return 1 instead of true, and 0 instead of false.
>         Adjust function documentation accordingly.
> 
> I will push this patch in the next few days, unless there are
> objections.

No objections here.

(guess we should reinstate the ARI bool rule too then.)

Thanks,
Pedro Alves

Re: [RFA/commit] Remove use of stdbool.h in GDB sources.

[Joel Brobecker]

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

> > gdb/ChangeLog:
> > 
> >         * utils.h: Remove <stdbool.h> #include.
> >         (producer_is_gcc): Change return type to "int".
> >         * utils.c (producer_is_gcc): Change return type to int.
> >         Return 1 instead of true, and 0 instead of false.
> >         Adjust function documentation accordingly.
> > 
> > I will push this patch in the next few days, unless there are
> > objections.
> 
> No objections here.

Thanks, Pedro. I just pushed the patch.

> (guess we should reinstate the ARI bool rule too then.)

Absolutely. Attached is the patch that I just pushed reinstating
them.

gdb/ChangeLog:

        * contrib/ari/gdb_ari.sh: Reinstate checks for "true" and "false".

-- 
Joel

[-- Attachment #2: 0001-Revert-Remove-true-and-false-ARI-checks-now-that-we-.patch --]
[-- Type: text/x-diff, Size: 1917 bytes --]

From 00e474c2e96f9a56f128ee29fbdc36ac32d5e14a Mon Sep 17 00:00:00 2001
From: Joel Brobecker <brobecker@adacore.com>
Date: Mon, 2 Mar 2015 06:05:01 -0800
Subject: [PATCH] Revert "Remove true and false ARI checks now that we use
 stdbool.h."

As we cannot use type bool until conversion to C++ is official,
this patch re-instates the ARI checks for "true/false".

gdb/ChangeLog:

        * contrib/ari/gdb_ari.sh: Reinstate checks for "true" and "false".
---
 gdb/ChangeLog              |  4 ++++
 gdb/contrib/ari/gdb_ari.sh | 20 ++++++++++++++++++++
 2 files changed, 24 insertions(+)

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index 56e7206..922b1d9 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,5 +1,9 @@
 2015-03-02  Joel Brobecker  <brobecker@adacore.com>
 
+	* contrib/ari/gdb_ari.sh: Reinstate checks for "true" and "false".
+
+2015-03-02  Joel Brobecker  <brobecker@adacore.com>
+
 	* utils.h: Remove <stdbool.h> #include.
 	(producer_is_gcc): Change return type to "int".
 	* utils.c (producer_is_gcc): Change return type to int.
diff --git a/gdb/contrib/ari/gdb_ari.sh b/gdb/contrib/ari/gdb_ari.sh
index 52d8ab1..b868a17 100644
--- a/gdb/contrib/ari/gdb_ari.sh
+++ b/gdb/contrib/ari/gdb_ari.sh
@@ -1145,6 +1145,26 @@ Do not use `boolean'\'',  use `int'\'' instead"
     }
 }
 
+BEGIN { doc["false"] = "\
+Definitely do not use `false'\'' in boolean expressions"
+    category["false"] = ari_regression
+}
+/(^|[^_[:alnum:]])false([^_[:alnum:]]|$)/ {
+    if (is_yacc_or_lex == 0) {
+       fail("false")
+    }
+}
+
+BEGIN { doc["true"] = "\
+Do not try to use `true'\'' in boolean expressions"
+    category["true"] = ari_regression
+}
+/(^|[^_[:alnum:]])true([^_[:alnum:]]|$)/ {
+    if (is_yacc_or_lex == 0) {
+       fail("true")
+    }
+}
+
 # Typedefs that are either redundant or can be reduced to `struct
 # type *''.
 # Must be placed before if assignment otherwise ARI exceptions
-- 
1.9.1