haskell / happy

The Happy parser generator for Haskell
Other
279 stars 84 forks source link

Support ArrayTarget as the default and only supported target. #276

Closed Kariiem closed 3 months ago

Kariiem commented 3 months ago

This PR removes all code related to HaskellTarget, makes ArrayTarget on by default independent of the command line arguments and removes extra arguments to several functions that needed to check for the target.

@int-index @Ericson2314 @sgraf812 I hope that a single commit PR is acceptable. I will do the same for --g/ghc flag and HAPPY_GHC.

int-index commented 3 months ago

I'm surprised to see no changes to the test suite.

sgraf812 commented 3 months ago

Good point; I think we can now get rid of all targets in tests/Makefile that use -a. Roughly

diff --git a/tests/Makefile b/tests/Makefile
index 84938bd..3e97c35 100644
--- a/tests/Makefile
+++ b/tests/Makefile
@@ -56,42 +56,24 @@ TEST_HAPPY_OPTS = --strict
 %.n.hs : %.ly
    $(HAPPY) $(TEST_HAPPY_OPTS) $< -o $@

-%.a.hs : %.ly
-   $(HAPPY) $(TEST_HAPPY_OPTS) -a $< -o $@
-
 %.g.hs : %.ly
    $(HAPPY) $(TEST_HAPPY_OPTS) -g $< -o $@

 %.gc.hs : %.ly
    $(HAPPY) $(TEST_HAPPY_OPTS) -gc $< -o $@

-%.ag.hs : %.ly
-   $(HAPPY) $(TEST_HAPPY_OPTS) -ag $< -o $@
-
-%.agc.hs : %.ly
-   $(HAPPY) $(TEST_HAPPY_OPTS) -agc $< -o $@
-
 %.n.hs : %.y
    $(HAPPY) $(TEST_HAPPY_OPTS) $< -o $@

-%.a.hs : %.y
-   $(HAPPY) $(TEST_HAPPY_OPTS) -a $< -o $@
-
 %.g.hs : %.y
    $(HAPPY) $(TEST_HAPPY_OPTS) -g $< -o $@

 %.gc.hs : %.y
    $(HAPPY) $(TEST_HAPPY_OPTS) -gc $< -o $@

-%.ag.hs : %.y
-   $(HAPPY) $(TEST_HAPPY_OPTS) -ag $< -o $@
-
-%.agc.hs : %.y
-   $(HAPPY) $(TEST_HAPPY_OPTS) -agc $< -o $@
-
-CLEAN_FILES += *.n.hs *.a.hs *.g.hs *.gc.hs *.ag.hs *.agc.hs *.info *.hi *.bin *.exe *.o *.run.stdout *.run.stderr
+CLEAN_FILES += *.n.hs *.g.hs *.gc.hs *.info *.hi *.bin *.exe *.o *.run.stdout *.run.stderr

-ALL_TEST_HS = $(shell echo $(TESTS) | sed -e 's/\([^\. ]*\)\.\(l\)\{0,1\}y/\1.n.hs \1.a.hs \1.g.hs \1.gc.hs \1.ag.hs \1.agc.hs/g')
+ALL_TEST_HS = $(shell echo $(TESTS) | sed -e 's/\([^\. ]*\)\.\(l\)\{0,1\}y/\1.n.hs \1.g.hs \1.gc.hs/g')

 ALL_TESTS = $(patsubst %.hs, %.run, $(ALL_TEST_HS))

@@ -118,15 +100,15 @@ check.%.y : %.y
 all :: $(CHECK_ERROR_TESTS) $(ALL_TESTS)

 check-todo::
-   $(HAPPY) $(TEST_HAPPY_OPTS) -ad Test.ly
+   $(HAPPY) $(TEST_HAPPY_OPTS) -d Test.ly
    $(HC) Test.hs -o happy_test
    ./happy_test
    -rm -f ./happy_test
-   $(HAPPY) $(TEST_HAPPY_OPTS) -agd Test.ly
+   $(HAPPY) $(TEST_HAPPY_OPTS) -gd Test.ly
    $(HC) Test.hs -o happy_test
    ./happy_test
    -rm -f ./happy_test
-   $(HAPPY) $(TEST_HAPPY_OPTS) -agcd Test.ly
+   $(HAPPY) $(TEST_HAPPY_OPTS) -gcd Test.ly
    $(HC) Test.hs -o happy_test
    ./happy_test
    -rm -f ./happy_test

(Some of those targets appear to be duplicated. Weird.)

Kariiem commented 3 months ago

I commited the changes to the Makefile.

sgraf812 commented 3 months ago

@Ericson2314 @int-index it would be great to make progress here, so that the PR stack that @Kariiem has to wrangle does not become too large. The goal is to merge the changes in #272 bit by bit, starting with the least controversial ones.

To me, this PR looks good. It's probably good to run the CI pipeline just in case (which needs maintainer approval.)

int-index commented 3 months ago

It's probably good to run the CI pipeline just in case (which needs maintainer approval.)

I approved the CI run (again). Is there any way to allow CI to run in all @Kariiem's PRs?

@sgraf812 I actually thought you had maintainer access, maybe you could request it? (Admins of the haskell group should be able to grant those permissions).

sgraf812 commented 3 months ago

@sgraf812 I actually thought you had maintainer access, maybe you could request it? (Admins of the haskell group should be able to grant those permissions).

Alright, I requested and got maintainer access on Matrix!

sgraf812 commented 3 months ago

I merged; the remaining CI errors appear to be unrelated and are hopefully fixed by https://github.com/haskell/happy/pull/279.