Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Make parens mandatory after with - vastly improving the error message #504

Merged
merged 2 commits into from Oct 1, 2013

Conversation

david-christiansen
Copy link
Contributor

Right now, parentheses are required around any non-atomic expressions that are
the scrutinee of a with-rule. In practice, this means that almost all
expressions require parentheses, as with is typically used to introduce more
information that was not available in the outer pattern match.

In the standard library, the only place where with rules occur without
parentheses is in Data.Bits.

Making the parentheses mandatory vastly improves parser errors in case they
are forgotten, RE #166 .

Sample of current message:

./WithRule.idr:4:14: error: expected: "{",
    function declaration
foo x with x == 0
             ^

Sample of message with the first patch:

./WithRule.idr:4:12: error: expected: "("
foo x with x == 0
           ^

Sample of message with the second patch as well:

./WithRule.idr:4:12: error: expected: parenthesized expression
foo x with x == 0 
           ^ 

Right now, parentheses are required around any non-atomic expressions that are
the scrutinee of a with-rule. In practice, this means that almost all
expressions require parentheses, as with is typically used to introduce more
information that was not available in the outer pattern match.

In the standard library, the only place where with rules occur without
parentheses is in Data.Bits.

Making the parentheses mandatory vastly improves parser errors in case they
are forgotten, RE idris-lang#166 .

Sample of current message:
./WithRule.idr:4:14: error: expected: "{",
    function declaration
foo x with x == 0
             ^

Sample of message with this patch:
./WithRule.idr:4:12: error: expected: "("
foo x with x == 0
           ^
Now, instead of saying "missing '('", the parser says "missing parenthesized
expression".
@edwinb
Copy link
Contributor

edwinb commented Sep 30, 2013

Personally I'd prefer not to require parens - if anything I'd like to find a way to go the other way and make parens optional for complex expressions but various things make that difficult (including, as you say, error messages).

Nevertheless, it's a good point about error messages, and I think it's pretty unlikely that you'd want an atomic expression in most cases, so maybe we should do this...

Um. Ah. Open to persuasion :).

@david-christiansen
Copy link
Contributor Author

Another possibility is to make parens mandatory for anything on more than
one line, perhaps, and go away from the strict off-side rule in this case.

I would prefer that as well, actually, but if we don't quickly find a way
to do that cleanly then I'd advocate for mandatory parens now and a way to
relax it later.

I guess another option is a token separating the scrutinee from the
subcases, like the of in case expressions. Perhaps =>? This would be a
much bigger change though, and it would break a lot of code.
Den 30/09/2013 05.56 skrev "Edwin Brady" notifications@github.com:

Personally I'd prefer not to require parens - if anything I'd like to find
a way to go the other way and make parens optional for complex expressions
but various things make that difficult (including, as you say, error
messages).

Nevertheless, it's a good point about error messages, and I think it's
pretty unlikely that you'd want an atomic expression in most cases, so
maybe we should do this...

Um. Ah. Open to persuasion :).


Reply to this email directly or view it on GitHubhttps://github.com//pull/504#issuecomment-25348946
.

@ahmadsalim
Copy link

@edwinb Is there any reason it should be a simpleExpr instead of an expr (i.e. is it intentional that it does not support operators or similar?)

edwinb added a commit that referenced this pull request Oct 1, 2013
Make parens mandatory after with - vastly improving the error message
@edwinb edwinb merged commit 45e0a55 into idris-lang:master Oct 1, 2013
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants