diff mbox

[C++] PR 52363

Message ID 4F8EFD66.10306@oracle.com
State New
Headers show

Commit Message

Paolo Carlini April 18, 2012, 5:44 p.m. UTC 
Hi,

thus I'm handling this issue per the discussion in the audit trail: by 
adding a tsubst_flags_t parameter to 'joust' and 'tourney' and adjusting 
the callers. Then 'joust' deals with SFINAE as-if pedantic mode were 
unconditionally active.

Tested x86_64-linux.

Thanks,
Paolo.

////////////////////////
/cp
2012-04-18  Paolo Carlini  <paolo.carlini@oracle.com>

	PR c++/52363
	* call.c (tourney): Add tsubst_flags_t parameter.
	(joust): Likewise, use it to handle SFINAE as if pedantic.
	(build_user_type_conversion_1, perform_overload_resolution,
	build_op_call_1, build_conditional_expr_1, build_new_op_1,
	build_over_call, build_new_method_call_1): Adjust.

/testsuite
2012-04-18  Paolo Carlini  <paolo.carlini@oracle.com>

	PR c++/52363
	* testsuite/g++.dg/cpp0x/sfinae35.C: New.
	* testsuite/g++.dg/cpp0x/sfinae36.C: Likewise.

Comments

Jason Merrill April 18, 2012, 7 p.m. UTC | #1 
On 04/18/2012 01:44 PM, Paolo Carlini wrote:
> +  cand = tourney (candidates, tf_warning_or_error);

This seems unlikely to do the right thing; we can get here in both 
SFINAE and non-SFINAE situations.  In build_user_type_conversion_1 I 
think we can get away with checking LOOKUP_COMPLAIN; for 
perform_overload_conversion we need to pass in complain.

Jason
Paolo Carlini April 18, 2012, 8:07 p.m. UTC | #2 
Hi
> On 04/18/2012 01:44 PM, Paolo Carlini wrote:
>> +  cand = tourney (candidates, tf_warning_or_error);
>
> This seems unlikely to do the right thing; we can get here in both 
> SFINAE and non-SFINAE situations.  In build_user_type_conversion_1 I 
> think we can get away with checking LOOKUP_COMPLAIN; for 
> perform_overload_conversion we need to pass in complain.
Ok. perform_overload_conversion can get a complain very smoothly, too 
bad I didn't try it to earlier today. build_user_type_conversion_1 seems 
more tricky: I understood your suggestion as something like:

-  cand = tourney (candidates);
+  cand = tourney (candidates, (flags & LOOKUP_COMPLAIN)
+          ? tf_warning_or_error : tf_none);

and didn't work well enough, I got these fails:

FAIL: g++.old-deja/g++.benjamin/16077.C -std=gnu++98 nic (test for 
warnings, line 24)
FAIL: g++.old-deja/g++.benjamin/16077.C -std=gnu++98 conv (test for 
warnings, line 24)
FAIL: g++.old-deja/g++.benjamin/16077.C -std=gnu++98 note (test for 
warnings, line 24)
FAIL: g++.old-deja/g++.benjamin/16077.C -std=gnu++11 nic (test for 
warnings, line 24)
FAIL: g++.old-deja/g++.benjamin/16077.C -std=gnu++11 conv (test for 
warnings, line 24)
FAIL: g++.old-deja/g++.benjamin/16077.C -std=gnu++11 note (test for 
warnings, line 24)
FAIL: g++.old-deja/g++.other/overcnv2.C -std=gnu++98 B (test for 
warnings, line 19)
FAIL: g++.old-deja/g++.other/overcnv2.C -std=gnu++98 conv (test for 
warnings, line 19)
FAIL: g++.old-deja/g++.other/overcnv2.C -std=gnu++98 note (test for 
warnings, line 19)
FAIL: g++.old-deja/g++.other/overcnv2.C -std=gnu++11 B (test for 
warnings, line 19)
FAIL: g++.old-deja/g++.other/overcnv2.C -std=gnu++11 conv (test for 
warnings, line 19)
FAIL: g++.old-deja/g++.other/overcnv2.C -std=gnu++11 note (test for 
warnings, line 19)

Thus, if I don't ear from you, I'm probably going to add complain to 
build_user_type_conversion_1, hopefully I can manage, I remember that 
earlier today I tried and was dragging in a lot of changes...

Thanks,
Paolo.
Paolo Carlini April 19, 2012, 12:30 a.m. UTC | #3 
On 04/18/2012 09:00 PM, Jason Merrill wrote:
> On 04/18/2012 01:44 PM, Paolo Carlini wrote:
>> +  cand = tourney (candidates, tf_warning_or_error);
>
> This seems unlikely to do the right thing; we can get here in both 
> SFINAE and non-SFINAE situations.  In build_user_type_conversion_1 I 
> think we can get away with checking LOOKUP_COMPLAIN;
... however, I must admit that I'm not sure to understand what exactly 
we are risking here, in terms of regressions: I mean, *currently*, if 
build_user_type_conversion_1 is called in a SFINAE situation, it seems 
indeed in principle possible that the above call leads to hard errors. 
With my change, tourney keeps on behaving exactly in the same way, that 
is - if I didn't make some stupid material mistake somewhere:

     tourney (candidates) == tourney (candidates, tf_warning_or_error)

