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

ghc-options: -Wall and doctest #30

Closed
angerman opened this issue May 2, 2017 · 8 comments
Closed

ghc-options: -Wall and doctest #30

angerman opened this issue May 2, 2017 · 8 comments

Comments

@angerman
Copy link

angerman commented May 2, 2017

With sol/doctest#156 and haskell/cabal#4480, the custom build type and setup could be dropped, and a simple cabal doctest would do the testing.

However, as this now passes the same flags as cabal, doctest is run with -Wall as well, as specified in the ghc-options:. This however results in the following test failure:

distributive-0.5.2 $ ../cabal/dist-newstyle/build/cabal-install-2.1.0.0/build/cabal/cabal doctest --with-doctest=../doctest/dist/build/doctest/doctest
Preprocessing library for distributive-0.5.2..
### Failure in src/Data/Distributive.hs:84: expression `distribute [(+1),(+2)] 1'
expected: [2,3]
 but got:
          <interactive>:23:1: warning: [-Wtype-defaults]
              • Defaulting the following constraints to type ‘Integer’
                  (Show t0) arising from a use of ‘print’ at <interactive>:23:1-24
                  (Num t0) arising from a use of ‘it’ at <interactive>:23:1-24
              • In a stmt of an interactive GHCi command: print it
          [2,3]
Examples: 1  Tried: 1  Errors: 0  Failures: 1

As an affected package by this, I'd like to solicit some feedback on this from your point of view, what would be the expected behaviour here?

@RyanGlScott
Copy link
Collaborator

I see two ways to fix this:

  1. Use a type annotation: [2,3] :: [Int]. This would squash that particular warning, but it makes the doctests noisier, and looking forward, I'm not sure how well this technique would scale.
  2. Put doctest-options: -Wno-type-defaults in distributive.cabal (à la Add support for x-doctest-options ulidtko/cabal-doctest#10). I'm personally more inclined towards this option, since running doctest is tantamount to having an extended GHCi session, wherein you'd typically not want all of the warnings associated with -Wall.

@angerman
Copy link
Author

angerman commented May 2, 2017

I'm in favor of the second option as well. Would you expect the behaviour to be that ghc-options AND doctest-options are passed? E.g. -Wall -Wno-type-defaults here?

@RyanGlScott
Copy link
Collaborator

To be honest, I'm a bit conflicted on whether we should be passing the same ghc-options that the library uses to doctest in the first place. In my mind, doctest is a downstream consumer of the library, and most options that I could think of that you'd want to apply in the library (e.g., -O2, -fmax-simplifier-iterations, etc.) aren't really relevant in an interpreted setting. But perhaps others feel differently about this...

If we do decide to pass both sets of options, I would expect doctest-options to override ghc-options, yes.

@angerman
Copy link
Author

angerman commented May 2, 2017

Thank you for the input! I hope you are ok with a doctest-ghc-options field; and ignoring ghc-options: when running cabal doctest. I'm still a bit unsure if ignoring is the right thing here, but your reasoning tipped me to the "let's ignore them" side. The argument I tried to make with myself was: doctest should run identical to how the library is compiled, and hence we need the ghc-options:; and the case that one would need to argue why cabal doctest would flat out ignore ghc-options:.

However, I'm happy with the explicit doctest-ghc-options: even though it might introduce minor redundancy.

@phadej
Copy link
Collaborator

phadej commented May 2, 2017

One might want or need to specify e.g. -fobject-code

@angerman
Copy link
Author

angerman commented May 2, 2017

@phadej right, that wouldn't work without doctest-ghc-options:. However it would work with either ghc-options: + doctest-ghc-options: or just doctest-ghc-options: (and magically ignoring ghc-options:)

@ekmett
Copy link
Owner

ekmett commented Oct 9, 2017

I'm not yet ready to drop the custom build mode I use. We'd need to get to a point where the case that just works has much much better user-base penetration lest I cut off users.

That said, I'm open to making the tests more verbose to have them run -Wall clean.

@RyanGlScott
Copy link
Collaborator

Now that we have migrated away from cabal-doctest to cabal-docspec in #60, we no longer use a custom Setup script. As a result, I believe the original motivation for this issue is now moot, so I'm going to close this issue. If you feel that there is still something to be addressed here, please re-open this.

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

No branches or pull requests

4 participants