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.

Sign up for GitHub

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

Use -XNoStrictData in generated parser (#273) #289

Merged
merged 2 commits into from
Sep 13, 2024
Merged

Use -XNoStrictData in generated parser (#273) #289

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
62a265b
Use -XNoStrictData in generated parser (#273)
sgraf812 Sep 13, 2024
c5a6328
Bump lower bound on base to 4.9
sgraf812 Sep 13, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions ChangeLog.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,10 @@

The main focus of this release was modularizing Happy.

* Generated parsers now activate the language extension `-XNoStrictData` without
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

-X(No)StrictData is available since GHC-8.0, so this change means that happy won't anymore generate code which works with older GHCs.

As far as I'm concerned, that is fine, but it's probably a good idea to tell explicitly in the changelog.

Copy link
Contributor

@phadej phadej Sep 13, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

... you don't test master with older than GHC-8.0, though it might be still buildable with older GHCs? If it weren't, then using new happy with old GHCs will become very unlikely.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The trouble is that it has become quite hard to test 7.10 and below in CI. ghcup doesn't provide it and I'm not a huge fan of manually setting it up. Besides it is not unlikely that it won't work due to some glibc incompatibility.

Besides, GHC 8.0 is quite ancient at this point and I would be happy to drop support for 7.10 and below. The cabal file says as much already, but perhaps we should announce it in the release notes as well.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The cabal file says as much already,

It doesn't

It only says that you test with GHC-8.0 and up, not that you don't support oldere GHCs. E.g. you have

  build-depends: base < 5,
                 array,
                 ...

so technically cabal would try compile even with GHC-7.0 etc.

And it actually fails, if you test it:

Building library for happy-backend-glr-2.0..
ghc: unrecognised flag: -Wincomplete-uni-patterns

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Anyone stuck on GHC < 8.0 is probably also fine with happy < 2.0. Let's not waste time on legacy compiler versions.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@int-index Correct the metadata. Write lower-bounds. base >=4.9. Not a hard thing to do. It's not a waste of time.

Copy link
Collaborator

@int-index int-index Sep 13, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Dependency bounds for the generator itself and dependency bounds for the generated code aren't the same thing. base >=4.9 is correct as long as happy itself builds with base >=4.9.

I agree that we should bump this bound because, as you point out, happy-backend-glr doesn't build with older GHCs. However it's a separate problem unrelated to this PR.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As I said, if happy itself is not buildable with older GHCs then it's make sense it isn't able to generate code for older GHCs.

It could be so that happy is buildable only with newer GHCs (say even GHC-9+ only) but supports code generation for older GHCs. (It would be trickier to test but not impossible. E.g. cabal is buildable with only newer GHCs, but can work with older than that).

It would be somewhat confusing if happy would be buildable with old GHCs, but not be able to generate code for those old GHCs. Given that cabal-install builds happy itself (doesn't reuse some global installation), those users would find themselves if complicated situations.

So, IMHO, it makes everything somewhat simpler if happy itself and generated code has the same support GHC windows. "If you can build a happy version with GHC-x.y, that happy will generate code which GHC-x.y will handle".

In particular, if/when GHC introduces a change, such that happy itself is still buildable, but code is not, I'd argue that happy shouldn't be buildable.

There is no way to declare in happy.cabal dependency metadata for the generated code, so at large it's simpler if happy's own build metadata is (re)used also as the generated code dependency metadata. (Similarly as we are currently imprecise in multi-staged or cross-compilation scenarios, using the same bounds for all stages and host&target).

which every use of a happy parser would lead to an immediate crash (#273).
This causes us to drop support for GHC < 8.0.

* Break up into libraries:

- `happy-backend-glr`
Expand Down
4 changes: 2 additions & 2 deletions happy.cabal
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -140,7 +140,7 @@ executable happy
hs-source-dirs: src
main-is: Main.lhs

build-depends: base < 5,
build-depends: base >= 4.9 && < 5,
array,
containers >= 0.4.2,
mtl >= 2.2.1,
Expand All @@ -164,5 +164,5 @@ test-suite tests
build-tools: happy
ghc-options: -threaded

build-depends: base < 5, process < 1.7
build-depends: base >= 4.9 && < 5, process < 1.7
default-language: Haskell98
2 changes: 1 addition & 1 deletion packages/backend-lalr/src/Happy/Backend/LALR.hs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ importsToInject debug = concat ["\n", import_array, import_bits, import_glaexts,
"import Control.Monad (ap)\n"

langExtsToInject :: [String]
langExtsToInject = ["MagicHash", "BangPatterns", "TypeSynonymInstances", "FlexibleInstances"]
langExtsToInject = ["MagicHash", "BangPatterns", "TypeSynonymInstances", "FlexibleInstances", "NoStrictData"]

defines :: Bool -> Bool -> String
defines debug coerce = unlines [ "#define " ++ d ++ " 1" | d <- vars_to_define ]
Expand Down