Thus, it seems to me that with above change it's just that we don't have 
any hope of fixing latent SFINAE bugs in turney called by 
build_user_type_conversion_1 per the above. And, at this time, we don't 
have any evidence of such bugs.

That said, if we want to pursue the approach you suggested above 
(instead of going for adding complain to build_user_type_conversion_1, 
as I did in my second proposal), we could for example add more to 
LOOKUP_COMPLAIN, eg. adding LOOKUP_NO_CONVERSION is enough for the 
testsuite:

   cand = tourney (candidates,
           (flags & (LOOKUP_COMPLAIN | LOOKUP_NO_CONVERSION))
           ? tf_warning_or_error : tf_none);

but then, this is something I would personally consider a bit risky, 
because we could definitely stop producing errors and warnings which we 
do produce now. Unless we are able to audit all the callers of 
build_user_type_conversion_1 and make sure we got the latter check right 
(not so many, just a few in call.c and cvt.c)

What do you think?

Thanks,
Paolo.
Jason Merrill April 19, 2012, 4:06 a.m. UTC | #4 
On 04/18/2012 08:30 PM, Paolo Carlini wrote:
> Thus, it seems to me that with above change it's just that we don't have
> any hope of fixing latent SFINAE bugs in turney called by
> build_user_type_conversion_1 per the above. And, at this time, we don't
> have any evidence of such bugs.

We've just been dealing with SFINAE bugs in tourney; I would expect that 
they can also occur for conversions using a single-argument constructor.

I think you're right that we should just pass complain through 
build_user_type_conversion_1.  Looking at the patch, I think we need to 
pass it through implicit_conversion and reference_binding as well.

Jason
diff mbox

Patch

Index: testsuite/g++.dg/cpp0x/sfinae35.C
===================================================================
--- testsuite/g++.dg/cpp0x/sfinae35.C	(revision 0)
+++ testsuite/g++.dg/cpp0x/sfinae35.C	(revision 0)
@@ -0,0 +1,13 @@ 
+// PR c++/52363
+// { dg-options -std=c++11 }
+
+#include <type_traits>
+
+struct proxy
+{
+  void operator=(int const&);
+  void operator=(int&&) const;
+};
+
+static_assert( !std::is_assignable<proxy, int>::value, "" );
+static_assert( std::is_assignable<const proxy, int>::value, "" );
Index: testsuite/g++.dg/cpp0x/sfinae36.C
===================================================================
--- testsuite/g++.dg/cpp0x/sfinae36.C	(revision 0)
+++ testsuite/g++.dg/cpp0x/sfinae36.C	(revision 0)
@@ -0,0 +1,13 @@ 
+// PR c++/52363
+// { dg-options "-std=c++11 -pedantic" }
+
+#include <type_traits>
+
+struct proxy
+{
+  void operator=(int const&);
+  void operator=(int&&) const;
+};
+
+static_assert( !std::is_assignable<proxy, int>::value, "" );
+static_assert( std::is_assignable<const proxy, int>::value, "" );
Index: cp/call.c
===================================================================
--- cp/call.c	(revision 186573)
+++ cp/call.c	(working copy)
@@ -142,9 +142,10 @@  static struct obstack conversion_obstack;
 static bool conversion_obstack_initialized;
 struct rejection_reason;
 
-static struct z_candidate * tourney (struct z_candidate *);
+static struct z_candidate * tourney (struct z_candidate *, tsubst_flags_t);
 static int equal_functions (tree, tree);
-static int joust (struct z_candidate *, struct z_candidate *, bool);
+static int joust (struct z_candidate *, struct z_candidate *, bool,
+		  tsubst_flags_t);
 static int compare_ics (conversion *, conversion *);
 static tree build_over_call (struct z_candidate *, int, tsubst_flags_t);
 static tree build_java_interface_fn_ref (tree, tree);
@@ -3582,7 +3583,7 @@  build_user_type_conversion_1 (tree totype, tree ex
       return NULL;
     }
 
