-
Notifications
You must be signed in to change notification settings - Fork 272
-
Notifications
You must be signed in to change notification settings - Fork 272
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Lens doesn't build with --ghc-options="-pgmP cpphs -optP " #415
Comments
This is part of my work on https://ghc.haskell.org/trac/ghc/ticket/8683 to decouple ghc from needing to use the C compiler for CPP. Looks like its a cpphs bug. reporting it here |
The problem is that lens defines operators with "//" which cpphs interprets as a single-line comment. |
You can turn off C++ style comments with an option to cpphs. |
@malcolmwallace which option flag is that? If we wind up shipping CPPHS with ghc for the 7.8 release cycle, would be nice to give it the right default flags! i was just using |
There actually appears to be a bug in how cpphs calls its "tokenise" [sic] function. The first two arguments are stripEol and stripC89 but they are swapped at the call sites. (Just from a cursory look at 1.8.3's source code) |
With that fix I was able to build lens using --ghc-options="-pgmP cpphs -optP-traditional -optP--cpp" |
Good spot. Thanks glguy. [P.S. "tokenise" is cromulent spelling for a Brit] |
@glguy @malcolmwallace i thought cpphs was always in |
When cpphs is called by ghc, it gets set to -traditional mode, yes. But traditional mode strips C89 comments, leaving C++ EOL comments intact. If the flags were accidentally swapped internally, that would explain the confusion. |
@malcolmwallace i don't understand the sentence either ghc passes --cpp --traditional to the CPP program or not, what flags must GHC pass to cpphs (such as would be set by -optP) to work as expected? Exactly what @glguy pasted before or something else? |
I do beg your pardon, I was mis-remembering the behaviour of the cpp-compatibility options for cpphs. So, -traditional is not the default, as glguy says. You can add it with -optP--cpp -optP-traditional. Or alternatively, you can use -optP--cpp -optP-CC to retain all C-style comments (-CC is a standard gcc cpp flag). |
to repeat my question: what flags should I have ghc default to passing to CPPHS for ALL haskell code? :) (I'll need to pass this info along @thoughtpolice and @hvr for the 7.8.1 release :) ) |
Well, that depends somewhat on the source code to be processed. If the Haskell source code contains valid Haskell symbols // or /* or */ then you need to turn off comment-stripping; but if the CPP directives contain C-style comments, then you need to strip them. However I think any package that contains both CPP and the Haskell symbol // must already add the extra option -optP-CC (in the cabal file), otherwise how could it compile correctly? |
@malcolmwallace: Since @cartazio: So it seems we're basically being killed by the I wonder if we can exile them to non-preprocessed modules as a hackish workaround in the meantime. Ugh. |
Hrmm, so I guess we can probably get away with just adding |
@ekmett you could totally do a setup.hs hack to detect what the CPP program is and if its cpphs add the right flags to optP |
Honestly, that sounds pretty hard to do correctly and portably, comparatively. |
@ekmett its not the ghc wrap ..... austin is tentatively thinking of bundling cpphs with ghc 7.8.... and what i've been doing also makes it easier to change the cpp in user land. but this does exhibit a good corner case if that happens. I don't think it will happen, but it could |
Soooo, if I had passed -cpp -traditional, rather than -cpp, this wouldn't have happened? |
@malcolmwallace could we have |
So, cpphs --cpp -traditional is supposed to act exactly as you want: stripping /**/ comments, but retaining // comments. But there was a bug, identified by glguy, which meant cpphs got this wrong. Once I release cpphs-1.18.4, all should be good, I hope. |
Thank you for clarifying; I'm looking forward to the |
OK, cpphs-1.18.4 is now on hackage. |
Thanks, Malcolm. Edward has already added -traditional to the cpp-options, so that should wrap up this ticket. |
should -traditional need be added? GHC should always be setup up to do that ... ? |
Added it as a prophylactic measure. Willing to revisit it later. |
``
Preprocessing library lens-4.0.7...
src/Control/Lens/Setter.hs:55:5: parse error on input ‘assign’
cabal: Error: some packages failed to install:
lens-4.0.7 failed during the building phase. The exception was:
ExitFailure 1
``
the associated CPPHS invocation being
I don't know if this is a lens issue or a cpphs issue, and I'm personally not equipped to sort it out myself.
The text was updated successfully, but these errors were encountered: