Re: [HACKERS] -Wcast-qual cleanup, part 1
On mån, 2011-11-07 at 10:07 -0500, Tom Lane wrote: > >> 2. Macros accessing structures should come in two variants: a > "get" > >> version, and a "set"/anything else version, so that the "get" > version > >> can preserve the const qualifier. > > I'm not prepared to buy into that as a general coding rule. > > Maybe it would be better to just add -Wno-cast-qual to CFLAGS. OK, I understand the concerns that have been raised here and in the other thread. I'll work instead on removing "lying" const qualifiers on the upper layers that were the causes of attempting to push the consts down. -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] -Wcast-qual cleanup, part 1
Robert Haas writes:
> On Mon, Nov 7, 2011 at 12:13 AM, Peter Eisentraut wrote:
>> typedef struct ErrorContextCallback
>> {
>> struct ErrorContextCallback *previous;
>> -void(*callback) (void *arg);
>> -void *arg;
>> +void(*callback) (const void *arg);
>> +const void *arg;
>> } ErrorContextCallback;
> Why should the callback be forced to treat its private argument as const?
Yes, the above seems like a seriously bad idea. It's probably true that
most existing callbacks could afford to declare their callback args as
const, but it does not follow that we should legislate that in the API.
All that that will accomplish is to move the need to cast away const
from some places to some other places.
>> #define XLogRecGetData(record) ((char*) (record) + SizeOfXLogRecord)
>> +#define XLogRecGetConstData(record) ((const char*) (record) +
>> SizeOfXLogRecord)
> IMHO, this is an example of everything that's wrong with const.
Yes, this seems pretty horrid as well. Not so much just the one instance
as the implications of this sweeping requirement:
>> 2. Macros accessing structures should come in two variants: a "get"
>> version, and a "set"/anything else version, so that the "get" version
>> can preserve the const qualifier.
I'm not prepared to buy into that as a general coding rule.
Maybe it would be better to just add -Wno-cast-qual to CFLAGS.
regards, tom lane
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] -Wcast-qual cleanup, part 1
On Mon, Nov 7, 2011 at 12:13 AM, Peter Eisentraut wrote:
> Anyway, attached is the first patch for your amusement.
I can't help but wonder if the cure isn't worse than the disease. I
mean, I very much like the fact that our code compiles without
warnings, and I'm glad you're willing to put in the time to make that
happen ... but aren't these warnings incredibly pedantic?
const is like kudzu. Once you start using it, you find that you need
it everywhere ... but your life is no better than it was before,
except that now you have const.
I'm suspicious of this hunk, for example:
typedef struct ErrorContextCallback
{
struct ErrorContextCallback *previous;
- void(*callback) (void *arg);
- void *arg;
+ void(*callback) (const void *arg);
+ const void *arg;
} ErrorContextCallback;
Why should the callback be forced to treat its private argument as const?
#define XLogRecGetData(record) ((char*) (record) + SizeOfXLogRecord)
+#define XLogRecGetConstData(record)((const char*) (record) +
SizeOfXLogRecord)
IMHO, this is an example of everything that's wrong with const. The
result will, I suppose, be const if and only if record is const. But
there's no way to express that cleanly, so we have to duplicate the
macro definition. And anyone who is not using the right compiler
version will have to stare at the code and scratch their head to
figure out which one to use.
--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