-  cand = tourney (candidates);
+  cand = tourney (candidates, tf_warning_or_error);
   if (cand == 0)
     {
       if (flags & LOOKUP_COMPLAIN)
@@ -3800,7 +3801,7 @@  perform_overload_resolution (tree fn,
 
   *candidates = splice_viable (*candidates, pedantic, any_viable_p);
   if (*any_viable_p)
-    cand = tourney (*candidates);
+    cand = tourney (*candidates, tf_warning_or_error);
   else
     cand = NULL;
 
@@ -4106,7 +4107,7 @@  build_op_call_1 (tree obj, VEC(tree,gc) **args, ts
     }
   else
     {
-      cand = tourney (candidates);
+      cand = tourney (candidates, complain);
       if (cand == 0)
 	{
           if (complain & tf_error)
@@ -4584,7 +4585,7 @@  build_conditional_expr_1 (tree arg1, tree arg2, tr
             }
 	  return error_mark_node;
 	}
-      cand = tourney (candidates);
+      cand = tourney (candidates, complain);
       if (!cand)
 	{
           if (complain & tf_error)
@@ -5113,7 +5114,7 @@  build_new_op_1 (enum tree_code code, int flags, tr
     }
   else
     {
-      cand = tourney (candidates);
+      cand = tourney (candidates, complain);
       if (cand == 0)
 	{
 	  if ((flags & LOOKUP_COMPLAIN) && (complain & tf_error))
@@ -5140,7 +5141,7 @@  build_new_op_1 (enum tree_code code, int flags, tr
 	    {
 	      struct candidate_warning *w;
 	      for (w = cand->warnings; w; w = w->next)
-		joust (cand, w->loser, 1);
+		joust (cand, w->loser, 1, complain);
 	    }
 
 	  /* Check for comparison of different enum types.  */
@@ -6383,7 +6384,7 @@  build_over_call (struct z_candidate *cand, int fla
     {
       struct candidate_warning *w;
       for (w = cand->warnings; w; w = w->next)
-	joust (cand, w->loser, 1);
+	joust (cand, w->loser, 1, complain);
     }
 
   /* Make =delete work with SFINAE.  */
@@ -7318,7 +7319,7 @@  build_new_method_call_1 (tree instance, tree fns,
     }
   else
     {
-      cand = tourney (candidates);
+      cand = tourney (candidates, complain);
       if (cand == 0)
 	{
 	  char *pretty_name;
@@ -8013,7 +8014,8 @@  add_warning (struct z_candidate *winner, struct z_
       0: cand1 and cand2 are indistinguishable */
 
 static int
-joust (struct z_candidate *cand1, struct z_candidate *cand2, bool warn)
+joust (struct z_candidate *cand1, struct z_candidate *cand2, bool warn,
+       tsubst_flags_t complain)
 {
   int winner = 0;
   int off1 = 0, off2 = 0;
@@ -8076,7 +8078,8 @@  static int
 
       if (comp != 0)
 	{
-	  if (warn_sign_promo
+	  if ((complain & tf_warning)
+	      && warn_sign_promo
 	      && (CONVERSION_RANK (t1) + CONVERSION_RANK (t2)
 		  == cr_std + cr_promotion)
 	      && t1->kind == ck_std
@@ -8121,7 +8124,8 @@  static int
   /* warn about confusing overload resolution for user-defined conversions,
      either between a constructor and a conversion op, or between two
      conversion ops.  */
-  if (winner && warn_conversion && cand1->second_conv
+  if ((complain & tf_warning)
+      && winner && warn_conversion && cand1->second_conv
       && (!DECL_CONSTRUCTOR_P (cand1->fn) || !DECL_CONSTRUCTOR_P (cand2->fn))
       && winner != compare_ics (cand1->second_conv, cand2->second_conv))
     {
@@ -8283,12 +8287,18 @@  static int
 	    {
 	      if (warn)
 		{
-		  permerror (input_location, "default argument mismatch in "
-			     "overload resolution");
-		  inform (input_location,
-			  " candidate 1: %q+#F", cand1->fn);
-		  inform (input_location,
-			  " candidate 2: %q+#F", cand2->fn);
+		  if (complain & tf_error)
+		    {
+		      permerror (input_location,
+				 "default argument mismatch in "
+				 "overload resolution");
+		      inform (input_location,
+			      " candidate 1: %q+#F", cand1->fn);
+		      inform (input_location,
+			      " candidate 2: %q+#F", cand2->fn);
+		    }
+		  else
+		    return 0;
 		}
 	      else
 		add_warning (cand1, cand2);
@@ -8305,7 +8315,7 @@  tweak:
 
   /* Extension: If the worst conversion for one candidate is worse than the
      worst conversion for the other, take the first.  */
-  if (!pedantic)
+  if (!pedantic && (complain & tf_warning_or_error))
     {
       conversion_rank rank1 = cr_identity, rank2 = cr_identity;
       struct z_candidate *w = 0, *l = 0;
@@ -8351,7 +8361,7 @@  tweak:
    algorithm.  */
 
 static struct z_candidate *
-tourney (struct z_candidate *candidates)
+tourney (struct z_candidate *candidates, tsubst_flags_t complain)
 {
   struct z_candidate *champ = candidates, *challenger;
   int fate;
@@ -8362,7 +8372,7 @@  static struct z_candidate *
 
   for (challenger = champ->next; challenger; )
     {
-      fate = joust (champ, challenger, 0);
+      fate = joust (champ, challenger, 0, complain);
       if (fate == 1)
 	challenger = challenger->next;
       else
@@ -8392,7 +8402,7 @@  static struct z_candidate *
 	 && !(champ_compared_to_predecessor && challenger->next == champ);
        challenger = challenger->next)
     {
-      fate = joust (champ, challenger, 0);
+      fate = joust (champ, challenger, 0, complain);
       if (fate != 1)
 	return NULL;
     }