Re: fixes for type-punning warnings in GCC 4.1

[Alexandre Oliva <aoliva@redhat.com>]

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

On Feb  9, 2006, Daniel Jacobowitz <drow@false.org> wrote:

> On Thu, Feb 09, 2006 at 10:32:48PM -0200, Alexandre Oliva wrote:
>> On Feb  8, 2006, Daniel Jacobowitz <drow@false.org> wrote:
>> 
>> > On Wed, Feb 08, 2006 at 12:48:11AM -0200, Alexandre Oliva wrote:
>> >> On Jan 22, 2006, Daniel Jacobowitz <drow@false.org> wrote:
>> >> 
>> >> > The output is always a DOUBLEST.  I don't know of any reason why we
>> >> > should enable HAVE_LONG_DOUBLE if we can't printf and scanf them; would
>> >> > this be simpler in that case?  Don't make DOUBLEST something we can't
>> >> > scan or print.
>> >> 
>> >> Sounds good to me.  Ok to install?
>> 
>> > Well, it's not right as-is; you need to look at the other uses of
>> > HAVE_LONG_DOUBLE.
>> 
>> Did.  The other uses of HAVE_LONG_DOUBLE are correct, since they do
>> not assume DOUBLEST is long double and they make no attempts at
>> printing long doubles directly.

> Disagree; did you read the bit of my message that you snipped?
> doublest.c jumps through unnecessary hoops casting to long double and
> back to handle a DOUBLEST if this is defined.

Yep.  It didn't look like those two occurrences were such a big deal.
I don't see why we should refuse to handle long double at all just
because we can't scan them in as such.  But it's not my call, I guess.

>> > Would you mind terribly fixing that, adding a changelog, and leaving
>> > out the tui-data change for now?

>> gdb won't build without the tui-data change.  What's wrong with adding
>> the temporary fix now, such that it builds, until someone with a
>> better understanding can go ahead and re-engineer the data structure
>> correctly?

> Sorry, use -Wno-error if you're in that much of a hurry.  I even
> offered to take care of it for you.  If your fix goes in, it will never
> leave; we have plenty of experience with FIXMEs in GDB to back that up,
> I think.

Ok, I tried changing the type in the struct declaration and that
seems to have worked, so I went ahead and removed the now-redundant
type casts.

Ok to install?


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: gdb-type-punning-4.patch --]
[-- Type: text/x-patch, Size: 12346 bytes --]

for gdb/ChangeLog
from  Alexandre Oliva  <aoliva@redhat.com>

	* doublest.h (DOUBLEST): Use long double only if we can scan
	it in.  Undefine HAVE_LONG_DOUBLE otherwise.
	(DOUBLEST_FORMAT): New.
	* c-exp.y (parse_number): Use it.
	* jv-exp.y (parse_number): Likewise.
	* objc-exp.y (parse_number): Likewise.
	* p-exp.y (parse_number): Likewise.
	* varobj.c (free_variable): Silence type-punning warnings.
	* tui/tui-data.h (struct tui_list): Change type of list member.
	* tui/tui-data.c: Remove no-longer-needed type casts.
	(source_windows): Silence type-punning warnings.
	* tui/tui-stack.c, tui/tui-win.c, tui/tui-winsource.c: Likewise.

