On 7/29/20 2:27 PM, Jason Merrill wrote:
> On 7/23/20 3:08 PM, Martin Sebor wrote:
>> Another test case added to the bug since I posted the patch shows
>> that the no-warning bit set by the C++ front end is easily lost
>> during early folding, triggering the -Wnonull again despite
>> the suppression.  The attached revision tweaks the folder in just
>> this one case to let the suppression take effect in this situation.
>>
>> In light of how pervasive this potential for stripping looks (none
>> of the other folders preserves the bit) the tweak feels like a band-
>> aid rather than a general solution.  But implementing something
>> better (and mainly developing tests to verify that it works in
>> general) would probably be quite the project.  So I leave it for
>> some other time.
> 
> How about in check_function_arguments_recurse COND_EXPR handling, 
> checking for TREE_NO_WARNING on the condition?  Then we wouldn't need to 
> deal with setting and copying it on the COND_EXPR itself.

The condition is already checked at the beginning of the function.

But the latest trunk doesn't seem to need the fold-const.c change
anymore to suppress the warning and the original patch is good
enough.  The updated test should catch the regression if
the COND_EXPR should again lose the no warning bit.

Martin

> 
>>
>> On 7/17/20 1:00 PM, Martin Sebor wrote:
>>> The recent enhancement to treat the implicit this pointer argument
>>> as nonnull in member functions triggers spurious -Wnonnull for
>>> the synthesized conditional expression the C++ front end replaces
>>> the pointer with in some static_cast expressions.  The front end
>>> already sets the no-warning bit for the test but not for the whole
>>> conditional expression, so the attached fix extends the same solution
>>> to it.
>>>
>>> The consequence of this fix is that user-written code like this:
>>>
>>>    static_cast<T*>(p ? p : 0)->f ();
>>> or
>>>    static_cast<T*>(p ? p : nullptr)->f ();
>>>
>>> don't trigger the warning because they are both transformed into
>>> the same expression as:
>>>
>>>    static_cast<T*>(p)->f ();
>>>
>>> What still does trigger it is this:
>>>
>>>    static_cast<T*>(p ? p : (T*)0)->f ();
>>>
>>> because here it's the inner COND_EXPR's no-warning bit that's set
>>> (the outer one is clear), whereas in the former expressions it's
>>> the other way around.  It would be nice if this worked consistently
>>> but I didn't see an easy way to do that and more than a quick fix
>>> seems outside the scope for this bug.
>>>
>>> Another case reported by someone else in the same bug involves
>>> a dynamic_cast.  A simplified test case goes something like this:
>>>
>>>    if (dynamic_cast<T*>(p))
>>>      dynamic_cast<T*>(p)->f ();
>>>
>>> The root cause is the same: the front end emitting the COND_EXPR
>>>
>>>    ((p != 0) ? ((T*)__dynamic_cast(p, (& _ZTI1B), (& _ZTI1C), 0)) : 0)
>>>
>>> I decided not to suppress the warning in this case because doing
>>> so would also suppress it in unconditional calls with the result
>>> of the cast:
>>>
>>>    dynamic_cast<T*>(p)->f ();
>>>
>>> and that doesn't seem helpful.  Instead, I'd suggest to make
>>> the second cast in the if statement to reference to T&:
>>>
>>>    if (dynamic_cast<T*>(p))
>>>      dynamic_cast<T&>(*p).f ();
>>>
>>> Martin
> 
>