0.4.1 build failure with cpphs 1.18.2: Cannot parse #if directive

[mpietrzak]

While trying to cabal install type-eq 0.4.1 using cpphs 1.18.2 I get "Cannot parse #if directive" error.

type-eq 0.4 with cpphs 1.18.2 works
type-eq 0.4.1 with cpphs 1.18.1 also works

Note that cpphs 1.18.2 incorrectly reports version 1.18.1 to console when invoked with --version parameter:

~/src/type-eq-0.4.1 $ cpphs --version
cpphs 1.18.1
~/src/type-eq-0.4.1 $ ghc-pkg list | grep -i cpphs
    cpphs-1.18.2
~/src/type-eq-0.4.1 $ cabal build
Building type-eq-0.4.1...
Preprocessing library type-eq-0.4.1...
cpphs: Cannot parse #if directive in file ./Type/Eq.hs  at line 28 col 1

I'm not sure if this is type-eq bug or cpphs bug, since I don't know if the offending line is correct cpphs code. Sorry if this should be reported to cpphs instead.

[glaebhoerl]

That's very strange, because the only change between 0.4 and 0.4.1 is in the Cabal file, informing it to use cpphs. So while I could possibly imagine there being a change between cpphs 1.18.1 and 1.18.2 which causes it to stop working for this code (which would be unfortunate), I can't possibly imagine what could cause the same version of cpphs to have different results with version 0.4 versus 0.4.1 of this package.

Are you sure that's actually the case, and it's not some other issue such as the Cabal file change causing it to select different cpphs versions or different cpp implementations entirely, and/or some issue which causes ghc-pkg to see one version of cpphs while another is being actually found in the PATH, or something of that nature?

[mpietrzak]

I didn't notice that type-eq 0.4 does not use cpphs at all.

However, I've tested type-eq-0.4.1, and:

1. It compiles with cpphs 1.18.1,
2. It does not compile with cpphs 1.18.2,
3. After commenting out cpphs lines (ghc-options, built-tools) it compiles again with cpphs 1.18.2 installed - suggesting that indeed cpphs is not used as specified in .cabal file.

I've also additionally checked if correct cpphs is used by removing execute bit from file permissions ("forbidden" during compilation confirms that $HOME/.cabal/bin/cpphs was used) and by manually checking file modification time.

I've noticed one more thing: changelog included in package at Hackage mentions change in 1.18.2, but changelog at cpphs' source repository does not:

http://code.haskell.org/cpphs/CHANGELOG
http://hackage.haskell.org/package/cpphs-1.18.2/src/CHANGELOG

First one starts with:

Version 1.18
------------
  * better lexing of Template Haskell single quotes (thanks to Stephan Wehr)
  * (1.18.1): fix incomplete pattern match

Version 1.17
------------
  * recursively evaluate #if expressions after macro expansion (fix)
  * (1.17.1): report the right version with cpphs --version

and second one starts with:

Version 1.18
------------
  * better lexing of Template Haskell single quotes (thanks to Stephan Wehr)
  * (1.18.1): fix incomplete pattern match
  * (1.18.2): bugfix for erroneous boolean intepretation of some macro
              expansions in #if clauses

Version 1.17
------------
  * recursively evaluate #if expressions after macro expansion (fix)
  * (1.17.1): report the right version with cpphs --version

The "bugfix for erroneous boolean intepretation of some macro expansions in #if clauses" looks suspicious :)

Also note version value at http://hackage.haskell.org/package/cpphs-1.18.2/src/cpphs.hs, there's a wrong version number as I mentioned in original report, which does not directly affect compilation issue, but does increase confusion ;)

There's a new test file at http://hackage.haskell.org/package/cpphs-1.18.2/src/tests/cheplyaka that suggests that a bug in boolean expressions support was fixed recently.

To my naked eye, Type/Eq.hs looks correct (and it works with cpp), so I guess this is most likely a regression introduced in recent version of cpphs. I'll try to notice author of cpphs about this issue.

Best Regards
Maciej

[glaebhoerl]

Thanks a bunch! Let me know if the author of cpphs agrees with this assessment, and if so I'll close this issue.

[snoyberg]

I just had an offline conversation with Malcolm about this. We both noticed that the problem seems to arise from parentheses, in particular the difference between the following two lines:

