search

haskell / alex

A lexical analyser generator for Haskell
https://hackage.haskell.org/package/alex
BSD 3-Clause "New" or "Revised" License
298 stars 82 forks source link

Fix #197 by only lexing numeric literals in multiplicity expressions. #202

Closed andreasabel closed 2 years ago

andreasabel commented 2 years ago

Fix #197 by only lexing numeric literals in multiplicity expressions.

In issue #141, multiplicity annotations in regexes where extended to the general, multi-digit case {nnn,mmm}. However, lexing numeric literals broke parsing of regexes like:

   32|64
   [01-89]

The solution here is to only lex numeric literals in a special lexerstate called multiplicity which is entered by the parser when parsing multiplicity braces {nnn,mmm}.

This restores alex' handling of digits as characters in the non-multiplicity situations.

andreasabel commented 2 years ago

Why is the new testcase failing with error make: *** No rule to make target 'issue_197.n.run', needed by 'all'. ? https://github.com/simonmar/alex/runs/4914643015?check_suite_focus=true#step:22:122

andreasabel commented 2 years ago

@Ericson2314 , could you please review this solution attempt?

andreasabel commented 2 years ago

... any idea for CI failures?

I am clueless. It also happened on your PR #200: https://github.com/simonmar/alex/runs/4911840593?check_suite_focus=true#step:22:132

Locally, on my machine, it works fine.

If I locally delete tests/issue_197.x. I get the same errors as the CI. So, it is almost as if this file is not present when CI is run. How is that possible? It is clearly included in the commit.

Ericson2314 commented 2 years ago

I am having even finding the error in the CI log, you might be ahead of me there?

andreasabel commented 2 years ago

I am having even finding the error in the CI log, you might be ahead of me there?

Does clicking the links I pasted not work?

Ericson2314 commented 2 years ago

OK sorry that does work, and while Control-F does poorly, github's log search for 197 works too so I can find it in both PRs. All good now.

Ericson2314 commented 2 years ago

I found the bug: test file needs to be listed in cabal file to go in sdist. Will push fix in a second.