[PATCH 0/2] More bool in ada-lang.c

[PATCH 0/2] More bool in ada-lang.c

[Tom Tromey]

I found a couple more spots in ada-lang.c that could use bool.

Signed-off-by: Tom Tromey <tromey@adacore.com>
---
Tom Tromey (2):
      Use bool in ada_add_all_symbols
      Use bool in map_matching_symbols

 gdb/ada-lang.c | 44 ++++++++++++++++++++++----------------------
 1 file changed, 22 insertions(+), 22 deletions(-)
---
base-commit: 989ac09520f7d421a6be515ea6b346c597111eab
change-id: 20260424-ada-bool-again-9f00dde8910d

Best regards,
-- 
Tom Tromey <tromey@adacore.com>

[PATCH 1/2] Use bool in ada_add_all_symbols

[Tom Tromey]

This changes ada_add_all_symbols to use bool, then fixes up the
callers.
---
 gdb/ada-lang.c | 36 ++++++++++++++++++------------------
 1 file changed, 18 insertions(+), 18 deletions(-)

diff --git a/gdb/ada-lang.c b/gdb/ada-lang.c
index 71a338ce17e..afdd59965fd 100644
--- a/gdb/ada-lang.c
+++ b/gdb/ada-lang.c
@@ -104,7 +104,7 @@ static void ada_add_block_symbols (std::vector<struct block_symbol> &,
 static void ada_add_all_symbols (std::vector<struct block_symbol> &,
 				 const struct block *,
 				 const lookup_name_info &lookup_name,
-				 domain_search_flags, int, int *);
+				 domain_search_flags, bool, bool *);
 
 static bool is_nonfunction (const std::vector<struct block_symbol> &);
 
@@ -5528,7 +5528,7 @@ ada_add_block_renamings (std::vector<struct block_symbol> &result,
 	  lookup_name_info decl_lookup_name (r_name,
 					     lookup_name.match_type ());
 	  ada_add_all_symbols (result, block, decl_lookup_name, domain,
-			       1, NULL);
+			       true, nullptr);
 	}
     }
   return result.size () != defns_mark;
@@ -5612,10 +5612,10 @@ add_nonlocal_symbols (std::vector<struct block_symbol> &result,
 }
 
 /* Find symbols in DOMAIN matching LOOKUP_NAME, in BLOCK and, if
-   FULL_SEARCH is non-zero, enclosing scope and in global scopes,
+   FULL_SEARCH is true, enclosing scope and in global scopes,
    returning the number of matches.  Add these to RESULT.
 
-   When FULL_SEARCH is non-zero, any non-function/non-enumeral
+   When FULL_SEARCH is false, any non-function/non-enumeral
    symbol match within the nest of blocks whose innermost member is BLOCK,
    is the one match returned (no other matches in that or
    enclosing blocks is returned).  If there are any matches in or
@@ -5633,13 +5633,13 @@ ada_add_all_symbols (std::vector<struct block_symbol> &result,
 		     const struct block *block,
 		     const lookup_name_info &lookup_name,
 		     domain_search_flags domain,
-		     int full_search,
-		     int *made_global_lookup_p)
+		     bool full_search,
+		     bool *made_global_lookup_p)
 {
   struct symbol *sym;
 
-  if (made_global_lookup_p)
-    *made_global_lookup_p = 0;
+  if (made_global_lookup_p != nullptr)
+    *made_global_lookup_p = false;
 
   /* Special case: If the user specifies a symbol name inside package
      Standard, do a non-wild matching of the symbol name without
@@ -5679,8 +5679,8 @@ ada_add_all_symbols (std::vector<struct block_symbol> &result,
       return;
     }
 
-  if (made_global_lookup_p)
-    *made_global_lookup_p = 1;
+  if (made_global_lookup_p != nullptr)
+    *made_global_lookup_p = true;
 
   /* Search symbols from all global blocks.  */
 
