[PATCH] Move value_true to value.h

[PATCH] Move value_true to value.h

[Tom Tromey]

I noticed that value_true is declared in language.h and defined in
language.c.  However, as part of the value API, I think it would be
better in one of those files.  And, because it is very short, I
changed it to be an inline function in value.h.  I've also removed a
comment from the implementation, on the basis that it seems obsolete
-- if the change it suggests was needed, it probably would have been
done by now; and if it is needed in the future, odds are it would be
done differently anyway.

Finally, this patch also changes value_true to return a bool, and
updates some uses.
---
 gdb/ada-lang.c       | 10 +++++-----
 gdb/cli/cli-script.c |  3 +--
 gdb/language.c       | 17 -----------------
 gdb/language.h       |  4 ----
 gdb/value.h          |  7 +++++++
 5 files changed, 13 insertions(+), 28 deletions(-)

diff --git a/gdb/ada-lang.c b/gdb/ada-lang.c
index 6680a4fd657..63e2d587440 100644
--- a/gdb/ada-lang.c
+++ b/gdb/ada-lang.c
@@ -11754,13 +11754,13 @@ re_set_exception (struct breakpoint *b)
    user specified a specific exception, we only want to cause a stop
    if the program thrown that exception.  */
 
-static int
+static bool
 should_stop_exception (const struct bp_location *bl)
 {
   struct ada_catchpoint *c = (struct ada_catchpoint *) bl->owner;
   const struct ada_catchpoint_location *ada_loc
     = (const struct ada_catchpoint_location *) bl;
-  int stop;
+  bool stop;
 
   struct internalvar *var = lookup_internalvar ("_ada_exception");
   if (c->m_kind == ada_catch_assert)
@@ -11788,16 +11788,16 @@ should_stop_exception (const struct bp_location *bl)
 
   /* With no specific exception, should always stop.  */
   if (c->excep_string.empty ())
-    return 1;
+    return true;
 
   if (ada_loc->excep_cond_expr == NULL)
     {
       /* We will have a NULL expression if back when we were creating
 	 the expressions, this location's had failed to parse.  */
-      return 1;
+      return true;
     }
 
-  stop = 1;
+  stop = true;
   try
     {
       struct value *mark;
diff --git a/gdb/cli/cli-script.c b/gdb/cli/cli-script.c
index 98463677902..0aaa59ea65f 100644
--- a/gdb/cli/cli-script.c
+++ b/gdb/cli/cli-script.c
@@ -19,7 +19,6 @@
 
 #include "defs.h"
 #include "value.h"
-#include "language.h"		/* For value_true */
 #include <ctype.h>
 
 #include "ui-out.h"
@@ -579,7 +578,7 @@ execute_control_command_1 (struct command_line *cmd, int from_tty)
 	/* Keep iterating so long as the expression is true.  */
 	while (loop == 1)
 	  {
-	    int cond_result;
+	    bool cond_result;
 
 	    QUIT;
 
diff --git a/gdb/language.c b/gdb/language.c
index 45ce2ebcc92..f52956022ac 100644
--- a/gdb/language.c
+++ b/gdb/language.c
@@ -394,23 +394,6 @@ pointer_type (struct type *type)
 {
   return type->code () == TYPE_CODE_PTR || TYPE_IS_REFERENCE (type);
 }
-
-\f
-/* This page contains functions that return info about
-   (struct value) values used in GDB.  */
-
-/* Returns non-zero if the value VAL represents a true value.  */
-int
-value_true (struct value *val)
-{
-  /* It is possible that we should have some sort of error if a non-boolean
-     value is used in this context.  Possibly dependent on some kind of
-     "boolean-checking" option like range checking.  But it should probably
-     not depend on the language except insofar as is necessary to identify
-     a "boolean" value (i.e. in C using a float, pointer, etc., as a boolean
-     should be an error, probably).  */
-  return !value_logical_not (val);
-}
 \f
 /* This page contains functions for the printing out of
    error messages that occur during type- and range-
diff --git a/gdb/language.h b/gdb/language.h
index 21ed47b3580..c6bef7494b3 100644
--- a/gdb/language.h
+++ b/gdb/language.h
@@ -774,10 +774,6 @@ extern int pointer_type (struct type *);
 
 extern void range_error (const char *, ...) ATTRIBUTE_PRINTF (1, 2);
 
-/* Data:  Does this value represent "truth" to the current language?  */
-
-extern int value_true (struct value *);
-
 /* Misc:  The string representing a particular enum language.  */
 
 extern enum language language_enum (const char *str);
diff --git a/gdb/value.h b/gdb/value.h
index abbb8d4268a..c5847290564 100644
--- a/gdb/value.h
+++ b/gdb/value.h
@@ -1024,6 +1024,13 @@ extern int value_less (struct value *arg1, struct value *arg2);
 
 extern bool value_logical_not (struct value *arg1);
 
+/* Returns non-zero if the value VAL represents a true value.  */
+static inline bool
+value_true (struct value *val)
+{
+  return !value_logical_not (val);
+}
+
 /* C++ */
 
 extern struct value *value_of_this (const struct language_defn *lang);
-- 
2.31.1

Re: [PATCH] Move value_true to value.h

[Pedro Alves]

On 2021-09-11 7:12 p.m., Tom Tromey wrote:
> I noticed that value_true is declared in language.h and defined in
> language.c.  However, as part of the value API, I think it would be
> better in one of those files.  And, because it is very short, I
> changed it to be an inline function in value.h.  I've also removed a
> comment from the implementation, on the basis that it seems obsolete
> -- if the change it suggests was needed, it probably would have been
> done by now; and if it is needed in the future, odds are it would be
> done differently anyway.
> 
> Finally, this patch also changes value_true to return a bool, and
> updates some uses.

Seems obvious enough.  OK.

Re: [PATCH] Move value_true to value.h

[Tom Tromey]

>> Finally, this patch also changes value_true to return a bool, and
>> updates some uses.

Pedro> Seems obvious enough.  OK.

I sent the wrong version -- I also converted value_logical_not to return
bool.

Tom

commit 12fb19e547b8352fb2fdaafc9e0895eb34804d0d
Author: Tom Tromey <tom@tromey.com>
Date:   Thu Aug 26 18:17:40 2021 -0600

    Move value_true to value.h
    
    I noticed that value_true is declared in language.h and defined in
    language.c.  However, as part of the value API, I think it would be
    better in one of those files.  And, because it is very short, I
    changed it to be an inline function in value.h.  I've also removed a
    comment from the implementation, on the basis that it seems obsolete
    -- if the change it suggests was needed, it probably would have been
    done by now; and if it is needed in the future, odds are it would be
    done differently anyway.
    
    Finally, this patch also changes value_true and value_logical_not to
    return a bool, and updates some uses.

diff --git a/gdb/ada-lang.c b/gdb/ada-lang.c
index 487d92be5c9..a00fa141156 100644
--- a/gdb/ada-lang.c
+++ b/gdb/ada-lang.c
@@ -11765,13 +11765,13 @@ re_set_exception (struct breakpoint *b)
    user specified a specific exception, we only want to cause a stop
    if the program thrown that exception.  */
 
-static int
+static bool
 should_stop_exception (const struct bp_location *bl)
 {
   struct ada_catchpoint *c = (struct ada_catchpoint *) bl->owner;
   const struct ada_catchpoint_location *ada_loc
     = (const struct ada_catchpoint_location *) bl;
-  int stop;
+  bool stop;
 
   struct internalvar *var = lookup_internalvar ("_ada_exception");
   if (c->m_kind == ada_catch_assert)
@@ -11799,16 +11799,16 @@ should_stop_exception (const struct bp_location *bl)
 
   /* With no specific exception, should always stop.  */
   if (c->excep_string.empty ())
-    return 1;
+    return true;
 
   if (ada_loc->excep_cond_expr == NULL)
     {
       /* We will have a NULL expression if back when we were creating
 	 the expressions, this location's had failed to parse.  */
-      return 1;
+      return true;
     }
 
-  stop = 1;
+  stop = true;
   try
     {
       struct value *mark;
diff --git a/gdb/cli/cli-script.c b/gdb/cli/cli-script.c
index 98463677902..0aaa59ea65f 100644
--- a/gdb/cli/cli-script.c
+++ b/gdb/cli/cli-script.c
@@ -19,7 +19,6 @@
 
 #include "defs.h"
 #include "value.h"
-#include "language.h"		/* For value_true */
 #include <ctype.h>
 
 #include "ui-out.h"
@@ -579,7 +578,7 @@ execute_control_command_1 (struct command_line *cmd, int from_tty)
 	/* Keep iterating so long as the expression is true.  */
 	while (loop == 1)
 	  {
-	    int cond_result;
+	    bool cond_result;
 
 	    QUIT;
 
diff --git a/gdb/eval.c b/gdb/eval.c
index 5c348c34e66..97710e73ece 100644
--- a/gdb/eval.c
+++ b/gdb/eval.c
@@ -2206,7 +2206,7 @@ logical_and_operation::evaluate (struct type *expect_type,
     }
   else
     {
-      int tem = value_logical_not (arg1);
+      bool tem = value_logical_not (arg1);
       if (!tem)
 	{
 	  arg2 = std::get<1> (m_storage)->evaluate (nullptr, exp, noside);
@@ -2235,7 +2235,7 @@ logical_or_operation::evaluate (struct type *expect_type,
     }
   else
     {
-      int tem = value_logical_not (arg1);
+      bool tem = value_logical_not (arg1);
       if (tem)
 	{
 	  arg2 = std::get<1> (m_storage)->evaluate (nullptr, exp, noside);
diff --git a/gdb/language.c b/gdb/language.c
index 45ce2ebcc92..f52956022ac 100644
--- a/gdb/language.c
+++ b/gdb/language.c
@@ -394,23 +394,6 @@ pointer_type (struct type *type)
 {
   return type->code () == TYPE_CODE_PTR || TYPE_IS_REFERENCE (type);
 }
-
-\f
-/* This page contains functions that return info about
-   (struct value) values used in GDB.  */
-
-/* Returns non-zero if the value VAL represents a true value.  */
-int
-value_true (struct value *val)
-{
-  /* It is possible that we should have some sort of error if a non-boolean
-     value is used in this context.  Possibly dependent on some kind of
-     "boolean-checking" option like range checking.  But it should probably
-     not depend on the language except insofar as is necessary to identify
-     a "boolean" value (i.e. in C using a float, pointer, etc., as a boolean
-     should be an error, probably).  */
-  return !value_logical_not (val);
-}
 \f
 /* This page contains functions for the printing out of
    error messages that occur during type- and range-
diff --git a/gdb/language.h b/gdb/language.h
index 40d22d20559..312662055e5 100644
--- a/gdb/language.h
+++ b/gdb/language.h
@@ -782,10 +782,6 @@ extern int pointer_type (struct type *);
 
 extern void range_error (const char *, ...) ATTRIBUTE_PRINTF (1, 2);
 
-/* Data:  Does this value represent "truth" to the current language?  */
-
-extern int value_true (struct value *);
-
 /* Misc:  The string representing a particular enum language.  */
 
 extern enum language language_enum (const char *str);
diff --git a/gdb/opencl-lang.c b/gdb/opencl-lang.c
index 136969ead76..b877de400d7 100644
--- a/gdb/opencl-lang.c
+++ b/gdb/opencl-lang.c
@@ -748,7 +748,7 @@ opencl_logical_binop_operation::evaluate (struct type *expect_type,
       /* For scalar built-in types, only evaluate the right
 	 hand operand if the left hand operand compares
 	 unequal(&&)/equal(||) to 0.  */
-      int tmp = value_logical_not (arg1);
+      bool tmp = value_logical_not (arg1);
 
       if (op == BINOP_LOGICAL_OR)
 	tmp = !tmp;
diff --git a/gdb/valarith.c b/gdb/valarith.c
index 9ebad648b27..8acd83545f7 100644
--- a/gdb/valarith.c
+++ b/gdb/valarith.c
@@ -1655,7 +1655,7 @@ value_binop (struct value *arg1, struct value *arg2, enum exp_opcode op)
 \f
 /* Simulate the C operator ! -- return 1 if ARG1 contains zero.  */
 
-int
+bool
 value_logical_not (struct value *arg1)
 {
   int len;
diff --git a/gdb/value.h b/gdb/value.h
index e1c6aabfa29..c5847290564 100644
--- a/gdb/value.h
+++ b/gdb/value.h
@@ -1022,7 +1022,14 @@ extern int value_equal_contents (struct value *arg1, struct value *arg2);
 
 extern int value_less (struct value *arg1, struct value *arg2);
 
-extern int value_logical_not (struct value *arg1);
+extern bool value_logical_not (struct value *arg1);
+
+/* Returns non-zero if the value VAL represents a true value.  */
+static inline bool
+value_true (struct value *val)
+{
+  return !value_logical_not (val);
+}
 
 /* C++ */
 

Re: [PATCH] Move value_true to value.h

[Pedro Alves]

On 2021-09-23 10:01 p.m., Tom Tromey wrote:

> commit 12fb19e547b8352fb2fdaafc9e0895eb34804d0d
> Author: Tom Tromey <tom@tromey.com>
> Date:   Thu Aug 26 18:17:40 2021 -0600
> 
>     Move value_true to value.h
>     
>     I noticed that value_true is declared in language.h and defined in
>     language.c.  However, as part of the value API, I think it would be
>     better in one of those files.  And, because it is very short, I
>     changed it to be an inline function in value.h.  I've also removed a
>     comment from the implementation, on the basis that it seems obsolete
>     -- if the change it suggests was needed, it probably would have been
>     done by now; and if it is needed in the future, odds are it would be
>     done differently anyway.
>     
>     Finally, this patch also changes value_true and value_logical_not to
>     return a bool, and updates some uses.

Still OK.  Though I noticed a couple comments that could use tweaking.


> diff --git a/gdb/valarith.c b/gdb/valarith.c
> index 9ebad648b27..8acd83545f7 100644
> --- a/gdb/valarith.c
> +++ b/gdb/valarith.c
> @@ -1655,7 +1655,7 @@ value_binop (struct value *arg1, struct value *arg2, enum exp_opcode op)
>  \f
>  /* Simulate the C operator ! -- return 1 if ARG1 contains zero.  */

"Return true".  Consider moving to .h file.

>  
> -int
> +bool
>  value_logical_not (struct value *arg1)
>  {
>    int len;
> diff --git a/gdb/value.h b/gdb/value.h
> index e1c6aabfa29..c5847290564 100644
> --- a/gdb/value.h
> +++ b/gdb/value.h

> @@ -1022,7 +1022,14 @@ extern int value_equal_contents (struct value *arg1, struct value *arg2);
>  
>  extern int value_less (struct value *arg1, struct value *arg2);
>  
> -extern int value_logical_not (struct value *arg1);
> +extern bool value_logical_not (struct value *arg1);
> +
> +/* Returns non-zero if the value VAL represents a true value.  */

"Returns true".

> +static inline bool
> +value_true (struct value *val)
> +{
> +  return !value_logical_not (val);
> +}
>  
>  /* C++ */
>  
> 

Re: [PATCH] Move value_true to value.h

[Tom Tromey]

Pedro> Still OK.  Though I noticed a couple comments that could use tweaking.

I've made these changes and will check it in shortly.  Thanks.

Tom