On Mar 30, 2018, Jason Merrill <ja...@redhat.com> wrote: > On Thu, Mar 29, 2018 at 6:24 PM, Alexandre Oliva <aol...@redhat.com> wrote:
>> AFAICT we wouldn't always know, within cp_parser_template_id, whether >> the id is a type or a function. > True, it looks like sometimes we build a TEMPLATE_ID_EXPR with an > IDENTIFIER_NODE. Looking at tsubst_copy_and_build, I see that we > don't call finish_id_expression when substituting such a > TEMPLATE_ID_EXPR. So maybe lookup_template_function and > lookup_template_variable are the right places for this test. lookup_template_function is no good as it is, it's called twice with the same (fns, arglist), from build_concept_check (within cp_parser_maybe_partial_concept_id), and then directly from cp_parser_template_id. With the patchlet below, we get two copies of the error message: diff --git a/gcc/cp/pt.c b/gcc/cp/pt.c index 284eaf3cab66..b35f8076cfbd 100644 --- a/gcc/cp/pt.c +++ b/gcc/cp/pt.c @@ -8788,6 +8788,18 @@ lookup_template_function (tree fns, tree arglist) return error_mark_node; } + if (flag_concepts) + { + tree xauto = type_uses_auto (arglist); + if (xauto) + { + error_at (DECL_SOURCE_LOCATION (TEMPLATE_TYPE_DECL (xauto)), + "invalid use of %<auto%> in template argument" + " for template function"); + return error_mark_node; + } + } + if (BASELINK_P (fns)) { BASELINK_FUNCTIONS (fns) = build2 (TEMPLATE_ID_EXPR, @@ -9395,6 +9407,19 @@ lookup_template_variable (tree templ, tree arglist) if (flag_concepts && variable_concept_p (templ)) /* Except that concepts are always bool. */ type = boolean_type_node; + + if (flag_concepts) + { + tree xauto = type_uses_auto (arglist); + if (xauto) + { + error_at (DECL_SOURCE_LOCATION (TEMPLATE_TYPE_DECL (xauto)), + "invalid use of %<auto%> in template argument" + " for template variable"); + return error_mark_node; + } + } + return build2 (TEMPLATE_ID_EXPR, type, templ, arglist); } So I tried the spot at cp_parser_template_id just after the spot that tries a partial_concept_id, and then instead of two copies of the error message, I got four: diff --git a/gcc/cp/parser.c b/gcc/cp/parser.c index e946d0b72292..f93b3bb21307 100644 --- a/gcc/cp/parser.c +++ b/gcc/cp/parser.c @@ -15856,6 +15856,13 @@ cp_parser_template_id (cp_parser *parser, && (template_id = (cp_parser_maybe_partial_concept_id (parser, templ, arguments)))) return template_id; + else if (flag_concepts + && (template_id = type_uses_auto (arguments))) + { + error_at (DECL_SOURCE_LOCATION (TEMPLATE_TYPE_DECL (template_id)), + "invalid use of %<auto%> in template argument"); + return error_mark_node; + } else if (variable_template_p (templ)) { template_id = lookup_template_variable (templ, arguments); I guess this means we don't want to fail to parse the thing as what it is and force the parser to try other things just because there's an auto in the arglist. Parsing it as what it is and then catching it later, like I proposed before, seems to be working better. It's not a parse error, after all, it's rather a constraint on what otherwise legitimate template arguments might contain. Can you elaborate on any reasons to not proceed with the patch I proposed? (if the reason is the failure to call finish_id_expression when tsubsting TEMPLATE_ID_EXPRs with identifiers in them, maybe we can have the test there as well?) -- Alexandre Oliva, freedom fighter http://FSFLA.org/~lxoliva/ You must be the change you wish to see in the world. -- Gandhi Be Free! -- http://FSFLA.org/ FSF Latin America board member Free Software Evangelist|Red Hat Brasil GNU Toolchain Engineer