logos: Silent ambiguous matches
As it stands now, ambiguous match in logos seems to produce last encountered token. I had a typo in a regex which caused two different tokens tokens to match at the same time:
#[token=" "]
White
#[token=" [\n ]*"
LongWhite
...
The outcome was that whenever ambiguity was encountered Logos silently did not update the token field. It is this expected/documented behaviour? I guess the speed is of concern, and ideally we don’t want to disambiguate at runtime, but perhaps at some point compile time checking could be introduced.
About this issue
- Original URL
- State: closed
- Created 6 years ago
- Comments: 17 (16 by maintainers)
Commits related to this issue
- Edition 2018 + fixes (#41) * Edition 2018, new broken test for #40 * Fix #42 * Bump versions and `toolshed` — committed to maciejhirsz/logos by maciejhirsz 6 years ago
Great, thanks for lighting fast fix! Will sure report everything I will find since I am switching lexing exclusively to Logos. The “unusual” Hoon syntax is apparently good at breaking things 😉
@mikolajpp released 0.9.2 with a fix. Thanks again for being patient with this and adding the test case. The fact that we were able to isolate few cases where things progressively break helped a lot, since there were a couple interesting things happening in the branching algo here:
~(m|h|s)
when a branch~[a-z]
is present, an intersection should have been detected, but that only worked while the~
token was absent. Adding~
definition turned the fork on second byte into aMaybe
variant which disabled this (this is now fixed with 0.9.2).~[
, where[
does not belong in the seta-z
disabled that part.(m|h|s)
worked (because it creates 3 single byte branches), but[mhs]
didn’t (because it created a single branch with a class pattern).To make things properly robust in the future:
Feel free to open a new issue if you find anything else!
This bug has something to do with Logos’ handling of regexes. This works:
This does not work
This also does not work
There seems to be crucial difference wrt how classes are handled v explicit forks. I have put test which triggers this into a branch here: https://github.com/mikolajpp/logos/tree/common-fork-bug
Right! Those are definitely bugs, thanks for reporting!