search

brownplt / pyret-lang

The Pyret language.
Other
1.06k stars 109 forks source link

bad error message for poorly-named function #255

Open shriram opened 10 years ago

shriram commented 10 years ago

It's perfectly reasonable to write

fun check(x): x == 3 end

(I know, because by accident I did.) The error ought to say something about check not being usable as a function name. Instead it says 

Pyret didn't understand your program around definitions: line 1, column 4
blerner commented 10 years ago 
That's a parse error, since check is a keyword, so you get a parsing
error.  On 8/19/2014 10:56 PM, Shriram
  Krishnamurthi wrote:

  It's perfectly reasonable to write
  fun check(x): x == 3 end

  (I know, because by accident I did.) The error ought to say
    something about check not being usable as a
    function name. Instead it says 
  Pyret didn't understand your program around definitions: line 1, column 4

  —
    Reply to this email directly or view
      it on GitHub.
shriram commented 10 years ago

My subject line said "bad error message". This is a BAD error message. A good one would say "check is a keyword, you can't use it here".

jpolitz commented 10 years ago

Ben is saying that we don't know how to make this better right now without writing a by-hand parser or something. It's an outstanding issue with the difficulty of parsing and grammar maintenance. On Aug 19, 2014 11:00 PM, "Shriram Krishnamurthi" notifications@github.com wrote:

My subject line said "bad error message". This is a BAD error message. A good one would say "check is a keyword, you can't use it here".

— Reply to this email directly or view it on GitHub https://github.com/brownplt/pyret-lang/issues/255#issuecomment-52728726.

jpolitz commented 10 years ago

(It's still an issue, but it won't get fixed right away.) On Aug 19, 2014 11:04 PM, "Joe Gibbs Politz" joe.politz@gmail.com wrote:

Ben is saying that we don't know how to make this better right now without writing a by-hand parser or something. It's an outstanding issue with the difficulty of parsing and grammar maintenance. On Aug 19, 2014 11:00 PM, "Shriram Krishnamurthi" < notifications@github.com> wrote:

My subject line said "bad error message". This is a BAD error message. A good one would say "check is a keyword, you can't use it here".

— Reply to this email directly or view it on GitHub https://github.com/brownplt/pyret-lang/issues/255#issuecomment-52728726 .

blerner commented 10 years ago 
Right.  There are an enormous number of ways to misuse keywords,
because all our keywords are valid identifiers.  Catching all those
cases requires abandoning the parser we have and writing a totally
new one that tries to diagnose parse errors, and that just isn't
feasible (or terribly reliable) right now.
One thing we can do is add another suggestion to the More... link in
the error message.  Currently it suggests "The program is missing
something" and "The program contains something extra"; to that we
could add "The program misuses a keyword".On 8/19/2014 11:04 PM, Joe Politz
  wrote:
(It's still an issue, but it won't get fixed right
  away.)

  On Aug 19, 2014 11:04 PM, "Joe Gibbs Politz"
  <joe.politz@gmail.com> wrote:

  > Ben is saying that we don't know how to make this better
  right now without

  > writing a by-hand parser or something. It's an outstanding
  issue with the

  > difficulty of parsing and grammar maintenance.

  > On Aug 19, 2014 11:00 PM, "Shriram Krishnamurthi" <

  > notifications@github.com> wrote:

  >

  >> My subject line said "bad error message". This is a BAD
  error message. A

  >> good one would say "check is a keyword, you can't use it
  here".

  >>

  >> —

  >> Reply to this email directly or view it on GitHub

  >>

https://github.com/brownplt/pyret-lang/issues/255#issuecomment-52728726

.

  >>

  >
  —
    Reply to this email directly or view
      it on GitHub.
shriram commented 7 years ago

Mmph. This hasn't yet improved. Not to reopen the whole thread, but maybe related to @jpolitz's recent question about hand-parsing…

schanzer commented 7 years ago

FWIW, this is now worse on horizon than it is on CPO: Internal errors prevented this error message from being shown. Please report this as a bug.

jswrenn commented 7 years ago

@schanzer I'm not able to replicate that in the definitions or interactions pane of the http://pyret-horizon.herokuapp.com/ instance.

blerner commented 7 years ago

This has not regressed on CPO; the error message is the same. Fixing this requires rewriting the parser from scratch, or writing an ungrammar. It's also ambiguous at the point of the parse error whether the mistake is the check or the fun keyword (requires lookahead to resolve). The parser only keeps track of the token on which parsing failed. It could potentially look ahead one token (so it would see the check and the lparen tokens in this case), but it can't look back to see the fun token. There's still no simple fix for this one.