[Top][All Lists]
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: Improve error messages for `$' or `@' followed by `.' or `-'.
From: |
Alex Rozenman |
Subject: |
Re: Improve error messages for `$' or `@' followed by `.' or `-'. |
Date: |
Sun, 9 Jan 2011 22:54:19 +0200 |
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).