* [PATCH] c++: Implement DR2303 [PR97453] @ 2020-10-22 17:31 kamlesh kumar 2020-10-27 17:38 ` Jason Merrill 0 siblings, 1 reply; 7+ messages in thread From: kamlesh kumar @ 2020-10-22 17:31 UTC (permalink / raw) To: Jason Merrill; +Cc: gcc-patches List, Marek Polacek [-- Attachment #1: Type: text/plain, Size: 5709 bytes --] 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. On Thu, Oct 22, 2020 at 3:23 AM Jason Merrill <jason@redhat.com> wrote: > > On 10/21/20 6:32 AM, kamlesh kumar wrote: > > gcc/cp/ChangeLog > > ----------------------------------- > > > > 2020-10-21 Kamlesh Kumar <kamleshbhalui@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 <kamleshbhalui@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 > > > [-- Attachment #2: pr97453.patch --] [-- Type: application/x-patch, Size: 3140 bytes --] ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] c++: Implement DR2303 [PR97453] 2020-10-22 17:31 [PATCH] c++: Implement DR2303 [PR97453] kamlesh kumar @ 2020-10-27 17:38 ` Jason Merrill 2020-11-02 15:10 ` kamlesh kumar 0 siblings, 1 reply; 7+ messages in thread From: Jason Merrill @ 2020-10-27 17:38 UTC (permalink / raw) To: kamlesh kumar; +Cc: gcc-patches List, Marek Polacek 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 <jason@redhat.com > <mailto:jason@redhat.com>> wrote: > > > > On 10/21/20 6:32 AM, kamlesh kumar wrote: > > > gcc/cp/ChangeLog > > > ----------------------------------- > > > > > > 2020-10-21 Kamlesh Kumar <kamleshbhalui@gmail.com > <mailto:kamleshbhalui@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 <kamleshbhalui@gmail.com > <mailto:kamleshbhalui@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 > > > > > ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] c++: Implement DR2303 [PR97453] 2020-10-27 17:38 ` Jason Merrill @ 2020-11-02 15:10 ` kamlesh kumar 2020-11-02 16:18 ` Jason Merrill 0 siblings, 1 reply; 7+ messages in thread From: kamlesh kumar @ 2020-11-02 15:10 UTC (permalink / raw) To: Jason Merrill; +Cc: gcc-patches List, Marek Polacek [-- Attachment #1: Type: text/plain, Size: 6975 bytes --] addressed jason comments. no regression due to this, tested on x86_64 linux. On Tue, Oct 27, 2020 at 11:09 PM Jason Merrill <jason@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 <jason@redhat.com > > <mailto:jason@redhat.com>> wrote: > > > > > > On 10/21/20 6:32 AM, kamlesh kumar wrote: > > > > gcc/cp/ChangeLog > > > > ----------------------------------- > > > > > > > > 2020-10-21 Kamlesh Kumar <kamleshbhalui@gmail.com > > <mailto:kamleshbhalui@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 <kamleshbhalui@gmail.com > > <mailto:kamleshbhalui@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 > > > > > > > > [-- Attachment #2: dr2303.patch --] [-- Type: text/x-patch, Size: 2857 bytes --] 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{}); +} ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] c++: Implement DR2303 [PR97453] 2020-11-02 15:10 ` kamlesh kumar @ 2020-11-02 16:18 ` Jason Merrill 2020-11-02 17:20 ` kamlesh kumar 0 siblings, 1 reply; 7+ messages in thread From: Jason Merrill @ 2020-11-02 16:18 UTC (permalink / raw) To: kamlesh kumar; +Cc: gcc-patches List, Marek Polacek [-- Attachment #1: Type: text/plain, Size: 3054 bytes --] 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 <jason@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 <jason@redhat.com >>> <mailto:jason@redhat.com>> wrote: >>> > >>> > On 10/21/20 6:32 AM, kamlesh kumar wrote: >>> > > gcc/cp/ChangeLog >>> > > ----------------------------------- >>> > > >>> > > 2020-10-21 Kamlesh Kumar <kamleshbhalui@gmail.com >>> <mailto:kamleshbhalui@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 <kamleshbhalui@gmail.com >>> <mailto:kamleshbhalui@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 [-- Attachment #2: dr2303.diff --] [-- Type: text/x-patch, Size: 2396 bytes --] commit 6b0fc692d57f38ec69ea739117ac3d4552cd0d23 Author: kamlesh kumar <kamleshbhalui@gmail.com> Date: Mon Nov 2 20:40:21 2020 +0530 c++: Implement DR2303 [PR97453] gcc/cp/ChangeLog 2020-10-21 Kamlesh Kumar <kamleshbhalui@gmail.com> Jason Merrill <jason@redhat.com> 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 <kamleshbhalui@gmail.com> 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 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{}); +} ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] c++: Implement DR2303 [PR97453] 2020-11-02 16:18 ` Jason Merrill @ 2020-11-02 17:20 ` kamlesh kumar 2020-11-03 8:11 ` kamlesh kumar 0 siblings, 1 reply; 7+ messages in thread From: kamlesh kumar @ 2020-11-02 17:20 UTC (permalink / raw) To: Jason Merrill; +Cc: gcc-patches List, Marek Polacek >>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 <jason@redhat.com> 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 <jason@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 <jason@redhat.com > >>> <mailto:jason@redhat.com>> wrote: > >>> > > >>> > On 10/21/20 6:32 AM, kamlesh kumar wrote: > >>> > > gcc/cp/ChangeLog > >>> > > ----------------------------------- > >>> > > > >>> > > 2020-10-21 Kamlesh Kumar <kamleshbhalui@gmail.com > >>> <mailto:kamleshbhalui@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 <kamleshbhalui@gmail.com > >>> <mailto:kamleshbhalui@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 ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] c++: Implement DR2303 [PR97453] 2020-11-02 17:20 ` kamlesh kumar @ 2020-11-03 8:11 ` kamlesh kumar 2020-11-03 19:40 ` Jason Merrill 0 siblings, 1 reply; 7+ messages in thread From: kamlesh kumar @ 2020-11-03 8:11 UTC (permalink / raw) To: Jason Merrill; +Cc: gcc-patches List, Marek Polacek [-- Attachment #1: Type: text/plain, Size: 3743 bytes --] Here is the copyright assignment under which i will be contributing. On Mon, Nov 2, 2020 at 10:50 PM kamlesh kumar <kamleshbhalui@gmail.com> 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 <jason@redhat.com> 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 <jason@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 <jason@redhat.com > > >>> <mailto:jason@redhat.com>> wrote: > > >>> > > > >>> > On 10/21/20 6:32 AM, kamlesh kumar wrote: > > >>> > > gcc/cp/ChangeLog > > >>> > > ----------------------------------- > > >>> > > > > >>> > > 2020-10-21 Kamlesh Kumar <kamleshbhalui@gmail.com > > >>> <mailto:kamleshbhalui@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 <kamleshbhalui@gmail.com > > >>> <mailto:kamleshbhalui@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 [-- Attachment #2: Wind.River.Systems.1368315.GNU.tar.gz --] [-- Type: application/gzip, Size: 216715 bytes --] ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] c++: Implement DR2303 [PR97453] 2020-11-03 8:11 ` kamlesh kumar @ 2020-11-03 19:40 ` Jason Merrill 0 siblings, 0 replies; 7+ messages in thread From: Jason Merrill @ 2020-11-03 19:40 UTC (permalink / raw) To: kamlesh kumar; +Cc: gcc-patches List, Marek Polacek 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 <kamleshbhalui@gmail.com> 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 <jason@redhat.com> 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 <jason@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 <jason@redhat.com >>>>>> <mailto:jason@redhat.com>> wrote: >>>>>> > >>>>>> > On 10/21/20 6:32 AM, kamlesh kumar wrote: >>>>>> > > gcc/cp/ChangeLog >>>>>> > > ----------------------------------- >>>>>> > > >>>>>> > > 2020-10-21 Kamlesh Kumar <kamleshbhalui@gmail.com >>>>>> <mailto:kamleshbhalui@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 <kamleshbhalui@gmail.com >>>>>> <mailto:kamleshbhalui@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 ^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2020-11-03 19:40 UTC | newest] Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2020-10-22 17:31 [PATCH] c++: Implement DR2303 [PR97453] kamlesh kumar 2020-10-27 17:38 ` Jason Merrill 2020-11-02 15:10 ` kamlesh kumar 2020-11-02 16:18 ` Jason Merrill 2020-11-02 17:20 ` kamlesh kumar 2020-11-03 8:11 ` kamlesh kumar 2020-11-03 19:40 ` Jason Merrill
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).