#if !(MIN_VERSION_base(4, 7, 0))
#if !MIN_VERSION_base(4, 7, 0)

Changing the type-eq codebase from the former to the latter seems to resolve the issue. I won't claim to be enough of an expert on CPP to understand the semantics of this, but at least gcc's CPP implementation agrees with Malcolm.

[snoyberg]

Also, out of curiosity: why are you using cpphs here at all? If I disable the cpphs preprocessor from the cabal file, everything still compiles.

[glaebhoerl]

Interesting. The reason (IIRC, but I think I do) the parentheses are there is because in an earlier version of Cabal (or maybe it was cabal-install, whichever thing provides it), the MIN_VERSION_foo macro has a bug such that !MIN_VERSION_foo(6, 6, 6) is parsed incorrectly, and the parentheses are a workaround for this issue. This is also done in some other packages for the same reason, see this module for example.

Also, out of curiosity: why are you using cpphs here at all? If I disable the cpphs preprocessor from the cabal file, everything still compiles.

Not on every platform. See bug #1. :)

[snoyberg]

Not the OS X issue! I thought we generally agreed to stop going crazy trying to work around a broken CPP implementation and instead gave OS X users a working CPP.

[glaebhoerl]

...a working CPP that's not cpphs?

[glaebhoerl]

(FWIW, I wasn't aware of any of this general goings-on.)

[snoyberg]

Here's the comment that informed me of this solution: snoyberg/conduit#124 (comment).

conduit was the second package where I got requests to totally redo the way comments were written to work around Maverick's new CPP processor.

[mpietrzak]

I think in CPP it's common to put parentheses around macro calls because of this: http://gcc.gnu.org/onlinedocs/cpp/Operator-Precedence-Problems.html.

In short, macro expansion order might expand !foo to !bar && baz, which is different than !(foo) expanded to !(bar && baz).

I'm not sure if it applies to cpphs.

[glaebhoerl]

@mpietrzak Yes, this is the essence of the earlier MIN_VERSION_foo problem I referenced.

Is there a reason why fixing/changing cpphs to accept this syntax would not be a good way forward? (Again, this is not the only package which does it like this.)

[snoyberg]

I'd take it up with Malcolm. I've never written my CPP macros that way, and don't really understand its syntax that well.

[snoyberg]

@glaebhoerl I think that the dependence on cpphs is worse than you realize: your MIN_VERSION macros probably haven't been working at all since you started using cpphs, since you aren't telling cpphs about your cabal_macros.h file.

[mpietrzak]

I've done a very simple test to see how/what gets evaluated:

I've modified Type/Eq.hs in following ways:

First, I've split #if !(...) into separate #define and #if to avoid having !(...) inside if.

So instead of:

#if !(MIN_VERSION_base(4, 7, 0))

I have:

#define mvb470 (MIN_VERSION_base(4, 7, 0))
#if !mvb470

I've done this at 28th and around 50th line.

Secondly, I've added following lines after imports:

minVersionBase470 :: Bool
minVersionBase470 = MIN_VERSION_base(4, 7, 0)

minVersionBase440 :: Bool
minVersionBase440 = MIN_VERSION_base(4, 4, 0)

After those changes Type/Eq.hs compilation no longer ends with "cannot parse" error. Also it seems that macro expands to proper Haskell boolean expression, even if ghc shows warnings ("Defaulting the following constraint(s) to type `Integer'").

I've installed this modified version to default package registry at $HOME/.cabal using cabal install command (no sandbox).

I've then switched to separate empty directory (so I don't accidentally compile/use local sources) and created a test program with following contents:

import Type.Eq

main :: IO ()
main = do
    putStrLn $ show minVersionBase470
    putStrLn $ show minVersionBase440

I'm using Homebrew's distribution of Haskell Platform and I believe version of my base package is 4.6.0.1.

I've compiled test code with ghc and executed it, and it printed:

~/src/tte $ ./t
False
True

I hope I didn't mix anything up, but I guess if I didn't install and compile type-eq properly, then my test program also wouldn't compile. I've double checked and "build-tools" and "ghc-options" are uncommented, as downloaded from Hackage. To triple check I've deleted cpphs and issued cabal clean followed by cabal build and I got "cabal: The program cpphs is required but it could not be found"…

