This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
Re: PR 15787 Poor error message with if and blocks
On 28/01/07, Manuel López-Ibáñez <lopezibanez@gmail.com> wrote:
On 28/01/07, Mark Mitchell <mark@codesourcery.com> wrote:
> So, I think a better solution might be to notice the "else" keyword,
> emit an error saying "'else' without a previous 'if'", skip the "else"
> keyword, and keep parsing. Or, do your idea only if within an
> "if"-statement, and my idea if not -- but that's more complicated that
> just doing my idea, and my idea will still make it reasonably clear to
> the user what is wrong.
[snip]
On the other hand, detecting whether we are in the body of an "if" or
not would need a new field in cp_parser (or is there other way to
check this?). We currently have in_unbraced_linkage_specification_p,
in_declarator_p, in_template_argument_list_p, in_statement,
in_switch_statement_p, in_type_id_in_expr_p, implicit_extern_c,
translate_strings_p, and in_function_body. I think we could have
in_if_body and detect the situation.
Do you think this would be overly complicated?
Actually, I couldn't refrain myself from trying your idea because I
think it is the nicest solution. With the patch below we give:
error: expected `}' before 'else'
for my testcase, while we give:
error: 'else' without a previous 'if'
for yours. Also, it passes all tests in g++.dg/parser/.
Of course, I will need a complete bootstrap and regression test to
check whether I broke something else. Then, I will submit it properly.
Does it look OK?
Cheers,
Manuel.
--- gcc/cp/parser.c (revision 121027)
+++ gcc/cp/parser.c (working copy)
@@ -1395,6 +1395,7 @@ typedef struct cp_parser GTY(())
#define IN_ITERATION_STMT 2
#define IN_OMP_BLOCK 4
#define IN_OMP_FOR 8
+#define IN_IF_STMT 16
unsigned char in_statement;
/* TRUE if we are presently parsing the body of a switch statement.
@@ -6522,6 +6523,19 @@ cp_parser_statement_seq_opt (cp_parser*
|| token->type == CPP_EOF
|| token->type == CPP_PRAGMA_EOL)
break;
+
+ /* If we are in a compound statement and find 'else' then
+ something went wrong. */
+ else if (token->type == CPP_KEYWORD && token->keyword == RID_ELSE)
+ {
+ if (parser->in_statement & IN_IF_STMT)
+ break;
+ else
+ {
+ token = cp_lexer_consume_token (parser->lexer);
+ error ("%<else%> without a previous %<if%>");
+ }
+ }
/* Parse the statement. */
cp_parser_statement (parser, in_statement_expr, true, NULL);
@@ -6587,12 +6601,17 @@ cp_parser_selection_statement (cp_parser
if (keyword == RID_IF)
{
bool nested_if;
+ unsigned char in_statement;
/* Add the condition. */
finish_if_stmt_cond (condition, statement);
/* Parse the then-clause. */
+ in_statement = parser->in_statement;
+ parser->in_statement |= IN_IF_STMT;
cp_parser_implicitly_scoped_statement (parser, &nested_if);
+ parser->in_statement = in_statement;
+
finish_then_clause (statement);
/* If the next token is `else', parse the else-clause. */
@@ -6952,11 +6971,12 @@ cp_parser_jump_statement (cp_parser* par
switch (parser->in_statement)
{
case 0:
+ case IN_IF_STMT:
error ("break statement not within loop or switch");
break;
default:
gcc_assert ((parser->in_statement & IN_SWITCH_STMT)
- || parser->in_statement == IN_ITERATION_STMT);
+ || (parser->in_statement & IN_ITERATION_STMT));
statement = finish_break_stmt ();
break;
case IN_OMP_BLOCK:
@@ -6970,7 +6990,7 @@ cp_parser_jump_statement (cp_parser* par
break;
case RID_CONTINUE:
- switch (parser->in_statement & ~IN_SWITCH_STMT)
+ switch (parser->in_statement & ~(IN_SWITCH_STMT | IN_IF_STMT))
{
case 0:
error ("continue statement not within a loop");
Cheers,
Manuel.