addressed jason comments.
no regression due to this, tested on x86_64 linux.

On Tue, Oct 27, 2020 at 11:09 PM Jason Merrill <ja...@redhat.com> wrote:
>
> On 10/22/20 1:31 PM, kamlesh kumar wrote:
> > Attaching the patch file.
> >
> >  >>Instead of building a hash table, would it work to handle ambiguity by
> >  >>checking whether one of the classes is a base of the other?
>
> > Fixing for cases like: struct B: A<int>,A<int,int> may not be cleaner
> > this way.
>
> Why not?  Your patch does extra work even when there's no ambiguity.
>
> > On Thu, Oct 22, 2020 at 3:23 AM Jason Merrill <ja...@redhat.com
> > <mailto:ja...@redhat.com>> wrote:
> >  >
> >  > On 10/21/20 6:32 AM, kamlesh kumar wrote:
> >  > > gcc/cp/ChangeLog
> >  > > -----------------------------------
> >  > >
> >  > > 2020-10-21  Kamlesh Kumar  <kamleshbha...@gmail.com
> > <mailto:kamleshbha...@gmail.com>>
> >  > >
> >  > > PR c++/97453
> >  > > * pt.c (get_template_base): Implement DR2303,
> >  > > Consider closest base while template
> >  > > deduction when base of base also matches.
> >  > >
> >  > > gcc/testsuite/ChangeLog
> >  > > ------------------------------------------
> >  > >
> >  > > 2020-10-21  Kamlesh Kumar  <kamleshbha...@gmail.com
> > <mailto:kamleshbha...@gmail.com>>
> >  > >
> >  > > * g++.dg/Drs/dr2303.C: New Test
> >  > >
> >  > > --------------------------------------------------
> >  > >
> >  > > As part of this patch I Implemented fix for below defect report in cwg
> >  > > https://wg21.cmeerw.net/cwg/issue2303 .
> >  >
> >  > Thanks!
> >  >
> >  > Please see https://gcc.gnu.org/contribute.html for guidance on email
> >  > subject lines; for this patch I'd think something like
> >  >
> >  > [PATCH] c++: Implement DR2303 [PR97453]
> >  >
> >  > Also, your patch was corrupted by word wrap; the easiest way to avoid
> >  > that is probably to attach the file rather than copy it into the message.
> >  >
> >  > > Reg tested on x86_64 and did not found any failure.
> >  > > Patch summary: Remove base of base from list of bases
> >  > >
> >  > > created a hash_set from list of bases and then iterate over each
> >  > > element of hash_set and find its  list of bases and remove this from
> >  > > hash_set if present.
> >  > > and finally, deduction succeeds if in hash_set remains only single
> >  > > element or it's empty.
> >  > > otherwise deduction is ambiguous.
> >  >
> >  > Instead of building a hash table, would it work to handle ambiguity by
> >  > checking whether one of the classes is a base of the other?
> >  >
> >  > > -------------------------------------------------------
> >  > > diff --git a/gcc/cp/pt.c b/gcc/cp/pt.c
> >  > > index dc664ec3798..7adf461e108 100644
> >  > > --- a/gcc/cp/pt.c
> >  > > +++ b/gcc/cp/pt.c
> >  > > @@ -22643,8 +22643,9 @@ static enum template_base_result
> >  > >   get_template_base (tree tparms, tree targs, tree parm, tree arg,
> >  > >       bool explain_p, tree *result)
> >  > >   {
> >  > > -  tree rval = NULL_TREE;
> >  > > +  *result = NULL_TREE;
> >  > >     tree binfo;
> >  > > +  hash_set<tree> binfo_set;
> >  > >
> >  > >     gcc_assert (RECORD_OR_UNION_CODE_P (TREE_CODE (arg)));
> >  > >
> >  > > @@ -22659,31 +22660,51 @@ get_template_base (tree tparms, tree targs,
> >  > > tree parm, tree arg,
> >  > >     /* Walk in inheritance graph order.  The search order is not
> >  > >        important, and this avoids multiple walks of virtual bases.  */
> >  > >     for (binfo = TREE_CHAIN (binfo); binfo; binfo = TREE_CHAIN (binfo))
> >  > > -    {
> >  > > -      tree r = try_class_unification (tparms, targs, parm,
> >  > > -       BINFO_TYPE (binfo), explain_p);
> >  > > -
> >  > > -      if (r)
> >  > > - {
> >  > > -   /* If there is more than one satisfactory baseclass, then:
> >  > > -
> >  > > -        [temp.deduct.call]
> >  > > +     {
> >  > > +       tree r = try_class_unification (tparms, targs, parm,
> >  > > +                       BINFO_TYPE (binfo), explain_p);
> >  > > +       if (r)
> >  > > +         {
> >  > > +           binfo_set.add(r);
> >  > > +         }
> >  > > +     }
> >  > >
> >  > > -       If they yield more than one possible deduced A, the type
> >  > > -       deduction fails.
> >  > > +  /* If there is more than one satisfactory baseclass, then:
> >  > > +     [temp.deduct.call]
> >  > > +          If they yield more than one possible deduced A, the type
> >  > > +          deduction fails.
> >  > > +     However, if there is a class C that is a (direct or indirect)
> >  > > base class of
> >  > > +     D and derived (directly or indirectly) from a class B and
> > that would be a
> >  > > +     valid deduced A, the deduced A cannot be B or pointer to B,
> >  > > respectively.  */
> >  > > +  for (hash_set<tree>::iterator it = binfo_set.begin();
> >  > > +                                it != binfo_set.end(); ++it)
> >  > > +    {
> >  > > +      binfo = TYPE_BINFO (*it);
> >  > > +      for (binfo = TREE_CHAIN (binfo); binfo; binfo = TREE_CHAIN
> > (binfo))
> >  > > +        {
> >  > > +          tree r = try_class_unification (tparms, targs, parm,
> >  > > +                          BINFO_TYPE (binfo), explain_p);
> >  > > +          if (r && binfo_set.contains(r))
> >  > > +            {
> >  > > +              binfo_set.remove(r);
> >  > > +            }
> >  > > +        }
> >  > > +    }
> >  > >
> >  > > -      applies.  */
> >  > > -   if (rval && !same_type_p (r, rval))
> >  > > -     {
> >  > > -       *result = NULL_TREE;
> >  > > -       return tbr_ambiguous_baseclass;
> >  > > -     }
> >  > > +  if (binfo_set.elements() > 1)
> >  > > +    {
> >  > > +      return tbr_ambiguous_baseclass;
> >  > > +    }
> >  > >
> >  > > -   rval = r;
> >  > > - }
> >  > > +  if (binfo_set.is_empty())
> >  > > +    {
> >  > > +      return tbr_success;
> >  > >       }
> >  > >
> >  > > -  *result = rval;
> >  > > +  if (binfo_set.elements() == 1)
> >  > > +    {
> >  > > +      *result = *binfo_set.begin();
> >  > > +    }
> >  > >     return tbr_success;
> >  > >   }
> >  > >
> >  > > diff --git a/gcc/testsuite/g++.dg/DRs/dr2303.C
> >  > > b/gcc/testsuite/g++.dg/DRs/dr2303.C
> >  > > new file mode 100644
> >  > > index 00000000000..b4c23332358
> >  > > --- /dev/null
> >  > > +++ b/gcc/testsuite/g++.dg/DRs/dr2303.C
> >  > > @@ -0,0 +1,20 @@
> >  > > +// DR 2303
> >  > > +// PR c++/97453
> >  > > +// { dg-do compile { target c++11 } }
> >  > > +
> >  > > +template <typename... T>
> >  > > +struct A;
> >  > > +template <>
> >  > > +struct A<> {};
> >  > > +template <typename T, typename... Ts>
> >  > > +struct A<T, Ts...> : A<Ts...> {};
> >  > > +struct B : A<int, int> {};
> >  > > +
> >  > > +template <typename... T>
> >  > > +void f(const A<T...> &) {
> >  > > +  static_assert(sizeof...(T) == 2, "it should duduce to A<int,int>");
> >  > > +}
> >  > > +
> >  > > +void g() {
> >  > > +  f(B{});
> >  > > +}
> >  > > --------------------------------
> >  > >
> >  > > ./kamlesh
> >  > >
> >  >
>
diff --git a/gcc/cp/pt.c b/gcc/cp/pt.c
index aa162d2a4f9..c8ce506ae61 100644
--- a/gcc/cp/pt.c
+++ b/gcc/cp/pt.c
@@ -22663,6 +22663,8 @@ get_template_base (tree tparms, tree targs, tree parm, tree arg,
 {
   tree rval = NULL_TREE;
   tree binfo;
+  hash_set<tree> bases;
+  bool ambigious_p = false;
 
   gcc_assert (RECORD_OR_UNION_CODE_P (TREE_CODE (arg)));
 
@@ -22683,24 +22685,50 @@ get_template_base (tree tparms, tree targs, tree parm, tree arg,
 
       if (r)
 	{
-	  /* If there is more than one satisfactory baseclass, then:
-
-	       [temp.deduct.call]
-
-	      If they yield more than one possible deduced A, the type
-	      deduction fails.
-
-	     applies.  */
 	  if (rval && !same_type_p (r, rval))
 	    {
-	      *result = NULL_TREE;
-	      return tbr_ambiguous_baseclass;
+	      ambigious_p = true;
+	      break;
 	    }
 
 	  rval = r;
 	}
     }
 
+  /* [temp.deduct.call] :
+     If there is a class C that is a (direct or indirect) base class of D and
+     derived (directly or indirectly) from a class B and that would be a valid
+     deduced A, the deduced A cannot be B or pointer to B, respectively. */
+  if (ambigious_p)
+    {
+      binfo = TYPE_BINFO (complete_type (arg));
+      for (binfo = TREE_CHAIN (binfo); binfo; binfo = TREE_CHAIN (binfo))
+	{
+	  tree r = try_class_unification (tparms, targs, parm,
+					  BINFO_TYPE (binfo), explain_p);
+	  bases.add (r);
+	}
+
+      for (hash_set<tree>::iterator it = bases.begin (); it != bases.end ();
+	   ++it)
+	{
+	  binfo = TYPE_BINFO (*it);
+	  for (binfo = TREE_CHAIN (binfo); binfo; binfo = TREE_CHAIN (binfo))
+	    {
+	      tree r = try_class_unification (tparms, targs, parm,
+					      BINFO_TYPE (binfo), explain_p);
+	      if (bases.contains (r))
+		bases.remove (r);
+	    }
+	}
+
+      if (bases.elements () > 1)
+	{
+	  *result = NULL_TREE;
+	  return tbr_ambiguous_baseclass;
+	}
+      rval = *bases.begin ();
+    }
   *result = rval;
   return tbr_success;
 }
diff --git a/gcc/testsuite/g++.dg/DRs/dr2303.C b/gcc/testsuite/g++.dg/DRs/dr2303.C
new file mode 100644
index 00000000000..b6acb6e2197
--- /dev/null
+++ b/gcc/testsuite/g++.dg/DRs/dr2303.C
@@ -0,0 +1,37 @@
+// DR 2303
+// PR c++/97453
+// { dg-do compile { target c++11 } }
+
+template <typename... T> struct A;
+template <> struct A<>
+{
+};
+template <typename T, typename... Ts> struct A<T, Ts...> : A<Ts...>
+{
+};
+struct B : A<int, int>
+{
+};
+
+struct C : A<int, int>, A<int> //  { dg-warning "direct base .A<int>. inaccessible in .C. due to ambiguity" }
+{
+};
+
+struct D : A<int>, A<int, int> //  { dg-warning "direct base .A<int>. inaccessible in .D. due to ambiguity" }
+{
+};
+template <typename... T>
+void
+f (const A<T...> &)
+{
+  static_assert (sizeof...(T) == 2, "it should duduce to A<int,int>");
+}
+
+
+void
+g ()
+{
+  f (B{});
+  f (C{});
+  f (D{});
+}

Reply via email to