Re: [PATCH 08/11] Make conversion warnings work on NULL with -ftrack-macro-expansion

[Dodji Seketeli <dodji at redhat dot com>]

Gabriel Dos Reis <gdr@integrable-solutions.net> writes:

> Thanks.  But a point of the suggestion was that we won't need the
> same comment/explanation duplicated over at least 3 places.  Just
> one.  All those three places deal exactly with one instance: null
> pointer constant.  That deserves a function in and of itself, which
> is documented by the duplicated comments.
> Please make that change.  Everything else is OK.  thanks.

I am sorry for the round trips.  Please find below a patch udpated
accordingly.

I am bootstrapping the whole patch set, but the impacted files of this
patch have built fine so far.

Thanks.

gcc/
	* input.h (expansion_point_location_if_in_system_header): Declare
	new function.
	* input.c (expansion_point_location_if_in_system_header): Define it.
gcc/cp/

	* call.c (conversion_null_warnings): Use the new
	expansion_point_location_if_in_system_header.
	* cvt.c (build_expr_type_conversion): Likewise.
	* typeck.c (cp_build_binary_op): Likewise.

gcc/testsuite/

	* g++.dg/warn/Wconversion-null-2.C: Add testing for __null,
	alongside the previous testing for NULL.
---
 gcc/cp/call.c                                  |    7 ++++-
 gcc/cp/cvt.c                                   |    9 +++++-
 gcc/cp/typeck.c                                |    9 ++++--
 gcc/input.c                                    |   20 +++++++++++++++
 gcc/input.h                                    |    1 +
 gcc/testsuite/g++.dg/warn/Wconversion-null-2.C |   31 +++++++++++++++++++++++-
 6 files changed, 69 insertions(+), 8 deletions(-)

