Re: [C++ Patch] PR 71737
On Thu, Feb 9, 2017 at 4:08 AM, Paolo Carlini wrote: > On 14/01/2017 15:43, Jason Merrill wrote: >> >> OK. > > As you may or may not have noticed, I had to revert the patch because it > caused the regression of > tr1/2_general_utilities/shared_ptr/cons/43820_neg.cc during error recovery: > an ICE happens in remap_decl when build_variant_type_copy is called on a > TREE_TYPE (== DECL_ORIGINAL_TYPE) which is error_mark_node. A possible > straightforward and safe fix for the new issue would be simply checking for > the special condition, as I did in patch_71737_3 below, which passes > testing. Alternately, I have been fiddling also with older approaches, and > patch_71737_4 below also passes testing: it simply renounces to preserve a > typedef which names an error_mark_node type. _4 seems consistent with the set_underlying_type behavior, I guess let's go with that. Jason
Re: [C++ Patch] PR 71737
Hi again,
On 14/01/2017 15:43, Jason Merrill wrote:
OK.
As you may or may not have noticed, I had to revert the patch because it
caused the regression of
tr1/2_general_utilities/shared_ptr/cons/43820_neg.cc during error
recovery: an ICE happens in remap_decl when build_variant_type_copy is
called on a TREE_TYPE (== DECL_ORIGINAL_TYPE) which is error_mark_node.
A possible straightforward and safe fix for the new issue would be
simply checking for the special condition, as I did in patch_71737_3
below, which passes testing. Alternately, I have been fiddling also with
older approaches, and patch_71737_4 below also passes testing: it simply
renounces to preserve a typedef which names an error_mark_node type.
Thanks again!
Paolo.
//
Index: c-family/c-common.c
===
--- c-family/c-common.c (revision 245274)
+++ c-family/c-common.c (working copy)
@@ -7420,16 +7420,18 @@ set_underlying_type (tree x)
if (TYPE_NAME (TREE_TYPE (x)) == 0)
TYPE_NAME (TREE_TYPE (x)) = x;
}
- else if (TREE_TYPE (x) != error_mark_node
- && DECL_ORIGINAL_TYPE (x) == NULL_TREE)
+ else if (DECL_ORIGINAL_TYPE (x) == NULL_TREE)
{
tree tt = TREE_TYPE (x);
DECL_ORIGINAL_TYPE (x) = tt;
- tt = build_variant_type_copy (tt);
- TYPE_STUB_DECL (tt) = TYPE_STUB_DECL (DECL_ORIGINAL_TYPE (x));
- TYPE_NAME (tt) = x;
- TREE_USED (tt) = TREE_USED (x);
- TREE_TYPE (x) = tt;
+ if (tt != error_mark_node)
+ {
+ tt = build_variant_type_copy (tt);
+ TYPE_STUB_DECL (tt) = TYPE_STUB_DECL (DECL_ORIGINAL_TYPE (x));
+ TYPE_NAME (tt) = x;
+ TREE_USED (tt) = TREE_USED (x);
+ TREE_TYPE (x) = tt;
+ }
}
}
Index: testsuite/g++.dg/cpp0x/pr71737.C
===
--- testsuite/g++.dg/cpp0x/pr71737.C(revision 245274)
+++ testsuite/g++.dg/cpp0x/pr71737.C(working copy)
@@ -0,0 +1,13 @@
+// PR c++/78765
+// { dg-do compile { target c++11 } }
+
+template class TT>
+struct quote {
+ template
+ using apply = TT; // { dg-error "pack expansion" }
+};
+
+template
+using to_int_t = int;
+
+using t = quote::apply>::apply;
Index: tree-inline.c
===
--- tree-inline.c (revision 245274)
+++ tree-inline.c (working copy)
@@ -375,7 +375,8 @@ remap_decl (tree decl, copy_body_data *id)
/* Preserve the invariant that DECL_ORIGINAL_TYPE != TREE_TYPE,
which is enforced in gen_typedef_die when DECL_ABSTRACT_ORIGIN
is not set on the TYPE_DECL, for example in LTO mode. */
- if (DECL_ORIGINAL_TYPE (t) == TREE_TYPE (t))
+ if (DECL_ORIGINAL_TYPE (t) == TREE_TYPE (t)
+ && TREE_TYPE (t) != error_mark_node)
{
tree x = build_variant_type_copy (TREE_TYPE (t));
TYPE_STUB_DECL (x) = TYPE_STUB_DECL (TREE_TYPE (t));
Index: cp/pt.c
===
--- cp/pt.c (revision 245274)
+++ cp/pt.c (working copy)
@@ -12876,11 +12876,11 @@ tsubst_decl (tree t, tree args, tsubst_flags_t com
args, complain, in_decl);
/* Preserve a typedef that names a type. */
- if (is_typedef_decl (r))
+ if (is_typedef_decl (r) && type != error_mark_node)
{
DECL_ORIGINAL_TYPE (r) = NULL_TREE;
set_underlying_type (r);
- if (TYPE_DECL_ALIAS_P (r) && type != error_mark_node)
+ if (TYPE_DECL_ALIAS_P (r))
/* An alias template specialization can be dependent
even if its underlying type is not. */
TYPE_DEPENDENT_P_VALID (TREE_TYPE (r)) = false;
Index: testsuite/g++.dg/cpp0x/pr71737.C
===
--- testsuite/g++.dg/cpp0x/pr71737.C(revision 245274)
+++ testsuite/g++.dg/cpp0x/pr71737.C(working copy)
@@ -0,0 +1,13 @@
+// PR c++/78765
+// { dg-do compile { target c++11 } }
+
+template class TT>
+struct quote {
+ template
+ using apply = TT; // { dg-error "pack expansion" }
+};
+
+template
+using to_int_t = int;
+
+using t = quote::apply>::apply;
Re: [C++ Patch] PR 71737
On 16/01/2017 16:11, David Edelsohn wrote: This patch caused a libstdc++ regression on AIX libstdc++-v3/include/tr1/shared_ptr.h:556: internal compiler error: tree check: expected class 'type', have 'exceptional' (error_mark) in build_variant_type_copy, at tree.c:6737 I noticed, patch reverted for the time being. Sorry about the annoyance, Paolo.
Re: [C++ Patch] PR 71737
This patch caused a libstdc++ regression on AIX libstdc++-v3/include/tr1/shared_ptr.h:556: internal compiler error: tree check: expected class 'type', have 'exceptional' (error_mark) in build_variant_type_copy, at tree.c:6737
Re: [C++ Patch] PR 71737
OK. On Fri, Jan 13, 2017 at 5:58 PM, Paolo Carlini wrote: > Hi, > > On 13/01/2017 18:33, Jason Merrill wrote: >> >> On Fri, Jan 13, 2017 at 11:42 AM, Paolo Carlini >> wrote: >>> >>> Hi, >>> >>> On 13/01/2017 15:51, Nathan Sidwell wrote: On 01/13/2017 09:45 AM, Paolo Carlini wrote: > > Hi, > > in this error recovery issue get_underlying_template crashes when > TYPE_TEMPLATE_INFO_MAYBE_ALIAS is applied to a null orig_type. Simply > checking for that condition appears to solve the issue in a > straightforward way. Tested x86_64-linux. Wouldn't it be better if a scrogged alias got error_mark_node as the underlying type? (I have no idea whether that's an easy thing to accomplish) >>> >>> Your reply, Nathan, led me to investigate where exactly >>> DECL_ORIGINAL_TYPE >>> becomes null, and turns out that in tsubst_decl we have code actively >>> doing >>> that. That same code, a few lines below, only sets TYPE_DEPENDENT_P_VALID >>> to >>> false if type != error_mark_node. I cannot say to fully understand yet >>> all >>> the details, but certainly the patchlet below also passes testing. Do you >>> have comments about this too? >> >> The clearing of DECL_ORIGINAL_TYPE is to allow set_underlying_type to >> then set it to something more appropriate. That function currently >> avoids setting DECL_ORIGINAL_TYPE to error_mark_node, perhaps that >> should be changed. > > I see, thanks a lot. The below passes testing on x86_64-linux. > > Paolo. > > ///
Re: [C++ Patch] PR 71737
Hi,
On 13/01/2017 18:33, Jason Merrill wrote:
On Fri, Jan 13, 2017 at 11:42 AM, Paolo Carlini
wrote:
Hi,
On 13/01/2017 15:51, Nathan Sidwell wrote:
On 01/13/2017 09:45 AM, Paolo Carlini wrote:
Hi,
in this error recovery issue get_underlying_template crashes when
TYPE_TEMPLATE_INFO_MAYBE_ALIAS is applied to a null orig_type. Simply
checking for that condition appears to solve the issue in a
straightforward way. Tested x86_64-linux.
Wouldn't it be better if a scrogged alias got error_mark_node as the
underlying type? (I have no idea whether that's an easy thing to
accomplish)
Your reply, Nathan, led me to investigate where exactly DECL_ORIGINAL_TYPE
becomes null, and turns out that in tsubst_decl we have code actively doing
that. That same code, a few lines below, only sets TYPE_DEPENDENT_P_VALID to
false if type != error_mark_node. I cannot say to fully understand yet all
the details, but certainly the patchlet below also passes testing. Do you
have comments about this too?
The clearing of DECL_ORIGINAL_TYPE is to allow set_underlying_type to
then set it to something more appropriate. That function currently
avoids setting DECL_ORIGINAL_TYPE to error_mark_node, perhaps that
should be changed.
I see, thanks a lot. The below passes testing on x86_64-linux.
Paolo.
///
Index: c-family/c-common.c
===
--- c-family/c-common.c (revision 244405)
+++ c-family/c-common.c (working copy)
@@ -7419,16 +7419,18 @@ set_underlying_type (tree x)
if (TYPE_NAME (TREE_TYPE (x)) == 0)
TYPE_NAME (TREE_TYPE (x)) = x;
}
- else if (TREE_TYPE (x) != error_mark_node
- && DECL_ORIGINAL_TYPE (x) == NULL_TREE)
+ else if (DECL_ORIGINAL_TYPE (x) == NULL_TREE)
{
tree tt = TREE_TYPE (x);
DECL_ORIGINAL_TYPE (x) = tt;
- tt = build_variant_type_copy (tt);
- TYPE_STUB_DECL (tt) = TYPE_STUB_DECL (DECL_ORIGINAL_TYPE (x));
- TYPE_NAME (tt) = x;
- TREE_USED (tt) = TREE_USED (x);
- TREE_TYPE (x) = tt;
+ if (tt != error_mark_node)
+ {
+ tt = build_variant_type_copy (tt);
+ TYPE_STUB_DECL (tt) = TYPE_STUB_DECL (DECL_ORIGINAL_TYPE (x));
+ TYPE_NAME (tt) = x;
+ TREE_USED (tt) = TREE_USED (x);
+ TREE_TYPE (x) = tt;
+ }
}
}
Index: testsuite/g++.dg/cpp0x/pr71737.C
===
--- testsuite/g++.dg/cpp0x/pr71737.C(revision 0)
+++ testsuite/g++.dg/cpp0x/pr71737.C(working copy)
@@ -0,0 +1,13 @@
+// PR c++/78765
+// { dg-do compile { target c++11 } }
+
+template class TT>
+struct quote {
+ template
+ using apply = TT; // { dg-error "pack expansion" }
+};
+
+template
+using to_int_t = int;
+
+using t = quote::apply>::apply;
Re: [C++ Patch] PR 71737
On Fri, Jan 13, 2017 at 11:42 AM, Paolo Carlini wrote: > Hi, > > On 13/01/2017 15:51, Nathan Sidwell wrote: >> >> On 01/13/2017 09:45 AM, Paolo Carlini wrote: >>> >>> Hi, >>> >>> in this error recovery issue get_underlying_template crashes when >>> TYPE_TEMPLATE_INFO_MAYBE_ALIAS is applied to a null orig_type. Simply >>> checking for that condition appears to solve the issue in a >>> straightforward way. Tested x86_64-linux. >> >> >> Wouldn't it be better if a scrogged alias got error_mark_node as the >> underlying type? (I have no idea whether that's an easy thing to >> accomplish) > > Your reply, Nathan, led me to investigate where exactly DECL_ORIGINAL_TYPE > becomes null, and turns out that in tsubst_decl we have code actively doing > that. That same code, a few lines below, only sets TYPE_DEPENDENT_P_VALID to > false if type != error_mark_node. I cannot say to fully understand yet all > the details, but certainly the patchlet below also passes testing. Do you > have comments about this too? The clearing of DECL_ORIGINAL_TYPE is to allow set_underlying_type to then set it to something more appropriate. That function currently avoids setting DECL_ORIGINAL_TYPE to error_mark_node, perhaps that should be changed. Jason
Re: [C++ Patch] PR 71737
Hi,
On 13/01/2017 15:51, Nathan Sidwell wrote:
On 01/13/2017 09:45 AM, Paolo Carlini wrote:
Hi,
in this error recovery issue get_underlying_template crashes when
TYPE_TEMPLATE_INFO_MAYBE_ALIAS is applied to a null orig_type. Simply
checking for that condition appears to solve the issue in a
straightforward way. Tested x86_64-linux.
Wouldn't it be better if a scrogged alias got error_mark_node as the
underlying type? (I have no idea whether that's an easy thing to
accomplish)
Your reply, Nathan, led me to investigate where exactly
DECL_ORIGINAL_TYPE becomes null, and turns out that in tsubst_decl we
have code actively doing that. That same code, a few lines below, only
sets TYPE_DEPENDENT_P_VALID to false if type != error_mark_node. I
cannot say to fully understand yet all the details, but certainly the
patchlet below also passes testing. Do you have comments about this too?
Thanks!
Paolo.
///
Index: pt.c
===
--- pt.c(revision 244405)
+++ pt.c(working copy)
@@ -12868,7 +12868,8 @@ tsubst_decl (tree t, tree args, tsubst_flags_t com
/* Preserve a typedef that names a type. */
if (is_typedef_decl (r))
{
- DECL_ORIGINAL_TYPE (r) = NULL_TREE;
+ if (type != error_mark_node)
+ DECL_ORIGINAL_TYPE (r) = NULL_TREE;
set_underlying_type (r);
if (TYPE_DECL_ALIAS_P (r) && type != error_mark_node)
/* An alias template specialization can be dependent
Re: [C++ Patch] PR 71737
On 01/13/2017 09:45 AM, Paolo Carlini wrote: Hi, in this error recovery issue get_underlying_template crashes when TYPE_TEMPLATE_INFO_MAYBE_ALIAS is applied to a null orig_type. Simply checking for that condition appears to solve the issue in a straightforward way. Tested x86_64-linux. Wouldn't it be better if a scrogged alias got error_mark_node as the underlying type? (I have no idea whether that's an easy thing to accomplish) nathan -- Nathan Sidwell
