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

Using the "not" keyword in variable names #115

Closed
eastlund opened this issue Apr 10, 2015 · 18 comments
Closed

Using the "not" keyword in variable names #115

eastlund opened this issue Apr 10, 2015 · 18 comments

Comments

@eastlund
Copy link
Contributor

When using the string "not" as a prefix of a variable name, interesting things happen.

See this example program:

class Main {
  def main() : void {
    let
      notice = "test"
    in
      print(notice)
  }
}

The output:

 *** Error during typechecking *** 
"notIssue.enc" (line 6, column 16)
Unbound variable 'ice'
In expression: 
  ice
In expression: 
  not ice
In expression: 
  print(not ice)
In expression: 
  let notice = "test"
  in
    print(not ice)
In expression: 
  {let notice = "test"
   in
     print(not ice)}
In method 'main' of type 'void'
In class 'Main'

The problem goes away if you add some char before "not", e.g. replacing "notice" with "a_notice".

@TheGrandmother
Copy link
Contributor

The space only gets inserted in if one uses not as a prefix. Tried with some of the other operators and keywords and it worked.

@EliasC
Copy link
Contributor

EliasC commented Apr 10, 2015

The reason for this is that not is an operator, and is thus handled by the expression parser that also handles +, ==, and etc. Operators generally don't need spaces (x+y == x + y), and since not is a prefix operator (actually our only prefix operator), notice is parsed just as ~ice would be parsed, if ~ was an operator. A similar symptom is noticable in the following code:

let anders = 42 in 
  true anders
 *** Error during typechecking *** 
Unbound variable 'ers'
In expression: 
  ers
In expression: 
  true and ers

The not case could be solved by moving it out of the expression parser, but I'm guessing there is a more general solution that would solve it all.

@TheGrandmother
Copy link
Contributor

@EliasC
Nope. It works for anders.
´´´
class Main {
def main() : void {
let
anders = "asd"
in
print(anders)
}
}
´´´
compiles and works

@EliasC
Copy link
Contributor

EliasC commented Apr 10, 2015

@TheGrandmother Check my example again. true anders is parsed as true and ers. This "only" gives an erroneous error message, but the cause is the same.

@TheGrandmother
Copy link
Contributor

@EliasC
Ahaa... i c.

Using my imaginary "knowlege" of the parser i traced it to this in Parser.hs

      opTable = [
                 [arrayAccess],
                 [prefix "not" Identifiers.NOT],
                 [op "*" TIMES, op "/" DIV, op "%" MOD],
                 [op "+" PLUS, op "-" MINUS],
                 [op "<" Identifiers.LT, op ">" Identifiers.GT, op "<=" Identifiers.LTE, op ">=" Identifiers.GTE, op "==" Identifiers.EQ, op "!=" NEQ],
                 [op "and" Identifiers.AND, op "or" Identifiers.OR],
                 [messageSend],
                 [typedExpression],
                 [chain],
                 [assignment]
                ]

not appears no be matched just to not and not taking any spaces into consideration.
Could one not just add white spaces to the matching?

@TheGrandmother
Copy link
Contributor

Just changing it to [prefix "not " Identifiers.NOT], fixes the problem. Although its pretty ugly.

@EliasC
Copy link
Contributor

EliasC commented Apr 10, 2015

@TheGrandmother Something like that*. Probably even making sure that the following character is not a letter (not(true) should be allowed).

*where "that" isn't your last comment :)

@TheGrandmother
Copy link
Contributor

How could we do that? Do we have any "regex" like stuffs in the parser?

@EliasC
Copy link
Contributor

EliasC commented Apr 10, 2015

We have a parser ;)

@TheGrandmother
Copy link
Contributor

I understand :P But how would one go about adding the check for something not being a letter?

@albertnetymk
Copy link
Contributor

Changing line 224 to this expression = expr <|> buildExpressionParser opTable expr?

@EliasC
Copy link
Contributor

EliasC commented Apr 10, 2015

@albertnetymk That breaks a bunch of other things it seems.

@albertnetymk
Copy link
Contributor

@EliasC Indeed. Need to deal with it systematically.

@albertnetymk
Copy link
Contributor

How about this?

expr = ...
     <|> notExpr
     <?> "expression"
  where
    ...
    notExpr = do
            pos <- getPosition
            reservedOp "not"
            x <- expression
            return $ Unary (meta pos) Identifiers.NOT x

@EliasC
Copy link
Contributor

EliasC commented Apr 10, 2015

@albertnetymk That would solve this particular instance of the problem, but it would be nice with a solution that is general enough to solve the problem for the infix operators as well (see above).

@albertnetymk
Copy link
Contributor

@EliasC Indeed. The problem seems a lexer problem, treating notx as not x instead of a whole word. The same goes for anders.

@EliasC
Copy link
Contributor

EliasC commented Apr 10, 2015

The solution turned out to be very simple! Here is the code for parsing prefix operators:

      prefix s operator = 
          Prefix (try(do pos <- getPosition
                         reservedOp s
                         return (\x -> Unary (meta pos) operator x)))

In our case s = "not" and operator is the data representing not. Since we're reading s (i.e. not) as a reservedOp, the whitespace rules for operators apply. Changing reservedOp to reserved changes it to the whitespace rules for keywords.

I think we should keep the prefix function as is and add a textualPrefix, and similarly for infix operators.

@albertnetymk
Copy link
Contributor

Neat. +1

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

No branches or pull requests

5 participants