Re: safer node casting

On Mon, Jan 2, 2017 at 2:10 PM, Andres Freund <andres@anarazel.de> 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));
>
>
>> I propose a macro castNode() that combines the assertion and the cast,
>> so this would become
>>
>>     RestrictInfo *rinfo = castNode(RestrictInfo, lfirst(lc));
>
> I'm quite a bit in favor of something like this, having proposed it
> before ;)
>
>> +#define castNode(_type_,nodeptr)     (AssertMacro(!nodeptr || IsA(nodeptr,_type_)), (_type_ *)(nodeptr))
>
> ISTM that we need to do the core part of this in an inline function, to
> avoid multiple evaluation hazards - which seem quite likely to occur
> here - it's pretty common to cast the result of a function after all.
>
> Something like
>
> static inline Node*
> castNodeImpl(void *c, enum NodeTag t)
> {
>     Assert(c == NULL || IsA(c, t));
>     return c;
> }
>
> #define castNode(_type_, nodeptr) ((_type_ *) castNodeImpl(nodeptr, _type_))
>
> should work without too much trouble afaics?
>
I tried this quickly as per attached patch. It gave a compiler error
createplan.c: In function ‘castNodeImpl’:
createplan.c:340:2: error: ‘T_t’ undeclared (first use in this function)
createplan.c:340:2: note: each undeclared identifier is reported only
once for each function it appears in
createplan.c: In function ‘create_plan_recurse’:
createplan.c:445:13: error: expected expression before ‘AggPath’

Is the attached patch as per your suggestion?

-- 
Best Wishes,
Ashutosh Bapat
EnterpriseDB Corporation
The Postgres Database Company