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.

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

Refactor happyDoActions #280

Merged
merged 2 commits into from
Jul 22, 2024
Merged

Conversation

Kariiem
Copy link
Contributor

@Kariiem Kariiem commented Jun 22, 2024

This PR refactors happyDoActions into several smaller functions, changes how the parsing tables are constructed, using 32-bit integers instead of 16-bit integers.

@sgraf812 sgraf812 force-pushed the refactor-happyDoActions branch 2 times, most recently from c6a4bc4 to d5bbb82 Compare June 23, 2024 09:11
@sgraf812
Copy link
Collaborator

I rebased and added a commit fixing the tests. Feel free to squash the changes in the commits where they belong.

@Kariiem Kariiem force-pushed the refactor-happyDoActions branch from d5bbb82 to dfdc863 Compare June 24, 2024 03:24
Copy link
Collaborator

@sgraf812 sgraf812 left a comment

Choose a reason for hiding this comment

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

Sigh. I just realised I had the following comments stuck in the review queue which we should discuss.

packages/backend-lalr/data/HappyTemplate.hs Outdated Show resolved Hide resolved
Copy link
Collaborator

@sgraf812 sgraf812 left a comment

Choose a reason for hiding this comment

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

I had a closer look at how WORDS_BIGENDIAN is to be used. alex simply pulls it from the GHC-provided header ghcconfig.h. So we need to add the following line to the begin of the template (after the GHC check):

#include "ghcconfig.h"

After that, we still need to add an emulated CI pipeline as suggested by @Bodigrim here: haskell/alex#260 (comment). I can do that as well, but probably not within the next 2 weeks.

@Bodigrim
Copy link

Bodigrim commented Jul 1, 2024

I had a closer look at how WORDS_BIGENDIAN is to be used. alex simply pulls it from the GHC-provided header ghcconfig.h.

There are several possible headers; I don't remember why but I think at some point we concluded that MachDeps.h is the best option.

There is also GHC.ByteOrder, but beware that it was broken before GHC 8.10. A compatibility package ghc-byteorder is available.

@Kariiem Kariiem force-pushed the refactor-happyDoActions branch from faae80f to 1447b5d Compare July 4, 2024 11:08
@sgraf812 sgraf812 force-pushed the refactor-happyDoActions branch from 1447b5d to 73465de Compare July 15, 2024 07:11
@sgraf812 sgraf812 force-pushed the refactor-happyDoActions branch from 73465de to 026cd35 Compare July 15, 2024 07:48
@sgraf812 sgraf812 force-pushed the refactor-happyDoActions branch from 026cd35 to d8a3db6 Compare July 15, 2024 07:51
@sgraf812
Copy link
Collaborator

Alright, I included MachDeps.h to import the WORDS_BIGENDIAN define and did a final review. I'm inclined to merge this.

@sgraf812 sgraf812 merged commit 80126af into haskell:master Jul 22, 2024
14 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants