On Thu, Jun 14, 2018 at 3:40 PM, Marek Polacek <pola...@redhat.com> wrote: > On Thu, Jun 14, 2018 at 12:59:00PM -0400, Jason Merrill wrote: >> On Thu, Jun 14, 2018 at 12:51 PM, Marek Polacek <pola...@redhat.com> wrote: >> > On Mon, Jun 11, 2018 at 03:00:04PM -0400, Jason Merrill wrote: >> >> On Wed, Jun 6, 2018 at 5:18 PM, Marek Polacek <pola...@redhat.com> wrote: >> >> > We crash on this testcase containing a bogus attribute, because >> >> > cp_check_const_attributes accessed TREE_VALUE of a tree that happened >> >> > to be >> >> > expr_pack_expansion. Since here we're merely trying to evaluate >> >> > constexpr >> >> > arguments, I thought we could skip such bogus arguments. >> >> >> >> Hmm, attributes should always be a TREE_LIST, lots of places assume >> >> that. Why isn't the pack expansion wrapped in a TREE_LIST? >> > >> > I believe you did that on purpose. There pack comes from >> > cp_parser_std_attribute_list. We could wrap it into a TREE_LIST, but then >> > tsubst_attribute would have to be tweaked to handle the pack expansion >> > correctly. >> >> How so? tsubst_attribute expects to find a pack expansion in the >> TREE_VALUE of a TREE_LIST. >> And cp_parser_std_attribute_list puts the pack expansion in TREE_VALUE. > > Exactly. But what tsubst_attribute gets currently is > > <tree_list 0x7ffff001c280 tree_0 > purpose <tree_list 0x7ffff0002f00 > purpose <identifier_node 0x7fffefec7d80 gnu > normal local bindings <(nil)>> > value <identifier_node 0x7ffff0014f00 aligned > normal local bindings <(nil)>>> > value <expr_pack_expansion 0x7fffefeada20 > arg:0 <tree_list 0x7ffff0002f50 > value <alignof_expr 0x7ffff0015600 type <integer_type > 0x7fffefecd7e0 long unsigned int> > readonly tree_0 arg:0 <template_type_parm 0x7ffff001a1f8 T> > alignas4.C:17:19 start: alignas4.C:17:19 finish: > alignas4.C:17:29>> > arg:1 <tree_list 0x7ffff0002f78 value <template_type_parm > 0x7ffff001a1f8 T>>>> > > so if I were to wrap the expr_pack_expansion in a TREE_LIST, I would have to > adjust > tsubst_attribute. But cp_check_const_attributes doesn't expect that the > TREE_VALUE > of the above is a non-list. Right?
Ah, of course, you're already looking at the arguments, I wasn't reading closely enough. The patch is OK. Jason
