On 9/21/21 20:34, Martin Sebor wrote:
On 9/21/21 3:40 PM, Jason Merrill wrote:
On 9/17/21 12:02, Martin Sebor wrote:
On 9/8/21 2:06 PM, Jason Merrill wrote:
On 9/2/21 7:53 PM, Martin Sebor wrote:
@@ -4622,28 +4622,94 @@ warn_for_null_address (location_t location,
tree op, tsubst_flags_t complain)
if (!warn_address
|| (complain & tf_warning) == 0
|| c_inhibit_evaluation_warnings != 0
- || warning_suppressed_p (op, OPT_Waddress))
+ || warning_suppressed_p (op, OPT_Waddress)
+ || processing_template_decl != 0)
Completely suppressing this warning in templates seems like a
regression; I'd think we could recognize many relevant cases before
instantiation. You just can't assume that ADDR_EXPR has the default
meaning if it has unknown type (i.e. because op0 is type-dependent).
I added the suppression to keep g++.dg/warn/pr101219.C from failing
but in hindsight I should have questioned the reasoning behind
the "no warning emitted here (no instantiation)" comment in the test.
I agree that it would be helpful to diagnose the type-independent
subset of the problem even in uninstantiated templates. Current
trunk doesn't (it never has), but with my patch and the suppression
above removed it does. I've updated the tests to expect it.
Please see the attached revision.
Martin
PS There are still more opportunities to issue -Waddress in templates
that this patch doesn't handle, e.g.,:
template <class T> bool f (T *p) { return &p == 0; }
Handling this will take more surgery.
PPS It seems that most other warnings (and even some errors, like
-Wnarrowing) are suppressed in uninstantiated templates as well,
even for non-dependent expressions. In the few test cases I looked
at Clang does better. It sounds like you'd like to see improvements
in this direction not just for -Waddress but in general. Just for
the avoidance of doubt, can you confirm that for future reference?
Yes, in general it's better to diagnose sooner.
+ if (TREE_CODE (cop) == NON_LVALUE_EXPR)
+ /* Unwrap the expression for C++ 98. */
+ cop = TREE_OPERAND (cop, 0);
What does this have to do with C++98?
The code is needed to avoid failures in C++ 98 in the test below
where COP is a NON_LVALUE_EXPR which isn't handled below otherwise.
I didn't investigate why that happens (it works fine if f() is
an ordinary member function).
void f (bool);
void g ()
{
struct A { virtual void vf (); };
f (&A::vf); // missing -Waddress in C++ 98 mode
}
+ if (TREE_CODE (cop) == PTRMEM_CST)
+ {
+ /* The address of a nonstatic data member is never null. */
+ warning_at (location, OPT_Waddress,
+ "the address %qE will never be NULL",
Capitalizing NULL when talking about pointers-to-members seems a bit
odd, but I guess it's fine.
I agree. My personal preference is for lowercase null (in all
languages) since that's the technical term for it. I used NULL
here only to conform to the existing style. I'm willing to
change all these warnings to either use null or to some form
that doesn't mention null (there are two in use, although if
I had my druthers I'd choose some other phrasing altogether).
Let me know if you would support such a change.
I don't feel strongly about it. I agree that lowercase or another
phrasing would be better, but probably better to avoid adding work for
the translators with the churn.
The C++ changes are OK.
Jeff, should I take your previous "Generally OK" as an approval
for the rest of the patch as well? (It has not changed in v2.)
I have just submitted a Glibc patch to suppress the new instances
there.
Martin
