Re: Improve error messages for `$' or `@' followed by `.' or `-'.

[Alex Rozenman]

Hi Joel,

If this is a syntax error, we'd prefer to that "$.field" will not be parsed
as a {ref} at all.
It would imply, for example that references without bracketing will not
start with dots.

For example:
letter    [.abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ_]
c_letter [abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ_]
id        -*(-|{letter}({letter}|[-0-9])*)
ref      -?[0-9]+|{c_letter}({letter}|[0-9])*|"["{id}"]"|"$"

Or, with "regular" letter definition:
letter    [abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ_]
id        -*(-|({letter}[.])({letter}|[.-0-9])*)
ref      -?[0-9]+|{letter}({letter}|[.-0-9])*|"["{id}"]"|"$"

Anyway, if we want to keep "possibly meant" error messages in this case,
your local solution is fine.
So no objections from my side.

Alex

On Sat, Jan 8, 2011 at 8:17 PM, Joel E. Denny <address@hidden>wrote:

> Hi,
>
> Given this test.y:
>
>  %%
>  start: .field { $start = $.field; }
>  .field: ;
>
> I find the second line of this error message confusing:
>
>  test.y:2.26-32: invalid reference: `$.field'
>  test.y:2.8-35:      symbol not found in production:
>  test.y:2.8-13:      possibly meant: $[.field] at $1
>
> I'd prefer this:
>
>  test.y:2.26-32: invalid reference: `$.field'
>  test.y:2.27:        syntax error after `$', expecting integer, letter,
> `_', or `['
>  test.y:2.8-13:      possibly meant: $[.field] at $1
>
> The following patch, written against branch-2.5, fixes that.  Any
> objections?
>
> From 9491fad8a45fb419cfb842a539c3e0588be3df94 Mon Sep 17 00:00:00 2001
> From: Joel E. Denny <address@hidden>
> Date: Sat, 8 Jan 2011 12:52:04 -0500
> Subject: [PATCH] Improve error messages for `$' or `@' followed by `.' or
> `-'.
>
> Previously, for this special case of an invalid reference, the
> usual "symbol not found in production:" was printed.  However,
> because the symbol name was parsed as the empty string, that
> message was followed immediately by a newline instead of a symbol
> name.  In reality, this is a syntax error, so the reference is
> invalid regardless of the symbols actually appearing in the
> production.
> * src/scan-code.l (parse_ref): Report the above case as a syntax
> error.  Other than that, continue to handle this case like any
> other invalid reference that Bison manages to parse because
> "possibly meant" messages can still be helpful to the user.
> * tests/named-refs.at ($ or @ followed by . or -): New test group.
> ---
>  ChangeLog           |   16 ++++++++++++++++
>  src/scan-code.l     |   12 +++++++++++-
>  tests/named-refs.at |   29 +++++++++++++++++++++++++++++
>  3 files changed, 56 insertions(+), 1 deletions(-)
>
> diff --git a/ChangeLog b/ChangeLog
> index a7d8a76..3eaa83d 100644
> --- a/ChangeLog
> +++ b/ChangeLog
> @@ -1,3 +1,19 @@
> +2011-01-08  Joel E. Denny  <address@hidden>
> +
> +       Improve error messages for `$' or `@' followed by `.' or `-'.
> +       Previously, for this special case of an invalid reference, the
> +       usual "symbol not found in production:" was printed.  However,
> +       because the symbol name was parsed as the empty string, that
> +       message was followed immediately by a newline instead of a symbol
> +       name.  In reality, this is a syntax error, so the reference is
> +       invalid regardless of the symbols actually appearing in the
> +       production.
> +       * src/scan-code.l (parse_ref): Report the above case as a syntax
> +       error.  Other than that, continue to handle this case like any
> +       other invalid reference that Bison manages to parse because
> +       "possibly meant" messages can still be helpful to the user.
> +       * tests/named-refs.at ($ or @ followed by . or -): New test group.
> +
>  2011-01-05  Alex Rozenman  <address@hidden>
>
>        Do not allow identifiers that start with a negative number.
> diff --git a/src/scan-code.l b/src/scan-code.l
> index e3e6b65..bfe3f66 100644
> --- a/src/scan-code.l
> +++ b/src/scan-code.l
> @@ -620,7 +620,17 @@ parse_ref (char *cp, symbol_list *rule, int
> rule_length,
>         complain_at_indent (text_loc, &indent, _("invalid reference: %s"),
>                             quote (text));
>         indent += SUB_INDENT;
> -        if (midrule_rhs_index)
> +        if (len == 0)
> +          {
> +            location sym_loc = text_loc;
> +            sym_loc.start.column += 1;
> +            sym_loc.end = sym_loc.start;
> +            const char *format =
> +              _("syntax error after `%c', expecting integer, letter,"
> +                " `_', or `['");
> +            complain_at_indent (sym_loc, &indent, format, dollar_or_at);
> +          }
> +        else if (midrule_rhs_index)
>           {
>             const char *format =
>               _("symbol not found in production before $%d: %.*s");
> diff --git a/tests/named-refs.at b/tests/named-refs.at
> index 25af97d..b3b4f5b 100644
> --- a/tests/named-refs.at
> +++ b/tests/named-refs.at
> @@ -561,3 +561,32 @@ test.y:57.8-17: invalid reference: `$<aa>[sym]'
>  test.y:54.1-57.21:  symbol not found in production: sym
>  ]])
>  AT_CLEANUP
> +
> +#######################################################################
> +
> +AT_SETUP([[$ or @ followed by . or -]])
> +AT_DATA([[test.y]],
> +[[
> +%%
> +start:
> +  .field { $.field; }
> +| -field { @-field; }
> +| 'a'    { @.field; }
> +| 'a'    { $-field; }
> +;
> +.field: ;
> +-field: ;
> +]])
> +AT_BISON_CHECK([[test.y]], [[1]], [],
> +[[test.y:4.12-18: invalid reference: `$.field'
> +test.y:4.13:        syntax error after `$', expecting integer, letter,
> `_', or `@<:@'
> +test.y:4.3-8:       possibly meant: $[.field] at $1
> +test.y:5.12-18: invalid reference: address@hidden'
> +test.y:5.13:        syntax error after `@', expecting integer, letter,
> `_', or `@<:@'
> +test.y:5.3-8:       possibly meant: @[-field] at $1
> +test.y:6.12-18: invalid reference: address@hidden'
> +test.y:6.13:        syntax error after `@', expecting integer, letter,
> `_', or `@<:@'
> +test.y:7.12-18: invalid reference: `$-field'
> +test.y:7.13:        syntax error after `$', expecting integer, letter,
> `_', or `@<:@'
> +]])
> +AT_CLEANUP
> --
> 1.7.0.4
>
>
>


-- 
Best regards,
Alex Rozenman (address@hidden).