Regenerate Lexer.hs with latest Alex (fix #8892) #8896

hsyl20 · 2023-04-04T07:33:55Z

Regenerate Lexer.hs with Alex 3.2.7.2 to fix issue ##8892 (out-of-bound access due to haskell/alex#223).

Please include the following checklist in your PR:

Patches conform to the coding conventions.
Any changes that could be relevant to users have been recorded in the changelog.
The documentation has been updated, if necessary.

Please also shortly describe how you tested your change. Bonus points for added tests!

ulysses4ever

Thanks! Sounds like something worth backporting to 3.10, @Mikolaj ?

Mikolaj

Yes, definitely, let's backport.

I guess some old test results require a bit of tweaking, too.

Mikolaj · 2023-04-04T18:47:34Z

@mergify backport 3.10

mergify · 2023-04-04T18:47:47Z

backport 3.10

✅ Backports have been created

#8921 Regenerate Lexer.hs with latest Alex (fix #8892) (backport #8896) has been created for branch 3.10

hsyl20 · 2023-04-14T15:18:03Z

I've updated to require Alex 3.2.7.3 which doesn't generate files that require PatternGuards (see upstream ticket)

ulysses4ever · 2023-04-15T14:24:45Z

@hsyl20 when you have time, could you look into the test failure? I guess, just updating the reference should be enough? (I haven't looked too close though) Updating (via --accept) is described in cabal-testsuite/README.md.

hsyl20 · 2023-04-17T09:38:35Z

I've updated the test. However why did we have to update this? It could be an Alex bug: previously, two unbreakable whitespaces were correctly reported at column 3; now they are reported at column 5, which makes no sense except if column is expressed in bytes instead of characters.

hsyl20 · 2023-04-17T10:22:12Z

The new failure seems to confirm this.

We use a Latin1 generated parser with Alex, but we also parses Unicode BOM, unbreakable spaces, etc. In recent Alex, the reported column isn't expressed in Unicode chars anymore but in bytes/ASCII chars (probably due to haskell/alex@ae525e3 but I haven't checked), which broke our tests (see haskell#8896). To work around this we report indentation warnings at token start position, instead of token end position (i.e. always 1). Otherwise position makes no sense anymore for the user.

hsyl20 · 2023-04-17T12:15:02Z

New commit with message:

Lexer: report indentation warnings at column 1

We use a Latin1 generated parser with Alex, but we also parses Unicode BOM, unbreakable spaces, etc. In recent Alex, the reported column isn't expressed in Unicode chars anymore but in bytes/ASCII chars (probably due to haskell/alex@ae525e3 but I haven't checked), which broke our tests (see #8896).

To work around this we report indentation warnings at token start position, instead of token end position (i.e. always 1). Otherwise position makes no sense anymore for the user.

I've tried to switch to a utf8 lexer instead of latin1 but it makes many more tests fail. I don't even know if .cabal files are supposed to be UTF-8 though.

ulysses4ever · 2023-04-17T12:51:52Z

This all sounds a bit worrying. @andreasabel, as the author of the corresponding alex commit, maybe you have a feeling for what's going on here?

For the sake of discussion, maybe we should preserve links to failing workflows. I know they will die at some point, but at least it gives us some time to contemplate on different approaches.

andreasabel · 2023-04-17T13:40:22Z

Cabal-syntax/src/Distribution/Fields/Lexer.hs

-                new_s = if GTE(offset,0#) && EQ(check,ord_c)
+                new_s = if GTE(offset,ILIT(0))
+                          && let check  = alexIndexInt16OffAddr alex_check offset
+                             in  EQ(check,ord_c)


This comes from this Alex commit: haskell/alex@e96c347 (#224)

I do not see how this should have introduced the problems we are seeing.
GTE ILIT and EQ are macros, yes, but they do not have side effects, so moving the check binding into a local let should not be a semantic change except making things lazier.

andreasabel · 2023-04-17T13:41:56Z

This all sounds a bit worrying. @andreasabel, as the author of the corresponding alex commit, maybe you have a feeling for what's going on here?

I currently do not have time to dig deeper, but I think the latest alex commit is not to be blamed, see my comment: https://github.com/haskell/cabal/pull/8896/files#r1168715008

hsyl20 · 2023-04-17T13:56:27Z

Yes I also think it's just the result of this change: haskell/alex@ae525e3#diff-007f894e1221eb8cafde8fdf0ee317bd32859704c27652c29cbb5417a9d5c37dR179-R185

Which has been introduced in Alex 3.2.7

We use a Latin1 generated parser with Alex, but we also parses Unicode BOM, unbreakable spaces, etc. In recent Alex, the reported column isn't expressed in Unicode chars anymore but in bytes/ASCII chars (probably due to haskell/alex@ae525e3 but I haven't checked), which broke our tests (see haskell#8896). To work around this we report indentation warnings at token start position, instead of token end position (i.e. always 1). Otherwise position makes no sense anymore for the user.

andreasabel · 2023-04-17T14:17:29Z

Ok, this was 3 years ago, I don't remember much of it.
If I introduced a bug in Alex, please report it.
But maybe the source here needs some adjustment to work with Alex 3.2.7. If this is the case, the Alex violates the PVP, please also report it then.

ulysses4ever · 2023-04-18T01:26:49Z

Technically, this has two approvals and can be merged: the author is expected to add either merge-me or merge+squash label to the PR. But the alex issue is concerning. Indeed, may be worth reporting...

Cabal-tests/tests/ParserTests/regressions/Octree-0.5.format

hsyl20 · 2023-04-18T08:40:37Z

I've opened an upstream ticket: haskell/alex#227

Regenerate Lexer.hs with Alex 3.2.7.2 to fix issue #haskell#8892 (out-of-bound access due to haskell/alex#223).

We use a Latin1 generated parser with Alex, but we also parses Unicode BOM, unbreakable spaces, etc. In recent Alex, the reported column isn't expressed in Unicode chars anymore but in bytes/ASCII chars (probably due to haskell/alex@ae525e3 but I haven't checked), which broke our tests (see haskell#8896). To work around this we report indentation warnings at token start position, instead of token end position (i.e. always 1). Otherwise position makes no sense anymore for the user.

We use a Latin1 generated parser with Alex, but we also parses Unicode BOM, unbreakable spaces, etc. In recent Alex, the reported column isn't expressed in Unicode chars anymore but in bytes/ASCII chars (probably due to haskell/alex@ae525e3 but I haven't checked), which broke our tests (see #8896). To work around this we report indentation warnings at token start position, instead of token end position (i.e. always 1). Otherwise position makes no sense anymore for the user. (cherry picked from commit 5f72880)

Regenerate Lexer.hs with latest Alex (fix #8892) (backport #8896)

andreabedini · 2023-04-27T08:31:51Z

@hsyl20

I don't even know if .cabal files are supposed to be UTF-8 though.

The documentation confirms they are UTF-8.

The package description file must have a name ending in “.cabal”. It must be a Unicode text file encoded using valid UTF-8.

hsyl20 · 2023-04-27T09:49:29Z

So in theory we could use Alex unicode support. But since then I've read in https://amelia.how/posts/parsing-layout.html that:

Nobody uses the Unicode support, not even GHC: The community wisdom is to trick Alex into reading Unicode by compressing Unicode classes down into high byte characters. Yeah, yikes.

- this matches GHC CI - Fixes a bug in the JS backend see - https://gitlab.haskell.org/ghc/ghc/-/issues/22356 - haskell/cabal#8896 - https://gitlab.haskell.org/ghc/ghc/-/merge_requests/11444#note_552521https://gitlab.haskell.org/ghc/ghc/-/merge_requests/11444#note_552521

hsyl20 force-pushed the hsyl20/fix-lexer branch 2 times, most recently from 779f6dc to b5717e6 Compare April 4, 2023 08:14

ulysses4ever approved these changes Apr 4, 2023

View reviewed changes

Mikolaj approved these changes Apr 4, 2023

View reviewed changes

Mikolaj added the attention: needs-backport 3.12 label Apr 4, 2023

hsyl20 force-pushed the hsyl20/fix-lexer branch from b5717e6 to 8a34cb2 Compare April 14, 2023 15:16

andreasabel reviewed Apr 17, 2023

View reviewed changes

hsyl20 force-pushed the hsyl20/fix-lexer branch from cef2d51 to 4574b38 Compare April 17, 2023 13:57

andreasabel reviewed Apr 18, 2023

View reviewed changes

Cabal-tests/tests/ParserTests/regressions/Octree-0.5.format Show resolved Hide resolved

hsyl20 mentioned this pull request Apr 18, 2023

Alex PVP violation? haskell/alex#227

Closed

hsyl20 added the merge me Tell Mergify Bot to merge label Apr 18, 2023

mergify bot added the merge delay passed Applied (usually by Mergify) when PR approved and received no updates for 2 days label Apr 20, 2023

hsyl20 added 2 commits April 26, 2023 11:48

Regenerate Lexer.hs with latest Alex (fix haskell#8892)

ca7a8e2

Regenerate Lexer.hs with Alex 3.2.7.2 to fix issue #haskell#8892 (out-of-bound access due to haskell/alex#223).

hsyl20 force-pushed the hsyl20/fix-lexer branch from 4574b38 to 5f72880 Compare April 26, 2023 11:49

mergify bot merged commit 3af1731 into haskell:master Apr 26, 2023

mergify bot mentioned this pull request Apr 26, 2023

Regenerate Lexer.hs with latest Alex (fix #8892) (backport #8896) #8921

Merged

mergify bot added a commit that referenced this pull request Apr 26, 2023

Merge pull request #8921 from haskell/mergify/bp/3.10/pr-8896

4bfd6a0

Regenerate Lexer.hs with latest Alex (fix #8892) (backport #8896)

ulysses4ever mentioned this pull request May 29, 2023

Warn about inconsistent indentation #8975

Closed

Mikolaj removed the attention: needs-backport 3.12 label Jun 17, 2023

doyougnu mentioned this pull request Mar 12, 2024

alex: bump to 3.2.7 alpmestan/ghc.nix#190

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Regenerate Lexer.hs with latest Alex (fix #8892) #8896

Regenerate Lexer.hs with latest Alex (fix #8892) #8896

hsyl20 commented Apr 4, 2023

ulysses4ever left a comment

Mikolaj left a comment

Mikolaj commented Apr 4, 2023

mergify bot commented Apr 4, 2023 •

edited

Loading

hsyl20 commented Apr 14, 2023

ulysses4ever commented Apr 15, 2023

hsyl20 commented Apr 17, 2023

hsyl20 commented Apr 17, 2023

hsyl20 commented Apr 17, 2023 •

edited

Loading

ulysses4ever commented Apr 17, 2023

andreasabel Apr 17, 2023

andreasabel commented Apr 17, 2023

hsyl20 commented Apr 17, 2023

andreasabel commented Apr 17, 2023

ulysses4ever commented Apr 18, 2023

hsyl20 commented Apr 18, 2023

andreabedini commented Apr 27, 2023

hsyl20 commented Apr 27, 2023

Regenerate Lexer.hs with latest Alex (fix #8892) #8896

Regenerate Lexer.hs with latest Alex (fix #8892) #8896

Conversation

hsyl20 commented Apr 4, 2023

ulysses4ever left a comment

Choose a reason for hiding this comment

Mikolaj left a comment

Choose a reason for hiding this comment

Mikolaj commented Apr 4, 2023

mergify bot commented Apr 4, 2023 • edited Loading

✅ Backports have been created

hsyl20 commented Apr 14, 2023

ulysses4ever commented Apr 15, 2023

hsyl20 commented Apr 17, 2023

hsyl20 commented Apr 17, 2023

hsyl20 commented Apr 17, 2023 • edited Loading

ulysses4ever commented Apr 17, 2023

andreasabel Apr 17, 2023

Choose a reason for hiding this comment

andreasabel commented Apr 17, 2023

hsyl20 commented Apr 17, 2023

andreasabel commented Apr 17, 2023

ulysses4ever commented Apr 18, 2023

hsyl20 commented Apr 18, 2023

andreabedini commented Apr 27, 2023

hsyl20 commented Apr 27, 2023

mergify bot commented Apr 4, 2023 •

edited

Loading

hsyl20 commented Apr 17, 2023 •

edited

Loading