Re: fix for strict-alias warnings

From: "Andrew Dunstan" <andrew(at)dunslane(dot)net>
To: "PG Patches" <pgsql-patches(at)postgresql(dot)org>
Subject: Re: fix for strict-alias warnings
Date: 2003-10-11 17:47:48
Message-ID: 012401c3901f$c95fa3a0$6401a8c0@DUNSLANE
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers pgsql-patches

Tough words! :-)

ISTM the best thing would be to back out the patch, add -fno-strict-aliasing
for gcc, and add a TODO to fix this thoroughly.

Having -fstrict-aliasing on and ignoring the warnings doesn't seem like a
sound strategy. I think we should fix it or turn it off. The web is littered
with projects that got bizzare happenings when they turned it on without any
accompanying code changes.

I agree with Tom that my patch isn't ideal (I thought I said as much).
Fixing it thoroughly will require some significant code changes, though. We
seem to be far too close to 7.4 release to contemplate that.

cheers

andrew

----- Original Message -----
From: "Tom Lane" <tgl(at)sss(dot)pgh(dot)pa(dot)us>
To: "Bruce Momjian" <pgman(at)candle(dot)pha(dot)pa(dot)us>
Cc: "Andrew Dunstan" <andrew(at)dunslane(dot)net>; "PG Patches"
<pgsql-patches(at)postgresql(dot)org>
Sent: Saturday, October 11, 2003 1:29 PM
Subject: Re: [PATCHES] fix for strict-alias warnings

> Bruce Momjian <pgman(at)candle(dot)pha(dot)pa(dot)us> writes:
> > I don't have a version that does the double-cast, but I still have the
> > patch to back out and put in a new one. Andrew's point was that we cast
> > to void * in many places, so this case is not unique. Is that wrong?
>
> I do not like code that uses cast to void* as a substitute for casting
> to the real destination type. I think it's a lazy substitute for
> providing the correct cast, and it renders the code more fragile because
> there is *no* possibility of the compiler detecting a problem should you
> change the source or destination datatype in a way that renders the cast
> wrong.
>
> I have not gone around and tried to fix all the places that are lazy in
> this way, but I don't want to introduce more, and for sure I don't want
> to set a precedent that we'll weaken our type checking any time gcc
> burps for ill-defined reasons.
>
> I agree completely with all of the objections you raised in your
> original comment on the patch. In particular, I don't think we
> understand why gcc is complaining about these few places and not any of
> the thousands of other casts in our code. Until we understand that
> difference completely, we are not "fixing a bug" by introducing void*
> casts. I'd have to call it cargo-cult programming instead.
>
> I am perfectly content to leave the warnings in place until we have a
> satisfactory explanation.
>
> regards, tom lane
>
> ---------------------------(end of broadcast)---------------------------
> TIP 2: you can get off all lists at once with the unregister command
> (send "unregister YourEmailAddressHere" to majordomo(at)postgresql(dot)org)

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Bruce Momjian 2003-10-11 18:10:03 Re: fix for strict-alias warnings
Previous Message Marko Karppinen 2003-10-11 17:46:40 Re: [HACKERS] Sun performance - Major discovery!

Browse pgsql-patches by date

  From Date Subject
Next Message Bruce Momjian 2003-10-11 18:10:03 Re: fix for strict-alias warnings
Previous Message Tom Lane 2003-10-11 17:29:07 Re: fix for strict-alias warnings