Conversation
|
The lexer works properly. All positions and tokens are resolved correctly, as documented by tests. Error handling needs improvements, but other than that, code is pretty much ready for review. |
dominik-muc
left a comment
dominik-muc
left a comment
There was a problem hiding this comment.
Looks like it's going in the right direction. I've left some nitpicks and possible discussion topic. It's a shame I wont be able to attend those. Good luck!
| , stack = emptyStack | ||
| } | ||
|
|
||
| pub let getTok (LexerState {caret, start, stack}) = |
There was a problem hiding this comment.
I think handling effects here just to expose a state-monad-like API to the user misses it's point. Why not make an abstract handler for lex, pos and brackets at let user decide where to handle it? I think it will be easier to use. I also wouldn't handle fatal errors here. Speaking of that, I think this „raise” effect should be a part of stdlib (and lexer should use the stdlib version rather than defining its own)
There was a problem hiding this comment.
No it wont be any easier. Literally this interface exposes LexerState constructor and iteration function. It does not get any easier than this.
| import List as L | ||
| import String as S | ||
|
|
||
| # dubugs |
There was a problem hiding this comment.
What's the purpose of this? Did you mean debugs?
| end | ||
|
|
||
| # ----------------------------------------------------------------------------- | ||
| # Fatal errors |
There was a problem hiding this comment.
I would move this to stdlib, see other comments.
There was a problem hiding this comment.
Soon this effect will be in stdlib
| import open Parser/Lexer | ||
| import List | ||
|
|
||
| method equal (t1 : Token) (t2 : Token) = |
There was a problem hiding this comment.
I would have loved deriving Eq here... Are macros ever coming to Fram? Should they though? Maybe some other solution? I think we should discuss this.
There was a problem hiding this comment.
Feel free to implement this attribute. I will gladly use it
There was a problem hiding this comment.
What is that? I don't think we want bugs in the compiler...
There was a problem hiding this comment.
My bad. I was reproducing issue related to recursive varaible inference
| else | ||
| raise "Invalid octal digit" | ||
|
|
||
| let parseBinDigit (dgt : Char) = |
There was a problem hiding this comment.
let parseBinDigit (dgt : Char) =
if '0' == dgt then 0
else if '1' == dgt then 1
else raise "Invalid binary digit"
Would be way better if we had literal pattern matching. This applies to so many functions here.
There was a problem hiding this comment.
It soon will be possible... heopefully
| (let digit = num.get i in | ||
| let acc = acc * base + parser digit in | ||
| if acc < 0 then | ||
| raise "Number is too big!" |
There was a problem hiding this comment.
Maybe „Literal out of bounds” or something like that?
Also, I would define error strings somewhere else, so they can be easily changed and reused.
| else if c == 'a' then '\a' | ||
| else if c == 'f' then '\f' | ||
| else if "xX".mem c then | ||
| (let ch1 = forcePopChar () in |
There was a problem hiding this comment.
I overlooked this before, but why do we need parentheses here? Is it the only way to solve dangling else? It looks horrific. Maybe we should look into python's elif?
There was a problem hiding this comment.
This issue is related to out parser. It can be solved, no one is willing to do this
| in | ||
| iter st | ||
|
|
||
| # This annotation speeds up tests massively! |
There was a problem hiding this comment.
Why? This doesn't seem intended.
There was a problem hiding this comment.
It stops propagation of effects related to Testing framework.
| method show (Tok {token, pos}) = | ||
| "Tok {token=\{token.show}, pos=\{pos.show}}" | ||
|
|
||
| let unwrapToksWithPos str = |
There was a problem hiding this comment.
Just use „Tokens”? Or „unwTksWPos”?
2 participants
I have decided to drop controversial backtracking effect and build lexer with more robust functions