diff mbox

Fix PR c++/60764

Message ID 1396904393-24080-1-git-send-email-patrick@parcs.ath.cx
State New
Headers show

Commit Message

Patrick Palka April 7, 2014, 8:59 p.m. UTC 
Hello,

This patch fixes fixes an erroneous -Wnonnull warning when calling a
constructor declared with __attribute__ ((nonnull (1))).  The issue is
that a NULL pointer is used as the placeholder object for the "this"
parameter of the constructor method call until a target object is
determined and substituted for the placeholder object during
gimplification.  Meanwhile, check_function_arguments is called in
build_over_call which notices that there is a NULL pointer argument for
a parameter that's been marked nonnull, thus issuing the -Wnonnull
warning.

This patch modifies the C++ frontend to use build_dummy_object() instead
of a NULL pointer as the placeholder object to a constructor method
call.  This suppresses the warning issued by check_function_arguments
because the object returned by build_dummy_object() is not determined to
be a NULL pointer.

I bootstrapped and regtested this patch on x86_64-unknown-linux-gnu.

	PR c++/60764
	* call.c (build_user_type_coversion): Use build_dummy_object
	to create the placeholder object for a constructor method call.
	(build_special_member_call): Likewise.
	(build_over_call): Check for the placeholder object with
	is_dummy_object.
	(build_new_method_call_1): Likewise.  Don't attempt to resolve
	a dummy object for a constructor method call.
---
 gcc/cp/call.c                        | 14 +++++---------
 gcc/testsuite/g++.dg/warn/nonnull2.C | 10 ++++++++++
 2 files changed, 15 insertions(+), 9 deletions(-)
 create mode 100644 gcc/testsuite/g++.dg/warn/nonnull2.C

Comments

Jason Merrill April 9, 2014, 8:40 p.m. UTC | #1 
Hmm, I would expect the parameter numbering for attribute nonnull and 
such to ignore the 'this' parameter.

Jason
Marc Glisse April 9, 2014, 9:27 p.m. UTC | #2 
On Wed, 9 Apr 2014, Jason Merrill wrote:

> Hmm, I would expect the parameter numbering for attribute nonnull and such to 
> ignore the 'this' parameter.

The doc for the "format" attribute says clearly:

"Since non-static C++ methods have an implicit this argument, the 
arguments of such methods should be counted from two, not one, when giving 
values for string-index and first-to-check."

It would be strange to count arguments differently for different 
attributes.
Jason Merrill April 10, 2014, 12:21 p.m. UTC | #3 
On 04/09/2014 05:27 PM, Marc Glisse wrote:
> The doc for the "format" attribute says clearly:
>
> "Since non-static C++ methods have an implicit this argument, the
> arguments of such methods should be counted from two, not one, when
> giving values for string-index and first-to-check."

Ah.  That seems wrong to me, but if it's already documented that way I 
guess we shouldn't change it.

> It would be strange to count arguments differently for different
> attributes.

Agreed.

The patch is OK for after 4.9 branches.

Jason
Patrick Palka April 16, 2014, 12:16 p.m. UTC | #4 
Could someone install this for me?
diff mbox

Patch

diff --git a/gcc/cp/call.c b/gcc/cp/call.c
index ae0d4ff..ff5c834 100644
--- a/gcc/cp/call.c
+++ b/gcc/cp/call.c
@@ -3519,8 +3519,7 @@  build_user_type_conversion_1 (tree totype, tree expr, int flags,
     {
       int ctorflags = flags;
 
-      first_arg = build_int_cst (build_pointer_type (totype), 0);
-      first_arg = build_fold_indirect_ref (first_arg);
+      first_arg = build_dummy_object (totype);
 
       /* We should never try to call the abstract or base constructor
 	 from here.  */
@@ -7096,7 +7095,7 @@  build_over_call (struct z_candidate *cand, int flags, tsubst_flags_t complain)
 	 ctor is trivial, do a bitwise copy with a simple TARGET_EXPR for a
 	 temp or an INIT_EXPR otherwise.  */
       fa = argarray[0];
-      if (integer_zerop (fa))
+      if (is_dummy_object (fa))
 	{
 	  if (TREE_CODE (arg) == TARGET_EXPR)
 	    return arg;
@@ -7434,10 +7433,7 @@  build_special_member_call (tree instance, tree name, vec<tree, va_gc> **args,
 
   /* Handle the special case where INSTANCE is NULL_TREE.  */
   if (name == complete_ctor_identifier && !instance)
-    {
-      instance = build_int_cst (build_pointer_type (class_type), 0);
-      instance = build1 (INDIRECT_REF, class_type, instance);
-    }
+    instance = build_dummy_object (class_type);
   else
     {
       if (name == complete_dtor_identifier
@@ -7747,8 +7743,7 @@  build_new_method_call_1 (tree instance, tree fns, vec<tree, va_gc> **args,
 
       if (init)
 	{
-	  if (INDIRECT_REF_P (instance)
-	      && integer_zerop (TREE_OPERAND (instance, 0)))
+	  if (is_dummy_object (instance))
 	    return get_target_expr_sfinae (init, complain);
 	  init = build2 (INIT_EXPR, TREE_TYPE (instance), instance, init);
 	  TREE_SIDE_EFFECTS (init) = true;
@@ -7847,6 +7842,7 @@  build_new_method_call_1 (tree instance, tree fns, vec<tree, va_gc> **args,
 	    }
 
 	  if (TREE_CODE (TREE_TYPE (fn)) == METHOD_TYPE
+	      && !DECL_CONSTRUCTOR_P (fn)
 	      && is_dummy_object (instance))
 	    {
 	      instance = maybe_resolve_dummy (instance);
diff --git a/gcc/testsuite/g++.dg/warn/nonnull2.C b/gcc/testsuite/g++.dg/warn/nonnull2.C
new file mode 100644
index 0000000..10515a4
--- /dev/null
+++ b/gcc/testsuite/g++.dg/warn/nonnull2.C
@@ -0,0 +1,10 @@ 
+// PR c++/60764
+// { dg-options "-Wall" }
+
+struct foo
+{
+  foo () __attribute__ ((nonnull (1)));
+};
+
+const foo &x = foo (); // { dg-bogus "null argument" }
+foo y = foo (); // { dg-bogus "null argument" }