On Fri, Dec 15, 2017 at 11:35 AM, David Malcolm <dmalc...@redhat.com> wrote: > On Fri, 2017-12-15 at 10:01 -0500, Jason Merrill wrote: >> On Thu, Dec 14, 2017 at 2:25 PM, David Malcolm <dmalc...@redhat.com> >> wrote: >> > On Mon, 2017-12-11 at 21:10 -0500, Jason Merrill wrote: >> > > On 11/10/2017 04:45 PM, David Malcolm wrote: >> > > > The initial version of the patch kit added location wrapper >> > > > nodes >> > > > around constants and uses-of-declarations, along with some >> > > > other >> > > > places in the parser (typeid, alignof, sizeof, offsetof). >> > > > >> > > > This version takes a much more minimal approach: it only adds >> > > > location wrapper nodes around the arguments at callsites, thus >> > > > not adding wrapper nodes around uses of constants and decls in >> > > > other >> > > > locations. >> > > > >> > > > It keeps them for the other places in the parser (typeid, >> > > > alignof, >> > > > sizeof, offsetof). >> > > > >> > > > In addition, for now, each site that adds wrapper nodes is >> > > > guarded >> > > > with !processing_template_decl, suppressing the creation of >> > > > wrapper >> > > > nodes when processing template declarations. This is to >> > > > simplify >> > > > the patch kit so that we don't have to support wrapper nodes >> > > > during >> > > > template expansion. >> > > >> > > Hmm, it should be easy to support them, since NON_LVALUE_EXPR and >> > > VIEW_CONVERT_EXPR don't otherwise appear in template trees. >> > > >> > > Jason >> > >> > I don't know if it's "easy"; it's at least non-trivial. >> > >> > I attempted to support them in the obvious way by adding the two >> > codes >> > to the switch statement tsubst_copy, reusing the case used by >> > NOP_EXPR >> > and others, but ran into a issue when dealing with template >> > parameter >> > packs. >> > Attached is the reproducer I've been testing with (minimized using >> > "delta" from a stdlib reproducer); my code was failing with: >> > >> > ../../src/cp-stdlib.ii: In instantiation of ‘struct >> > allocator_traits<allocator<char> >’: >> > ../../src/cp-stdlib.ii:31:8: required from ‘struct >> > __alloc_traits<allocator<char>, char>’ >> > ../../src/cp-stdlib.ii:43:75: required from ‘class >> > basic_string<char, allocator<char> >’ >> > ../../src/cp-stdlib.ii:47:58: required from here >> > ../../src/cp-stdlib.ii:27:55: sorry, unimplemented: use of >> > ‘type_pack_expansion’ in template >> > -> decltype(_S_construct(__a, __p, >> > forward<_Args>(__args)...)) { } >> > ^~~~~~ >> > >> > The issue is that normally "__args" would be a PARM_DECL of type >> > TYPE_PACK_EXPANSION, and that's handled by tsubst_decl, but on >> > adding a >> > wrapper node we now have a VIEW_CONVERT_EXPR of the same type i.e. >> > TYPE_PACK_EXPANSION wrapping the PARM_DECL. >> > >> > When tsubst traverses the tree, the VIEW_CONVERT_EXPR is reached >> > first, >> > and it attempts to substitute the type TYPE_PACK_EXPANSION, which >> > leads >> > to the "sorry". >> > >> > If I understand things right, during substitution, only tsubst_decl >> > on >> > PARM_DECL can handle nodes with type with code TYPE_PACK_EXPANSION. >> > >> > The simplest approach seems to be to not create wrapper nodes for >> > decls >> > of type TYPE_PACK_EXPANSION, and that seems to fix the issue. >> >> That does seem simplest. >> >> > Alternatively I can handle TYPE_PACK_EXPANSION for >> > VIEW_CONVERT_EXPR in >> > tsubst by remapping the type to that of what they wrap after >> > substitution; doing so also fixes the issue. >> >> This will be more correct. For the wrappers you don't need all the >> handling that we currently have for NOP_EXPR and such; since we know >> they don't change the type, we can substitute what they wrap, and >> then >> rewrap the result. > > (nods; I have this working) > > I've been debugging the other issues that I ran into when removing the > "!processing_template_decl" filter on making wrapper nodes (ICEs and > other errors on valid code). They turn out to relate to wrappers > around decls of type TEMPLATE_TYPE_PARM; having these wrappers leads to > such VIEW_CONVERT_EXPRs turning up in unexpected places.
Hmm, that's odd. What kind of decls? A variable which happens to have a template parameter for a type shouldn't be a problem. > I could try to track all those places down, but it seems much simpler > to just add an exclusion to adding wrapper nodes around decls of type > TEMPLATE_TYPE_PARM. On doing that my smoketests with the C++ stdlib > work again. Does that sound reasonable? Jason