diff mbox series

Do not change type in get_discrete_low_bound

Message ID 20230427193515.1060101-1-tromey@adacore.com
State New
Headers
Series Do not change type in get_discrete_low_bound |

Commit Message

Tom Tromey April 27, 2023, 7:35 p.m. UTC 
  get_discrete_low_bound has this code:

    /* Set unsigned indicator if warranted.  */
    if (low >= 0)
      type->set_is_unsigned (true);

It's bad to modify a type in a getter like this, so this patch removes
this code.  FWIW I looked and this code has been there since at least
1999 (it was in the initial sourceware import).

Types in general would benefit from const-ification, which would
probably reveal more code like this, but I haven't attempted that.

Regression tested on x86-64 Fedora 36.
---
 gdb/gdbtypes.c | 4 ----
 1 file changed, 4 deletions(-)

Comments

Kevin Buettner April 28, 2023, 4:07 p.m. UTC | #1 
On Thu, 27 Apr 2023 13:35:15 -0600
Tom Tromey via Gdb-patches <gdb-patches@sourceware.org> wrote:

> get_discrete_low_bound has this code:
> 
>     /* Set unsigned indicator if warranted.  */
>     if (low >= 0)
>       type->set_is_unsigned (true);
> 
> It's bad to modify a type in a getter like this, so this patch removes
> this code.  FWIW I looked and this code has been there since at least
> 1999 (it was in the initial sourceware import).

LGTM.

Reviewed-by: Kevin Buettner <kevinb@redhat.com>
diff mbox series

Patch

diff --git a/gdb/gdbtypes.c b/gdb/gdbtypes.c
index 6af59351b76..c8d5314bce7 100644
--- a/gdb/gdbtypes.c
+++ b/gdb/gdbtypes.c
@@ -1084,10 +1084,6 @@  get_discrete_low_bound (struct type *type)
 		  low = type->field (i).loc_enumval ();
 	      }
 
-	    /* Set unsigned indicator if warranted.  */
-	    if (low >= 0)
-	      type->set_is_unsigned (true);
-
 	    return low;
 	  }
 	else