Index: gdb/c-exp.y
===================================================================
--- gdb/c-exp.y.orig	2006-02-13 16:33:29.000000000 -0200
+++ gdb/c-exp.y	2006-02-13 16:34:55.000000000 -0200
@@ -1080,24 +1080,8 @@ parse_number (p, len, parsed_float, puti
       char saved_char = p[len];
 
       p[len] = 0;	/* null-terminate the token */
-
-      if (sizeof (putithere->typed_val_float.dval) <= sizeof (float))
-	num = sscanf (p, "%g%s", (float *) &putithere->typed_val_float.dval,s);
-      else if (sizeof (putithere->typed_val_float.dval) <= sizeof (double))
-	num = sscanf (p, "%lg%s", (double *) &putithere->typed_val_float.dval,s);
-      else
-	{
-#ifdef SCANF_HAS_LONG_DOUBLE
-	  num = sscanf (p, "%Lg%s", &putithere->typed_val_float.dval,s);
-#else
-	  /* Scan it into a double, then assign it to the long double.
-	     This at least wins with values representable in the range
-	     of doubles. */
-	  double temp;
-	  num = sscanf (p, "%lg%s", &temp,s);
-	  putithere->typed_val_float.dval = temp;
-#endif
-	}
+      num = sscanf (p, DOUBLEST_FORMAT "%s",
+		    &putithere->typed_val_float.dval, s);
       p[len] = saved_char;	/* restore the input stream */
 
       if (num == 1)
Index: gdb/jv-exp.y
===================================================================
--- gdb/jv-exp.y.orig	2006-02-13 16:33:29.000000000 -0200
+++ gdb/jv-exp.y	2006-02-13 16:34:55.000000000 -0200
@@ -713,23 +713,8 @@ parse_number (p, len, parsed_float, puti
       char saved_char = p[len];
 
       p[len] = 0;	/* null-terminate the token */
-      if (sizeof (putithere->typed_val_float.dval) <= sizeof (float))
-	num = sscanf (p, "%g%c", (float *) &putithere->typed_val_float.dval, &c);
-      else if (sizeof (putithere->typed_val_float.dval) <= sizeof (double))
-	num = sscanf (p, "%lg%c", (double *) &putithere->typed_val_float.dval, &c);
-      else
-	{
-#ifdef SCANF_HAS_LONG_DOUBLE
-	  num = sscanf (p, "%Lg%c", &putithere->typed_val_float.dval, &c);
-#else
-	  /* Scan it into a double, then assign it to the long double.
-	     This at least wins with values representable in the range
-	     of doubles. */
-	  double temp;
-	  num = sscanf (p, "%lg%c", &temp, &c);
-	  putithere->typed_val_float.dval = temp;
-#endif
-	}
+      num = sscanf (p, DOUBLEST_FORMAT "%c",
+		    &putithere->typed_val_float.dval, &c);
       p[len] = saved_char;	/* restore the input stream */
       if (num != 1) 		/* check scanf found ONLY a float ... */
 	return ERROR;
Index: gdb/objc-exp.y
===================================================================
--- gdb/objc-exp.y.orig	2006-02-13 16:33:29.000000000 -0200
+++ gdb/objc-exp.y	2006-02-13 16:34:55.000000000 -0200
@@ -1025,23 +1025,8 @@ parse_number (p, len, parsed_float, puti
 
       /* It's a float since it contains a point or an exponent.  */
 
-      if (sizeof (putithere->typed_val_float.dval) <= sizeof (float))
-	sscanf (p, "%g", (float *)&putithere->typed_val_float.dval);
-      else if (sizeof (putithere->typed_val_float.dval) <= sizeof (double))
-	sscanf (p, "%lg", (double *)&putithere->typed_val_float.dval);
-      else
-	{
-#ifdef PRINTF_HAS_LONG_DOUBLE
-	  sscanf (p, "%Lg", &putithere->typed_val_float.dval);
-#else
-	  /* Scan it into a double, then assign it to the long double.
-	     This at least wins with values representable in the range
-	     of doubles.  */
-	  double temp;
-	  sscanf (p, "%lg", &temp);
-	  putithere->typed_val_float.dval = temp;
-#endif
-	}
+      sscanf (p, DOUBLEST_FORMAT "%c",
+	      &putithere->typed_val_float.dval, &c);
 
       /* See if it has `f' or `l' suffix (float or long double).  */
 
Index: gdb/p-exp.y
===================================================================
--- gdb/p-exp.y.orig	2006-02-13 16:33:29.000000000 -0200
+++ gdb/p-exp.y	2006-02-13 16:34:55.000000000 -0200
@@ -799,23 +799,8 @@ parse_number (p, len, parsed_float, puti
       char saved_char = p[len];
 
       p[len] = 0;	/* null-terminate the token */
-      if (sizeof (putithere->typed_val_float.dval) <= sizeof (float))
-	num = sscanf (p, "%g%c", (float *) &putithere->typed_val_float.dval,&c);
-      else if (sizeof (putithere->typed_val_float.dval) <= sizeof (double))
-	num = sscanf (p, "%lg%c", (double *) &putithere->typed_val_float.dval,&c);
-      else
-	{
-#ifdef SCANF_HAS_LONG_DOUBLE
-	  num = sscanf (p, "%Lg%c", &putithere->typed_val_float.dval,&c);
-#else
-	  /* Scan it into a double, then assign it to the long double.
-	     This at least wins with values representable in the range
-	     of doubles. */
-	  double temp;
-	  num = sscanf (p, "%lg%c", &temp,&c);
-	  putithere->typed_val_float.dval = temp;
-#endif
-	}
+      num = sscanf (p, DOUBLEST_FORMAT "%c",
+		    &putithere->typed_val_float.dval, &c);
       p[len] = saved_char;	/* restore the input stream */
       if (num != 1) 		/* check scanf found ONLY a float ... */
 	return ERROR;
Index: gdb/varobj.c
===================================================================
--- gdb/varobj.c.orig	2006-02-13 16:33:29.000000000 -0200
+++ gdb/varobj.c	2006-02-13 16:34:55.000000000 -0200
@@ -1374,7 +1374,7 @@ free_variable (struct varobj *var)
   /* Free the expression if this is a root variable. */
   if (var->root->rootvar == var)
     {
-      free_current_contents ((char **) &var->root->exp);
+      free_current_contents (&var->root->exp);
       xfree (var->root);
     }
 
Index: gdb/doublest.h
===================================================================
--- gdb/doublest.h.orig	2006-02-13 16:33:29.000000000 -0200
+++ gdb/doublest.h	2006-02-13 16:34:55.000000000 -0200
@@ -48,10 +48,15 @@ struct floatformat;
    host's `long double'.  In general, we'll probably reduce the precision of
    any such values and print a warning.  */
 
-#ifdef HAVE_LONG_DOUBLE
+#if defined HAVE_LONG_DOUBLE && defined SCANF_HAS_LONG_DOUBLE
 typedef long double DOUBLEST;
+# define DOUBLEST_FORMAT "%Lg"
 #else
 typedef double DOUBLEST;
+# define DOUBLEST_FORMAT "%g"
+/* If we can't scan or print long double, we don't want to use it
+   anywhere.  */
+# undef HAVE_LONG_DOUBLE
 #endif
 
 extern void floatformat_to_doublest (const struct floatformat *,
Index: gdb/tui/tui-data.c
===================================================================
--- gdb/tui/tui-data.c.orig	2006-02-13 16:51:35.000000000 -0200
+++ gdb/tui/tui-data.c	2006-02-13 16:52:07.000000000 -0200
@@ -44,7 +44,7 @@ static int term_height, term_width;
 static struct tui_gen_win_info _locator;
 static struct tui_gen_win_info exec_info[2];
 static struct tui_win_info * src_win_list[2];
-static struct tui_list source_windows = {(void **) src_win_list, 0};
+static struct tui_list source_windows = {src_win_list, 0};
 static int default_tab_len = DEFAULT_TAB_LEN;
 static struct tui_win_info * win_with_focus = (struct tui_win_info *) NULL;
 static struct tui_layout_def layout_def =
@@ -182,7 +182,7 @@ tui_clear_source_windows_detail (void)
   int i;
 
   for (i = 0; i < (tui_source_windows ())->count; i++)
-    tui_clear_win_detail ((struct tui_win_info *) (tui_source_windows ())->list[i]);
+    tui_clear_win_detail ((tui_source_windows ())->list[i]);
 }
 
 
@@ -782,7 +782,7 @@ tui_free_all_source_wins_content (void)
 
   for (i = 0; i < (tui_source_windows ())->count; i++)
     {
-      struct tui_win_info * win_info = (struct tui_win_info *) (tui_source_windows ())->list[i];
+      struct tui_win_info * win_info = (tui_source_windows ())->list[i];
 
       if (win_info != NULL)
 	{
Index: gdb/tui/tui-data.h
===================================================================
--- gdb/tui/tui-data.h.orig	2006-02-13 16:51:35.000000000 -0200
+++ gdb/tui/tui-data.h	2006-02-13 16:52:07.000000000 -0200
@@ -110,7 +110,7 @@ enum tui_scroll_direction
 /* General list struct.  */
 struct tui_list
 {
-  void **list;
+  struct tui_win_info **list;
   int count;
 };
 
Index: gdb/tui/tui-stack.c
===================================================================
--- gdb/tui/tui-stack.c.orig	2006-02-13 16:51:35.000000000 -0200
+++ gdb/tui/tui-stack.c	2006-02-13 16:52:07.000000000 -0200
@@ -343,7 +343,7 @@ tui_show_frame_info (struct frame_info *
       for (i = 0; i < (tui_source_windows ())->count; i++)
 	{
 	  union tui_which_element *item;
-	  win_info = (struct tui_win_info *) (tui_source_windows ())->list[i];
+	  win_info = (tui_source_windows ())->list[i];
 
 	  item = &((struct tui_win_element *) locator->content[0])->which_element;
 	  if (win_info == TUI_SRC_WIN)
@@ -401,7 +401,7 @@ tui_show_frame_info (struct frame_info *
       tui_show_locator_content ();
       for (i = 0; i < (tui_source_windows ())->count; i++)
 	{
-	  win_info = (struct tui_win_info *) (tui_source_windows ())->list[i];
+	  win_info = (tui_source_windows ())->list[i];
 	  tui_clear_source_content (win_info, EMPTY_SOURCE_PROMPT);
 	  tui_update_exec_info (win_info);
 	}
Index: gdb/tui/tui-win.c
===================================================================
--- gdb/tui/tui-win.c.orig	2006-02-13 16:51:35.000000000 -0200
+++ gdb/tui/tui-win.c	2006-02-13 16:52:07.000000000 -0200
@@ -1119,7 +1119,7 @@ tui_adjust_win_heights (struct tui_win_i
 	      make_invisible_and_set_new_height (primary_win_info, new_height);
 	      if (primary_win_info->generic.type == CMD_WIN)
 		{
-		  win_info = (struct tui_win_info *) (tui_source_windows ())->list[0];
+		  win_info = (tui_source_windows ())->list[0];
 		  src_win_info = win_info;
 		}
 	      else
@@ -1148,7 +1148,7 @@ tui_adjust_win_heights (struct tui_win_i
 	      else
 		{
 		  first_win = TUI_DATA_WIN;
-		  second_win = (struct tui_win_info *) (tui_source_windows ())->list[0];
+		  second_win = (tui_source_windows ())->list[0];
 		}
 	      if (primary_win_info == TUI_CMD_WIN)
 		{		/*
@@ -1393,7 +1393,7 @@ new_height_ok (struct tui_win_info * pri
 	      struct tui_win_info * win_info;
 
 	      if (primary_win_info == TUI_CMD_WIN)
-		win_info = (struct tui_win_info *) (tui_source_windows ())->list[0];
+		win_info = (tui_source_windows ())->list[0];
 	      else
 		win_info = TUI_CMD_WIN;
 	      ok = ((new_height +
@@ -1414,7 +1414,7 @@ new_height_ok (struct tui_win_info * pri
 	  else
 	    {
 	      first_win = TUI_DATA_WIN;
-	      second_win = (struct tui_win_info *) (tui_source_windows ())->list[0];
+	      second_win = (tui_source_windows ())->list[0];
 	    }
 	  /*
 	     ** We could simply add all the heights to obtain the same result
@@ -1543,7 +1543,7 @@ parse_scrolling_args (char *arg, struct 
 	    error (_("Invalid window specified. \n\
 The window name specified must be valid and visible.\n"));
 	  else if (*win_to_scroll == TUI_CMD_WIN)
-	    *win_to_scroll = (struct tui_win_info *) (tui_source_windows ())->list[0];
+	    *win_to_scroll = (tui_source_windows ())->list[0];
 	}
       xfree (buf);
     }
Index: gdb/tui/tui-winsource.c
===================================================================
--- gdb/tui/tui-winsource.c.orig	2006-02-13 16:51:35.000000000 -0200
+++ gdb/tui/tui-winsource.c	2006-02-13 16:52:07.000000000 -0200
@@ -160,7 +160,7 @@ tui_update_source_windows_with_addr (COR
 
       for (i = 0; i < (tui_source_windows ())->count; i++)
 	{
-	  struct tui_win_info * win_info = (struct tui_win_info *) (tui_source_windows ())->list[i];
+	  struct tui_win_info * win_info = (tui_source_windows ())->list[i];
 
 	  tui_clear_source_content (win_info, EMPTY_SOURCE_PROMPT);
 	  tui_clear_exec_info_content (win_info);
@@ -384,7 +384,7 @@ tui_update_all_breakpoint_info (void)
 
   for (i = 0; i < list->count; i++)
     {
-      struct tui_win_info * win = (struct tui_win_info *) list->list[i];
+      struct tui_win_info * win = list->list[i];
 
       if (tui_update_breakpoint_info (win, FALSE))
         {

[-- Attachment #3: Type: text/plain, Size: 250 bytes --]



-- 
Alexandre Oliva         http://www.lsd.ic.unicamp.br/~oliva/
Secretary for FSF Latin America        http://www.fsfla.org/
Red Hat Compiler Engineer   aoliva@{redhat.com, gcc.gnu.org}
Free Software Evangelist  oliva@{lsd.ic.unicamp.br, gnu.org}