In r11-6900 (PR 98646 - static_cast confuses -Wnonnull) we decided
that issuing -Wnonnull for dereferencing the result of dynamic_cast
was helpful despite the false positives it causes when the pointer
is guaranteed not to be null because of a prior test.

The test case in PR 99251 along with the feedback I got from Martin
Liska have convinced me it was not the right decision.

The attached patch arranges for dynamic_cast to also suppress -Wnonnull
analogously to static_cast.  Since there already is a helper function
that builds the if-not-null test (ifnonnull) and sets TREE_NO_WARNING,
I factored out the corresponding code from build_base_path that sets
the additional TREE_NO_WARNING bit for static_cast into the function
and called it from both places.  I also renamed the function to make
its purpose clearer and for consistency with other build_xxx APIs.

Tested on x86_64-linux.

Martin
PR c++/99251 - inconsistent -Wnonnull warning behaviour with dynamic_cast

gcc/cp/ChangeLog:

	PR c++/99251
	* class.c (build_base_path): Call build_nonnull_test.
	* cp-tree.h (build_nonnull_test): Declare.
	* rtti.c (ifnonnull): Rename...
	(build_nonnull_test): ...to this.  Set no-warning bit on COND_EXPR.
	(build_dynamic_cast_1): Adjust to name change.

gcc/testsuite/ChangeLog:

	PR c++/99251
	* g++.dg/warn/Wnonnull9.C: Expect no warnings.
	* g++.dg/warn/Wnonnull12.C: New test.

diff --git a/gcc/cp/class.c b/gcc/cp/class.c
index 40f5fef7baa..6c6e0564bf9 100644
--- a/gcc/cp/class.c
+++ b/gcc/cp/class.c
@@ -402,16 +402,9 @@ build_base_path (enum tree_code code,
   if (TREE_SIDE_EFFECTS (expr) && (null_test || virtual_access))
     expr = save_expr (expr);
 
-  /* Now that we've saved expr, build the real null test.  */
+  /* Store EXPR and build the real null test just before returning.  */
   if (null_test)
-    {
-      tree zero = cp_convert (TREE_TYPE (expr), nullptr_node, complain);
-      null_test = build2_loc (input_location, NE_EXPR, boolean_type_node,
-			      expr, zero);
-      /* This is a compiler generated comparison, don't emit
-	 e.g. -Wnonnull-compare warning for it.  */
-      TREE_NO_WARNING (null_test) = 1;
-    }
+    null_test = expr;
 
   /* If this is a simple base reference, express it as a COMPONENT_REF.  */
   if (code == PLUS_EXPR && !virtual_access
@@ -516,14 +509,8 @@ build_base_path (enum tree_code code,
 
  out:
   if (null_test)
-    {
-      expr = fold_build3_loc (input_location, COND_EXPR, target_type, null_test,
-			      expr, build_zero_cst (target_type));
-      /* Avoid warning for the whole conditional expression (in addition
-	 to NULL_TEST itself -- see above) in case the result is used in
-	 a nonnull context that the front end -Wnonnull checks.  */
-      TREE_NO_WARNING (expr) = 1;
-    }
+    /* Wrap EXPR in a null test.  */
+    expr = build_nonnull_test (null_test, expr, complain);
 
   return expr;
 }
diff --git a/gcc/cp/cp-tree.h b/gcc/cp/cp-tree.h
index 38b31e3908f..8c6cda8d1a6 100644
--- a/gcc/cp/cp-tree.h
+++ b/gcc/cp/cp-tree.h
@@ -7271,6 +7271,7 @@ extern void emit_support_tinfos			(void);
 extern bool emit_tinfo_decl			(tree);
 extern unsigned get_pseudo_tinfo_index		(tree);
 extern tree get_pseudo_tinfo_type		(unsigned);
+extern tree build_nonnull_test			(tree, tree, tsubst_flags_t);
 
 /* in search.c */
 extern tree get_parent_with_private_access 	(tree decl, tree binfo);
diff --git a/gcc/cp/rtti.c b/gcc/cp/rtti.c
index b41d95469c6..84482743392 100644
--- a/gcc/cp/rtti.c
+++ b/gcc/cp/rtti.c
@@ -121,7 +121,6 @@ vec<tree, va_gc> *unemitted_tinfo_decls;
    and are generated as needed. */
 static GTY (()) vec<tinfo_s, va_gc> *tinfo_descs;
 
-static tree ifnonnull (tree, tree, tsubst_flags_t);
 static tree tinfo_name (tree, bool);
 static tree build_dynamic_cast_1 (location_t, tree, tree, tsubst_flags_t);
 static tree throw_bad_cast (void);
@@ -529,16 +528,23 @@ get_typeid (tree type, tsubst_flags_t complain)
 /* Check whether TEST is null before returning RESULT.  If TEST is used in
    RESULT, it must have previously had a save_expr applied to it.  */
 
-static tree
-ifnonnull (tree test, tree result, tsubst_flags_t complain)
+tree
+build_nonnull_test (tree test, tree result, tsubst_flags_t complain)
 {
-  tree cond = build2 (NE_EXPR, boolean_type_node, test,
-		      cp_convert (TREE_TYPE (test), nullptr_node, complain));
+  tree null_ptr = cp_convert (TREE_TYPE (test), nullptr_node, complain);
+  tree cond = build2 (NE_EXPR, boolean_type_node, test, null_ptr);
+
   /* This is a compiler generated comparison, don't emit
      e.g. -Wnonnull-compare warning for it.  */
   TREE_NO_WARNING (cond) = 1;
-  return build3 (COND_EXPR, TREE_TYPE (result), cond, result,
-		 cp_convert (TREE_TYPE (result), nullptr_node, complain));
+
+  null_ptr = cp_convert (TREE_TYPE (result), nullptr_node, complain);
+  cond = build3 (COND_EXPR, TREE_TYPE (result), cond, result, null_ptr);
+
+  /* Likewise, don't emit -Wnonnull for using the result to call
+     a member function.  */
+  TREE_NO_WARNING (cond) = 1;
+  return cond;
 }
 
 /* Execute a dynamic cast, as described in section 5.2.6 of the 9/93 working
@@ -671,7 +677,7 @@ build_dynamic_cast_1 (location_t loc, tree type, tree expr,
 	  expr1 = build_headof (expr);
 	  if (TREE_TYPE (expr1) != type)
 	    expr1 = build1 (NOP_EXPR, type, expr1);
-	  return ifnonnull (expr, expr1, complain);
+	  return build_nonnull_test (expr, expr1, complain);
 	}
       else
 	{
@@ -786,7 +792,7 @@ build_dynamic_cast_1 (location_t loc, tree type, tree expr,
 
 	  /* Now back to the type we want from a void*.  */
 	  result = cp_convert (type, result, complain);
-	  return ifnonnull (expr, result, complain);
+	  return build_nonnull_test (expr, result, complain);
 	}
     }
   else
