Hi,
On 19/05/2018 01:40, Jason Merrill wrote:
On Fri, May 18, 2018 at 1:40 PM, Paolo Carlini <paolo.carl...@oracle.com> wrote:
Hi again,
I'm playing with a wild, wild idea: would it make sense to try *first* an
expression? Because, a quickly hacked draft appears to handle very elegantly
all the possible cases of "junk" after the declarator, eg:
void foo()
{
if (void bar()JUNK);
}
and the parenthesized case, etc, etc. Before trying to seriously work on
that I wanted to ask...
We'd need to try parsing as a declaration whether or not parsing as an
expression works, since any ambiguous cases are treated as
declarations [stmt.ambig].
Yeah, that complicates the implementation of my idea. However, I'm
thinking that at least *in the specific case of cp_parse_condition* from
the computational complexity point of view probably we wouldn't regress
much, because declarations are rare anyway, thus in most of the cases we
end up doing both anyway. If we do expressions first and we save the
result, then I believe when we can handle the declarator as something
which cannot be a function or an array even if we don't see the
initializer much more easily, we easily have a better diagnostic for
things like
if (int x);
(another case we currently handle pretty badly, we don't talk about the
missing initializer at all!), we cope elegantly with any junk following
the wrong function/array declaration, etc. See that attached, if you are
curious, which essentially passes the testsuite modulo a nit (*) which
doesn't have anything to do with [stmt.ambig] per se (which of course is
*not* correctly implemented in the wip).
Can you give me your opinion about the more detailed idea, in particular
whether we already have good infrastructure to implement [stmt.ambig] in
this context, thus to keep the first parsing as an expression around,
possibly returning to it if the parsing as a declaration fails??
I hope I'm making sense, it's again late here... in any case the wip
patch I did much earlier today ;)
Paolo.
(*) Has to do with David's access failure hint not handling deferred
access checks (accessor-fixits-6.C).
Index: parser.c
===================================================================
--- parser.c (revision 260347)
+++ parser.c (working copy)
@@ -11527,6 +11527,33 @@ cp_parser_selection_statement (cp_parser* parser,
}
}
+/* Helper function for cp_parser_condition. Enforces [stmt.stmt]/2:
+ The declarator shall not specify a function or an array. Returns
+ TRUE if the declarator is valid, FALSE otherwise. */
+
+static bool
+cp_parser_check_condition_declarator (cp_parser* parser,
+ cp_declarator *declarator,
+ location_t loc)
+{
+ if (function_declarator_p (declarator)
+ || declarator->kind == cdk_array)
+ {
+ if (declarator->kind == cdk_array)
+ error_at (loc, "condition declares an array");
+ else
+ error_at (loc, "condition declares a function");
+ if (parser->fully_implicit_function_template_p)
+ abort_fully_implicit_template (parser);
+ cp_parser_skip_to_closing_parenthesis (parser, /*recovering=*/true,
+ /*or_comma=*/false,
+ /*consume_paren=*/false);
+ return false;
+ }
+ else
+ return true;
+}
+
/* Parse a condition.
condition:
@@ -11545,12 +11572,20 @@ cp_parser_selection_statement (cp_parser* parser,
static tree
cp_parser_condition (cp_parser* parser)
{
+ cp_parser_parse_tentatively (parser);
+
+ /* Try the expression first. */
+ tree expr = cp_parser_expression (parser);
+
+ if (cp_parser_parse_definitely (parser))
+ return expr;
+
+ cp_parser_parse_tentatively (parser);
+
cp_decl_specifier_seq type_specifiers;
const char *saved_message;
int declares_class_or_enum;
- /* Try the declaration first. */
- cp_parser_parse_tentatively (parser);
/* New types are not allowed in the type-specifier-seq for a
condition. */
saved_message = parser->type_definition_forbidden_message;
@@ -11563,6 +11598,7 @@ cp_parser_condition (cp_parser* parser)
&declares_class_or_enum);
/* Restore the saved message. */
parser->type_definition_forbidden_message = saved_message;
+
/* If all is well, we might be looking at a declaration. */
if (!cp_parser_error_occurred (parser))
{
@@ -11570,7 +11606,8 @@ cp_parser_condition (cp_parser* parser)
tree asm_specification;
tree attributes;
cp_declarator *declarator;
- tree initializer = NULL_TREE;
+ tree initializer;
+ location_t loc = cp_lexer_peek_token (parser->lexer)->location;
/* Parse the declarator. */
declarator = cp_parser_declarator (parser, CP_PARSER_DECLARATOR_NAMED,
@@ -11582,19 +11619,7 @@ cp_parser_condition (cp_parser* parser)
attributes = cp_parser_attributes_opt (parser);
/* Parse the asm-specification. */
asm_specification = cp_parser_asm_specification_opt (parser);
- /* If the next token is not an `=' or '{', then we might still be
- looking at an expression. For example:
- if (A(a).x)
-
- looks like a decl-specifier-seq and a declarator -- but then
- there is no `=', so this is an expression. */
- if (cp_lexer_next_token_is_not (parser->lexer, CPP_EQ)
- && cp_lexer_next_token_is_not (parser->lexer, CPP_OPEN_BRACE))
- cp_parser_simulate_error (parser);
-
- /* If we did see an `=' or '{', then we are looking at a declaration
- for sure. */
if (cp_parser_parse_definitely (parser))
{
tree pushed_scope;
@@ -11601,6 +11626,9 @@ cp_parser_condition (cp_parser* parser)
bool non_constant_p;
int flags = LOOKUP_ONLYCONVERTING;
+ if (!cp_parser_check_condition_declarator (parser, declarator, loc))
+ return error_mark_node;
+
/* Create the declaration. */
decl = start_decl (declarator, &type_specifiers,
/*initialized_p=*/true,
@@ -11614,12 +11642,18 @@ cp_parser_condition (cp_parser* parser)
CONSTRUCTOR_IS_DIRECT_INIT (initializer) = 1;
flags = 0;
}
- else
+ else if (cp_lexer_next_token_is (parser->lexer, CPP_EQ))
{
/* Consume the `='. */
cp_parser_require (parser, CPP_EQ, RT_EQ);
- initializer = cp_parser_initializer_clause (parser,
&non_constant_p);
+ initializer = cp_parser_initializer_clause (parser,
+ &non_constant_p);
}
+ else
+ {
+ cp_parser_error (parser, "expected initializer");
+ initializer = error_mark_node;
+ }
if (BRACE_ENCLOSED_INITIALIZER_P (initializer))
maybe_warn_cpp0x (CPP0X_INITIALIZER_LISTS);
@@ -11640,8 +11674,8 @@ cp_parser_condition (cp_parser* parser)
else
cp_parser_abort_tentative_parse (parser);
- /* Otherwise, we are looking at an expression. */
- return cp_parser_expression (parser);
+ cp_parser_error (parser, "expected condition");
+ return error_mark_node;
}
/* Parses a for-statement or range-for-statement until the closing ')',
