Re: Patch: New implementation of -Wstrict-aliasing

[Andrew Pinski <pinskia at gmail dot com>]

On Fri, 2007-01-19 at 18:12 -0800, Silvius Rus wrote:
> -  return *((int *)buf); /* { dg-warning "strict-aliasing" } */
> +  return *((int *)buf); /* { dg-bogus "XXX: should really be a
> warning" } */

If this should be a warning and we don't warn anymore, then this should
be instead xfail and a bug should be filed after this patch goes in.
the way to fail it is to do:
/* { dg-warning "strict-aliasing" "" { xfail *-*-* } } */


One more comment about another testcase or two:
g++.dg/warn/Wstrict-aliasing-bogus-char-2.C 

that testcase is actually violating C/C++ aliasing rules.
+  char buffer[4];
+  int* pi = reinterpret_cast<int*> (buffer);  /* { dg-bogus "char" } */
+  buffer[1] = 0;
+  return *pi;

The reason is that you are accessing a character as an integer which is
not allowed, you can do the opposite; that is you can access an integer
as a character.

Take the following C++ code example:

char buffer[4];
int* pi = reinterpret_cast<int*> (buffer);
float* pi1 = reinterpret_cast<float*> (buffer);
buffer[1] = 0;
*pi1 = *pi;
return *pi;

Do we warn at all for this case?


There should be a warning for g
++.dg/warn/Wstrict-aliasing-bogus-union.C 

union U {
  int i;
  float f;
};
float g(int *a, float *b)
{
  *a = 1;
  return *b;
}

float foo () {
  union U u;
  return g(&u.i, &u.f);
}

The above code is undefined, even though unions are "holly" in GCC.

I would actually not use "strict" in the warning at all because it is
wrong as "strict" is not really anything except saying GCC follows the C
standard here (except we don't fully for character types).

You have a couple of style issues in your patch:
+		 get_ref_type_str(rt1),

space between the function name and '('.


Also even though you already make sure the warning is only emitted with
-Wstrict-aliasing, it is still good to include Wstrict_aliasing as the
first argument for warning so that people can do
-Werror=Wstrict-aliasing as documented in the manual. 

I also think the warning should not really be using %s at all, there are
pretty-printers for a reason, they really should be quoted if %s is to
be used.  Also I noticed you will print out (unknown) if the decl is a
RESULT_DECL, I rather not warn about any artificial decls at all, or
maybe even ICE at that point with checking enabled because that means
the middle-end is producing invalid stores/loads.

-- Pinski