Patchwork C++ PATCH for c++/48113 (SFINAE bug causing bind regression in libstdc++)

login
register
mail settings
Submitter Jason Merrill
Date March 16, 2011, 7:51 p.m.
Message ID <4D8114D5.6070907@redhat.com>
Download mbox | patch
Permalink /patch/87295/
State New
Headers show

Comments

Jason Merrill - March 16, 2011, 7:51 p.m.
convert_for_initialization is sfinae-enabled, but it was calling 
ocp_convert, which is not.  This patch changes it to use the newer 
perform_implicit_conversion_flags instead.  To make that work, I needed 
to add support for LOOKUP_PREFER_RVALUE to that code path.

Tested x86_64-pc-linux-gnu, applying to trunk.

Also OK for 4.6.0?  The risk is that swapping to 
perform_implicit_conversion_flags will have other differences of 
behavior, but the library regression seems pretty major, so I think it's 
worth the risk.
commit fb46d045ce1b2ee71402942a42b61921cec7c41f
Author: Jason Merrill <jason@redhat.com>
Date:   Mon Mar 14 14:49:58 2011 -0400

    	PR c++/48113
    	* typeck.c (convert_for_initialization): Use
    	perform_implicit_conversion_flags.
    	* call.c (standard_conversion): If LOOKUP_PREFER_RVALUE, set
    	rvaluedness_matches_p on ck_rvalue.
    	(convert_like_real) [ck_rvalue]: And restore it here.
Paolo Carlini - March 16, 2011, 9:23 p.m.
Hi,
> 
> Also OK for 4.6.0?  The risk is that swapping to perform_implicit_conversion_flags will have other differences of behavior, but the library regression seems pretty major, so I think it's worth the risk.

Thanks Jason. Indeed, I agree that the library issue is pretty serious. If you think I can help somehow in terms of testing, just let me know.

Thanks again,
Paolo.
Richard Guenther - March 17, 2011, 8:26 a.m.
On Wed, Mar 16, 2011 at 8:51 PM, Jason Merrill <jason@redhat.com> wrote:
> convert_for_initialization is sfinae-enabled, but it was calling
> ocp_convert, which is not.  This patch changes it to use the newer
> perform_implicit_conversion_flags instead.  To make that work, I needed to
> add support for LOOKUP_PREFER_RVALUE to that code path.
>
> Tested x86_64-pc-linux-gnu, applying to trunk.
>
> Also OK for 4.6.0?  The risk is that swapping to
> perform_implicit_conversion_flags will have other differences of behavior,
> but the library regression seems pretty major, so I think it's worth the
> risk.

Ok for 4.6.0.  I guess we need to do a RC2 anyway.

Thanks,
Richard.