diff --git a/gcc/cp/call.c b/gcc/cp/call.c
index f9a7f08..85e45c2 100644
--- a/gcc/cp/call.c
+++ b/gcc/cp/call.c
@@ -5598,12 +5598,15 @@ conversion_null_warnings (tree totype, tree expr, tree fn, int argnum)
   if (expr == null_node && TREE_CODE (totype) != BOOLEAN_TYPE
       && ARITHMETIC_TYPE_P (totype))
     {
+      source_location loc =
+	expansion_point_location_if_in_system_header (input_location);
+
       if (fn)
-	warning_at (input_location, OPT_Wconversion_null,
+	warning_at (loc, OPT_Wconversion_null,
 		    "passing NULL to non-pointer argument %P of %qD",
 		    argnum, fn);
       else
-	warning_at (input_location, OPT_Wconversion_null,
+	warning_at (loc, OPT_Wconversion_null,
 		    "converting to non-pointer type %qT from NULL", totype);
     }
 
diff --git a/gcc/cp/cvt.c b/gcc/cp/cvt.c
index 3dab372..49ba38a 100644
--- a/gcc/cp/cvt.c
+++ b/gcc/cp/cvt.c
@@ -1472,8 +1472,13 @@ build_expr_type_conversion (int desires, tree expr, bool complain)
   if (expr == null_node
       && (desires & WANT_INT)
       && !(desires & WANT_NULL))
-    warning_at (input_location, OPT_Wconversion_null,
-		"converting NULL to non-pointer type");
+    {
+      source_location loc =
+	expansion_point_location_if_in_system_header (input_location);
+
+      warning_at (loc, OPT_Wconversion_null,
+		  "converting NULL to non-pointer type");
+    }
 
   basetype = TREE_TYPE (expr);
 
diff --git a/gcc/cp/typeck.c b/gcc/cp/typeck.c
index fb2f1bc..52d264b 100644
--- a/gcc/cp/typeck.c
+++ b/gcc/cp/typeck.c
@@ -3838,9 +3838,12 @@ cp_build_binary_op (location_t location,
 	  || (!null_ptr_cst_p (orig_op1) 
 	      && !TYPE_PTR_P (type1) && !TYPE_PTR_TO_MEMBER_P (type1)))
       && (complain & tf_warning))
-    /* Some sort of arithmetic operation involving NULL was
-       performed.  */
-    warning (OPT_Wpointer_arith, "NULL used in arithmetic");
+    {
+      source_location loc =
+	expansion_point_location_if_in_system_header (input_location);
+
+      warning_at (loc, OPT_Wpointer_arith, "NULL used in arithmetic");
+    }
 
   switch (code)
     {
diff --git a/gcc/input.c b/gcc/input.c
index 260be7e..5f14489 100644
--- a/gcc/input.c
+++ b/gcc/input.c
@@ -162,6 +162,26 @@ expand_location_to_spelling_point (source_location loc)
   return expand_location_1 (loc, /*expansion_piont_p=*/false);
 }
 
+/* If LOCATION is in a sytem header and if it's a virtual location for
+   a token coming from the expansion of a macro M, unwind it to the
+   location of the expansion point of M.  Otherwise, just return
+   LOCATION.
+
+   This is used for instance when we want to emit diagnostics about a
+   token that is located in a macro that is itself defined in a system
+   header -- e.g for the NULL macro.  In that case, if LOCATION is
+   passed to diagnostics emitting functions like warning_at as is, no
+   diagnostic won't be emitted.  */
+
+source_location
+expansion_point_location_if_in_system_header (source_location location)
+{
+  if (in_system_header_at (location))
+    location = linemap_resolve_location (line_table, location,
+					 LRK_MACRO_EXPANSION_POINT,
+					 NULL);
+  return location;
+}
 
 #define ONE_K 1024
 #define ONE_M (ONE_K * ONE_K)
diff --git a/gcc/input.h b/gcc/input.h
index f755cdf..f588838 100644
--- a/gcc/input.h
+++ b/gcc/input.h
@@ -40,6 +40,7 @@ extern char builtins_location_check[(BUILTINS_LOCATION
 extern expanded_location expand_location (source_location);
 extern const char * location_get_source_line(expanded_location xloc);
 extern expanded_location expand_location_to_spelling_point (source_location);
+extern source_location expansion_point_location_if_in_system_header (source_location);
 
 /* Historically GCC used location_t, while cpp used source_location.
    This could be removed but it hardly seems worth the effort.  */
diff --git a/gcc/testsuite/g++.dg/warn/Wconversion-null-2.C b/gcc/testsuite/g++.dg/warn/Wconversion-null-2.C
index dd498c1..a71551f 100644
--- a/gcc/testsuite/g++.dg/warn/Wconversion-null-2.C
+++ b/gcc/testsuite/g++.dg/warn/Wconversion-null-2.C
@@ -25,7 +25,7 @@ void l(long) {}
 template <>
 void l(long long) {}
 
-int main()
+void warn_for_NULL()
 {
   int i = NULL; // { dg-warning "" } converting NULL to non-pointer type
   float z = NULL; // { dg-warning "" } converting NULL to non-pointer type
@@ -47,3 +47,32 @@ int main()
   l(NULL);   // No warning: NULL is used to implicitly instantiate the template
   NULL && NULL; // No warning: converting NULL to bool is OK
 }
+
+int warn_for___null()
+{
+  int i = __null; // { dg-warning "" } converting __null to non-pointer type
+  float z = __null; // { dg-warning "" } converting __null to non-pointer type
+  int a[2];
+
+  i != __null; // { dg-warning "" } __null used in arithmetic
+  __null != z; // { dg-warning "" } __null used in arithmetic
+  k != __null; // No warning: decay conversion
+  __null != a; // Likewise.
+  -__null;     // { dg-warning "" } converting __null to non-pointer type
+  +__null;     // { dg-warning "" } converting __null to non-pointer type
+  ~__null;     // { dg-warning "" } converting __null to non-pointer type
+  a[__null] = 3; // { dg-warning "" } converting __null to non-pointer-type
+  i = __null;  // { dg-warning "" } converting __null to non-pointer type
+  z = __null;  // { dg-warning "" } converting __null to non-pointer type
+  k(__null);   // { dg-warning "" } converting __null to int
+  g(__null);   // { dg-warning "" } converting __null to int
+  h<__null>(); // No warning: __null bound to integer template parameter
+  l(__null);   // No warning: __null is used to implicitly instantiate the template
+  __null && __null; // No warning: converting NULL to bool is OK
+}
+
+int main()
+{
+  warn_for_NULL();
+  warn_for___null();
+}
-- 
		Dodji