@@ -5694,12 +5694,12 @@ ada_add_all_symbols (std::vector<struct block_symbol> &result,
 }
 
 /* Find symbols in DOMAIN matching LOOKUP_NAME, in BLOCK and, if FULL_SEARCH
-   is non-zero, enclosing scope and in global scopes.
+   is true, enclosing scope and in global scopes.
 
    Returns (SYM,BLOCK) tuples, indicating the symbols found and the
    blocks and symbol tables (if any) in which they were found.
 
-   When full_search is non-zero, any non-function/non-enumeral
+   When full_search is false, any non-function/non-enumeral
    symbol match within the nest of blocks whose innermost member is BLOCK,
    is the one match returned (no other matches in that or
    enclosing blocks is returned).  If there are any matches in or
@@ -5712,9 +5712,9 @@ static std::vector<struct block_symbol>
 ada_lookup_symbol_list_worker (const lookup_name_info &lookup_name,
 			       const struct block *block,
 			       domain_search_flags domain,
-			       int full_search)
+			       bool full_search)
 {
-  int syms_from_global_search;
+  bool syms_from_global_search;
   std::vector<struct block_symbol> results;
 
   ada_add_all_symbols (results, block, lookup_name,
@@ -5745,11 +5745,11 @@ ada_lookup_symbol_list (const char *name, const struct block *block,
   symbol_name_match_type name_match_type = name_match_type_from_name (name);
   lookup_name_info lookup_name (name, name_match_type);
 
-  return ada_lookup_symbol_list_worker (lookup_name, block, domain, 1);
+  return ada_lookup_symbol_list_worker (lookup_name, block, domain, true);
 }
 
 /* The result is as for ada_lookup_symbol_list with FULL_SEARCH set
-   to 1, but choosing the first symbol found if there are multiple
+   to true, but choosing the first symbol found if there are multiple
    choices.  */
 
 block_symbol
@@ -11510,7 +11510,7 @@ get_var_value (const char *name, const char *err_msg)
   std::vector<struct block_symbol> syms
     = ada_lookup_symbol_list_worker (lookup_name,
 				     get_selected_block (0),
-				     SEARCH_VFT, 1);
+				     SEARCH_VFT, true);
 
   if (syms.size () != 1)
     {
@@ -13702,7 +13702,7 @@ class ada_language : public language_defn
 	 for_each_symbol_callback_ftype callback) const override
   {
     std::vector<struct block_symbol> results
-      = ada_lookup_symbol_list_worker (name, block, domain, 0);
+      = ada_lookup_symbol_list_worker (name, block, domain, false);
     for (block_symbol &sym : results)
       callback (&sym);
   }

-- 
2.53.0

Re: [PATCH 1/2] Use bool in ada_add_all_symbols

[Andrew Burgess]

Tom Tromey <tromey@adacore.com> writes:

