Re: [PATCH] c++: Implement DR2303 [PR97453]
On 11/3/20 3:11 AM, kamlesh kumar wrote: Here is the copyright assignment under which i will be contributing. Ah, great. I've now committed the patch. On Mon, Nov 2, 2020 at 10:50 PM kamlesh kumar wrote: Do you see a reason this wouldn't work? No, I do not see any.This is good. so it's probably simplest to go ahead with mine. Yes, thank you. On Mon, Nov 2, 2020 at 9:48 PM Jason Merrill wrote: On 11/2/20 10:10 AM, kamlesh kumar wrote: addressed jason comments. no regression due to this, tested on x86_64 linux. On Tue, Oct 27, 2020 at 11:09 PM Jason Merrill 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,A 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 mailto:ja...@redhat.com>> wrote: > > On 10/21/20 6:32 AM, kamlesh kumar wrote: > > gcc/cp/ChangeLog > > --- > > > > 2020-10-21 Kamlesh Kumar 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 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? This is what I had in mind; it seems clearer to me. Do you see a reason this wouldn't work? Also, I notice that you still don't seem to have a copyright assignment on file with the FSF. I and Jonathan Wakely both asked about it last year; has there been any progress on that? Your patch is too large to go in without a copyright assignment, so it's probably simplest to go ahead with mine. Thanks, Jason
Re: [PATCH] c++: Implement DR2303 [PR97453]
>>Do you see a reason this wouldn't work? No, I do not see any.This is good. >>so it's probably simplest to go ahead with mine. Yes, thank you. On Mon, Nov 2, 2020 at 9:48 PM Jason Merrill wrote: > > On 11/2/20 10:10 AM, kamlesh kumar wrote: > > addressed jason comments. > > no regression due to this, tested on x86_64 linux. > > > > On Tue, Oct 27, 2020 at 11:09 PM Jason Merrill 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,A 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 >>> <mailto:ja...@redhat.com>> wrote: > >>> > > >>> > On 10/21/20 6:32 AM, kamlesh kumar wrote: > >>> > > gcc/cp/ChangeLog > >>> > > --- > >>> > > > >>> > > 2020-10-21 Kamlesh Kumar >>> <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 >>> <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? > > This is what I had in mind; it seems clearer to me. Do you see a reason > this wouldn't work? > > Also, I notice that you still don't seem to have a copyright assignment > on file with the FSF. I and Jonathan Wakely both asked about it last > year; has there been any progress on that? Your patch is too large to > go in without a copyright assignment, so it's probably simplest to go > ahead with mine. > > Thanks, > Jason
Re: [PATCH] c++: Implement DR2303 [PR97453]
On 11/2/20 10:10 AM, kamlesh kumar wrote:
addressed jason comments.
no regression due to this, tested on x86_64 linux.
On Tue, Oct 27, 2020 at 11:09 PM Jason Merrill 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,A 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 mailto:ja...@redhat.com>> wrote:
>
> On 10/21/20 6:32 AM, kamlesh kumar wrote:
> > gcc/cp/ChangeLog
> > ---
> >
> > 2020-10-21 Kamlesh Kumar 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 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?
This is what I had in mind; it seems clearer to me. Do you see a reason
this wouldn't work?
Also, I notice that you still don't seem to have a copyright assignment
on file with the FSF. I and Jonathan Wakely both asked about it last
year; has there been any progress on that? Your patch is too large to
go in without a copyright assignment, so it's probably simplest to go
ahead with mine.
Thanks,
Jason
commit 6b0fc692d57f38ec69ea739117ac3d4552cd0d23
Author: kamlesh kumar
Date: Mon Nov 2 20:40:21 2020 +0530
c++: Implement DR2303 [PR97453]
gcc/cp/ChangeLog
2020-10-21 Kamlesh Kumar
Jason Merrill
PR c++/97453
DR2303
* 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
g++.dg/DRs/dr2303.C: New test.
diff --git a/gcc/cp/pt.c b/gcc/cp/pt.c
index f31a1a70473..245b7a83a92 100644
--- a/gcc/cp/pt.c
+++ b/gcc/cp/pt.c
@@ -22693,8 +22693,20 @@ get_template_base (tree tparms, tree targs, tree parm, tree arg,
applies. */
if (rval && !same_type_p (r, rval))
{
- *result = NULL_TREE;
- return tbr_ambiguous_baseclass;
+ /* [temp.deduct.call]/4.3: 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 (DERIVED_FROM_P (r, rval))
+ /* Ignore r. */
+ continue;
+ else if (DERIVED_FROM_P (rval, r))
+ /* Discard rval. */;
+ else
+ {
+ *result = NULL_TREE;
+ return tbr_ambiguous_baseclass;
+ }
}
rval = r;
diff --git a/gcc/testsuite/g++.dg/DRs/dr2303.C b/gcc/testsuite/g++.dg/DRs/dr2303.C
new file mode 100644
index 000..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 struct A;
+template <> struct A<>
+{
+};
+template struct A : A
+{
+};
+struct B : A
+{
+};
+
+struct C : A, A // { dg-warning "direct base .A. inaccessible in .C. due to ambiguity" }
+{
+};
+
+struct D : A, A // { dg-warning "direct base .A. inaccessible in .D. due to ambiguity" }
+{
+};
+template
+void
+f (const A &)
+{
+ static_assert (sizeof...(T) == 2, "it should duduce to A");
+}
+
+
+void
+g ()
+{
+ f (B{});
+ f (C{});
+ f (D{});
+}
Re: [PATCH] c++: Implement DR2303 [PR97453]
addressed jason comments.
no regression due to this, tested on x86_64 linux.
On Tue, Oct 27, 2020 at 11:09 PM Jason Merrill 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,A 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 > <mailto:ja...@redhat.com>> wrote:
> > >
> > > On 10/21/20 6:32 AM, kamlesh kumar wrote:
> > > > gcc/cp/ChangeLog
> > > > ---
> > > >
> > > > 2020-10-21 Kamlesh Kumar > <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 > <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 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)
> > > > + {
> > > >
Re: [PATCH] c++: Implement DR2303 [PR97453]
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,A 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 <mailto:ja...@redhat.com>> wrote:
>
> On 10/21/20 6:32 AM, kamlesh kumar wrote:
> > gcc/cp/ChangeLog
> > ---
> >
> > 2020-10-21 Kamlesh Kumar <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 <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 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::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
[PATCH] c++: Implement DR2303 [PR97453]
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,A may not be cleaner this
way.
On Thu, Oct 22, 2020 at 3:23 AM Jason Merrill wrote:
>
> On 10/21/20 6:32 AM, kamlesh kumar wrote:
> > gcc/cp/ChangeLog
> > ---
> >
> > 2020-10-21 Kamlesh Kumar
> >
> > 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
> >
> > * 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 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::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 &a
