Re: C++ PATCH for c++/91889 - follow-up fix for DR 2352

2019-09-28 Thread Jason Merrill
On Sat, Sep 28, 2019 at 7:33 AM Marek Polacek  wrote:
> On Fri, Sep 27, 2019 at 10:08:49PM -0400, Jason Merrill wrote:
> > On 9/26/19 9:45 PM, Marek Polacek wrote:
> > > Jason, you remarked that adding a ck_qual under the ck_ref_bind might be
> > > too much trouble, but it seems it's actually fairly simple.  The comments
> > > hopefully explain my thinking.  Is this what you had in mind?
> >
> > Yes, I'm glad to hear it was simpler than I worried it would be.
> >
> > > +   /* direct_reference_binding might have inserted a ck_qual under
> > > +  this ck_ref_bind for the benefit of conversion sequence ranking.
> > > +  Ignore the conversion; we'll create our own below.  */
> > > +   if (next_conversion (convs)->kind == ck_qual)
> > > + {
> > > +   gcc_assert (same_type_p (TREE_TYPE (expr),
> > > +next_conversion (convs)->type));
> > > +   STRIP_NOPS (expr);
> >
> > Why STRIP_NOPS?
>
> I needed to strip the cast the ck_qual had created so that the call to
> cp_build_addr_expr below won't fail with "lvalue required as the operand of 
> &".
> Perhaps a comment should explain it.

OK with the comment.

Jason


Re: C++ PATCH for c++/91889 - follow-up fix for DR 2352

2019-09-28 Thread Marek Polacek
On Fri, Sep 27, 2019 at 10:08:49PM -0400, Jason Merrill wrote:
> On 9/26/19 9:45 PM, Marek Polacek wrote:
> > Jason, you remarked that adding a ck_qual under the ck_ref_bind might be
> > too much trouble, but it seems it's actually fairly simple.  The comments
> > hopefully explain my thinking.  Is this what you had in mind?
> 
> Yes, I'm glad to hear it was simpler than I worried it would be.
> 
> > +   /* direct_reference_binding might have inserted a ck_qual under
> > +  this ck_ref_bind for the benefit of conversion sequence ranking.
> > +  Ignore the conversion; we'll create our own below.  */
> > +   if (next_conversion (convs)->kind == ck_qual)
> > + {
> > +   gcc_assert (same_type_p (TREE_TYPE (expr),
> > +next_conversion (convs)->type));
> > +   STRIP_NOPS (expr);
> 
> Why STRIP_NOPS?

I needed to strip the cast the ck_qual had created so that the call to
cp_build_addr_expr below won't fail with "lvalue required as the operand of &".
Perhaps a comment should explain it.

Marek


Re: C++ PATCH for c++/91889 - follow-up fix for DR 2352

2019-09-27 Thread Jason Merrill

On 9/26/19 9:45 PM, Marek Polacek wrote:

Jason, you remarked that adding a ck_qual under the ck_ref_bind might be
too much trouble, but it seems it's actually fairly simple.  The comments
hopefully explain my thinking.  Is this what you had in mind?


Yes, I'm glad to hear it was simpler than I worried it would be.


+   /* direct_reference_binding might have inserted a ck_qual under
+  this ck_ref_bind for the benefit of conversion sequence ranking.
+  Ignore the conversion; we'll create our own below.  */
+   if (next_conversion (convs)->kind == ck_qual)
+ {
+   gcc_assert (same_type_p (TREE_TYPE (expr),
+next_conversion (convs)->type));
+   STRIP_NOPS (expr);


Why STRIP_NOPS?

Jason



C++ PATCH for c++/91889 - follow-up fix for DR 2352

2019-09-26 Thread Marek Polacek
It turned out that DR 2352 needs some fixing: as of now, it means that in

  void f(int*); // #1
  void f(const int* const &); // #2
  void g(int* p) { f(p); }

the call to f is ambiguous.  This broke e.g. Boost.  I raised this on the CWG
reflector and Jason suggested wording that would break this up; this patch is
an attempt to implement that suggestion.

Jason, you remarked that adding a ck_qual under the ck_ref_bind might be
too much trouble, but it seems it's actually fairly simple.  The comments
hopefully explain my thinking.  Is this what you had in mind?

ref-bind6.C is something reduced from libstdc++ I came across when testing
this patch.

Bootstrapped/regtested on x86_64-linux, ok for trunk?

2019-09-26  Marek Polacek  

PR c++/91889 - follow-up fix for DR 2352.
* call.c (involves_qualification_conversion_p): New function.
(direct_reference_binding): Build a ck_qual if the conversion
would involve a qualification conversion.
(convert_like_real): Strip the conversion created by the ck_qual
in direct_reference_binding.

* g++.dg/cpp0x/ref-bind3.C: Add dg-error.
* g++.dg/cpp0x/ref-bind4.C: New test.
* g++.dg/cpp0x/ref-bind5.C: New test.
* g++.dg/cpp0x/ref-bind6.C: New test.
* g++.old-deja/g++.pt/spec35.C: Revert earlier change.

diff --git gcc/cp/call.c gcc/cp/call.c
index 45b984ecb11..09976369aed 100644
--- gcc/cp/call.c
+++ gcc/cp/call.c
@@ -1555,6 +1555,27 @@ reference_compatible_p (tree t1, tree t2)
   return true;
 }
 
+/* Return true if converting FROM to TO would involve a qualification
+   conversion.  */
+
+static bool
+involves_qualification_conversion_p (tree to, tree from)
+{
+  /* If we're not convering a pointer to another one, we won't get
+ a qualification conversion.  */
+  if (!((TYPE_PTR_P (to) && TYPE_PTR_P (from))
+   || (TYPE_PTRDATAMEM_P (to) && TYPE_PTRDATAMEM_P (from
+return false;
+
+  conversion *conv = standard_conversion (to, from, NULL_TREE,
+ /*c_cast_p=*/false, 0, tf_none);
+  for (conversion *t = conv; t; t = next_conversion (t))
+if (t->kind == ck_qual)
+  return true;
+
+  return false;
+}
+
 /* A reference of the indicated TYPE is being bound directly to the
expression represented by the implicit conversion sequence CONV.
Return a conversion sequence for this binding.  */
@@ -1598,6 +1619,19 @@ direct_reference_binding (tree type, conversion *conv)
 That way, convert_like knows not to generate a temporary.  */
   conv->need_temporary_p = false;
 }
+  else if (involves_qualification_conversion_p (t, conv->type))
+/* Represent the qualification conversion.  After DR 2352
+   #1 and #2 were indistinguishable conversion sequences:
+
+void f(int*); // #1
+void f(const int* const &); // #2
+void g(int* p) { f(p); }
+
+   because the types "int *" and "const int *const" are
+   reference-related and we were binding both directly and they
+   had the same rank.  To break it up, we add a ck_qual under the
+   ck_ref_bind so that conversion sequence ranking chooses #1.  */
+conv = build_conv (ck_qual, t, conv);
 
   return build_conv (ck_ref_bind, type, conv);
 }
@@ -7342,6 +7376,16 @@ convert_like_real (conversion *convs, tree expr, tree 
fn, int argnum,
   {
tree ref_type = totype;
 
+   /* direct_reference_binding might have inserted a ck_qual under
+  this ck_ref_bind for the benefit of conversion sequence ranking.
+  Ignore the conversion; we'll create our own below.  */
+   if (next_conversion (convs)->kind == ck_qual)
+ {
+   gcc_assert (same_type_p (TREE_TYPE (expr),
+next_conversion (convs)->type));
+   STRIP_NOPS (expr);
+ }
+
if (convs->bad_p && !next_conversion (convs)->bad_p)
  {
tree extype = TREE_TYPE (expr);
diff --git gcc/testsuite/g++.dg/cpp0x/ref-bind3.C 
gcc/testsuite/g++.dg/cpp0x/ref-bind3.C
index 16e1bfe6ccc..b2c85ec684a 100644
--- gcc/testsuite/g++.dg/cpp0x/ref-bind3.C
+++ gcc/testsuite/g++.dg/cpp0x/ref-bind3.C
@@ -1,22 +1,18 @@
 // PR c++/91844 - Implement CWG 2352, Similar types and reference binding.
 // { dg-do compile { target c++11 } }
 
-template int f (const T *const &); // (1)
-template int f (T *const &); // (2)
-template int f (T *); // (3)
-
-/* Before CWG 2352, (2) was a better match than (1), but (2) and (3) were
-   equally good, so there was an ambiguity.  (2) was better than (1) because
-   (1) required a qualification conversion whereas (2) didn't.  But with this
-   CWG, (1) no longer requires a qualification conversion, because the types
-   "const int* const" and "int *" are now considered reference-related and we
-   bind directly, and (1) is more specialized than (2).  And (1) is also a
-   better match than (3).  */
+template int f (const T *const &); // 1
+template int f (T