Closed Kellador closed 5 years ago
Thanks for doing this! Two suggestions:
Maybe it would be better for Happy to inject import qualified Prelude as HappyPrelude
in the imports, and then qualify everything with HappyPrelude
? That way we aren't relying on the user to import Prelude with a known name.
I note that you copied all the tests. Rather than copying them in the repository, would it be possible to auto-generate the modified tests, perhaps using CPP or something? I'd rather not have multiple copies of the tests if possible, since they could easily diverge accidentally.
Thanks for the suggestions, injecting the import definitely sounds like a good idea, and I will look into how to best auto-generate the modified tests as well.
import qualified Prelude as HappyPrelude
is now injected and everything qualified with Prelude
before is now qualified with HappyPrelude
instead.
This alone would however make all old grammars incompatible, since they would then have to import Prelude themselves without qualification, so I've added a new cli option: --noprelude
https://github.com/Kellador/happy/blob/master/src/Main.lhs#L439-L440
Without this flag the default behaviour is injection of both:
import qualified Prelude as HappyPrelude
and import Prelude
,
whereas with the flag, the latter import is omitted;
A side effect of this is that with the flag it is not necessary to declare {-# LANGUAGE NoImplicitPrelude #-}
.
Both tests for HappyPrelude
usage with the --noprelude
flag and basic Prelude
are now generated via the C preprocessor.
For this all relevant tests are preprended with
`#ifndef QUALIFIEDPRELUDE
and new targets were added to the tests Makefile (https://github.com/Kellador/happy/blob/master/tests/Makefile#L40-L126).
This in effect doubles up all tests, which might be why the Travis CI build is currently timing out.
import qualified Prelude as HappyPrelude is now injected and everything qualified with Prelude before is now qualified with HappyPrelude instead. This alone would however make all old grammars incompatible, since they would then have to import Prelude themselves without qualification
I don't follow that - why wouldn't it work to just inject import qualified Preliude as HappyPrelude
and then refer to all Prelude identifiers qualified by HappyPrelude.
? We don't have to add {-# LANGUAGE NoImplicitPrelude #-}
too, we can let the user choose whether to do that or not.
I don't follow that - why wouldn't it work to just inject
import qualified Preliude as HappyPrelude
and then refer to all Prelude identifiers qualified byHappyPrelude.
? We don't have to add{-# LANGUAGE NoImplicitPrelude #-}
too, we can let the user choose whether to do that or not.
Because once Prelude is imported under a qualified name it is not implicitly imported anymore;
If the extra unqualified import Prelude
is omitted all tests without the HappyPrelude qualifiers fail, even without {-# LANGUAGE NoImplicitPrelude #-}
, which is not injected atm.
Oh, I'd forgotten about that. How unfortunate.
Giving Travis CI a little more time before timing out did the trick, so I pushed that along with the last few fixes to make it work for the older GHC versions.
Not sure about appveyor, as that looks to have been broken for a while now and fixing that would not be related to this pull request.
Thanks @Kellador! The work done here was essential for getting https://github.com/simonmar/happy/pull/156 right.
Should resolve #131; allowing the usage of qualified Prelude in templates, as specified in the issue.
A set of all testcases modified to use NoImplicitPrelude and qualified Prelude is included in a seperate directory inside the tests directory.