> commit fb46d045ce1b2ee71402942a42b61921cec7c41f
> Author: Jason Merrill <jason@redhat.com>
> Date:   Mon Mar 14 14:49:58 2011 -0400
>
>        PR c++/48113
>        * typeck.c (convert_for_initialization): Use
>        perform_implicit_conversion_flags.
>        * call.c (standard_conversion): If LOOKUP_PREFER_RVALUE, set
>        rvaluedness_matches_p on ck_rvalue.
>        (convert_like_real) [ck_rvalue]: And restore it here.
>
> diff --git a/gcc/cp/call.c b/gcc/cp/call.c
> index f75c248..436c956 100644
> --- a/gcc/cp/call.c
> +++ b/gcc/cp/call.c
> @@ -98,7 +98,9 @@ struct conversion {
>   BOOL_BITFIELD base_p : 1;
>   /* If KIND is ck_ref_bind, true when either an lvalue reference is
>      being bound to an lvalue expression or an rvalue reference is
> -     being bound to an rvalue expression. */
> +     being bound to an rvalue expression.  If KIND is ck_rvalue,
> +     true when we should treat an lvalue as an rvalue (12.8p33).  If
> +     KIND is ck_base, always false.  */
>   BOOL_BITFIELD rvaluedness_matches_p: 1;
>   BOOL_BITFIELD check_narrowing: 1;
>   /* The type of the expression resulting from the conversion.  */
> @@ -897,6 +899,8 @@ standard_conversion (tree to, tree from, tree expr, bool
> c_cast_p,
>            }
>        }
>       conv = build_conv (ck_rvalue, from, conv);
> +      if (flags & LOOKUP_PREFER_RVALUE)
> +       conv->rvaluedness_matches_p = true;
>     }
>
>    /* Allow conversion between `__complex__' data types.  */
> @@ -5489,6 +5493,8 @@ convert_like_real (conversion *convs, tree expr, tree
> fn, int argnum,
>           conversion (i.e. the second step of copy-initialization), so
>           don't allow any more.  */
>        flags |= LOOKUP_NO_CONVERSION;
> +      if (convs->rvaluedness_matches_p)
> +       flags |= LOOKUP_PREFER_RVALUE;
>       if (TREE_CODE (expr) == TARGET_EXPR
>          && TARGET_EXPR_LIST_INIT_P (expr))
>        /* Copy-list-initialization doesn't actually involve a copy.  */
> diff --git a/gcc/cp/typeck.c b/gcc/cp/typeck.c
> index c062f0f..0e8a6d7 100644
> --- a/gcc/cp/typeck.c
> +++ b/gcc/cp/typeck.c
> @@ -7454,7 +7454,7 @@ convert_for_initialization (tree exp, tree type, tree
> rhs, int flags,
>     return rhs;
>
>   if (MAYBE_CLASS_TYPE_P (type))
> -    return ocp_convert (type, rhs, CONV_IMPLICIT|CONV_FORCE_TEMP, flags);
> +    return perform_implicit_conversion_flags (type, rhs, complain, flags);
>
>   return convert_for_assignment (type, rhs, errtype, fndecl, parmnum,
>                                 complain, flags);
> diff --git a/gcc/testsuite/g++.dg/cpp0x/initlist38.C
> b/gcc/testsuite/g++.dg/cpp0x/initlist38.C
> index 818d69a..32e20d5 100644
> --- a/gcc/testsuite/g++.dg/cpp0x/initlist38.C
> +++ b/gcc/testsuite/g++.dg/cpp0x/initlist38.C
> @@ -17,5 +17,5 @@ int main()
>   f({});
>   B b0 = { };
>   B b1 { };    // OK, uses #1
> -  B b2 { 1 };  // { dg-error "conversion" }
> +  B b2 { 1 };  // { dg-error "could not convert" }
>  }
> diff --git a/gcc/testsuite/g++.dg/cpp0x/pr45908.C
> b/gcc/testsuite/g++.dg/cpp0x/pr45908.C
> index 1a821e5..3a85088 100644
> --- a/gcc/testsuite/g++.dg/cpp0x/pr45908.C
> +++ b/gcc/testsuite/g++.dg/cpp0x/pr45908.C
> @@ -14,5 +14,5 @@ struct vector {
>  class block {
>     vector v;
>     auto end() const -> decltype(v.begin())
> -    { return v.begin(); } // { dg-error "conversion" }
> +    { return v.begin(); } // { dg-error "could not convert" }
>  };
> diff --git a/gcc/testsuite/g++.dg/cpp0x/sfinae6.C
> b/gcc/testsuite/g++.dg/cpp0x/sfinae6.C
> new file mode 100644
> index 0000000..401d536
> --- /dev/null
> +++ b/gcc/testsuite/g++.dg/cpp0x/sfinae6.C
> @@ -0,0 +1,31 @@
> +// PR c++/48113
> +// { dg-options -std=c++0x }
> +
> +template<typename T> T declval();
> +
> +struct tuple { };
> +
> +struct F1
> +{
> +    void operator()(tuple, int);
> +};
> +
> +typedef void (*F2)(tuple, int);
> +
> +template<typename F, typename T>
> +struct Bind
> +{
> +    template<typename A,
> +             typename R = decltype( F()(declval<T&>(), A()) )>
> +    R f(A);
> +
> +    template<typename A,
> +             typename R = decltype( F()(declval<volatile T&>(), A()) )>
> +    R f(A) volatile;
> +};
> +
> +int main()
> +{
> +    Bind<F1, tuple>().f(0);  // OK
> +    Bind<F2, tuple>().f(0);  // ERROR, should be OK
> +}
> diff --git a/gcc/testsuite/g++.old-deja/g++.jason/conversion11.C
> b/gcc/testsuite/g++.old-deja/g++.jason/conversion11.C
> index 607cf9c..6621a27 100644
> --- a/gcc/testsuite/g++.old-deja/g++.jason/conversion11.C
> +++ b/gcc/testsuite/g++.old-deja/g++.jason/conversion11.C
> @@ -21,7 +21,7 @@ void DoSomething(Ding A);
>
>  void foo(Something* pX)
>  {
> -  DoSomething(1);              // { dg-error "conversion" }
> +  DoSomething(1);              // { dg-error "could not convert" }
>   pX->DoSomething(1);          // { dg-error "no matching" }
>   // { dg-message "candidate" "candidate note" { target *-*-* } 25 }
>   (*pX).DoSomething(1);                // { dg-error "no matching" }
> diff --git a/gcc/testsuite/g++.old-deja/g++.law/arg11.C
> b/gcc/testsuite/g++.old-deja/g++.law/arg11.C
> index fc590c4..fc93579 100644
> --- a/gcc/testsuite/g++.old-deja/g++.law/arg11.C
> +++ b/gcc/testsuite/g++.old-deja/g++.law/arg11.C
> @@ -16,7 +16,7 @@ void function(Ack);
>  int
>  foo(S *o)
>  { // Neither call has a usable constructor for conversions of char[5] to
> Ack.
> -  function("adsf");// { dg-error "conversion" }
> +  function("adsf");// { dg-error "could not convert" }
>   o->method("adsf");// { dg-error "no matching" }
>   // { dg-message "candidate" "candidate note" { target *-*-* } 20 }
>   return 0;
>
>
Jakub Jelinek - March 17, 2011, 8:34 a.m.
On Thu, Mar 17, 2011 at 09:26:36AM +0100, Richard Guenther wrote:
> Ok for 4.6.0.  I guess we need to do a RC2 anyway.

Yeah, certainly.  I'll roll one during the weekend, it would be nice if all
the pending fixes still desirable for 4.6.0 make it till then and
after RC2 we'd try to allow only very high severity bugfixes.

Some sort of safe workaround for PR47571 is IMHO e.g. acceptable before RC2,
but not afterwards, and probably want PR48161 fixed too.

	Jakub

Patch

diff --git a/gcc/cp/call.c b/gcc/cp/call.c
index f75c248..436c956 100644
--- a/gcc/cp/call.c
+++ b/gcc/cp/call.c
@@ -98,7 +98,9 @@  struct conversion {
   BOOL_BITFIELD base_p : 1;
   /* If KIND is ck_ref_bind, true when either an lvalue reference is
      being bound to an lvalue expression or an rvalue reference is
-     being bound to an rvalue expression. */
+     being bound to an rvalue expression.  If KIND is ck_rvalue,
+     true when we should treat an lvalue as an rvalue (12.8p33).  If
+     KIND is ck_base, always false.  */
   BOOL_BITFIELD rvaluedness_matches_p: 1;
   BOOL_BITFIELD check_narrowing: 1;
   /* The type of the expression resulting from the conversion.  */
@@ -897,6 +899,8 @@  standard_conversion (tree to, tree from, tree expr, bool c_cast_p,
 	    }
 	}
       conv = build_conv (ck_rvalue, from, conv);
+      if (flags & LOOKUP_PREFER_RVALUE)
+	conv->rvaluedness_matches_p = true;
     }
 
    /* Allow conversion between `__complex__' data types.  */
@@ -5489,6 +5493,8 @@  convert_like_real (conversion *convs, tree expr, tree fn, int argnum,
 	   conversion (i.e. the second step of copy-initialization), so
 	   don't allow any more.  */
 	flags |= LOOKUP_NO_CONVERSION;
+      if (convs->rvaluedness_matches_p)
+	flags |= LOOKUP_PREFER_RVALUE;
       if (TREE_CODE (expr) == TARGET_EXPR
 	  && TARGET_EXPR_LIST_INIT_P (expr))
 	/* Copy-list-initialization doesn't actually involve a copy.  */
diff --git a/gcc/cp/typeck.c b/gcc/cp/typeck.c
index c062f0f..0e8a6d7 100644
--- a/gcc/cp/typeck.c
+++ b/gcc/cp/typeck.c
@@ -7454,7 +7454,7 @@  convert_for_initialization (tree exp, tree type, tree rhs, int flags,
     return rhs;
 
   if (MAYBE_CLASS_TYPE_P (type))
-    return ocp_convert (type, rhs, CONV_IMPLICIT|CONV_FORCE_TEMP, flags);
+    return perform_implicit_conversion_flags (type, rhs, complain, flags);
 
   return convert_for_assignment (type, rhs, errtype, fndecl, parmnum,
 				 complain, flags);
diff --git a/gcc/testsuite/g++.dg/cpp0x/initlist38.C b/gcc/testsuite/g++.dg/cpp0x/initlist38.C
index 818d69a..32e20d5 100644
--- a/gcc/testsuite/g++.dg/cpp0x/initlist38.C
+++ b/gcc/testsuite/g++.dg/cpp0x/initlist38.C
@@ -17,5 +17,5 @@  int main()
   f({});
   B b0 = { };
   B b1 { };    // OK, uses #1
-  B b2 { 1 };  // { dg-error "conversion" }
+  B b2 { 1 };  // { dg-error "could not convert" }
 }
diff --git a/gcc/testsuite/g++.dg/cpp0x/pr45908.C b/gcc/testsuite/g++.dg/cpp0x/pr45908.C
index 1a821e5..3a85088 100644
--- a/gcc/testsuite/g++.dg/cpp0x/pr45908.C
+++ b/gcc/testsuite/g++.dg/cpp0x/pr45908.C
@@ -14,5 +14,5 @@  struct vector {
 class block {
     vector v;
     auto end() const -> decltype(v.begin())
-    { return v.begin(); } // { dg-error "conversion" }
+    { return v.begin(); } // { dg-error "could not convert" }
 };
diff --git a/gcc/testsuite/g++.dg/cpp0x/sfinae6.C b/gcc/testsuite/g++.dg/cpp0x/sfinae6.C
new file mode 100644
index 0000000..401d536
--- /dev/null
+++ b/gcc/testsuite/g++.dg/cpp0x/sfinae6.C
@@ -0,0 +1,31 @@ 
+// PR c++/48113
+// { dg-options -std=c++0x }
+
+template<typename T> T declval();
+
+struct tuple { };
+
+struct F1
+{
+    void operator()(tuple, int);
+};
+
+typedef void (*F2)(tuple, int);
+
+template<typename F, typename T>
+struct Bind
+{
+    template<typename A,
+             typename R = decltype( F()(declval<T&>(), A()) )>
+    R f(A);
+
+    template<typename A,
+             typename R = decltype( F()(declval<volatile T&>(), A()) )>
+    R f(A) volatile;
+};
+
+int main()
+{
+    Bind<F1, tuple>().f(0);  // OK
+    Bind<F2, tuple>().f(0);  // ERROR, should be OK
+}
diff --git a/gcc/testsuite/g++.old-deja/g++.jason/conversion11.C b/gcc/testsuite/g++.old-deja/g++.jason/conversion11.C
index 607cf9c..6621a27 100644
--- a/gcc/testsuite/g++.old-deja/g++.jason/conversion11.C
+++ b/gcc/testsuite/g++.old-deja/g++.jason/conversion11.C
@@ -21,7 +21,7 @@  void DoSomething(Ding A);
 
 void foo(Something* pX)
 {
-  DoSomething(1);		// { dg-error "conversion" } 
+  DoSomething(1);		// { dg-error "could not convert" }
   pX->DoSomething(1);		// { dg-error "no matching" } 
   // { dg-message "candidate" "candidate note" { target *-*-* } 25 }
   (*pX).DoSomething(1);		// { dg-error "no matching" } 
diff --git a/gcc/testsuite/g++.old-deja/g++.law/arg11.C b/gcc/testsuite/g++.old-deja/g++.law/arg11.C
index fc590c4..fc93579 100644
--- a/gcc/testsuite/g++.old-deja/g++.law/arg11.C
+++ b/gcc/testsuite/g++.old-deja/g++.law/arg11.C
@@ -16,7 +16,7 @@  void function(Ack);
 int
 foo(S *o)
 { // Neither call has a usable constructor for conversions of char[5] to Ack.
-  function("adsf");// { dg-error "conversion" } 
+  function("adsf");// { dg-error "could not convert" }
   o->method("adsf");// { dg-error "no matching" } 
   // { dg-message "candidate" "candidate note" { target *-*-* } 20 }
   return 0;