This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: PR 15787 Poor error message with if and blocks


On 28/01/07, Mark Mitchell <mark@codesourcery.com> wrote:
Manuel López-Ibáñez wrote:

> 2007-01-27  Manuel Lopez-Ibanez  <manu@gcc.gnu.org>
>
>  PR c++/15787
> cp/
>  * parser.c (cp_parser_statement_seq_opt): Return as soon as we find
> an unexpected 'else'.
>
> testsuite/
>  * g++.dg/parse/else.C: New.

I think this an interesting idea, but not complete.  If we're inside an
if-statement, and find an "else", that might indicate the user forgot
the "}" and justify your patch.  But, if we see:

  void f() {
    ...
    else {
    }
  }

it doesn't make sense just to stop parsing statements, as your patch
does, since then we'll think we've reached the end of the function.
Instead, it might make more sense just to skip the "else" keyword, and
parse the dependent statements.


I see.


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.

Well, in my testcase there is actually and 'if' previously to the else, what is missing is '}'. In that case, if we skip the 'else' and continue, we report:

gcc/testsuite/g++.dg/parse/else.C:8: error: 'else' without a previous 'if'
gcc/testsuite/g++.dg/parse/else.C:13: error: expected `}' at end of input

Which is wrong, since there is a previous 'if' and we are not really
expecting anything at end of input.  But it is better than what we
have now, so if this is the best we can do for now, I will implement
it.

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?

Cheers,

Manuel.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]