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

Why does the lexer allow relations that start with an underscore? #1163

Closed
stefjoosten opened this issue Apr 6, 2021 · 3 comments
Closed
Assignees
Labels
bug Indicates an unexpected problem or unintended behavior component: compiler component:parser priority:normal Syntax

Comments

@stefjoosten
Copy link
Contributor

stefjoosten commented Apr 6, 2021

What happened

I noticed that a relation name in Ampersand may start with an underscore. The compiler accepts the following code fragment:

RELATION _takes [Student*Course]
MEANING "A student takes a complete course"

The compiler lets me work with this underscored identifier without problems.

What I expected

I expected an error because the documentation states clearly that relation names start with a lower case letter.

Version of ampersand that was used

Ampersand-v4.1.4 [no git info], build time: 10-Mar-21 13:53:13 UTC

Steps to reproduce

  1. Log into RAP4
  2. compile an arbitrary script that has a relation with a name that starts with an underscore.

Analysis

I found this error by code inspection.
I traced it back to the lexer, in the function isIdStart:
image
It is used in the following code fragment in file lexer.hs:

mainLexer p cs@(c:s)
     | isIdStart c || isUpper c
         = let (name', p', s')    = scanIdent (addPos 1 p) s
               name''               = c:name'
               tokt   | iskw name'' = LexKeyword name''
                      | otherwise = if isIdStart c
                                    then LexVarId name''
                                    else LexConId name''
           in returnToken tokt p mainLexer p' s'

It is clear that the function isIdStart is deliberate. So I wonder why it was inserted. Maybe @DanielSchiavini knows?
In the meantime I suggest we treat this like an error and fix it. I propose the following change:

image

This change is ready for testing in commit 7c4d1ac

@DanielSchiavini
Copy link
Collaborator

@stefjoosten I think it was a wrong assumption on my side as most programming languages accept underlines.

@stefjoosten
Copy link
Contributor Author

Thanks, @DanielSchiavini, we'll fix it.

@stefjoosten stefjoosten added bug Indicates an unexpected problem or unintended behavior and removed question Indicates that an issue or pull request needs more information labels Apr 6, 2021
stefjoosten added a commit that referenced this issue Jun 7, 2021
@hanjoosten
Copy link
Member

@stefjoosten , I fixed the readability of the error message that is thrown due to your latest change (02e04a0). Please fix this bug yourself, and make the ci pipeline happy 😃.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Indicates an unexpected problem or unintended behavior component: compiler component:parser priority:normal Syntax
Projects
None yet
Development

No branches or pull requests

3 participants