| Message ID | 20181128154849.GD3269@redhat.com |
|---|---|
| State | New |
| Headers | show |
| Series | C++ PATCH for c++/88184, ICE when treating name as template-name | expand |
On 11/28/18 10:48 AM, Marek Polacek wrote: > Since P0846R0 was implemented, a name will be treated as a template-name when > it is an unqualified-id followed by a < and name lookup finds either one or > more functions or finds nothing, in order to potentially cause ADL to be performed. > > In this case, we had > > f<T> (); > > where we'd found a decl for f (not a TEMPLATE_DECL) and < follows. From the backtrace in the PR, it seems as though we're treating f<T> as non-dependent, which is wrong. type_dependent_expression_p only looks at the arguments of a TEMPLATE_ID_EXPR if it has unknown_type_node, so we probably want to give it that type. Jason
On Wed, Nov 28, 2018 at 10:34:11PM -0500, Jason Merrill wrote: > On 11/28/18 10:48 AM, Marek Polacek wrote: > > Since P0846R0 was implemented, a name will be treated as a template-name when > > it is an unqualified-id followed by a < and name lookup finds either one or > > more functions or finds nothing, in order to potentially cause ADL to be performed. > > > > In this case, we had > > > > f<T> (); > > > > where we'd found a decl for f (not a TEMPLATE_DECL) and < follows. > > From the backtrace in the PR, it seems as though we're treating f<T> as > non-dependent, which is wrong. type_dependent_expression_p only looks at > the arguments of a TEMPLATE_ID_EXPR if it has unknown_type_node, so we > probably want to give it that type. That was my first attempt but it was crashing everywhere, so I abandoned it. But I was setting unknown_type_node in cp_parser_template_name whereas this patch sets it in cp_parser_postfix_expression, which works and is arguably better because it reuses the diagnostic in finish_call_expr. Bootstrapped/regtested on x86_64-linux, ok for trunk? 2018-11-29 Marek Polacek <polacek@redhat.com> PR c++/88184, ICE when treating name as template-name. * parser.c (cp_parser_postfix_expression): Set the function decl type to unknown type when the name-as-template-name doesn't have any arguments. * g++.dg/cpp2a/fn-template17.C: New test. * g++.dg/cpp2a/fn-template18.C: New test. diff --git gcc/cp/parser.c gcc/cp/parser.c index 3ef1eda4518..fc328c28f69 100644 --- gcc/cp/parser.c +++ gcc/cp/parser.c @@ -7241,6 +7241,20 @@ cp_parser_postfix_expression (cp_parser *parser, bool address_p, bool cast_p, complain); } } + else if (TREE_CODE (postfix_expression) == TEMPLATE_ID_EXPR + && (TREE_CODE (TREE_OPERAND (postfix_expression, 0)) + == FUNCTION_DECL) + && args->is_empty ()) + { + /* We can get here when treating a name as a template name + (because it was an unqualified-id followed by a < and + name lookup found a decl), but now we see that there + are no arguments, so ADL isn't possible. Tweak the type + so that the TEMPLATE_ID_EXPR is type-dependent and we get + proper diagnostic down in finish_call_expr. */ + TREE_TYPE (TREE_OPERAND (postfix_expression, 0)) + = unknown_type_node; + } } if (TREE_CODE (postfix_expression) == COMPONENT_REF) diff --git gcc/testsuite/g++.dg/cpp2a/fn-template17.C gcc/testsuite/g++.dg/cpp2a/fn-template17.C new file mode 100644 index 00000000000..f0d24107682 --- /dev/null +++ gcc/testsuite/g++.dg/cpp2a/fn-template17.C @@ -0,0 +1,21 @@ +// PR c++/88184 +// { dg-do compile } +// { dg-options "-std=c++2a -fchecking=2" } + +namespace A +{ + void f (); +} + +using A::f; + +template <typename T> void g () +{ + f<T> (); // { dg-error "no matching function for call" } +} + +void +fn () +{ + g<int>(); +} diff --git gcc/testsuite/g++.dg/cpp2a/fn-template18.C gcc/testsuite/g++.dg/cpp2a/fn-template18.C new file mode 100644 index 00000000000..7fe5e89ace3 --- /dev/null +++ gcc/testsuite/g++.dg/cpp2a/fn-template18.C @@ -0,0 +1,23 @@ +// PR c++/88184 +// { dg-do compile } +// { dg-options "-std=c++2a -fchecking=2" } + +namespace A +{ + void f (); + void f (int); + void f (int, int); +} + +using A::f; + +template <typename T> void g () +{ + f<T> (); // { dg-error "no matching function for call" } +} + +void +fn () +{ + g<int>(); +}
On 11/29/18 1:28 PM, Marek Polacek wrote: > On Wed, Nov 28, 2018 at 10:34:11PM -0500, Jason Merrill wrote: >> On 11/28/18 10:48 AM, Marek Polacek wrote: >>> Since P0846R0 was implemented, a name will be treated as a template-name when >>> it is an unqualified-id followed by a < and name lookup finds either one or >>> more functions or finds nothing, in order to potentially cause ADL to be performed. >>> >>> In this case, we had >>> >>> f<T> (); >>> >>> where we'd found a decl for f (not a TEMPLATE_DECL) and < follows. >> >> From the backtrace in the PR, it seems as though we're treating f<T> as >> non-dependent, which is wrong. type_dependent_expression_p only looks at >> the arguments of a TEMPLATE_ID_EXPR if it has unknown_type_node, so we >> probably want to give it that type. > > That was my first attempt but it was crashing everywhere, so I abandoned it. > But I was setting unknown_type_node in cp_parser_template_name whereas this > patch sets it in cp_parser_postfix_expression, which works and is arguably > better because it reuses the diagnostic in finish_call_expr. Would it work for lookup_template_function to always use unknown_type_node? Jason
On Thu, Nov 29, 2018 at 04:03:50PM -0500, Jason Merrill wrote: > On 11/29/18 1:28 PM, Marek Polacek wrote: > > On Wed, Nov 28, 2018 at 10:34:11PM -0500, Jason Merrill wrote: > > > On 11/28/18 10:48 AM, Marek Polacek wrote: > > > > Since P0846R0 was implemented, a name will be treated as a template-name when > > > > it is an unqualified-id followed by a < and name lookup finds either one or > > > > more functions or finds nothing, in order to potentially cause ADL to be performed. > > > > > > > > In this case, we had > > > > > > > > f<T> (); > > > > > > > > where we'd found a decl for f (not a TEMPLATE_DECL) and < follows. > > > > > > From the backtrace in the PR, it seems as though we're treating f<T> as > > > non-dependent, which is wrong. type_dependent_expression_p only looks at > > > the arguments of a TEMPLATE_ID_EXPR if it has unknown_type_node, so we > > > probably want to give it that type. > > > > That was my first attempt but it was crashing everywhere, so I abandoned it. > > But I was setting unknown_type_node in cp_parser_template_name whereas this > > patch sets it in cp_parser_postfix_expression, which works and is arguably > > better because it reuses the diagnostic in finish_call_expr. > > Would it work for lookup_template_function to always use unknown_type_node? Luckily, that seems to work! I wish I'd thought of that, I like fixing bugs by simplifying code. Bootstrapped/regtested on x86_64-linux, ok for trunk? 2018-12-04 Marek Polacek <polacek@redhat.com> PR c++/88184 - ICE when treating name as template-name. * pt.c (lookup_template_function): Always build the TEMPLATE_ID_EXPR with unknown_type_node. * g++.dg/cpp2a/fn-template17.C: New test. * g++.dg/cpp2a/fn-template18.C: New test. diff --git gcc/cp/pt.c gcc/cp/pt.c index a0d899f594b..78addc6c1c2 100644 --- gcc/cp/pt.c +++ gcc/cp/pt.c @@ -9068,8 +9068,6 @@ add_pending_template (tree d) tree lookup_template_function (tree fns, tree arglist) { - tree type; - if (fns == error_mark_node || arglist == error_mark_node) return error_mark_node; @@ -9090,11 +9088,7 @@ lookup_template_function (tree fns, tree arglist) return fns; } - type = TREE_TYPE (fns); - if (TREE_CODE (fns) == OVERLOAD || !type) - type = unknown_type_node; - - return build2 (TEMPLATE_ID_EXPR, type, fns, arglist); + return build2 (TEMPLATE_ID_EXPR, unknown_type_node, fns, arglist); } /* Within the scope of a template class S<T>, the name S gets bound diff --git gcc/testsuite/g++.dg/cpp2a/fn-template17.C gcc/testsuite/g++.dg/cpp2a/fn-template17.C new file mode 100644 index 00000000000..f0d24107682 --- /dev/null +++ gcc/testsuite/g++.dg/cpp2a/fn-template17.C @@ -0,0 +1,21 @@ +// PR c++/88184 +// { dg-do compile } +// { dg-options "-std=c++2a -fchecking=2" } + +namespace A +{ + void f (); +} + +using A::f; + +template <typename T> void g () +{ + f<T> (); // { dg-error "no matching function for call" } +} + +void +fn () +{ + g<int>(); +} diff --git gcc/testsuite/g++.dg/cpp2a/fn-template18.C gcc/testsuite/g++.dg/cpp2a/fn-template18.C new file mode 100644 index 00000000000..7fe5e89ace3 --- /dev/null +++ gcc/testsuite/g++.dg/cpp2a/fn-template18.C @@ -0,0 +1,23 @@ +// PR c++/88184 +// { dg-do compile } +// { dg-options "-std=c++2a -fchecking=2" } + +namespace A +{ + void f (); + void f (int); + void f (int, int); +} + +using A::f; + +template <typename T> void g () +{ + f<T> (); // { dg-error "no matching function for call" } +} + +void +fn () +{ + g<int>(); +}
OK. On Tue, Dec 4, 2018 at 12:27 PM Marek Polacek <polacek@redhat.com> wrote: > > On Thu, Nov 29, 2018 at 04:03:50PM -0500, Jason Merrill wrote: > > On 11/29/18 1:28 PM, Marek Polacek wrote: > > > On Wed, Nov 28, 2018 at 10:34:11PM -0500, Jason Merrill wrote: > > > > On 11/28/18 10:48 AM, Marek Polacek wrote: > > > > > Since P0846R0 was implemented, a name will be treated as a template-name when > > > > > it is an unqualified-id followed by a < and name lookup finds either one or > > > > > more functions or finds nothing, in order to potentially cause ADL to be performed. > > > > > > > > > > In this case, we had > > > > > > > > > > f<T> (); > > > > > > > > > > where we'd found a decl for f (not a TEMPLATE_DECL) and < follows. > > > > > > > > From the backtrace in the PR, it seems as though we're treating f<T> as > > > > non-dependent, which is wrong. type_dependent_expression_p only looks at > > > > the arguments of a TEMPLATE_ID_EXPR if it has unknown_type_node, so we > > > > probably want to give it that type. > > > > > > That was my first attempt but it was crashing everywhere, so I abandoned it. > > > But I was setting unknown_type_node in cp_parser_template_name whereas this > > > patch sets it in cp_parser_postfix_expression, which works and is arguably > > > better because it reuses the diagnostic in finish_call_expr. > > > > Would it work for lookup_template_function to always use unknown_type_node? > > Luckily, that seems to work! I wish I'd thought of that, I like fixing bugs by > simplifying code. > > Bootstrapped/regtested on x86_64-linux, ok for trunk? > > 2018-12-04 Marek Polacek <polacek@redhat.com> > > PR c++/88184 - ICE when treating name as template-name. > * pt.c (lookup_template_function): Always build the TEMPLATE_ID_EXPR > with unknown_type_node. > > * g++.dg/cpp2a/fn-template17.C: New test. > * g++.dg/cpp2a/fn-template18.C: New test. > > diff --git gcc/cp/pt.c gcc/cp/pt.c > index a0d899f594b..78addc6c1c2 100644 > --- gcc/cp/pt.c > +++ gcc/cp/pt.c > @@ -9068,8 +9068,6 @@ add_pending_template (tree d) > tree > lookup_template_function (tree fns, tree arglist) > { > - tree type; > - > if (fns == error_mark_node || arglist == error_mark_node) > return error_mark_node; > > @@ -9090,11 +9088,7 @@ lookup_template_function (tree fns, tree arglist) > return fns; > } > > - type = TREE_TYPE (fns); > - if (TREE_CODE (fns) == OVERLOAD || !type) > - type = unknown_type_node; > - > - return build2 (TEMPLATE_ID_EXPR, type, fns, arglist); > + return build2 (TEMPLATE_ID_EXPR, unknown_type_node, fns, arglist); > } > > /* Within the scope of a template class S<T>, the name S gets bound > diff --git gcc/testsuite/g++.dg/cpp2a/fn-template17.C gcc/testsuite/g++.dg/cpp2a/fn-template17.C > new file mode 100644 > index 00000000000..f0d24107682 > --- /dev/null > +++ gcc/testsuite/g++.dg/cpp2a/fn-template17.C > @@ -0,0 +1,21 @@ > +// PR c++/88184 > +// { dg-do compile } > +// { dg-options "-std=c++2a -fchecking=2" } > + > +namespace A > +{ > + void f (); > +} > + > +using A::f; > + > +template <typename T> void g () > +{ > + f<T> (); // { dg-error "no matching function for call" } > +} > + > +void > +fn () > +{ > + g<int>(); > +} > diff --git gcc/testsuite/g++.dg/cpp2a/fn-template18.C gcc/testsuite/g++.dg/cpp2a/fn-template18.C > new file mode 100644 > index 00000000000..7fe5e89ace3 > --- /dev/null > +++ gcc/testsuite/g++.dg/cpp2a/fn-template18.C > @@ -0,0 +1,23 @@ > +// PR c++/88184 > +// { dg-do compile } > +// { dg-options "-std=c++2a -fchecking=2" } > + > +namespace A > +{ > + void f (); > + void f (int); > + void f (int, int); > +} > + > +using A::f; > + > +template <typename T> void g () > +{ > + f<T> (); // { dg-error "no matching function for call" } > +} > + > +void > +fn () > +{ > + g<int>(); > +}
diff --git gcc/cp/parser.c gcc/cp/parser.c index 3a98ed900cd..01097b57eb9 100644 --- gcc/cp/parser.c +++ gcc/cp/parser.c @@ -7241,6 +7241,20 @@ cp_parser_postfix_expression (cp_parser *parser, bool address_p, bool cast_p, complain); } } + else if (TREE_CODE (postfix_expression) == TEMPLATE_ID_EXPR + && (TREE_CODE (TREE_OPERAND (postfix_expression, 0)) + == FUNCTION_DECL) + && args->is_empty ()) + { + /* We can get here when treating a name as a template name + (because it was an unqualified-id followed by a < and + name lookup found a decl), but now we see that there + are no arguments, so ADL isn't possible. */ + auto_diagnostic_group d; + error_at (loc, "no matching function for call to %<%D()%>", + postfix_expression.get_value ()); + postfix_expression = error_mark_node; + } } if (TREE_CODE (postfix_expression) == COMPONENT_REF) diff --git gcc/testsuite/g++.dg/cpp2a/fn-template17.C gcc/testsuite/g++.dg/cpp2a/fn-template17.C new file mode 100644 index 00000000000..f0d24107682 --- /dev/null +++ gcc/testsuite/g++.dg/cpp2a/fn-template17.C @@ -0,0 +1,21 @@ +// PR c++/88184 +// { dg-do compile } +// { dg-options "-std=c++2a -fchecking=2" } + +namespace A +{ + void f (); +} + +using A::f; + +template <typename T> void g () +{ + f<T> (); // { dg-error "no matching function for call" } +} + +void +fn () +{ + g<int>(); +} diff --git gcc/testsuite/g++.dg/cpp2a/fn-template18.C gcc/testsuite/g++.dg/cpp2a/fn-template18.C new file mode 100644 index 00000000000..7fe5e89ace3 --- /dev/null +++ gcc/testsuite/g++.dg/cpp2a/fn-template18.C @@ -0,0 +1,23 @@ +// PR c++/88184 +// { dg-do compile } +// { dg-options "-std=c++2a -fchecking=2" } + +namespace A +{ + void f (); + void f (int); + void f (int, int); +} + +using A::f; + +template <typename T> void g () +{ + f<T> (); // { dg-error "no matching function for call" } +} + +void +fn () +{ + g<int>(); +}