happy
happy copied to clipboard
haskell
qualify Prelude functions in templates?
In a .y file, I would like to define my own version of the div function. So this is what I have there:
import Prelude hiding (div)
import qualified Prelude
...
define and use my own div-function somewhere, possibly defined via Prelude.div in some way
...
But this does not work, because the generated .hs file does use the original Prelude version of div without qualification, so actually my new version of div is chosen, which happens to have a different type.
The concrete code that triggers the conflict in my case appears to be from the lines
readArrayBit arr bit =
Bits.testBit IBOX(indexShortOffAddr arr (bit `div` 16)) (bit `mod` 16)
in templates/GenericTemplate.hs, but more generally the issue seems to be various uses of Prelude functions in Happy generated code.
Wouldn't it be better if the above and similar references were to Prelude.div instead of just div, and likewise for other Prelude functions?
Are the .hs files in the templates subdirectory all one would have to touch/look through?
Or are there other places, in src, that also generate code that has both of the following properties:
- calls
Preludeor other standard functions not also generated by Happy itself; - is joined together, in a single output module, with user code from an input
.yfile?
The point in 2. being: "declarations from an input .y file". Of course, all the expressions in productions' semantic actions are also user code, but these should be harmless for the issue at hand. The declarations of functions in a user's Haskell code blocks taken over from .y into output are the problem.
So places, in src, where semantic action code is taken over into the output module are not problematic. But if something in src generates its own calls to div or other Prelude functions, then those "something" places could be relevant to fix. I don't know how to search for such places. But anyway, I could focus on a fix for the files in templates first.
I'm fairly sure that you would also need to look at the parts of Happy that generate code, in particular https://github.com/simonmar/happy/blob/master/src/ProduceCode.lhs, and the part where we generate the imports, https://github.com/simonmar/happy/blob/master/src/Main.lhs#L490
But if you add {-# LANGUAGE NoImplicitPrelude #-} and import qualified Prelude and then fix everything enough to make the tests pass, that should be enough.
Okay, I guess I will ask a TA to work on this, to be ready for my next compiler construction course. :smile:
One remaining question right now: Do the tests exercise the different versions of the code that are run/produced for different Happy settings, such as "use (or not use) GHC-specific extensions"?
For example, I see in these lines that under certain compile time conditions the comparison operations are already some qualified imported ones, while under others they will just reference "==" etc., potentially triggering the issue here.
Do the tests exercise the different versions of the code that are run/produced for different Happy settings, such as "use (or not use) GHC-specific extensions"?
Yes!
Not really. Or at least not fixed properly.
The underlying issue was that whenever Happy generates code that uses a Prelude function unqualified, then clashes can occur with user grammar code.
A proper fix would be to make sure that this cannot happen. So, not only 1) making sure that at some point the current code in this repo does not suffer from this issue, but also 2) making sure that going forward from that point it is impossible to introduce code into this repo that brings the issue back.
Typically, 2) would mean that some CI testing is put in place that would automatically reject any code change that brings unqualified Prelude functions back into the output of code generation.
Over in https://github.com/simonmar/happy/pull/135, an attempt was made to do both 1) and 2). That did not come to fruition, so instead in the commit https://github.com/simonmar/happy/commit/1ec0c8e78d4681270feed4b66ec45b0815d31597 you referenced, I did at least the 1) part, for the code that was in this repo at the time. So at that point in the distant past, the issue was fixed for a moment. If in the meantime any change was made to the generation code that again writes out an unqualified Prelude function name - which might easily have happened, without 2) in place - then the issue would be back.
I do believe that in https://github.com/simonmar/happy/issues/131#issuecomment-447256189, by "fix this properly", @simonmar meant the combination of 1) and 2). I do not know whether anything was done about 2) at any point (beayond the attempt in https://github.com/simonmar/happy/pull/135), but at least my commit https://github.com/simonmar/happy/commit/1ec0c8e78d4681270feed4b66ec45b0815d31597 does not do 2) at all.
Alright, so if I understand correctly, the issue is fixed, but ideally we need a method to prevent regressions in the future, hence we'll keep it open.
Yes, the specific instance of that problem at the time, with the example code I used to demonstrate the issue, was fixed. And actually due to the work of @Kellador in #135 we even know that at that time no other example code could have provoked the issue anymore (because his work in #135 made sure via testing that my commit caught all unqualified Prelude function uses present then at once). But whether there have been regressions since then is potentially an open question.