Re: [HACKERS] Coding style point: const in function parameter declarations
On 22.06.2011 01:51, Tom Lane wrote: I notice that the SSI code is rather heavily invested in function declarations like this: extern bool PageIsPredicateLocked(const Relation relation, const BlockNumber blkno); I find this to be poor style, and would like to see if there's any support for getting rid of the const keywords. My objections are twofold: 1. What such a const marking actually does is to forbid the function from changing the value of its local variable that received the passed parameter value. While you may or may not think that it's good style to avoid doing so, whether the function chooses to do that or not is surely no business of its callers'. Putting such a marking into the extern declaration doesn't create any useful API contract, it just means you'll have to change the declaration if you change the function's implementation. Yeah, that makes no sense. 2. In cases such as const Relation foo where the parameter type is a typedeffed pointer, it is easy for readers to arrive at the false conclusion that this guarantees the function doesn't change the pointed-to structure. But it guarantees no such thing, because that construction is *not* equivalent to const struct RelationData *foo; rather it means struct RelationData * const foo, ie only the pointer is being const-ified, not that to which it points. The function can hack the struct contents, or pass the pointer to functions that do arbitrary things, and the compiler won't make a whimper. So I think that declarations like this are positively pernicious --- they'll mislead all but the most language-lawyerly readers. Agreed. I did not realize the difference in the SSI patch. The intention of the const declarations was clearly to document that the function doesn't modify the data the pointer points to, but if the const qualifier doesn't accomplish that, it's just wrong. -- Heikki Linnakangas EnterpriseDB http://www.enterprisedb.com -- 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] Coding style point: const in function parameter declarations
On 22.06.2011 02:58, Dan Ports wrote: On Tue, Jun 21, 2011 at 06:51:20PM -0400, Tom Lane wrote: I find this to be poor style, and would like to see if there's any support for getting rid of the const keywords. I'm in favor of removing them too. Ok, I've removed all the useless const qualifiers. -- Heikki Linnakangas EnterpriseDB http://www.enterprisedb.com -- 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] Coding style point: const in function parameter declarations
On Tue, Jun 21, 2011 at 5:51 PM, Tom Lane t...@sss.pgh.pa.us wrote: Declarations like const structtype *param are fine, because those create a real, enforced contract on what the function can do to data that is visible to its caller. But I don't see any value at all in const-ifying the parameter itself. Comments? What about making a separate typedef for that (like ConstRelation)? Maybe the const contract is useful, and aren't there occasional performance enhancements the compiler can make when it function arguments are const? merlin -- 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] Coding style point: const in function parameter declarations
Merlin Moncure mmonc...@gmail.com writes: On Tue, Jun 21, 2011 at 5:51 PM, Tom Lane t...@sss.pgh.pa.us wrote: Declarations like const structtype *param are fine, because those create a real, enforced contract on what the function can do to data that is visible to its caller. But I don't see any value at all in const-ifying the parameter itself. What about making a separate typedef for that (like ConstRelation)? Well, any change along this line would require pretty widespread changes, as you'd have to const-ify from the bottom up, or else hold your nose and cast-away-const in assorted calls (which I think is so evil you might as well not have the const in the first place). If we were thinking of moving in that direction, I would argue that we should get rid of typedef'd pointers altogether, ie, change Relation to be a typedef for the struct and write Relation *rel not Relation rel. Then, attaching a const to the front is easy, natural, and does what the naive reader would expect. But this would be a sufficiently sweeping change that I'm not sure the benefits would be worth the cost. 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] Coding style point: const in function parameter declarations
On Wed, Jun 22, 2011 at 12:41 PM, Tom Lane t...@sss.pgh.pa.us wrote: Merlin Moncure mmonc...@gmail.com writes: On Tue, Jun 21, 2011 at 5:51 PM, Tom Lane t...@sss.pgh.pa.us wrote: Declarations like const structtype *param are fine, because those create a real, enforced contract on what the function can do to data that is visible to its caller. But I don't see any value at all in const-ifying the parameter itself. What about making a separate typedef for that (like ConstRelation)? Well, any change along this line would require pretty widespread changes, as you'd have to const-ify from the bottom up, or else hold your nose and cast-away-const in assorted calls (which I think is so evil you might as well not have the const in the first place). If we were thinking of moving in that direction, I would argue that we should get rid of typedef'd pointers altogether, ie, change Relation to be a typedef for the struct and write Relation *rel not Relation rel. Then, attaching a const to the front is easy, natural, and does what the naive reader would expect. But this would be a sufficiently sweeping change that I'm not sure the benefits would be worth the cost. I don't really see the point, either. I mean, the Relation is basically a cache. And so it could happen that we want to start caching something we don't cache today. Then someone for whom it was const suddenly needs it to be non-const. Of course I don't think there's much call for monkeying with rel-rd_id but how likely is that to be a real source of coding errors? -- 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
Re: [HACKERS] Coding style point: const in function parameter declarations
On Wed, Jun 22, 2011 at 5:41 PM, Tom Lane t...@sss.pgh.pa.us wrote: If we were thinking of moving in that direction, I would argue that we should get rid of typedef'd pointers altogether, ie, change Relation to be a typedef for the struct and write Relation *rel not Relation rel. Hm. I have to say the single most confusing thing about the Postgres source that took me a *long* time to get over was remembering that some of the typedefs were already pointers and some weren't. It seems silly now but when I was trying to understand what the intent of a function was and it wasn't obvious that some of the arguments appeared to be pass by value but were actually pass by reference it made things really surprising. -- greg -- 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] Coding style point: const in function parameter declarations
Greg Stark st...@mit.edu writes: On Wed, Jun 22, 2011 at 5:41 PM, Tom Lane t...@sss.pgh.pa.us wrote: If we were thinking of moving in that direction, I would argue that we should get rid of typedef'd pointers altogether, ie, change Relation to be a typedef for the struct and write Relation *rel not Relation rel. Hm. I have to say the single most confusing thing about the Postgres source that took me a *long* time to get over was remembering that some of the typedefs were already pointers and some weren't. Yeah, the lack of consistency about that is annoying. 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] Coding style point: const in function parameter declarations
On 23 June 2011 00:37, Greg Stark st...@mit.edu wrote: Hm. I have to say the single most confusing thing about the Postgres source that took me a *long* time to get over was remembering that some of the typedefs were already pointers and some weren't. It seems silly now but when I was trying to understand what the intent of a function was and it wasn't obvious that some of the arguments appeared to be pass by value but were actually pass by reference it made things really surprising. I agree that the inconsistency is annoying. If we were starting from scratch, I'd encourage abandoning the convention. It's a leaky abstraction. At the risk of saying something that will raise eyebrows, I'll point out that the win32 API follows a similar practice. The difference is that they have consistent conventions in how they name their typdefs, so that you get to learn the patterns. -- Peter Geoghegan http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training and Services -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] Coding style point: const in function parameter declarations
I notice that the SSI code is rather heavily invested in function declarations like this: extern bool PageIsPredicateLocked(const Relation relation, const BlockNumber blkno); I find this to be poor style, and would like to see if there's any support for getting rid of the const keywords. My objections are twofold: 1. What such a const marking actually does is to forbid the function from changing the value of its local variable that received the passed parameter value. While you may or may not think that it's good style to avoid doing so, whether the function chooses to do that or not is surely no business of its callers'. Putting such a marking into the extern declaration doesn't create any useful API contract, it just means you'll have to change the declaration if you change the function's implementation. 2. In cases such as const Relation foo where the parameter type is a typedeffed pointer, it is easy for readers to arrive at the false conclusion that this guarantees the function doesn't change the pointed-to structure. But it guarantees no such thing, because that construction is *not* equivalent to const struct RelationData *foo; rather it means struct RelationData * const foo, ie only the pointer is being const-ified, not that to which it points. The function can hack the struct contents, or pass the pointer to functions that do arbitrary things, and the compiler won't make a whimper. So I think that declarations like this are positively pernicious --- they'll mislead all but the most language-lawyerly readers. Declarations like const structtype *param are fine, because those create a real, enforced contract on what the function can do to data that is visible to its caller. But I don't see any value at all in const-ifying the parameter itself. Comments? 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] Coding style point: const in function parameter declarations
On 21 June 2011 23:51, Tom Lane t...@sss.pgh.pa.us wrote: I notice that the SSI code is rather heavily invested in function declarations like this: extern bool PageIsPredicateLocked(const Relation relation, const BlockNumber blkno); I find this to be poor style, and would like to see if there's any support for getting rid of the const keywords. My objections are twofold: 1. What such a const marking actually does is to forbid the function from changing the value of its local variable that received the passed parameter value. While you may or may not think that it's good style to avoid doing so, whether the function chooses to do that or not is surely no business of its callers'. Putting such a marking into the extern declaration doesn't create any useful API contract, it just means you'll have to change the declaration if you change the function's implementation. People may be confused by that inconsistency though. I agree that it generally doesn't make sense to const-qualify a passed-by-value parameter. I've seen it done, but I struggle to recall a case where I thought that it made much sense. 2. In cases such as const Relation foo where the parameter type is a typedeffed pointer, it is easy for readers to arrive at the false conclusion that this guarantees the function doesn't change the pointed-to structure. But it guarantees no such thing, because that construction is *not* equivalent to const struct RelationData *foo; rather it means struct RelationData * const foo, ie only the pointer is being const-ified, not that to which it points. The function can hack the struct contents, or pass the pointer to functions that do arbitrary things, and the compiler won't make a whimper. So I think that declarations like this are positively pernicious --- they'll mislead all but the most language-lawyerly readers. IMHO, you don't have to be that much of a language lawyer to pick up on that, because it's not as if the typedef has very effectively encapsulated the fact that the underlying type is actually a pointer anyway. I for one find it intuitively obvious that the pointed-to data isn't declared const in your example. Declarations like const structtype *param are fine, because those create a real, enforced contract on what the function can do to data that is visible to its caller. But I don't see any value at all in const-ifying the parameter itself. +1 const is actually an example of C++ influencing C. It tends to work very well with C++, where it cascades usefully throughout an entire system, and where a distinction can be made between logical and bit-wise const-ness. In C, it is a bit wishy-washy, particularly the fact that what happens when const-ness is violated is undefined, rather than just having the code fail to compile. const is also supposed to be able to facilitate certain compiler optimisations, which is why you have the undefined behaviour thing (what if the variable is stored in read-only memory?), but that isn't very relevant in practice. -- Peter Geoghegan http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training and Services -- 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] Coding style point: const in function parameter declarations
On Tue, Jun 21, 2011 at 06:51:20PM -0400, Tom Lane wrote: I find this to be poor style, and would like to see if there's any support for getting rid of the const keywords. I'm in favor of removing them too. Dan -- Dan R. K. Ports MIT CSAILhttp://drkp.net/ -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
