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)
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! |
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 |