> This changes ada_add_all_symbols to use bool, then fixes up the
> callers.
> ---
>  gdb/ada-lang.c | 36 ++++++++++++++++++------------------
>  1 file changed, 18 insertions(+), 18 deletions(-)
>
> diff --git a/gdb/ada-lang.c b/gdb/ada-lang.c
> index 71a338ce17e..afdd59965fd 100644
> --- a/gdb/ada-lang.c
> +++ b/gdb/ada-lang.c
> @@ -104,7 +104,7 @@ static void ada_add_block_symbols (std::vector<struct block_symbol> &,
>  static void ada_add_all_symbols (std::vector<struct block_symbol> &,
>  				 const struct block *,
>  				 const lookup_name_info &lookup_name,
> -				 domain_search_flags, int, int *);
> +				 domain_search_flags, bool, bool *);
>  
>  static bool is_nonfunction (const std::vector<struct block_symbol> &);
>  
> @@ -5528,7 +5528,7 @@ ada_add_block_renamings (std::vector<struct block_symbol> &result,
>  	  lookup_name_info decl_lookup_name (r_name,
>  					     lookup_name.match_type ());
>  	  ada_add_all_symbols (result, block, decl_lookup_name, domain,
> -			       1, NULL);
> +			       true, nullptr);
>  	}
>      }
>    return result.size () != defns_mark;
> @@ -5612,10 +5612,10 @@ add_nonlocal_symbols (std::vector<struct block_symbol> &result,
>  }
>  
>  /* Find symbols in DOMAIN matching LOOKUP_NAME, in BLOCK and, if
> -   FULL_SEARCH is non-zero, enclosing scope and in global scopes,
> +   FULL_SEARCH is true, enclosing scope and in global scopes,
>     returning the number of matches.  Add these to RESULT.
>  
> -   When FULL_SEARCH is non-zero, any non-function/non-enumeral
> +   When FULL_SEARCH is false, any non-function/non-enumeral

I found the description in this comment for what FULL_SEARCH actually
does really confusing.

I noticed that you replaced non-zero with false not true, which is an
inversion of the logic.  I thought I'd just read the comment and check
the code, then I'd know if this was a fix or not.  But I cannot tell
from the comment what the code is supposed to do, so I'm stuck.

All I know is that the code didn't change, but the comment has flipped
logic, and the commit message doesn't call this out as a fix, so I
wanted to check with you.

>     symbol match within the nest of blocks whose innermost member is BLOCK,
>     is the one match returned (no other matches in that or
>     enclosing blocks is returned).  If there are any matches in or
> @@ -5633,13 +5633,13 @@ ada_add_all_symbols (std::vector<struct block_symbol> &result,
>  		     const struct block *block,
>  		     const lookup_name_info &lookup_name,
>  		     domain_search_flags domain,
> -		     int full_search,
> -		     int *made_global_lookup_p)
> +		     bool full_search,
> +		     bool *made_global_lookup_p)
>  {
>    struct symbol *sym;
>  
> -  if (made_global_lookup_p)
> -    *made_global_lookup_p = 0;
> +  if (made_global_lookup_p != nullptr)
> +    *made_global_lookup_p = false;
>  
>    /* Special case: If the user specifies a symbol name inside package
>       Standard, do a non-wild matching of the symbol name without
> @@ -5679,8 +5679,8 @@ ada_add_all_symbols (std::vector<struct block_symbol> &result,
>        return;
>      }
>  
> -  if (made_global_lookup_p)
> -    *made_global_lookup_p = 1;
> +  if (made_global_lookup_p != nullptr)
> +    *made_global_lookup_p = true;
>  
>    /* Search symbols from all global blocks.  */
>  
> @@ -5694,12 +5694,12 @@ ada_add_all_symbols (std::vector<struct block_symbol> &result,
>  }
>  
>  /* Find symbols in DOMAIN matching LOOKUP_NAME, in BLOCK and, if FULL_SEARCH
> -   is non-zero, enclosing scope and in global scopes.
> +   is true, enclosing scope and in global scopes.
>  
>     Returns (SYM,BLOCK) tuples, indicating the symbols found and the
>     blocks and symbol tables (if any) in which they were found.
>  
> -   When full_search is non-zero, any non-function/non-enumeral
> +   When full_search is false, any non-function/non-enumeral

While here it would be good to capitalise FULL_SEARCH.  Also non-zero
became false here instead of true, as with ada_add_all_symbols, is this
intentional?

The actual code changes look fine though.

Approved-By: Andrew Burgess <aburgess@redhat.com>

Thanks,
Andrew

Re: [PATCH 1/2] Use bool in ada_add_all_symbols

[Tom Tromey]

>>>>> "Andrew" == Andrew Burgess <aburgess@redhat.com> writes:

>> -   FULL_SEARCH is non-zero, enclosing scope and in global scopes,
>> +   FULL_SEARCH is true, enclosing scope and in global scopes,
>> returning the number of matches.  Add these to RESULT.
>> 
>> -   When FULL_SEARCH is non-zero, any non-function/non-enumeral
>> +   When FULL_SEARCH is false, any non-function/non-enumeral

Andrew> I found the description in this comment for what FULL_SEARCH actually
Andrew> does really confusing.

Andrew> I noticed that you replaced non-zero with false not true, which is an
Andrew> inversion of the logic.  I thought I'd just read the comment and check
Andrew> the code, then I'd know if this was a fix or not.  But I cannot tell
Andrew> from the comment what the code is supposed to do, so I'm stuck.

Andrew> All I know is that the code didn't change, but the comment has flipped
Andrew> logic, and the commit message doesn't call this out as a fix, so I
Andrew> wanted to check with you.

Yeah, that was just a mistake in the comment update.

>> -   When full_search is non-zero, any non-function/non-enumeral
>> +   When full_search is false, any non-function/non-enumeral

Andrew> While here it would be good to capitalise FULL_SEARCH.  Also non-zero
Andrew> became false here instead of true, as with ada_add_all_symbols, is this
Andrew> intentional?

I fixed this too, it was not intentional.

Tom

[PATCH 2/2] Use bool in map_matching_symbols

[Tom Tromey]

This changes ada-lang.c:map_matching_symbols to take a bool parameter,
then fixes up the callers.
---
 gdb/ada-lang.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/gdb/ada-lang.c b/gdb/ada-lang.c
index afdd59965fd..47433ccf6b7 100644
--- a/gdb/ada-lang.c
+++ b/gdb/ada-lang.c
@@ -5551,7 +5551,7 @@ static void
 map_matching_symbols (struct objfile *objfile,
 		      const lookup_name_info &lookup_name,
 		      domain_search_flags domain,
-		      int global,
+		      bool global,
 		      match_data &data)
 {
   data.objfile = objfile;
@@ -5579,7 +5579,7 @@ map_matching_symbols (struct objfile *objfile,
 static void
 add_nonlocal_symbols (std::vector<struct block_symbol> &result,
 		      const lookup_name_info &lookup_name,
-		      domain_search_flags domain, int global)
+		      domain_search_flags domain, bool global)
 {
   struct match_data data (&result);
 
@@ -5684,13 +5684,13 @@ ada_add_all_symbols (std::vector<struct block_symbol> &result,
 
   /* Search symbols from all global blocks.  */
 