mp

[glaebhoerl]

@snoyberg

• Why would I have to specially inform cpphs of cabal_macros.h, but not the system cpp?
• If this were so, wouldn't the package be completely broken for GHC 7.8 because of all the #if MIN_VERSION_base I have to do around Data.Typeable vs. Data.OldTypeable?

@mpietrzak Thanks! That's cool, I didn't know MIN_VERSION_foo is valid as both CPP and Haskell code :). So I take it that this is a conclusive proof that I don't need to specially inform cpphs of cabal_macros.h? And the other takeaway is that I should be able to retain compatibility with both old Cabal/cabal-install and new cpphs by splitting !(MIN_VERSION...) off into a separate #define?

(Again, I suspect the pain from this apparent-regression will be wider than just this one package, but maybe I'll do it anyways.)

[snoyberg]

Why would I have to specially inform cpphs of cabal_macros.h, but not the system cpp?

Doh, I see the problem: I misread the cabal file. For some reason, I thought you were using cpphs as a code formatter. I see that you're passing it in as a replacement for the system CPP. I withdraw my comments. This seems like a straight cpphs bug regarding parentheses.

Nonetheless, I'd drop the usage of cpphs here. The workaround is to deal with problems with Mac OS X's CPP, but the accepted solution in the community seems to be to install GCC's CPP.

[glaebhoerl]

@snoyberg I'm also using cpphs-style /**/ token pasting rather than the "official" ##, so I think the fact that cpphs wasn't in the cabal file was a bug either way, and I'm surprised it worked otherwise. The precise details are lost to my memory, but I know I spent a bunch of time dicking around with the various mutually-incompatible flavors of cpp when originally developing the package, and eventually settled on just requiring cpphs. (I think I even had it in the cabal file, but either it got lost in some kind of mixup, or I only wanted to add it and forgot.)

[snoyberg]

In that case, I think your options are:

1. Change your MIN_VERSION syntax to remove the extra parentheses and conform to the new cpphs.
2. Get the regression in cpphs resolved.
3. Advertise that the newer cpphs is not supported.

I'd recommend going for (1).

[glaebhoerl]

Okay, upon further testing it seems to me like the MIN_VERSION_base macro doesn't work with cpphs at all, and if so I have no idea how anything ever worked (and why switching to cpphs apparently fixed the OS X users' problems, etc.). I think I'll give this a rest for today and hope the world will make more sense tomorrow.

[glaebhoerl]

@mpietrzak Could you please check if #if MIN_VERSION_base(...) works for you at all using cpphs 1.18.1/2? I.e. does it evaluate to true when it should? It's mystifying to me that (a) over here it always appears to evaluate to false, but (b) you apparently compiled type-eq with it successfully, and (c) so did the OS X users for who I added cpphs to the cabal file. (What GHC are you using? Maybe it just so happens to be one where it works if all the MIN_VERSIONs evaluate to false? I was trying with 7.8.)

[snoyberg]

@glaebhoerl I'd recommend moving ahead with a patch that drops the cpphs dependency for now. The current release on Hackage is guaranteed to fail for anyone installing from scratch, and dropping cpphs will most likely work for everyone, the one exception being Mavericks users who haven't installed the CPP fix.

[bergmark]

The safe thing for OS X users is to install haskell platform through homebrew, that always worked with type-eq. Three people had problems and at least two of them used the binary installer and then applied the ghc-clang-wrapper linked from the platform website. Maybe that path is slightly broken.

[mbrock]

Hello! So at @bergmark's request I did the tiny pull request that became 0.4.1. I don't really know what's going on, but I happen to have a Mavericks system with Platform from the binary installer. So if you want some patch tested on that setup, just mention me. :)

[bergmark]

Thanks @mbrock!

My fear is that dropping the cpphs dependency will break a lot of users again, but perhaps there is no short term solution that works for everyone.

Here's what I've gathered:

• cpphs-1.18.2 and type-eq-0.4.1 are incompatible.
• type-eq-0.4 is incompatible with standard Mavericks installations, so just dropping the cpphs dependency would bring us back here and these users would have to reinstall HP to use the latest version.
• cpphs-1.18.1 and type-eq-0.4.1 works for everyone running GHC 7.6, but doesn't work with GHC 7.8 (not sure if any of the configurations work with GHC7.8 yet)

