Re: [HACKERS] safer node casting

2017-02-24 Thread Peter Eisentraut
On 2/24/17 10:54, Tom Lane wrote: > Andres Freund writes: >> Those aren't actually equivalent, because of the !nodeptr. IsA() crashes >> for NULL pointers, but the new code won't. Which means 9ba8a9ce4548b et >> al actually weakened some asserts. > >> Should we perhaps have

Re: [HACKERS] safer node casting

2017-02-24 Thread Tom Lane
Andres Freund writes: > Those aren't actually equivalent, because of the !nodeptr. IsA() crashes > for NULL pointers, but the new code won't. Which means 9ba8a9ce4548b et > al actually weakened some asserts. > Should we perhaps have one NULL accepting version (castNodeNull?)

Re: [HACKERS] safer node casting

2017-02-24 Thread Andres Freund
Hi, I was about to add a few more uses of castNode, which made me think. You proposed replacing: On 2016-12-31 12:08:22 -0500, Peter Eisentraut wrote: > There is a common coding pattern that goes like this: > > RestrictInfo *rinfo = (RestrictInfo *) lfirst(lc); > Assert(IsA(rinfo,

Re: [HACKERS] safer node casting

2017-02-21 Thread Tom Lane
Jeff Janes writes: > With commit 38d103763d14baddf3cbfe4b00b501059fc9447f, I'm now getting a > compiler warning: > indxpath.c: In function 'generate_bitmap_or_paths': > indxpath.c:1312: warning: unused variable 'rinfo' Me too, without asserts. Fixed.

Re: [HACKERS] safer node casting

2017-02-21 Thread Jeff Janes
On Tue, Feb 21, 2017 at 9:00 AM, Peter Eisentraut < peter.eisentr...@2ndquadrant.com> wrote: > On 1/26/17 16:15, Andres Freund wrote: > > On 2017-01-25 19:21:40 -0500, Tom Lane wrote: > >> Andres Freund writes: > >>> On 2016-12-31 12:08:22 -0500, Peter Eisentraut wrote: >

Re: [HACKERS] safer node casting

2017-02-21 Thread Peter Eisentraut
On 1/26/17 16:15, Andres Freund wrote: > On 2017-01-25 19:21:40 -0500, Tom Lane wrote: >> Andres Freund writes: >>> On 2016-12-31 12:08:22 -0500, Peter Eisentraut wrote: RestrictInfo *rinfo = castNode(RestrictInfo, lfirst(lc)); >> >>> Are you planning to add this / update

Re: [HACKERS] safer node casting

2017-01-26 Thread Andres Freund
On 2017-01-26 20:29:06 -0500, Tom Lane wrote: > Andres Freund writes: > > On 2016-12-31 12:08:22 -0500, Peter Eisentraut wrote: > >> This is inspired by the dynamic_cast operator in C++, but follows the > >> syntax of the well-known makeNode() macro. > > > The analogy to

Re: [HACKERS] safer node casting

2017-01-26 Thread Tom Lane
Andres Freund writes: > On 2016-12-31 12:08:22 -0500, Peter Eisentraut wrote: >> This is inspired by the dynamic_cast operator in C++, but follows the >> syntax of the well-known makeNode() macro. > The analogy to dynamic_cast goes only so far, because we don't actually >

Re: [HACKERS] safer node casting

2017-01-26 Thread Andres Freund
Hi, On 2016-12-31 12:08:22 -0500, Peter Eisentraut wrote: > This is inspired by the dynamic_cast operator in C++, but follows the > syntax of the well-known makeNode() macro. The analogy to dynamic_cast goes only so far, because we don't actually support inheritance. I.e. in c++ we could

Re: [HACKERS] safer node casting

2017-01-26 Thread Andres Freund
On 2017-01-26 17:27:45 -0500, Tom Lane wrote: > Andres Freund writes: > > #if defined(USE_ASSERT_CHECKING) && defined(PG_USE_INLINE) > > is probably a better gatekeeper in the back-branches, than gcc? > > Ah, yeah, that would work --- I'd already swapped out that business ;-)

Re: [HACKERS] safer node casting

2017-01-26 Thread Tom Lane
Andres Freund writes: > #if defined(USE_ASSERT_CHECKING) && defined(PG_USE_INLINE) > is probably a better gatekeeper in the back-branches, than gcc? Ah, yeah, that would work --- I'd already swapped out that business ;-) regards, tom lane -- Sent

Re: [HACKERS] safer node casting

2017-01-26 Thread Andres Freund
On 2017-01-26 16:55:33 -0500, Tom Lane wrote: > Andres Freund writes: > > How about something like the attached? The first patch just contains > > castNode(), the second one a rebased version of Peter's changes (with > > some long lines broken up). > > Looks generally good.

Re: [HACKERS] safer node casting

2017-01-26 Thread Tom Lane
Andres Freund writes: > How about something like the attached? The first patch just contains > castNode(), the second one a rebased version of Peter's changes (with > some long lines broken up). Looks generally good. A couple quibbles from a quick read-through: * All but

Re: [HACKERS] safer node casting

2017-01-26 Thread Andres Freund
On 2017-01-25 19:21:40 -0500, Tom Lane wrote: > Andres Freund writes: > > On 2016-12-31 12:08:22 -0500, Peter Eisentraut wrote: > >> RestrictInfo *rinfo = castNode(RestrictInfo, lfirst(lc)); > > > Are you planning to add this / update this patch? Because I really would > >

Re: [HACKERS] safer node casting

2017-01-25 Thread Tom Lane
Andres Freund writes: > On 2016-12-31 12:08:22 -0500, Peter Eisentraut wrote: >> RestrictInfo *rinfo = castNode(RestrictInfo, lfirst(lc)); > Are you planning to add this / update this patch? Because I really would > have liked this a number of times already... I can update

Re: [HACKERS] safer node casting

2017-01-25 Thread Andres Freund
Hi Peter, On 2016-12-31 12:08:22 -0500, Peter Eisentraut wrote: > RestrictInfo *rinfo = castNode(RestrictInfo, lfirst(lc)); Are you planning to add this / update this patch? Because I really would have liked this a number of times already... I can update it according to my suggestions (to

Re: [HACKERS] safer node casting

2017-01-04 Thread Andres Freund
Hi, On 2017-01-03 11:00:47 +0530, Ashutosh Bapat wrote: > On Mon, Jan 2, 2017 at 2:10 PM, Andres Freund wrote: > > Hi, > > > > > > On 2016-12-31 12:08:22 -0500, Peter Eisentraut wrote: > >> There is a common coding pattern that goes like this: > >> > >> RestrictInfo

Re: [HACKERS] safer node casting

2017-01-02 Thread Ashutosh Bapat
On Mon, Jan 2, 2017 at 2:10 PM, Andres Freund wrote: > Hi, > > > On 2016-12-31 12:08:22 -0500, Peter Eisentraut wrote: >> There is a common coding pattern that goes like this: >> >> RestrictInfo *rinfo = (RestrictInfo *) lfirst(lc); >> Assert(IsA(rinfo, RestrictInfo));

Re: [HACKERS] safer node casting

2017-01-02 Thread Andres Freund
Hi, On 2016-12-31 12:08:22 -0500, Peter Eisentraut wrote: > There is a common coding pattern that goes like this: > > RestrictInfo *rinfo = (RestrictInfo *) lfirst(lc); > Assert(IsA(rinfo, RestrictInfo)); > I propose a macro castNode() that combines the assertion and the cast, > so

Re: [HACKERS] safer node casting

2017-01-01 Thread Ashutosh Bapat
On Sat, Dec 31, 2016 at 11:30 PM, Tom Lane wrote: > Peter Eisentraut writes: >> I propose a macro castNode() that combines the assertion and the cast, >> so this would become >> RestrictInfo *rinfo = castNode(RestrictInfo, lfirst(lc)); >

Re: [HACKERS] safer node casting

2016-12-31 Thread Tom Lane
I wrote: > Just to bikeshed a bit ... would "castNode" be a better name? Um ... -ENOCAFFEINE. Never mind that bit. regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription:

Re: [HACKERS] safer node casting

2016-12-31 Thread Tom Lane
Peter Eisentraut writes: > I propose a macro castNode() that combines the assertion and the cast, > so this would become > RestrictInfo *rinfo = castNode(RestrictInfo, lfirst(lc)); Seems like an OK idea, but I'm concerned by the implied multiple evaluations,

[HACKERS] safer node casting

2016-12-31 Thread Peter Eisentraut
There is a common coding pattern that goes like this: RestrictInfo *rinfo = (RestrictInfo *) lfirst(lc); Assert(IsA(rinfo, RestrictInfo)); (Arguably, the Assert should come before the cast, but I guess it's done this way out of convenience.) (Not to mention the other common coding