Re: [HACKERS] lots of unused variable warnings in assert-free builds

2012-03-21 Thread Peter Eisentraut
On tis, 2012-03-20 at 15:04 -0400, Tom Lane wrote: Hm. I bet it thinks that PG_USED_FOR_ASSERTS_ONLY is the variable name, which means that the behavior might be more exciting for multi-word type names (for instance struct foo or volatile int *. Could you check a few cases like that? Tested,

Re: [HACKERS] lots of unused variable warnings in assert-free builds

2012-03-20 Thread Peter Eisentraut
On tis, 2012-01-24 at 13:18 -0500, Tom Lane wrote: Robert Haas robertmh...@gmail.com writes: Yes, that's what I meant when I suggested it originally. I'm just not sure it's any nicer than adding ifdefs for USE_ASSERT_CHECKING. I'm inclined to think that it probably is nicer, just because

Re: [HACKERS] lots of unused variable warnings in assert-free builds

2012-03-20 Thread Tom Lane
Peter Eisentraut pete...@gmx.net writes: As you can see, pgindent adds whitespace on top of #ifdef USE_ASSERT_CHECKING, and messes up the vertical alignment of variable definitions that contain extra attributes. Hm. I bet it thinks that PG_USED_FOR_ASSERTS_ONLY is the variable name, which

Re: [HACKERS] lots of unused variable warnings in assert-free builds

2012-01-24 Thread Robert Haas
On Sat, Jan 21, 2012 at 1:06 PM, Peter Eisentraut pete...@gmx.net wrote: So, here are three patches that could solve this issue. pg-cassert-unused-attribute.patch, the one I already showed, using __attribute__((unused). pg-cassert-unused-ifdef.patch, using only additional #ifdef

Re: [HACKERS] lots of unused variable warnings in assert-free builds

2012-01-24 Thread Tom Lane
Robert Haas robertmh...@gmail.com writes: Spraying the code with __attribute__((unused)) is somewhat undesirable because it could mask a failure to properly initialize the variable in an assert-enabled build. Ouch. Is it really true that __attribute__((unused)) disables detection of use of

Re: [HACKERS] lots of unused variable warnings in assert-free builds

2012-01-24 Thread Robert Haas
On Tue, Jan 24, 2012 at 12:12 PM, Tom Lane t...@sss.pgh.pa.us wrote: Robert Haas robertmh...@gmail.com writes: Spraying the code with __attribute__((unused)) is somewhat undesirable because it could mask a failure to properly initialize the variable in an assert-enabled build. Ouch.  Is it

Re: [HACKERS] lots of unused variable warnings in assert-free builds

2012-01-24 Thread Tom Lane
Robert Haas robertmh...@gmail.com writes: On Tue, Jan 24, 2012 at 12:12 PM, Tom Lane t...@sss.pgh.pa.us wrote: Ouch. Is it really true that __attribute__((unused)) disables detection of use of uninitialized variables? Oh, I think maybe I am confused. The downsides of disabling *unused*

Re: [HACKERS] lots of unused variable warnings in assert-free builds

2012-01-24 Thread Tom Lane
I wrote: Also, it occurs to me that an intermediate macro PG_USED_FOR_ASSERTS_ONLY would be a good idea, first because it documents *why* you want to mark the variable as possibly unused, and second because changing the macro definition would provide an easy way to check for totally-unused

Re: [HACKERS] lots of unused variable warnings in assert-free builds

2012-01-24 Thread Robert Haas
On Tue, Jan 24, 2012 at 1:03 PM, Tom Lane t...@sss.pgh.pa.us wrote: I wrote: Also, it occurs to me that an intermediate macro PG_USED_FOR_ASSERTS_ONLY would be a good idea, first because it documents *why* you want to mark the variable as possibly unused, and second because changing the macro

Re: [HACKERS] lots of unused variable warnings in assert-free builds

2012-01-24 Thread Tom Lane
Robert Haas robertmh...@gmail.com writes: Yes, that's what I meant when I suggested it originally. I'm just not sure it's any nicer than adding ifdefs for USE_ASSERT_CHECKING. I'm inclined to think that it probably is nicer, just because of less vertical space used. But again, this opinion is

Re: [HACKERS] lots of unused variable warnings in assert-free builds

2012-01-21 Thread Peter Eisentraut
On ons, 2012-01-18 at 21:16 +0200, Peter Eisentraut wrote: On sön, 2012-01-15 at 08:14 -0500, Andrew Dunstan wrote: It would possibly have some documentary value too. Just looking very quickly at Peter's patch, I don't really understand his assertion that this would significantly butcher

Re: [HACKERS] lots of unused variable warnings in assert-free builds

2012-01-18 Thread Peter Eisentraut
On sön, 2012-01-15 at 01:37 -0500, Tom Lane wrote: Peter Eisentraut pete...@gmx.net writes: I see that in some places our code already uses #ifdef USE_ASSERT_CHECKING, presumably to hide similar issues. But in most cases using this would significantly butcher the code. I found that

Re: [HACKERS] lots of unused variable warnings in assert-free builds

2012-01-18 Thread Peter Eisentraut
On sön, 2012-01-15 at 08:14 -0500, Andrew Dunstan wrote: It would possibly have some documentary value too. Just looking very quickly at Peter's patch, I don't really understand his assertion that this would significantly butcher the code. The worst effect would be that in a few cases we'd

Re: [HACKERS] lots of unused variable warnings in assert-free builds

2012-01-18 Thread Tom Lane
Peter Eisentraut pete...@gmx.net writes: On sön, 2012-01-15 at 01:37 -0500, Tom Lane wrote: Surely this will fail entirely on most non-gcc compilers? No, because __attribute__() is defined to empty for other compilers. We use this pattern already. Oh, duh. In that case my only objection

Re: [HACKERS] lots of unused variable warnings in assert-free builds

2012-01-15 Thread Andrew Dunstan
On 01/15/2012 01:37 AM, Tom Lane wrote: Peter Eisentrautpete...@gmx.net writes: I see that in some places our code already uses #ifdef USE_ASSERT_CHECKING, presumably to hide similar issues. But in most cases using this would significantly butcher the code. I found that adding

Re: [HACKERS] lots of unused variable warnings in assert-free builds

2012-01-15 Thread Simon Riggs
On Sun, Jan 15, 2012 at 1:14 PM, Andrew Dunstan and...@dunslane.net wrote: On 01/15/2012 01:37 AM, Tom Lane wrote: Peter Eisentrautpete...@gmx.net  writes: I see that in some places our code already uses #ifdef USE_ASSERT_CHECKING, presumably to hide similar issues.  But in most cases

[HACKERS] lots of unused variable warnings in assert-free builds

2012-01-14 Thread Peter Eisentraut
In builds without --enable-cassert (I guess not many developers use those a lot), there are quite a few unused variable warnings. These usually hold some intermediate result that the assert checks later. I see that in some places our code already uses #ifdef USE_ASSERT_CHECKING, presumably to

Re: [HACKERS] lots of unused variable warnings in assert-free builds

2012-01-14 Thread Tom Lane
Peter Eisentraut pete...@gmx.net writes: I see that in some places our code already uses #ifdef USE_ASSERT_CHECKING, presumably to hide similar issues. But in most cases using this would significantly butcher the code. I found that adding __attribute__((unused)) is cleaner. Attached is a