[glaebhoerl]

Again, my experiments with all of cpphs 1.11, 1.18.1, and 1.18.2 show that MIN_VERSION_base doesn't work with cpphs at all.

If I put this near the beginning of Type/Eq.hs:

#ifndef MIN_VERSION_base
#error "no MIN_VERSION_base"
#endif

#if MIN_VERSION_base(4, 4, 0)
#error "4.4.0"
#else
#error "no 4.4.0"
#endif

and try to build the package, I get cpphs: #error "no 4.4.0" every time, regardless of what cpphs versions (the mentioned ones) or GHC versions (7.6 and 7.8) I try it with, and totally regardless of what numbers I put in as arguments to MIN_VERSION_base. If I use system cpp, it works fine.

I'm trying to get confirmation of this from someone else, because it's surprising to me that Cabal's MIN_VERSION_foo macros would not work with cpphs, given that being suitable for preprocessing Haskell code is the reason why cpphs even exists.

[snoyberg]

I can confirm your results.

[glaebhoerl]

Thanks!

Given that the only package whose version needs to be branched on is base, and the version of base is always tied to that of GHC, and the version of GHC can be branched on in the Cabal file, I think I'm going to do that, and introduce (or not) preprocessor defines that can be simply #ifdefed over from the Cabal file, based on the GHC version.

[snoyberg]

In that case, why not just use GLASGOW_HASKELL?

[glaebhoerl]

It seems e.g. #if __GLASGOW_HASKELL__ >= 706 does work with cpphs, so good point, that might be an easier option.

[malcolmwallace]

I will soon be releasing a new version of cpphs (1.18.3), to fix a bunch of integer arithmetic and precedence issues in the #if conditional evaluator. (And yes, 1.18.2 incorrectly reports itself as 1.18.1).

However, the main issue seems to be that ghc does not pass the cabal_macros.h file to the preprocessor, so there is simply no visible definition of MIN_VERSION_base(x,y,z) as far as cpphs can see.

[glaebhoerl]

@malcolmwallace I believe I ruled that possibility out (see also above):

#ifndef MIN_VERSION_base
#error "no MIN_VERSION_base"
#endif

This doesn't trigger, ergo MIN_VERSION_base is defined. If I change the name to a non-existent package (#ifndef MIN_VERSION_blah e.g.), it does trigger.

I also tried explicit #include "cabal_macros.h", #include "dist/build/autogen/cabal_macros.h", and ghc-options: -optP-include -optPdist/build/autogen/cabal_macros.h in the cabal file (in addition to the -pgmPcpphs -optP--cpp options), and none of them made a difference. (If I tried including a non-existent file, again I got an error as expected.)

So my conclusion is that cabal_macros.h is being automatically included, and the problem is around evaluating the MIN_VERSION_base macro itself.

[glaebhoerl]

I pushed a new version that uses __GLASGOW_HASKELL__, it appears to work with everything I've tried throwing at it, could others test it out? If all is in order, I'll put it up on Hackage.

[mbrock]

I successfully built a4676ed on Mavericks with the Platform from the installer and the Clang wrapper script. 💃

[bergmark]

a4676ed builds on my homebrew installed HP on Mavericks as well. Looking good, thanks!
Pinging @feuerbach, he was trying to get type-eq running on 7.6 iirc.

[malcolmwallace]

With cpphs-1.18.3, I think everything works as expected:

$ ../cpphs gabor
cpphs: #error "no MIN_VERSION_base"
in gabor at line 2 col 1

$ ../cpphs '-DMIN_VERSION_base(x,y,z)=foo' gabor
cpphs: #error "no 4.4.0"
in gabor at line 8 col 1

$ ../cpphs '-DMIN_VERSION_base(x,y,z)=1' gabor
cpphs: #error "4.4.0"
in gabor at line 6 col 1

[snoyberg]

I can confirm a successful install with cpphs-1.18.3 and type-eq 0.4.1.

[glaebhoerl]

Uploaded 0.4.2 to Hackage.

@malcolmwallace Great! I'm sticking with the approach that avoids MIN_VERSION_base to keep compatibility with older versions. Now there's multiple combinations that should work.

Thanks everyone for the assistance.