bison-patches
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: symbolic names - an updated version


From: Alex Rozenman
Subject: Re: symbolic names - an updated version
Date: Sat, 23 May 2009 19:56:14 +0300

Hi Joel,

Thanks for your comments.
I attached another version of the patch. I added some testing (its ~10
test-cases now) and made other modification, mainly according to your
suggestions. See my comments below.


> The "misleading reference" concept has disappeared from your code.  For
> example:
>
>  start: foo foo.bar { $foo.bar; }
>  tmp.y:2.22-29: reference is invalid: `$foo.bar'
>  tmp.y:2.8-10:   refers to: $foo at $1
>  tmp.y:2.12-18:   possibly meant: $[foo.bar] at $2
>

Now, it writes "misleading" instead of "invalid". My algorithm for choosing
"misleading" versus "invalid" looks like the following:
a. no variants: "not found", see my comments below
b. single variant: "invalid" or OK
b. >1 variant, all variants correct: "ambiguous"
c. >1 variant, all variants wrong: "invalid"
d. >1 variant, some correct, some wrong: "misleading".


>
> > if_stmt1: IF expr[cond] THEN stmt[then] ELSE stmt.list[else] FI
> >           { $if_stmt1 = new IfStmt($cond1, $then.f1, $else); };
> > interpreter.ypp:67.36-41: symbol not found: `$cond1'
> I still think it would be more consistent to say "reference is invalid"
> here.  Also, in other cases, "symbol not found" doesn't make sense anyway:
>  start: { $foo.bar; }
>  tmp.y:2.10-17: symbol not found: `$foo.bar'
> Bison does not parse $foo.bar as a symbol.  Bison parses it as a reference
> to a field of a semantic value of a symbol, and that reference is invalid.
>
Theoretically, I can agree that "$foo" is not a symbol - it is a reference.
I agree that "symbol not found: $foo.bar" is not 100% correct. But, it very
clear to me, that we just *must* explain what is the real problem in this
case. The real problem is that the reference cannot be resolved because a
symbol was not found. We must remember that the "not found" case is the most
important error for 90% of users 90% of the time.
My current implementation handles it like the following:
start: { $foo.bar; }
not_found.y:3.10-17: reference is invalid: `$foo.bar', symbol not found


> > if_stmt10: IF expr[cond] THEN stmt[stmt.x] FI
> >           { $if_stmt10 = new IfStmt($cond, $stmt.x, 0); };
> > interpreter.ypp:94.44-50: reference is invalid: `$stmt.x'
> > interpreter.ypp:93.36-41:   possibly meant: $[stmt.x].x at $4, `stmt' is
> > hidden
> > interpreter.ypp:93.36-41:   possibly meant: $[stmt.x] at $4
>
The first "possibly meant" seems confusing.  I think giving the exact
> hidden reference makes the connection more immediately obvious:
>   interpreter.ypp:94.44-50: reference is invalid: `$stmt.x'
>   interpreter.ypp:93.36-41:   possibly meant: $[stmt.x].x hiding $[stmt].x
> at $4
>   interpreter.ypp:93.36-41:   possibly meant: $[stmt.x] at $4
>
Now:
errors.y:46.44-50: reference is invalid: `$stmt.x'
errors.y:45.36-41:   possibly meant: $[stmt.x].x, hiding $stmt.x at $4
errors.y:45.36-41:   possibly meant: $[stmt.x] at $4


> > I sent my documents to FSF a two weeks ago, I hope they're already there.
> I don't see your information listed yet.  Let's give it a few more days,
> and then I'll ask about it.  In my experience, it can take more than two
> weeks.

I got the confirmation. I think the process is completed.

Three points:
1. There is a failed test case "Numbered tokens" in the first group. It also
fails without my modifications.
2. I also tried to add more test cases for not balanced brackets and for an
EOF after open bracket, but I was not able to write correct M4 code. When I
had an unbalanced brackets in the example grammar, M4 failed. I tried to
change quoting characters by "changequote" macro, but it is also failed, I
am not 100% understand why. I decided not to spend time on this, but it is
crucial to have a test case for these issues.
3. I wanted to say one more time: I don't like so much the current
formulation of error messages. In my opinion it's hard to understand. The
good thing is that most of the users will barely face these messages. It
seems important to hear some real feedback here. So let's push it :)

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

Attachment: May-23.git.patch
Description: Text Data


reply via email to

[Prev in Thread] Current Thread [Next in Thread]