-  add_nonlocal_symbols (result, lookup_name, domain, 1);
+  add_nonlocal_symbols (result, lookup_name, domain, true);
 
   /* Now add symbols from all per-file blocks if we've gotten no hits
      (not strictly correct, but perhaps better than an error).  */
 
   if (result.empty ())
-    add_nonlocal_symbols (result, lookup_name, domain, 0);
+    add_nonlocal_symbols (result, lookup_name, domain, false);
 }
 
 /* Find symbols in DOMAIN matching LOOKUP_NAME, in BLOCK and, if FULL_SEARCH

-- 
2.53.0

Re: [PATCH 2/2] Use bool in map_matching_symbols

[Andrew Burgess]

Tom Tromey <tromey@adacore.com> writes:

> This changes ada-lang.c:map_matching_symbols to take a bool parameter,
> then fixes up the callers.
> ---
>  gdb/ada-lang.c | 8 ++++----
>  1 file changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/gdb/ada-lang.c b/gdb/ada-lang.c
> index afdd59965fd..47433ccf6b7 100644
> --- a/gdb/ada-lang.c
> +++ b/gdb/ada-lang.c
> @@ -5551,7 +5551,7 @@ static void
>  map_matching_symbols (struct objfile *objfile,
>  		      const lookup_name_info &lookup_name,
>  		      domain_search_flags domain,
> -		      int global,
> +		      bool global,
>  		      match_data &data)
>  {
>    data.objfile = objfile;
> @@ -5579,7 +5579,7 @@ map_matching_symbols (struct objfile *objfile,
>  static void
>  add_nonlocal_symbols (std::vector<struct block_symbol> &result,
>  		      const lookup_name_info &lookup_name,
> -		      domain_search_flags domain, int global)
> +		      domain_search_flags domain, bool global)

The comment on this function needs updating, it still says non-zero when
discussing GLOBAL.

Otherwise,

Approved-By: Andrew Burgess <aburgess@redhat.com>

Thanks,
Andrew

Re: [PATCH 2/2] Use bool in map_matching_symbols

[Tom Tromey]

>>>>> "Andrew" == Andrew Burgess <aburgess@redhat.com> writes:

Andrew> The comment on this function needs updating, it still says non-zero when
Andrew> discussing GLOBAL.

Thanks, I made this change.

Tom