https://gcc.gnu.org/bugzilla/show_bug.cgi?id=102967

jbeulich at suse dot com changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |jbeulich at suse dot com

--- Comment #3 from jbeulich at suse dot com ---
(In reply to Martin Sebor from comment #1)
> The warning is intended: it points out that the second operand of the
> conditional expression is necessarily true:
> 
>   if ( !(pa ? &pa->c : NULL) )
>               ^^^^^^
> 
> There's no point in testing the address of a member for equality to null
> because  the member of no object can reside at that address.

Except that if pa is NULL and c is the first member of typeof(*pa), then &pa->c
is also NULL, isn't it?

>  The above can
> be simplified to
> 
>   if (!pa)

If there was no macro expansion involved in the former case, I'd agree with the
option of simplifying the expression. But the use of the macro expanding to
this construct may be intended, perhaps at least for the code to be
self-documenting. The macro itself, in turn, may be (and in our case is) also
used in contexts where the produced pointer actually gets used as such, not for
boolean NULL / non-NULL checks.

In any event I don't see how it matters here that, as you say, "the second
operand of the conditional expression is necessarily true". Especially
considering the macro aspect, changing to "pa ? 1 : NULL" or (to make the
construct type-correct) "pa ? 1 : 0" isn't an option, yet that's what you
appear to be suggesting (as intermediate step to arrive at simply "pa").

Imo the compiler wrongly connects the use of &pa->c in the conditional
expression to the boolean context of the enclosing if(), even irrespective of
the use of !() around the conditional operator. IOW

    if ( pa ? &pa->c : NULL )

should also go through without any diagnostic (at least as long as the compiler
doesn't also take into account whether the expression is the result of macro
expansion). Otherwise all you do is force people to write less legible code,
e.g. the macro becoming

#define a_to_c(pa) ({ \
    type_of_c *pc_ = (pa) ? &(pa)->c : NULL; \
    pc_; \
})

based on Andrew's observation that breaking out the expression doesn't result
in any warning (didn't check whether that in practice extends to the case
above).

Reply via email to