Message ID | 20131212133611.GN892@tucnak.redhat.com |
---|---|
State | New |
Headers | show |
On Thu, Dec 12, 2013 at 5:36 AM, Jakub Jelinek <jakub@redhat.com> wrote: > On Wed, Dec 11, 2013 at 11:51:55AM -0500, Jason Merrill wrote: >> It's only safe to free the targs if they weren't used to instantiate >> any templates, so I lean toward option #1. Did you test this with >> strict gc? > > Ok, after IRC discussion and another bootstrap/regtest I've installed > this variant instead: > > 2013-12-12 Jakub Jelinek <jakub@redhat.com> > > PR c++/58627 > * call.c (add_template_candidate_real): Don't call ggc_free on targs. > > --- gcc/cp/class.c.jj 2013-11-28 08:18:58.000000000 +0100 > +++ gcc/cp/class.c 2013-12-11 20:57:40.155059669 +0100 > @@ -7475,8 +7475,6 @@ resolve_address_of_overloaded_function ( > /* See if there's a match. */ > if (same_type_p (target_fn_type, static_fn_type (instantiation))) > matches = tree_cons (instantiation, fn, matches); > - > - ggc_free (targs); > } > > /* Now, remove all but the most specialized of the matches. */ > Has this been checked in? I still see random: FAIL: 17_intro/headers/c++200x/stdc++.cc (test for excess errors) FAIL: 17_intro/headers/c++200x/stdc++_multiple_inclusion.cc (test for excess errors) http://gcc.gnu.org/ml/gcc-testresults/2013-12/msg01230.html
On Thu, Dec 12, 2013 at 5:36 AM, Jakub Jelinek <jakub@redhat.com> wrote: > On Wed, Dec 11, 2013 at 11:51:55AM -0500, Jason Merrill wrote: >> It's only safe to free the targs if they weren't used to instantiate >> any templates, so I lean toward option #1. Did you test this with >> strict gc? > > Ok, after IRC discussion and another bootstrap/regtest I've installed > this variant instead: > > 2013-12-12 Jakub Jelinek <jakub@redhat.com> > > PR c++/58627 > * call.c (add_template_candidate_real): Don't call ggc_free on targs. ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Hi Jakub, This checkin doesn't match its ChangeLog entry and it doesn't fix FAIL: 17_intro/headers/c++200x/stdc++.cc (test for excess errors) FAIL: 17_intro/headers/c++200x/stdc++_multiple_inclusion.cc (test for excess errors) Is this an oversight? > > --- gcc/cp/class.c.jj 2013-11-28 08:18:58.000000000 +0100 > +++ gcc/cp/class.c 2013-12-11 20:57:40.155059669 +0100 > @@ -7475,8 +7475,6 @@ resolve_address_of_overloaded_function ( > /* See if there's a match. */ > if (same_type_p (target_fn_type, static_fn_type (instantiation))) > matches = tree_cons (instantiation, fn, matches); > - > - ggc_free (targs); > } > > /* Now, remove all but the most specialized of the matches. */ > > > Jakub
On Mon, Dec 16, 2013 at 09:51:38AM -0800, H.J. Lu wrote: > On Thu, Dec 12, 2013 at 5:36 AM, Jakub Jelinek <jakub@redhat.com> wrote: > > On Wed, Dec 11, 2013 at 11:51:55AM -0500, Jason Merrill wrote: > >> It's only safe to free the targs if they weren't used to instantiate > >> any templates, so I lean toward option #1. Did you test this with > >> strict gc? > > > > Ok, after IRC discussion and another bootstrap/regtest I've installed > > this variant instead: > > > > 2013-12-12 Jakub Jelinek <jakub@redhat.com> > > > > PR c++/58627 > > * call.c (add_template_candidate_real): Don't call ggc_free on targs. > ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ > > This checkin doesn't match its ChangeLog entry and it doesn't fix I have fixed up the function name in the ChangeLog entry. As for the stdc++.cc failure, I've commented in the PR, it is PCH related, the relation to PR58627 was just a guess from me and proved to be wrong guess. > FAIL: 17_intro/headers/c++200x/stdc++.cc (test for excess errors) > FAIL: 17_intro/headers/c++200x/stdc++_multiple_inclusion.cc (test for > excess errors) Jakub
On Mon, Dec 16, 2013 at 9:56 AM, Jakub Jelinek <jakub@redhat.com> wrote: > On Mon, Dec 16, 2013 at 09:51:38AM -0800, H.J. Lu wrote: >> On Thu, Dec 12, 2013 at 5:36 AM, Jakub Jelinek <jakub@redhat.com> wrote: >> > On Wed, Dec 11, 2013 at 11:51:55AM -0500, Jason Merrill wrote: >> >> It's only safe to free the targs if they weren't used to instantiate >> >> any templates, so I lean toward option #1. Did you test this with >> >> strict gc? >> > >> > Ok, after IRC discussion and another bootstrap/regtest I've installed >> > this variant instead: >> > >> > 2013-12-12 Jakub Jelinek <jakub@redhat.com> >> > >> > PR c++/58627 >> > * call.c (add_template_candidate_real): Don't call ggc_free on targs. >> ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ >> >> This checkin doesn't match its ChangeLog entry and it doesn't fix > > I have fixed up the function name in the ChangeLog entry. > > As for the stdc++.cc failure, I've commented in the PR, it is PCH related, The filename is class.c, not call.c.
--- gcc/cp/class.c.jj 2013-11-28 08:18:58.000000000 +0100 +++ gcc/cp/class.c 2013-12-11 20:57:40.155059669 +0100 @@ -7475,8 +7475,6 @@ resolve_address_of_overloaded_function ( /* See if there's a match. */ if (same_type_p (target_fn_type, static_fn_type (instantiation))) matches = tree_cons (instantiation, fn, matches); - - ggc_free (targs); } /* Now, remove all but the most specialized of the matches. */