Skip to content
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

"cabal haddock" uses CPP overzealously #1808

Closed
iainnicol opened this issue Apr 23, 2014 · 7 comments · Fixed by #1854
Closed

"cabal haddock" uses CPP overzealously #1808

iainnicol opened this issue Apr 23, 2014 · 7 comments · Fixed by #1854

Comments

@iainnicol
Copy link
Contributor

The Distribution.Simple.Haddock module determines whether to run CPP before Haddock by looking at Distribution.Package.allExtensions.

Instead it should look at usedExtensions, which is oldExtensions ++ defaultExtensions. It should also consider whether the individual source file has a {-# LANGUAGE CPP #-} pragma.

Concretely, suppose I have a Foo.hs which looks like this:

module Foo (bar) where

bar :: String
bar = "\
\A"

And suppose I have CPP listed in Cabal's other-extensions because some other .hs file uses CPP. Then "cabal haddock" will fail:

Foo.hs:4:9:
    lexical error in string/character literal at character 'A'
@tibbe
Copy link
Member

tibbe commented Apr 23, 2014

Is this a regression since 1.18?

I believe we never look inside the source file for extensions now so that would be a larger change.

@iainnicol
Copy link
Contributor Author

The good news is it's not a regression since 1.18. I think it might have always been an issue.

I was afraid you might say it's a larger change. Oh well.

@23Skidoo
Copy link
Member

I believe that haddock 2.x can pass its input through the C pre-processor itself, since it uses the GHC API. If we drop support for haddock 0.x (as suggested in #1718), we can just pass -optghc=-XCPP to haddock when CPP is in usedExtensions. It should be able to handle {-# LANGUAGE CPP #-} without our help.

@tibbe
Copy link
Member

tibbe commented Apr 24, 2014

I think dropping haddock 0.x is fine.

@iainnicol
Copy link
Contributor Author

In the interests of avoiding duplication of effort, please note I've been working on this issue, and hence also Issue #1718. Thanks @23Skidoo for the approach.

My code seems to be working, but needs slightly cleaned up before submission. Also, I'd like to add some tests if I can, before submission.

@iainnicol
Copy link
Contributor Author

Hmm. I could write cabal tests which invoke haddock. But this seems wrong from a bootstrapping PoV, because haddock I presume is built with cabal. Thoughts?

@23Skidoo
Copy link
Member

23Skidoo commented May 9, 2014

@iainnicol

But this seems wrong from a bootstrapping PoV, because haddock I presume is built with cabal.

I don't think there's a problem - it's not like you need to run the test suite to install Cabal.

iainnicol added a commit to iainnicol/cabal that referenced this issue May 9, 2014
Until recently we supported ancient versions of Haddock, pre v2.0.  To
support the CPP extension with such versions, cabal had to invoke the
CPP before invoking Haddock on the output.  For simplicity cabal would
invoke the CPP on all Haskell files, if any Haskell file required CPP.
However, invoking CPP on a file which does not require it can cause
build failures.

Haddock v2.0+ supports the CPP via GHC, and even automatically
preprocesses any file with the {-# LANGUAGE CPP #-} pragma. Hence we
simply need only tell Haddock to enable the CPP when the CPP is a
package level default extension.

Fixes issue haskell#1808.
iainnicol added a commit to iainnicol/cabal that referenced this issue May 10, 2014
Until recently we supported ancient versions of Haddock, pre v2.0.  To
support the CPP extension with such versions, cabal had to invoke the
CPP before invoking Haddock on the output.  For simplicity cabal would
invoke the CPP on all Haskell files, if any Haskell file required CPP.
However, invoking CPP on a file which does not require it can cause
build failures.

Haddock v2.0+ supports the CPP via GHC, and even automatically
preprocesses any file with the {-# LANGUAGE CPP #-} pragma. Hence we
simply need only tell Haddock to enable the CPP when the CPP is a
package level default extension.

Fixes issue haskell#1808.
mzero pushed a commit to mzero/cabal that referenced this issue Jul 23, 2014
Until recently we supported ancient versions of Haddock, pre v2.0.  To
support the CPP extension with such versions, cabal had to invoke the
CPP before invoking Haddock on the output.  For simplicity cabal would
invoke the CPP on all Haskell files, if any Haskell file required CPP.
However, invoking CPP on a file which does not require it can cause
build failures.

Haddock v2.0+ supports the CPP via GHC, and even automatically
preprocesses any file with the {-# LANGUAGE CPP #-} pragma. Hence we
simply need only tell Haddock to enable the CPP when the CPP is a
package level default extension.

Fixes issue haskell#1808.
tibbe pushed a commit that referenced this issue Jul 27, 2014
Until recently we supported ancient versions of Haddock, pre v2.0.  To
support the CPP extension with such versions, cabal had to invoke the
CPP before invoking Haddock on the output.  For simplicity cabal would
invoke the CPP on all Haskell files, if any Haskell file required CPP.
However, invoking CPP on a file which does not require it can cause
build failures.

Haddock v2.0+ supports the CPP via GHC, and even automatically
preprocesses any file with the {-# LANGUAGE CPP #-} pragma. Hence we
simply need only tell Haddock to enable the CPP when the CPP is a
package level default extension.

Fixes issue #1808.

(cherry picked from commit ba4ae3d)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants