Hacking on GHC
After visiting CUFP and meeting a bunch of exciting folks, my desire to contribute to GHC and get some of my own changes into the Haskell language rose drastically. Starting contributing is quite intimidating, though; although I’d done it a bit before, it was quite a big challenge to do so again.
This time, I figured I’d document the process in its entirety, so that others like me could follow and commit their own changes to GHC. To be honest, much of this document is useless and is already documentd on the GHC wiki; treat it more of as a story or experience report rather than as a factual reference for anything useful.
Day 1
Day 1 of the adventure begins now!
Setting up an environment
The first large barrier to working on GHC is getting it set up. I had most of my tools set up already, so it was a matter of following the instructions on the Newcomers page.
I started by cloning GHC. I keep all my code in ~/code, so I went ahead and cloned to
~/code/ghc. This took about ten minutes.
$ cd ~/code/ghc $ git clone --recursive git://git.haskell.org/ghc.git Cloning into 'ghc'... Submodule 'libffi-tarballs' registered for path 'libffi-tarballs' Submodule 'libraries/Cabal' registered for path 'libraries/Cabal' Submodule 'libraries/Win32' registered for path 'libraries/Win32' Submodule 'libraries/array' registered for path 'libraries/array' Submodule 'libraries/binary' registered for path 'libraries/binary' Submodule 'libraries/bytestring' registered for path 'libraries/bytestring' Submodule 'libraries/containers' registered for path 'libraries/containers' Submodule 'libraries/deepseq' registered for path 'libraries/deepseq' Submodule 'libraries/directory' registered for path 'libraries/directory' Submodule 'libraries/dph' registered for path 'libraries/dph' Submodule 'libraries/filepath' registered for path 'libraries/filepath' Submodule 'libraries/haskeline' registered for path 'libraries/haskeline' Submodule 'libraries/hoopl' registered for path 'libraries/hoopl' Submodule 'libraries/hpc' registered for path 'libraries/hpc' Submodule 'libraries/parallel' registered for path 'libraries/parallel' Submodule 'libraries/pretty' registered for path 'libraries/pretty' Submodule 'libraries/primitive' registered for path 'libraries/primitive' Submodule 'libraries/process' registered for path 'libraries/process' Submodule 'libraries/random' registered for path 'libraries/random' Submodule 'libraries/stm' registered for path 'libraries/stm' Submodule 'libraries/terminfo' registered for path 'libraries/terminfo' Submodule 'libraries/time' registered for path 'libraries/time' Submodule 'libraries/transformers' registered for path 'libraries/transformers' Submodule 'libraries/unix' registered for path 'libraries/unix' Submodule 'libraries/vector' registered for path 'libraries/vector' Submodule 'libraries/xhtml' registered for path 'libraries/xhtml' Submodule 'nofib' registered for path 'nofib' Submodule 'utils/haddock' registered for path 'utils/haddock' Submodule 'utils/hsc2hs' registered for path 'utils/hsc2hs' Cloning into 'libffi-tarballs'... Cloning into 'libraries/Cabal'... Cloning into 'libraries/Win32'... Cloning into 'libraries/array'... Cloning into 'libraries/binary'... Cloning into 'libraries/bytestring'... Cloning into 'libraries/containers'... Cloning into 'libraries/deepseq'... Cloning into 'libraries/directory'... Cloning into 'libraries/dph'... Cloning into 'libraries/filepath'... Cloning into 'libraries/haskeline'... Cloning into 'libraries/hoopl'... Cloning into 'libraries/hpc'... Cloning into 'libraries/parallel'... Cloning into 'libraries/pretty'... Cloning into 'libraries/primitive'... Cloning into 'libraries/process'... Cloning into 'libraries/random'... Cloning into 'libraries/stm'... Cloning into 'libraries/terminfo'... Cloning into 'libraries/time'... Cloning into 'libraries/transformers'... Cloning into 'libraries/unix'... Cloning into 'libraries/vector'... Cloning into 'libraries/xhtml'... Cloning into 'nofib'... Cloning into 'utils/haddock'... Cloning into 'utils/hsc2hs'...
Once the GHC repository and all submodules are cloned, you must set up how you want to build GHC. To
do so, you create a build.mk file with your own configuration; in my case, I wanted to build my
compiler quickly, and so stole the configuration from the
GHC wiki page on how
to make a quickly building stage 2 compiler. There are also a bunch of pre-configured builds, so you
can edit build.mk and just uncomment one of the lines to choose a build configuration. I went
ahead and copied the GHC wiki suggestion and just inserted the following at the top of build.mk (a
copied version of build.mk.sample):
# My build settings for hacking on stage 2 SRC_HC_OPTS = -H32m -O -fasm GhcStage1HcOpts = -O -fasm GhcStage2HcOpts = -O0 -DDEBUG -Wall GhcLibHcOpts = -O -fasm GhcLibWays = v SplitObjs = NO
I next ran ./boot:
$ ./boot Creating libraries/array/ghc.mk Creating libraries/array/GNUmakefile Creating libraries/base/ghc.mk Creating libraries/base/GNUmakefile Creating libraries/bin-package-db/ghc.mk ... and so on... Booting . Booting libraries/base/ Booting libraries/directory/ Booting libraries/integer-gmp/ Booting libraries/process/ Booting libraries/terminfo/ Booting libraries/time/ Booting libraries/unix/
After that comes ./configure:
$ ./configure
... everything was ok until...
checking for DocBook DTD... I/O error : Attempt to load network entity http://www.oasis-open.org/docbook/xml/4.5/docbookx.dtd
conftest.xml:5: warning: failed to load external entity "http://www.oasis-open.org/docbook/xml/4.5/docbookx.dtd"
]>
^
conftest.xml:6: element book: validity error : No declaration for attribute id of element book
<book id="test">
^
conftest-book.xml:2: parser error : Entity 'ldquo' not defined
<title>A DocBook “Test Document”</title>
^
conftest-book.xml:2: parser error : Entity 'rdquo' not defined
<title>A DocBook “Test Document”</title>
^
conftest-book.xml:11: parser error : chunk is not well balanced
^
conftest.xml:7: parser error : Failure to process entity conftest-book
&conftest-book;
^
conftest.xml:7: parser error : Entity 'conftest-book' not defined
&conftest-book;
^
failed
configure: WARNING: cannot find a DTD for DocBook XML V4.5, you will not be able to validate your documentation
configure: WARNING: check your XML_CATALOG_FILES environment variable and/or /etc/xml/catalog
checking for xsltproc... /usr/bin/xsltproc
checking for DocBook XSL stylesheet... no
configure: WARNING: cannot find DocBook XSL stylesheets, you will not be able to build the documentation
checking for dblatex... /usr/local/bin/dblatex
checking for ghc-pkg matching /Users/silver/.stack/programs/x86_64-osx/ghc-7.10.1/bin/ghc... /Users/silver/.stack/programs/x86_64-osx/ghc-7.10.1/bin/ghc-pkg
checking for happy... no
checking for version of happy...
configure: error: Happy version 1.19.4 or later is required to compile GHC.
Turns out I was missing a few things: happy, alex, and something related to docbook. Installing happy
was easy with stack install happy; same for alex. After doing so ./configure succeeded, so
perhaps DocBook wasn’t really an issue.
Finally we get to the long-running part: make
$ make + test -f mk/config.mk.old + cp -p mk/config.mk mk/config.mk.old touch -r mk/config.mk.old mk/config.mk + test -f mk/project.mk.old + cp -p mk/project.mk mk/project.mk.old touch -r mk/project.mk.old mk/project.mk + test -f compiler/ghc.cabal.old + cp -p compiler/ghc.cabal compiler/ghc.cabal.old touch -r compiler/ghc.cabal.old compiler/ghc.cabal ===--- building phase 0 /Applications/Xcode.app/Contents/Developer/usr/bin/make --no-print-directory -f ghc.mk phase=0 phase_0_builds ghc.mk:153: *** dyn is not in $(GhcLibWays), but $(DYNAMIC_GHC_PROGRAMS) is YES. Stop. make: *** [all] Error 2
This quickly fails with the error message above. It looks like the issue is that the variable
DYNAMIC_GHC_PROGRAMS defaults to YES which contradicts the fact that my copied build.mk sets
GhcLibWays to v and not something including dyn. I have no idea what this means, and after
Googling, stumble upon this page. This
still doesn’t make things quite clear, but in the interest of time I just add dyn to GhcLibWays
and hope it fixes things, and run make -j8 again. This time, it starts compiling things!
Looking at htop, it seems like at least for the first while, nothing really gets parallelized, at
least while building Cabal. Once Cabal is compiled though, the compile happily pegs my CPUs at
100% utilization.
Everything seems to go well for about 15-20 minutes until this happens:
sed 1d docs/users_guide/flags.xml >> docs/man/flags.xml
"inplace/bin/ghc-stage1" -optc-m64 -optc-fno-stack-protector -optc-Wall -optc-Wall -optc-Wextra -optc-Wstrict-prototypes -optc-Wmissing-prototypes -optc-Wmissing-declarations -optc-Winline -optc-Waggregate-return -optc-Wpointer-arith -optc-Wmissing-noreturn -optc-Wnested-externs -optc-Wredundant-decls -optc-Iincludes -optc-Iincludes/dist -optc-Iincludes/dist-derivedconstants/header -optc-Iincludes/dist-ghcconstants/header -optc-Irts -optc-Irts/dist/build -optc-DCOMPILING_RTS -optc-fno-strict-aliasing -optc-fno-common -optc-DDTRACE -optc-Irts/dist/build/autogen -optc-Wno-unknown-pragmas -optc-O2 -optc-fomit-frame-pointer -optc-g -optc-DRtsWay=\"rts_v\" -static -H32m -O -fasm -Iincludes -Iincludes/dist -Iincludes/dist-derivedconstants/header -Iincludes/dist-ghcconstants/header -Irts -Irts/dist/build -DCOMPILING_RTS -this-package-key rts -dcmm-lint -DDTRACE -i -irts -irts/dist/build -irts/dist/build/autogen -Irts/dist/build -Irts/dist/build/autogen -O2 -c rts/hooks/OutOfHeap.c -o rts/dist/build/hooks/OutOfHeap.o
"inplace/bin/ghc-stage1" -optc-m64 -optc-fno-stack-protector -optc-Wall -optc-Wall -optc-Wextra -optc-Wstrict-prototypes -optc-Wmissing-prototypes -optc-Wmissing-declarations -optc-Winline -optc-Waggregate-return -optc-Wpointer-arith -optc-Wmissing-noreturn -optc-Wnested-externs -optc-Wredundant-decls -optc-Iincludes -optc-Iincludes/dist -optc-Iincludes/dist-derivedconstants/header -optc-Iincludes/dist-ghcconstants/header -optc-Irts -optc-Irts/dist/build -optc-DCOMPILING_RTS -optc-fno-strict-aliasing -optc-fno-common -optc-DDTRACE -optc-Irts/dist/build/autogen -optc-Wno-unknown-pragmas -optc-O2 -optc-fomit-frame-pointer -optc-g -optc-DRtsWay=\"rts_v\" -static -H32m -O -fasm -Iincludes -Iincludes/dist -Iincludes/dist-derivedconstants/header -Iincludes/dist-ghcconstants/header -Irts -Irts/dist/build -DCOMPILING_RTS -this-package-key rts -dcmm-lint -DDTRACE -i -irts -irts/dist/build -irts/dist/build/autogen -Irts/dist/build -Irts/dist/build/autogen -O2 -c rts/hooks/StackOverflow.c -o rts/dist/build/hooks/StackOverflow.o
Warning: -rtsopts and -with-rtsopts have no effect with -no-hs-main.
Call hs_init_ghc() from your main() function to set these options.
Build the book set list...
Build the book set list...
warning: failed to load external entity "/System/Library/Frameworks/Python.framework/Versions/2.7/share/dblatex/xsl/latex_book_fast.xsl"
compilation error: file /private/var/folders/0l/vg6g5pdd32l6ggh_xdngnkwh0000gn/T/tmpi_nTXo/custom.xsl line 6 element import
xsl:import : unable to load /System/Library/Frameworks/Python.framework/Versions/2.7/share/dblatex/xsl/latex_book_fast.xsl
warning: failed to load external entity "/System/Library/Frameworks/Python.framework/Versions/2.7/share/dblatex/xsl/common/mkdoclist.xsl"
compilation error: file doclist.xsl line 7 element import
xsl:import : unable to load /System/Library/Frameworks/Python.framework/Versions/2.7/share/dblatex/xsl/common/mkdoclist.xsl
warning: failed to load external entity "/System/Library/Frameworks/Python.framework/Versions/2.7/share/dblatex/xsl/latex_book_fast.xsl"
compilation error: file /private/var/folders/0l/vg6g5pdd32l6ggh_xdngnkwh0000gn/T/tmphVadXL/custom.xsl line 6 element import
xsl:import : unable to load /System/Library/FramewoUnexpected error occured
Error: xsltproc failed
rks/Python.framework/Versions/2.7/share/dblatex/xsl/latex_book_fast.xsl
warning: failed to load external entity "/System/Library/Frameworks/Python.framework/Versions/2.7/share/dblatex/xsl/common/mkdoclist.xsl"
compilation error: file doclist.xsl line 7 element import
xsl:import : unable to load /System/Library/Frameworks/Python.framework/Versions/2.7/share/dblatex/xsl/common/mkdoclist.xsl
Unexpected error occured
Error: xsltproc failed
make[1]: *** [utils/haddock/doc/haddock.ps] Error 1
make[1]: *** Waiting for unfinished jobs....
make[1]: *** [utils/haddock/doc/haddock.pdf] Error 1
make: *** [all] Error 2
This seems once more related to the DocBook issues from before. Perhaps I don’t have xsltproc,
whatever that is?
$ which xsltproc /usr/bin/xsltproc
Nope, that’s not it. It does look like it’s unable to load some sort of libraries though, looking at these lines:
xsl:import : unable to load /System/Library/Frameworks/Python.framework/Versions/2.7/share/dblatex/xsl/latex_book_fast.xsl xsl:import : unable to load /System/Library/FramewoUnexpected error occured Error: xsltproc failed
On a whim, I tried adding DocBook support:
$ brew install docbook $ export XML_CATALOG_FILES=`brew --prefix`/etc/xml/catalog
Sadly this didn’t seem to affect much — still the same errors…
I tried with another package, which perhaps provides some data files for DocBook, maybe the ones that the GHC build can’t import:
$ brew install docbook-xsl
Nope, this is still useless. At this point, I gave up on fixing this issue through getting DocBook
to work and instead decided to disable whatever Haddock was trying to do, which
this page told me how to do: add HADDOCK_DOCKS = NO to my build.mk.
I then realized that in order for this to affect anything, I’d have to run ./configure again, and
it would maybe have to rerun a bunch of make stuff. Instead of risking that, I edited
mk/config.mk, and found these lines:
BUILD_DOCBOOK_PS = YES BUILD_DOCBOOK_PDF = YES
and changed YES to NO.
At this point, compilation happily proceeded onwards. This was probably the wrong way to handle
things, but it worked. After a little while, I see it configuring base-4.8 - great!
After all of this is done, I test that it works:
$ inplace/bin/ghc-stage2 --version The Glorious Glasgow Haskell Compilation System, version 7.11.20150904
Hooray!
Making a Test Parser Change
The changes I’d like to make pertain to the parser. In order to make sure I can change the parser,
I’m going to make a tiny change to the parser file Parser.y and see if I can prove that the change
goes through to the final compiler. I think that even though I’m fiddling with the parser, I can
stick to just rebuilding the Stage 2 compiler, so I edit the build.mk to reflect that. From the
Newcomers page, it’s not quite clear exactly how to
make it only build Stage 2, even though it suggests doing so. There’s some more info on the
Build System page, which
suggests using the commands make stage=2 or make 2 from the ghc subdirectory. I try it and it
completes quickly.
My parser change concerns expressions; I find that the awkwardly named exp10 rule in
compiler/parser/Parser.y is the main rule for expressions. My first nonsensical change (as a test)
is to add the following option:
| 'soup' stmtlist {% ams (L (comb2 $1 $2)
(mkHsDo DoExpr (snd $ unLoc $2)))
(mj AnnDo $1:(fst $ unLoc $2)) }
This is a rule that treats the new soup keyword as a do block.
Unsurprisingly, this doesn’t work — the lexer isn’t updated. Alright, I guess I have to make a more reasonable first change. As suggested by the GHC wiki, I find an error message:
"parse error in lambda: no expression after '->'"
and change it to something obviously different
"HELP ME! What's going on?! parse error in lambda: no expression after '->'"
make 2 happily reruns happy and generates me a new ghc. I create Test.hs:
module Test where test = \x ->
And run a command to test my error message:
# ../inplace/bin/ghc-stage2 Test.hs
[1 of 1] Compiling Test ( Test.hs, Test.o )
Test.hs:3:8: error:
HELP ME! What's going on?! parse error in lambda: no expression after '->'
Hooray! I’ve proven that my development cycle work.
Making my actual change
Today, I’d like to make a parser change that allows code like this:
main = do
fileExists <- doesFileExist "my_file.txt"
when fileExists do
putStrLn "It exists!"
The main change is in the when: I’d like the traditional $ to be optional, allowing a naked
do. I imagine there is a good reason why this hasn’t been done, so I expect to fail due to
shift-reduce conflicts or some other parser nonsense.
I start off by making a test file containing the code above, calling it Test1.hs.
I trace down function application to a rule named fexp, which expands to a left-recursive rule
fexp aexp, with aexp probably standing for "argument expression" and fexp probably standing
for "function expression". It seems like I want aexp to accept raw do blocks; looking at that
rule, it seems to boil down to aexp1, which in turn redirects to aexp2, which is quite meaty.
As a first pass, I just copy the do rule from exp to aexp2. Let’s see what happens!
$ ../inplace/bin/ghc-stage2 Test1.hs
[1 of 1] Compiling Main ( Test1.hs, Test1.o )
Test1.hs:2:17: error:
Variable not in scope: doesFileExist :: [Char] -> m t0
Test1.hs:3:3: error:
Variable not in scope: when :: t0 -> IO () -> m b
It parses!
However, when building Parser.y I noticed one some worrying lines:
unused rules: 1 reduce/reduce conflicts: 39
These didn’t appear before, so before declaring victory I’d like to know what’s going on. Surely it’s not this easy! I’m not 100% sure what reduce/reduce conflicts are, and googling brings me to a Bison documentation page on the issue. The key phrase is:
A reduce/reduce conflict occurs if there are two or more rules that apply to the same sequence of input. This usually indicates a serious error in the grammar.
Well, that’s sad, although pretty expected, since we literally duplicated a rule.
In order to resolve this, we really need to understand why aexp2 ane exp10 are different rules.
exp10 has fexp as one of its rules; fexp can be just a aexp, which can be just an aexp1,
which can be just an aexp2, so as a result aexp2 can parse directly as an exp10. Not the other
way around, however!
This means that in order to make do blocks behave the way we want, we need to move them from
exp10 to aexp2.
This is much better. Running make 2 now, I don’t get any reduce/reduce conflicts or unused rules,
and my Test1.hs still passes my brief test (it parses).
This still seems too easy! How can I discover where this breaks?
There’s a clear answer: use GHC’s parser test suite.
Sadly, it’s midnight, and I need to be awake much too soon, and I haven’t gone to sleep yet. So this is the end of day one. Day two will consist of running my new compiler on a test suite, making some changes, and maybe trying to add a LANGUAGE pragma for this change and then making a Phabricator patch/diff/commit/PR/whatever they’re called in GHC land.
End of Day 1
Day 2
I quickly find a page about running the test suite,
which tells me exactly what I need to know. I cd to testsuite/tests/parser and run make THREADS=8
and quickly get a result:
OVERALL SUMMARY for test run started at Sat Sep 5 00:07:17 2015 PDT
0:00:13 spent to go through
193 total tests, which gave rise to
449 test cases, of which
257 were skipped
0 had missing libraries
189 expected passes
3 expected failures
0 caused framework failures
0 unexpected passes
0 unexpected failures
0 unexpected stat failures
No changes in the parser test suite from my minor change.
I guess I should add a few tests!
My first test will just check that my feature does what it claims to do. It goes in
should_compile. It seems like there are files called EmptyDecls.hs and DoAndIfThenElse.hs,
so I decide I should come up with a name for my extension and use it to name the test case. The
extension will be called ArgumentDo (bikeshedding later) and goes in ArgumentDo.hs:
{-# LANGUAGE ArgumentDo #-}
module ArgumentDo where
when :: Bool -> IO () -> IO ()
when True a = a
when False _ = return ()
foo :: IO ()
foo = when True do
return ()
Running make again, I get the same numbers, so it seems just adding the file isn’t enough. Luckily
there’s a page on adding new test cases,
and it says I have to edit all.T. I add the line:
test('ArgumentDo', normal, compile, [''])
Running make now yields:
Unexpected results from:
TEST="ArgumentDo"
OVERALL SUMMARY for test run started at Sat Sep 5 00:15:59 2015 PDT
0:00:14 spent to go through
194 total tests, which gave rise to
452 test cases, of which
259 were skipped
0 had missing libraries
189 expected passes
3 expected failures
0 caused framework failures
0 unexpected passes
1 unexpected failures
0 unexpected stat failures
Unexpected failures:
should_compile ArgumentDo [exit code non-0] (normal)
Good news: it picked up the test. Bad news: the test seems to fail! Luckily, the test suite dumps
X.comp.stderr files (where X is the filename of the test), so I can look in
should_compile/ArgumentDo.comp.stderr to see what went wrong:
ArgumentDo.hs:1:14: error: Unsupported extension: ArgumentDo
Oh, of course! I forgot to add the ArgumentDo language extension. Let’s add this extension, even
though for now it will do nothing (I don’t know how to make the parser depend on extensions yet).
Adding a Language Pragma
I take DoAndIfThenElse as an example: it’s a simple extension. Grepping through the source code
shows me that I need to change main/DynFlags.hs. I search for DoAndIfThenElse in that file and
make some very similar lines in each place for ArgumentDo, and run make 2 again. Compilation
takes somewhat longer this time; more things depend on DynFlags.hs than I expected. I guess it
makes sense, since language extensions pervade the entire way the compiler operates. I begin to
wonder if I should have attached -j8 to make make 2 command; restarting it tells me that yes, I
do indeed what that. It goes 8 times faster (ish).
Once that’s done, I run make in the test directory again. This time it succeeds:
OVERALL SUMMARY for test run started at Sat Sep 5 00:26:34 2015 PDT
0:00:14 spent to go through
194 total tests, which gave rise to
452 test cases, of which
259 were skipped
0 had missing libraries
190 expected passes
3 expected failures
0 caused framework failures
0 unexpected passes
0 unexpected failures
0 unexpected stat failures
That said, I know these tests aren’t enough. I’ve almost certainly enabled some weird parses that weren’t valid before, and I’ll need to write code to eliminate those parses and then test that those parses were indeed elimianted. But that can come later; first, let’s make this extension actually enable and disable the feature.
To test this, I add a corresponding test in should_fail, that mirrors ArgumentDo.hs but lacks
the LANGUAGE pragma at the top, and then edit all.T.
After adding that test, I see that it fails (that is, the program unexpectedly works):
Unexpected results from:
TEST="ArgumentDo"
OVERALL SUMMARY for test run started at Sat Sep 5 00:30:02 2015 PDT
0:00:14 spent to go through
195 total tests, which gave rise to
453 test cases, of which
259 were skipped
0 had missing libraries
190 expected passes
3 expected failures
1 caused framework failures
0 unexpected passes
1 unexpected failures
0 unexpected stat failures
Unexpected failures:
should_fail ArgumentDo [exit code 0] (normal)
In order to make this fail as expected, we’ll check that a do block doesn’t occur in places it’s
not supposed to. Looking at the parser around lines 2245 in Parser.y, it seems like the new places we’ve allowed the do blocks
are:
-
as an argument to
fexp -
after
static -
after a
@(is that parsing a pattern as an expression and then reinterpreting it?) see the section on overparsing -
after a
~(same thing?) -
as the left hand side to a record constructor, e.g.
a { x = y}, where theais -
in some arrow notation that uses
(|and|)
For now let me just take care of the first case. From the
Parser page, it seems like
RdrHsSyn.hs is the right place to do parse tree validation, so I stick a checkArgumentDo in
there and export it. These changes take a very long back-and-forth with the compiler which makes me
grateful I’m working in Haskell; I have no idea how the parser or its API works, but I can just
muddle forward because I can rely on the type checker catching terrible mistakes.
-- | Yield a parse error if we have a function applied directly to a do block
-- and ArgumentDo is not enabled.
checkArgumentDo :: LHsExpr RdrName -> P (LHsExpr RdrName)
checkArgumentDo appExpr = do
pState <- getPState
if isNakedDo
then if xopt Opt_ArgumentDo (dflags pState)
then return appExpr
else parseErr
else return appExpr
where
isNakedDo =
case unLoc appExpr of
HsApp _ arg -> case unLoc arg of
HsDo{} -> True
_ -> False
_ -> False
parseErr =
parseErrorSDoc (getLoc appExpr)
(text "Unexpected do block in function application:"
$$ nest 4 (ppr appExpr)
$$ text "Perhaps you meant to use ArgumentDo?")
Next I need to use this. Since it checks for function application, I can use it directly in the rule
for function application where it says fexp aexp. That section goes from:
fexp :: { LHsExpr RdrName }
: fexp aexp { sLL $1 $> $ HsApp $1 $2 }
: ...
and becomes:
fexp :: { LHsExpr RdrName }
: fexp aexp {% checkArgumentDo (sLL $1 $> $ (HsApp $1 $2)) }
: ...
I believe I’m done! I’m going to go ahead and test again. Let’s see how badly everything breaks.
My test still fails:
Unexpected results from:
TEST="ArgumentDo"
OVERALL SUMMARY for test run started at Sat Sep 5 01:13:42 2015 PDT
0:00:14 spent to go through
195 total tests, which gave rise to
453 test cases, of which
259 were skipped
0 had missing libraries
190 expected passes
3 expected failures
1 caused framework failures
0 unexpected passes
1 unexpected failures
0 unexpected stat failures
Unexpected failures:
should_fail ArgumentDo [stderr mismatch] (normal)
This time, it’s because it returns stderr:
ArgumentDo.hs:8:7: error:
Unexpected do block in function application:
when True (do { return () })
Perhaps you meant to use ArgumentDo?
That’s awesome! I just need to stick that into a golden file and I’m good to go. I put it in
should_fail/ArgumentDo.stderr and rerun. It works!
OVERALL SUMMARY for test run started at Sat Sep 5 01:15:20 2015 PDT
0:00:14 spent to go through
195 total tests, which gave rise to
453 test cases, of which
259 were skipped
0 had missing libraries
191 expected passes
3 expected failures
1 caused framework failures
0 unexpected passes
0 unexpected failures
0 unexpected stat failures
Phabricator and review
Next up, let’s make a Phabricator patch to get this in front of some other Haskell devs. I’d like to get some feedback and see what I’m missing! This still seems way too easy — there has to be a reason this hasn’t been done. Why haven’t I run into it yet?
I’ll follow the instructions on the contributing a patch page.
First, it has to be on Trac. Alright, I can make a bug report. I made one here.
Next up, I need to make a commit! I do so:
$ git checkout -b argument-do
$ git add compiler/main/DynFlags.hs compiler/parser/Parser.y compiler/parser/RdrHsSyn.hs \
testsuite/tests/parser/should_compile/all.T testsuite/tests/parser/should_fail/all.T \
testsuite/tests/parser/should_compile/ArgumentDo.hs \
testsuite/tests/parser/should_fail/ArgumentDo.stderr \
testsuite/tests/parser/should_fail/ArgumentDo.hs \
$ git commit -m 'Support ArgumentDo (fixes #10843)'
I try to follow the Git commit message structure suggestions.
Alright, next up: Phabricator! I’ve already signed up before, so I just need to use the guide to figure out the Arcanist CLI commands or whatever.
Wow, it looks like all I have to do is run arc diff. That’s easy and awesome! Sadly… I get the
following response:
zsh: segmentation fault arc diff
I should just reinstall Arcanist. It’s been a while.
That doesn’t help, but for some reason brew uninstall php55 does.
arc diff tells me:
You have untracked files in this working copy.
Working copy: /Users/silver/code/ghc/
Untracked changes in working copy:
(To ignore this change, add it to ".git/info/exclude".)
libraries/integer-gmp/gmp/objs/__.SYMDEF SORTED
Sure, I’ll just rm them. arc diff asks me a few questions and for some info, which is all easy
to fill in, and finally gives me a URL for my revision!
Hooray!