diff --git a/gcc/testsuite/g++.dg/warn/Wnonnull12.C b/gcc/testsuite/g++.dg/warn/Wnonnull12.C
new file mode 100644
index 00000000000..7b2606302f5
--- /dev/null
+++ b/gcc/testsuite/g++.dg/warn/Wnonnull12.C
@@ -0,0 +1,29 @@
+/* PR c++/99251 - inconsistent -Wnonnull warning behaviour with dynamic_cast
+   { dg-do compile }
+   { dg-options "-Wall" } */
+
+struct A
+{
+  virtual ~A ();
+};
+
+struct B: A
+{
+  int f (int);
+};
+
+int f1 (A *p)
+{
+  if (!p)
+    return 0;
+
+  return (dynamic_cast<B *>(p))->f (1);
+}
+
+int f2 (A *p)
+{
+  if (!p)
+    return 0;
+
+  return dynamic_cast<B *>(p)->f (2);   // { dg-bogus "\\\[-Wnonnull" }
+}
diff --git a/gcc/testsuite/g++.dg/warn/Wnonnull9.C b/gcc/testsuite/g++.dg/warn/Wnonnull9.C
index b6135c4a624..e7c0dda2502 100644
--- a/gcc/testsuite/g++.dg/warn/Wnonnull9.C
+++ b/gcc/testsuite/g++.dg/warn/Wnonnull9.C
@@ -38,13 +38,16 @@ void static_cast_const_C_ptr (B *p)
 
 void dynamic_cast_C_ptr (B *p)
 {
-  // The dynamic_cast might fail so a warning is justified.
-  dynamic_cast<C*>(p->bptr ())->g ();         // { dg-warning "\\\[-Wnonnull" }
+  /* The dynamic_cast might fail but so a warning might be justified
+     but not when the operand is guarded.  See pr99251.  */
+  if (B *q = p->bptr ())
+    dynamic_cast<C*>(q)->g ();                // { dg-bogus "\\\[-Wnonnull" }
 }
 
 void dynamic_cast_const_C_ptr (B *p)
 {
-  dynamic_cast<const C*>(p->bptr ())->g ();   // { dg-warning "\\\[-Wnonnull" }
+  if (B *q = p->bptr ())
+    dynamic_cast<const C*>(q)->g ();          // { dg-bogus "\\\[-Wnonnull" }
 }
 
 
@@ -107,11 +110,14 @@ void static_cast_const_D_ptr (B *p)
 
 void dynamic_cast_D_ptr (B *p)
 {
-  // The dynamic_cast might fail so a warning is justified.
-  dynamic_cast<D*>(p->bptr ())->g ();         // { dg-warning "\\\[-Wnonnull" }
+  /* The dynamic_cast might fail but so a warning might be justified
+     but not when the operand is guarded.  See pr99251.  */
+  if (B *q = p->bptr ())
+    dynamic_cast<D*>(q)->g ();                // { dg-bogus "\\\[-Wnonnull" }
 }
 
 void dynamic_cast_const_D_ptr (B *p)
 {
-  dynamic_cast<const D*>(p->bptr ())->g ();   // { dg-warning "\\\[-Wnonnull" }
+  if (B *q = p->bptr ())
+    dynamic_cast<const D*>(q)->g ();          // { dg-bogus "\\\[-Wnonnull" }
 }

Reply via email to