* [C++ Patch/RFC] PR 89900 ("[9 Regression] ICE: Segmentation fault (in check_instantiated_arg)")
@ 2019-04-11 15:21 Paolo Carlini
2019-04-12 10:02 ` Paolo Carlini
2019-04-12 18:46 ` Jason Merrill
0 siblings, 2 replies; 5+ messages in thread
From: Paolo Carlini @ 2019-04-11 15:21 UTC (permalink / raw)
To: gcc-patches; +Cc: Jason Merrill
[-- Attachment #1: Type: text/plain, Size: 1669 bytes --]
Hi,
over the last few days I spent some time on this regression, which at
first seemed just a minor error-recovery issue, but then I noticed that
very slightly tweeking the original testcase uncovered a pretty serious
ICE on valid:
template<typename SX, typename ...XE> void
fk (XE..., int/*SW*/);
void
w9 (void)
{
 fk<int> (0);
}
The regression has to do with the changes committed by Jason for
c++/86932, in particular with the condition in coerce_template_parms:
  if (template_parameter_pack_p (TREE_VALUE (parm))
    && (arg || !(complain & tf_partial))
    && !(arg && ARGUMENT_PACK_P (arg)))
which has the additional (arg || !complain & tf_partial)) false for the
present testcase, thus the null arg is not changed into an empty pack,
thus later instantiate_template calls check_instantiated_args which
finds it still null and crashes. Now, likely some additional analysis is
in order, but for sure there is an important difference between the
testcase which came with c++/86932 and the above: non-type vs type
template parameter pack. It seems to me that the kind of problem fixed
in c++/86932 cannot occur with type packs, because it boils down to a
reference to a previous parm (full disclosure: the comments and logic in
fixed_parameter_pack_p helped me a lot here). Thus I had the idea of
simply restricting the scope of the new condition above by adding an ||
TREE_CODE (TREE_VALUE (parm)) == TYPE_DECL, which definitely leads to a
clean testsuite and a proper behavior on the new testcases, AFAICS. I'm
attaching what I tested on x86_64-linux.
Thanks, Paolo.
//////////////////////
[-- Attachment #2: patch_89900_draft --]
[-- Type: text/plain, Size: 1310 bytes --]
Index: cp/pt.c
===================================================================
--- cp/pt.c (revision 270279)
+++ cp/pt.c (working copy)
@@ -8475,7 +8475,8 @@ coerce_template_parms (tree parms,
arg = NULL_TREE;
if (template_parameter_pack_p (TREE_VALUE (parm))
- && (arg || !(complain & tf_partial))
+ && (arg || !(complain & tf_partial)
+ || TREE_CODE (TREE_VALUE (parm)) == TYPE_DECL)
&& !(arg && ARGUMENT_PACK_P (arg)))
{
/* Some arguments will be placed in the
Index: testsuite/g++.dg/cpp0x/pr89900-1.C
===================================================================
--- testsuite/g++.dg/cpp0x/pr89900-1.C (nonexistent)
+++ testsuite/g++.dg/cpp0x/pr89900-1.C (working copy)
@@ -0,0 +1,10 @@
+// { dg-do compile { target c++11 } }
+
+template<typename SX, typename ...XE> void
+fk (XE..., SW); // { dg-error "12:.SW. has not been declared" }
+
+void
+w9 (void)
+{
+ fk<int> (0);
+}
Index: testsuite/g++.dg/cpp0x/pr89900-2.C
===================================================================
--- testsuite/g++.dg/cpp0x/pr89900-2.C (nonexistent)
+++ testsuite/g++.dg/cpp0x/pr89900-2.C (working copy)
@@ -0,0 +1,10 @@
+// { dg-do compile { target c++11 } }
+
+template<typename SX, typename ...XE> void
+fk (XE..., int);
+
+void
+w9 (void)
+{
+ fk<int> (0);
+}
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [C++ Patch/RFC] PR 89900 ("[9 Regression] ICE: Segmentation fault (in check_instantiated_arg)")
2019-04-11 15:21 [C++ Patch/RFC] PR 89900 ("[9 Regression] ICE: Segmentation fault (in check_instantiated_arg)") Paolo Carlini
@ 2019-04-12 10:02 ` Paolo Carlini
2019-04-12 18:46 ` Jason Merrill
1 sibling, 0 replies; 5+ messages in thread
From: Paolo Carlini @ 2019-04-12 10:02 UTC (permalink / raw)
To: gcc-patches; +Cc: Jason Merrill
.. an additional observation. Over the last couple of days I wondered
if the amended testcase was really valid, given the non-terminal
parameter pack, beyond the evidence that all the compilers I have at
hand accept it. Note anyway, that we - and all the compilers - already
accept a version of the testcase without the explicit argument and
deduce the pack as empty:
template<typename ...XE> void
fk (XE..., int);
void
w9 (void)
{
 fk (0);
}
Thus, it seems to me that at least we have a consistency issue, at some
level. Are we already "inadvertently" implementing P0478R0 or I'm
missing something else?!?
Paolo.
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [C++ Patch/RFC] PR 89900 ("[9 Regression] ICE: Segmentation fault (in check_instantiated_arg)")
2019-04-11 15:21 [C++ Patch/RFC] PR 89900 ("[9 Regression] ICE: Segmentation fault (in check_instantiated_arg)") Paolo Carlini
2019-04-12 10:02 ` Paolo Carlini
@ 2019-04-12 18:46 ` Jason Merrill
2019-04-15 20:36 ` Paolo Carlini
1 sibling, 1 reply; 5+ messages in thread
From: Jason Merrill @ 2019-04-12 18:46 UTC (permalink / raw)
To: Paolo Carlini, gcc-patches
On 4/11/19 11:20 AM, Paolo Carlini wrote:
> Hi,
>
> over the last few days I spent some time on this regression, which at
> first seemed just a minor error-recovery issue, but then I noticed that
> very slightly tweeking the original testcase uncovered a pretty serious
> ICE on valid:
>
> template<typename SX, typename ...XE> void
> fk (XE..., int/*SW*/);
>
> void
> w9 (void)
> {
> Â fk<int> (0);
> }
>
> The regression has to do with the changes committed by Jason for
> c++/86932, in particular with the condition in coerce_template_parms:
>
> Â Â if (template_parameter_pack_p (TREE_VALUE (parm))
> Â Â Â Â && (arg || !(complain & tf_partial))
> Â Â Â Â && !(arg && ARGUMENT_PACK_P (arg)))
>
> which has the additional (arg || !complain & tf_partial)) false for the
> present testcase, thus the null arg is not changed into an empty pack,
> thus later instantiate_template calls check_instantiated_args which
> finds it still null and crashes. Now, likely some additional analysis is
> in order, but for sure there is an important difference between the
> testcase which came with c++/86932 and the above: non-type vs type
> template parameter pack. It seems to me that the kind of problem fixed
> in c++/86932 cannot occur with type packs, because it boils down to a
> reference to a previous parm (full disclosure: the comments and logic in
> fixed_parameter_pack_p helped me a lot here). Thus I had the idea of
> simply restricting the scope of the new condition above by adding an ||
> TREE_CODE (TREE_VALUE (parm)) == TYPE_DECL, which definitely leads to a
> clean testsuite and a proper behavior on the new testcases, AFAICS. I'm
> attaching what I tested on x86_64-linux.
I think the important property here is that it's non-terminal, not that
it's a type pack. We can't deduce anything for a non-terminal pack, so
we should go ahead and make an empty pack.
Jason
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [C++ Patch/RFC] PR 89900 ("[9 Regression] ICE: Segmentation fault (in check_instantiated_arg)")
2019-04-12 18:46 ` Jason Merrill
@ 2019-04-15 20:36 ` Paolo Carlini
2019-04-18 17:45 ` Jason Merrill
0 siblings, 1 reply; 5+ messages in thread
From: Paolo Carlini @ 2019-04-15 20:36 UTC (permalink / raw)
To: Jason Merrill, gcc-patches
[-- Attachment #1: Type: text/plain, Size: 2739 bytes --]
Hi,
On 12/04/19 20:29, Jason Merrill wrote:
> On 4/11/19 11:20 AM, Paolo Carlini wrote:
>> Hi,
>>
>> over the last few days I spent some time on this regression, which at
>> first seemed just a minor error-recovery issue, but then I noticed
>> that very slightly tweeking the original testcase uncovered a pretty
>> serious ICE on valid:
>>
>> template<typename SX, typename ...XE> void
>> fk (XE..., int/*SW*/);
>>
>> void
>> w9 (void)
>> {
>> Â Â fk<int> (0);
>> }
>>
>> The regression has to do with the changes committed by Jason for
>> c++/86932, in particular with the condition in coerce_template_parms:
>>
>> Â Â Â if (template_parameter_pack_p (TREE_VALUE (parm))
>> Â Â Â Â Â && (arg || !(complain & tf_partial))
>> Â Â Â Â Â && !(arg && ARGUMENT_PACK_P (arg)))
>>
>> which has the additional (arg || !complain & tf_partial)) false for
>> the present testcase, thus the null arg is not changed into an empty
>> pack, thus later instantiate_template calls check_instantiated_args
>> which finds it still null and crashes. Now, likely some additional
>> analysis is in order, but for sure there is an important difference
>> between the testcase which came with c++/86932 and the above:
>> non-type vs type template parameter pack. It seems to me that the
>> kind of problem fixed in c++/86932 cannot occur with type packs,
>> because it boils down to a reference to a previous parm (full
>> disclosure: the comments and logic in fixed_parameter_pack_p helped
>> me a lot here). Thus I had the idea of simply restricting the scope
>> of the new condition above by adding an || TREE_CODE (TREE_VALUE
>> (parm)) == TYPE_DECL, which definitely leads to a clean testsuite and
>> a proper behavior on the new testcases, AFAICS. I'm attaching what I
>> tested on x86_64-linux.
>
> I think the important property here is that it's non-terminal, not
> that it's a type pack. We can't deduce anything for a non-terminal
> pack, so we should go ahead and make an empty pack.
I see.
Then what about something bolder, like the below? Instead of fiddling
with the details of coerce_template_parms - how it handles the explicit
arguments - in fn_type_unification we deal with both parameter_pack ==
true and false in the same way when targ == NULL_TREE, thus we set
incomplete. Then, for the new testcases, since incomplete is true, there
is no jump to the deduced label and type_unification_real takes care of
making the empty pack - the same happens already when there are no
explicit arguments. Tested x86_64-linux. I also checked quite a few
other variants of the tests but nothing new, nothing interesting, showed
up...
Thanks, Paolo.
/////////////////////////
[-- Attachment #2: patch_89900_draft_2 --]
[-- Type: text/plain, Size: 2792 bytes --]
Index: cp/pt.c
===================================================================
--- cp/pt.c (revision 270364)
+++ cp/pt.c (working copy)
@@ -20176,21 +20176,17 @@ fn_type_unification (tree fn,
parameter_pack = TEMPLATE_PARM_PARAMETER_PACK (parm);
}
- if (!parameter_pack && targ == NULL_TREE)
+ if (targ == NULL_TREE)
/* No explicit argument for this template parameter. */
incomplete = true;
-
- if (parameter_pack && pack_deducible_p (parm, fn))
+ else if (parameter_pack && pack_deducible_p (parm, fn))
{
/* Mark the argument pack as "incomplete". We could
still deduce more arguments during unification.
We remove this mark in type_unification_real. */
- if (targ)
- {
- ARGUMENT_PACK_INCOMPLETE_P(targ) = 1;
- ARGUMENT_PACK_EXPLICIT_ARGS (targ)
- = ARGUMENT_PACK_ARGS (targ);
- }
+ ARGUMENT_PACK_INCOMPLETE_P(targ) = 1;
+ ARGUMENT_PACK_EXPLICIT_ARGS (targ)
+ = ARGUMENT_PACK_ARGS (targ);
/* We have some incomplete argument packs. */
incomplete = true;
Index: testsuite/g++.dg/cpp0x/pr89900-1.C
===================================================================
--- testsuite/g++.dg/cpp0x/pr89900-1.C (nonexistent)
+++ testsuite/g++.dg/cpp0x/pr89900-1.C (working copy)
@@ -0,0 +1,10 @@
+// { dg-do compile { target c++11 } }
+
+template<typename SX, typename ...XE> void
+fk (XE..., SW); // { dg-error "12:.SW. has not been declared" }
+
+void
+w9 (void)
+{
+ fk<int> (0);
+}
Index: testsuite/g++.dg/cpp0x/pr89900-2.C
===================================================================
--- testsuite/g++.dg/cpp0x/pr89900-2.C (nonexistent)
+++ testsuite/g++.dg/cpp0x/pr89900-2.C (working copy)
@@ -0,0 +1,10 @@
+// { dg-do compile { target c++11 } }
+
+template<typename SX, typename ...XE> void
+fk (XE..., int);
+
+void
+w9 (void)
+{
+ fk<int> (0);
+}
Index: testsuite/g++.dg/cpp0x/pr89900-3.C
===================================================================
--- testsuite/g++.dg/cpp0x/pr89900-3.C (nonexistent)
+++ testsuite/g++.dg/cpp0x/pr89900-3.C (working copy)
@@ -0,0 +1,10 @@
+// { dg-do compile { target c++11 } }
+
+template<typename ...XE> void
+fk (XE..., SW); // { dg-error "12:.SW. has not been declared" }
+
+void
+w9 (void)
+{
+ fk (0);
+}
Index: testsuite/g++.dg/cpp0x/pr89900-4.C
===================================================================
--- testsuite/g++.dg/cpp0x/pr89900-4.C (nonexistent)
+++ testsuite/g++.dg/cpp0x/pr89900-4.C (working copy)
@@ -0,0 +1,10 @@
+// { dg-do compile { target c++11 } }
+
+template<typename ...XE> void
+fk (XE..., int);
+
+void
+w9 (void)
+{
+ fk (0);
+}
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [C++ Patch/RFC] PR 89900 ("[9 Regression] ICE: Segmentation fault (in check_instantiated_arg)")
2019-04-15 20:36 ` Paolo Carlini
@ 2019-04-18 17:45 ` Jason Merrill
0 siblings, 0 replies; 5+ messages in thread
From: Jason Merrill @ 2019-04-18 17:45 UTC (permalink / raw)
To: Paolo Carlini; +Cc: gcc-patches
On Mon, Apr 15, 2019 at 1:08 PM Paolo Carlini <paolo.carlini@oracle.com> wrote:
>
> Hi,
>
> On 12/04/19 20:29, Jason Merrill wrote:
> > On 4/11/19 11:20 AM, Paolo Carlini wrote:
> >> Hi,
> >>
> >> over the last few days I spent some time on this regression, which at
> >> first seemed just a minor error-recovery issue, but then I noticed
> >> that very slightly tweeking the original testcase uncovered a pretty
> >> serious ICE on valid:
> >>
> >> template<typename SX, typename ...XE> void
> >> fk (XE..., int/*SW*/);
> >>
> >> void
> >> w9 (void)
> >> {
> >> fk<int> (0);
> >> }
> >>
> >> The regression has to do with the changes committed by Jason for
> >> c++/86932, in particular with the condition in coerce_template_parms:
> >>
> >> if (template_parameter_pack_p (TREE_VALUE (parm))
> >> && (arg || !(complain & tf_partial))
> >> && !(arg && ARGUMENT_PACK_P (arg)))
> >>
> >> which has the additional (arg || !complain & tf_partial)) false for
> >> the present testcase, thus the null arg is not changed into an empty
> >> pack, thus later instantiate_template calls check_instantiated_args
> >> which finds it still null and crashes. Now, likely some additional
> >> analysis is in order, but for sure there is an important difference
> >> between the testcase which came with c++/86932 and the above:
> >> non-type vs type template parameter pack. It seems to me that the
> >> kind of problem fixed in c++/86932 cannot occur with type packs,
> >> because it boils down to a reference to a previous parm (full
> >> disclosure: the comments and logic in fixed_parameter_pack_p helped
> >> me a lot here). Thus I had the idea of simply restricting the scope
> >> of the new condition above by adding an || TREE_CODE (TREE_VALUE
> >> (parm)) == TYPE_DECL, which definitely leads to a clean testsuite and
> >> a proper behavior on the new testcases, AFAICS. I'm attaching what I
> >> tested on x86_64-linux.
> >
> > I think the important property here is that it's non-terminal, not
> > that it's a type pack. We can't deduce anything for a non-terminal
> > pack, so we should go ahead and make an empty pack.
>
> I see.
>
> Then what about something bolder, like the below? Instead of fiddling
> with the details of coerce_template_parms - how it handles the explicit
> arguments - in fn_type_unification we deal with both parameter_pack ==
> true and false in the same way when targ == NULL_TREE, thus we set
> incomplete. Then, for the new testcases, since incomplete is true, there
> is no jump to the deduced label and type_unification_real takes care of
> making the empty pack - the same happens already when there are no
> explicit arguments. Tested x86_64-linux. I also checked quite a few
> other variants of the tests but nothing new, nothing interesting, showed
> up...
OK.
Jason
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2019-04-18 17:44 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-04-11 15:21 [C++ Patch/RFC] PR 89900 ("[9 Regression] ICE: Segmentation fault (in check_instantiated_arg)") Paolo Carlini
2019-04-12 10:02 ` Paolo Carlini
2019-04-12 18:46 ` Jason Merrill
2019-04-15 20:36 ` Paolo Carlini
2019-04-18 17:45 ` 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).