This patch improves -Waddress warnings to be clearer and more actionable:
- Use clearer wording: "testing the address of" instead of "the address of"
- Add fix-it hints suggesting to call functions/lambdas with ()
- Only suggest () for functions/lambdas with no parameters
- Don't show internal names for lambdas. If the lambda is a variable,
show the identifier.
Test case:
bool no_args();
int with_args(int a, int b);
void test_functions()
{
if (no_args) {}
if (!no_args) {}
if (with_args) {}
}
void test_lambdas()
{
if ([] { return true; }) {}
auto lambda = [] { return true; };
if (lambda) {}
auto lambda_with_args = [](int x) { return x > 0; };
if (lambda_with_args) {}
}
Before (GCC 15.2.1):
test.cpp: In function 'void test_functions()':
test.cpp:6:7: warning: the address of 'bool no_args()' will never be NULL
[-Waddress]
6 | if (no_args) {}
| ^~~~~~~
test.cpp:1:6: note: 'bool no_args()' declared here
1 | bool no_args();
| ^~~~~~~
test.cpp:7:8: warning: the address of 'bool no_args()' will never be NULL
[-Waddress]
7 | if (!no_args) {}
| ^~~~~~~
test.cpp:1:6: note: 'bool no_args()' declared here
1 | bool no_args();
| ^~~~~~~
test.cpp:8:7: warning: the address of 'int with_args(int, int)' will never be
NULL [-Waddress]
8 | if (with_args) {}
| ^~~~~~~~~
test.cpp:2:5: note: 'int with_args(int, int)' declared here
2 | int with_args(int a, int b);
| ^~~~~~~~~
test.cpp: In function 'void test_lambdas()':
test.cpp:13:7: warning: the address of 'static constexpr bool
test_lambdas()::<lambda()>::_FUN()' will never be NULL [-Waddress]
13 | if ([] { return true; }) {}
| ^~~~~~~~~~~~~~~~~~~
test.cpp:13:7: note: 'static constexpr bool
test_lambdas()::<lambda()>::_FUN()' declared here
13 | if ([] { return true; }) {}
| ^
test.cpp:16:7: warning: the address of 'static constexpr bool
test_lambdas()::<lambda()>::_FUN()' will never be NULL [-Waddress]
16 | if (lambda) {}
| ^~~~~~
test.cpp:15:17: note: 'static constexpr bool
test_lambdas()::<lambda()>::_FUN()' declared here
15 | auto lambda = [] { return true; };
| ^
test.cpp:19:7: warning: the address of 'static constexpr bool
test_lambdas()::<lambda(int)>::_FUN(int)' will never be NULL [-Waddress]
19 | if (lambda_with_args) {}
| ^~~~~~~~~~~~~~~~
test.cpp:18:27: note: 'static constexpr bool
test_lambdas()::<lambda(int)>::_FUN(int)' declared here
18 | auto lambda_with_args = [](int x) { return x > 0; };
| ^
After (with this patch):
test.cpp: In function 'void test_functions()':
test.cpp:6:7: warning: testing the address of 'bool no_args()' will always
evaluate as 'true' [-Waddress]
6 | if (no_args) {}
| ^~~~~~~
test.cpp:6:13: note: did you mean to call 'bool no_args()'?
6 | if (no_args) {}
| ~~~~~~^
| ()
test.cpp:1:6: note: 'bool no_args()' declared here
1 | bool no_args();
| ^~~~~~~
test.cpp:7:8: warning: testing the address of 'bool no_args()' will always
evaluate as 'true' [-Waddress]
7 | if (!no_args) {}
| ^~~~~~~
test.cpp:7:14: note: did you mean to call 'bool no_args()'?
7 | if (!no_args) {}
| ~~~~~~^
| ()
test.cpp:1:6: note: 'bool no_args()' declared here
1 | bool no_args();
| ^~~~~~~
test.cpp:8:7: warning: testing the address of 'int with_args(int, int)' will
always evaluate as 'true' [-Waddress]
8 | if (with_args) {}
| ^~~~~~~~~
test.cpp:2:5: note: 'int with_args(int, int)' declared here
2 | int with_args(int a, int b);
| ^~~~~~~~~
test.cpp: In function 'void test_lambdas()':
test.cpp:13:7: warning: testing a lambda expression converted to a function
pointer will always evaluate as 'true' [-Waddress]
13 | if ([] { return true; }) {}
| ^~~~~~~~~~~~~~~~~~~
test.cpp:13:25: note: did you mean to call it?
13 | if ([] { return true; }) {}
| ~~~~~~~~~~~~~~~~~~^
| ()
test.cpp:16:7: warning: testing 'lambda' converted to a function pointer will
always evaluate as 'true' [-Waddress]
16 | if (lambda) {}
| ^~~~~~
test.cpp:16:12: note: did you mean to call 'lambda'?
16 | if (lambda) {}
| ~~~~~^
| ()
test.cpp:19:7: warning: testing 'lambda_with_args' converted to a function
pointer will always evaluate as 'true' [-Waddress]
19 | if (lambda_with_args) {}
| ^~~~~~~~~~~~~~~~
Signed-off-by: Peter Damianov <[email protected]>
---
NOTE: This patch lacks a ChangeLog and tests because I expect it to
require revision.
gcc/c-family/c-common.cc | 31 ++++++++++++++++--
gcc/c/c-typeck.cc | 26 ++++++++++++---
gcc/cp/typeck.cc | 70 ++++++++++++++++++++++++++++++++++++++--
3 files changed, 118 insertions(+), 9 deletions(-)
diff --git a/gcc/c-family/c-common.cc b/gcc/c-family/c-common.cc
index 3cec729c901..35967ecc458 100644
--- a/gcc/c-family/c-common.cc
+++ b/gcc/c-family/c-common.cc
@@ -3662,11 +3662,38 @@ c_common_truthvalue_conversion (location_t location,
tree expr)
&& !warning_suppressed_p (expr, OPT_Waddress)
&& !warning_suppressed_p (inner, OPT_Waddress))
{
+ /* Use the location from the ADDR_EXPR if available, as it should
+ point to the actual function name usage. */
+ location_t warn_loc = EXPR_HAS_LOCATION (expr)
+ ? EXPR_LOCATION (expr) : location;
/* Common Ada programmer's mistake. */
- warning_at (location,
+ warning_at (warn_loc,
OPT_Waddress,
- "the address of %qD will always evaluate as %<true%>",
+ "testing the address of %qD will always evaluate as
%<true%>",
inner);
+ if (TREE_CODE (inner) == FUNCTION_DECL)
+ {
+ tree fntype = TREE_TYPE (inner);
+ tree parms = TYPE_ARG_TYPES (fntype);
+ /* Use the location from the ADDR_EXPR if available, as it
should
+ point to the actual function name usage. */
+ location_t fn_loc = EXPR_HAS_LOCATION (expr)
+ ? EXPR_LOCATION (expr) : location;
+ location_t start = get_start (fn_loc);
+ location_t finish = get_finish (fn_loc);
+ /* Check if function takes no arguments (parms is
void_list_node)
+ or takes arguments. */
+ if (parms == void_list_node)
+ {
+ /* Create a location with caret at the end, range from start
+ to finish, giving ~~~^ underlining. */
+ location_t insert_loc = make_location (finish, start,
finish);
+ gcc_rich_location richloc (insert_loc);
+ richloc.add_fixit_insert_after ("()");
+ inform (&richloc,
+ "did you mean to call %qD?", inner);
+ }
+ }
suppress_warning (inner, OPT_Waddress);
return truthvalue_true_node;
}
diff --git a/gcc/c/c-typeck.cc b/gcc/c/c-typeck.cc
index d7c9a324d7a..5e84ab253f2 100644
--- a/gcc/c/c-typeck.cc
+++ b/gcc/c/c-typeck.cc
@@ -14090,7 +14090,7 @@ maybe_warn_for_null_address (location_t loc, tree op,
tree_code code)
else
warning_at (loc, OPT_Waddress,
"the comparison will always evaluate as %<true%> "
- "for the address of %qE will never be NULL",
+ "because the address of %qE will never be NULL",
op);
return;
}
@@ -14123,16 +14123,32 @@ maybe_warn_for_null_address (location_t loc, tree op,
tree_code code)
if (code == EQ_EXPR)
w = warning_at (loc, OPT_Waddress,
"the comparison will always evaluate as %<false%> "
- "for the address of %qE will never be NULL",
+ "because the address of %qE will never be NULL",
op);
else
w = warning_at (loc, OPT_Waddress,
"the comparison will always evaluate as %<true%> "
- "for the address of %qE will never be NULL",
+ "because the address of %qE will never be NULL",
op);
- if (w && DECL_P (op))
- inform (DECL_SOURCE_LOCATION (op), "%qD declared here", op);
+ if (w && TREE_CODE (op) == FUNCTION_DECL)
+ {
+ tree fntype = TREE_TYPE (op);
+ tree parms = TYPE_ARG_TYPES (fntype);
+ location_t start = get_start (loc);
+ location_t finish = get_finish (loc);
+ /* Only suggest adding parentheses if the function takes no arguments.
*/
+ if (parms == void_list_node)
+ {
+ /* Create a location with caret at the end, range from start
+ to finish, giving ~~~^ underlining. */
+ location_t insert_loc = make_location (finish, start, finish);
+ gcc_rich_location richloc (insert_loc);
+ richloc.add_fixit_insert_after ("()");
+ inform (&richloc, "did you mean to call %qD?", op);
+ }
+ inform (DECL_SOURCE_LOCATION (op), "%qD declared here", op);
+ }
}
/* Build a binary-operation expression without default conversions.
diff --git a/gcc/cp/typeck.cc b/gcc/cp/typeck.cc
index 65e08eaa645..bc694a2ddd7 100644
--- a/gcc/cp/typeck.cc
+++ b/gcc/cp/typeck.cc
@@ -5182,9 +5182,75 @@ warn_for_null_address (location_t location, tree op,
tsubst_flags_t complain)
|| warning_suppressed_p (cop, OPT_Waddress))
return;
- warned = warning_at (location, OPT_Waddress,
- "the address of %qD will never be NULL", cop);
+ /* Check if this is a lambda static thunk to avoid showing ugly internal
names. */
+ bool is_lambda = (TREE_CODE (cop) == FUNCTION_DECL
+ && lambda_static_thunk_p (cop));
+ tree var_decl = NULL_TREE;
+
+ if (is_lambda)
+ {
+ /* For lambda in a variable vs direct lambda expression:
+ - Direct: op is CALL_EXPR with TARGET_EXPR inside
+ - Variable: op is CALL_EXPR with VAR_DECL inside
+ We need to dig into the CALL_EXPR to find the VAR_DECL. */
+ tree orig = op;
+ STRIP_NOPS (orig);
+
+ /* If op is a CALL_EXPR (lambda conversion operator call),
+ check its first argument which is ADDR_EXPR of the lambda object.
*/
+ if (TREE_CODE (orig) == CALL_EXPR && call_expr_nargs (orig) > 0)
+ {
+ tree arg0 = CALL_EXPR_ARG (orig, 0);
+ if (TREE_CODE (arg0) == ADDR_EXPR)
+ {
+ tree inner = TREE_OPERAND (arg0, 0);
+ /* Check if it's a VAR_DECL (named variable) or TARGET_EXPR
(temporary). */
+ if (TREE_CODE (inner) == VAR_DECL && DECL_NAME (inner))
+ var_decl = inner;
+ }
+ }
+
+ if (var_decl)
+ warned = warning_at (location, OPT_Waddress,
+ "testing %qD converted to a function pointer "
+ "will always evaluate as %<true%>", var_decl);
+ else
+ warned = warning_at (location, OPT_Waddress,
+ "testing a lambda expression converted to a "
+ "function pointer will always evaluate as
%<true%>");
+ }
+ else
+ warned = warning_at (location, OPT_Waddress,
+ "testing the address of %qD will always "
+ "evaluate as %<true%>", cop);
op = cop;
+
+ /* Add fix-it hints for functions. */
+ if (warned && TREE_CODE (cop) == FUNCTION_DECL)
+ {
+ tree fntype = TREE_TYPE (cop);
+ tree parms = TYPE_ARG_TYPES (fntype);
+ location_t start = get_start (location);
+ location_t finish = get_finish (location);
+ /* Only suggest adding parentheses if the function takes no
arguments. */
+ if (parms == void_list_node)
+ {
+ /* Create a location with caret at the end, range from start
+ to finish, giving ~~~^ underlining. */
+ location_t insert_loc = make_location (finish, start, finish);
+ gcc_rich_location richloc (insert_loc);
+ richloc.add_fixit_insert_after ("()");
+ if (is_lambda && var_decl)
+ inform (&richloc, "did you mean to call %qD?", var_decl);
+ else if (is_lambda)
+ inform (&richloc, "did you mean to call it?");
+ else
+ inform (&richloc, "did you mean to call %qD?", cop);
+ }
+ if (!is_lambda)
+ inform (DECL_SOURCE_LOCATION (cop), "%qD declared here", cop);
+ }
+ return;
}
else if (TREE_CODE (cop) == POINTER_PLUS_EXPR)
{
--
2.52.0
