rubik / argon

Monitor cyclomatic complexity in Haskell programs
BSD 3-Clause "New" or "Revised" License
97 stars 8 forks source link

CPP preprocessing error in the hint library #27

Closed meditans closed 8 years ago

meditans commented 8 years ago

I tried argon with hint (in particular the src directory) and I get a bunch of errors related to the cpp directives, like:

src/Language/Haskell/Interpreter/Unsafe.hs
    error: 28:2 lexical error at character 'i'
src/Hint/InterpreterT.hs
    error: 38:2 lexical error at character 'i'
src/Hint/CompatPlatform.hs
    error: 1:2 lexical error at character 'i'

A sample error which generates this is:

#if __GLASGOW_HASKELL__ < 800
                                 , Functor m
#endif

Can you reproduce this error?

More generally, I'm searching for a stable way to remove all the cpp preprocessing, so that I can than analyze the code with the ghc api or haskell-src-exts. Do you have any advice?

rubik commented 8 years ago

Yes, I can reproduce it. The error lexical error at character 'i' is an Haskell error. That means that CPP is not run on the files. In fact, I can confirm this, because adding {-# LANGUAGE CPP #-} in those files makes argon work.

Usually, if one does not want to litter all the files with a language pragma, they'll put the extensions used in the Cabal file. I can see that CPP is listed in hint.cabal. In theory the --cabal-file option should come in handy just in cases like this one, but unfortunately it does not work here.

So it seems that we have a bug in how extensions are read from the Cabal file, not related to CPP.

rubik commented 8 years ago

I found the problem. Argon reads the field default-extensions from the Cabal file: https://github.com/rubik/argon/blob/master/src/Argon/Cabal.hs#L34

However, even if the field extensions has been deprecated in favor of default-extensions, no one uses it. And all extensions specified under the field extensions are treated by Cabal as "old extensions".

To further complicate the matters, there's yet another field, "other-extensions", for the extensions that are used in a few files (default-extensions should list extensions that are used in all files). So it's probably useful to read them all and concatenate the results, even if it's not best (we would enable extensions which are not needed).

rubik commented 8 years ago

I tested on Hint's source and it works. You'll get additional CPP errors, like

hint/src/Hint/Base.hs
    error: 29:0  error: missing binary operator before token "("
         #if MIN_VERSION_base(4,8,0)

because MIN_VERSION_* are macros defined by Cabal in the file cabal_macros.h.

I see that Hint does not use Stack, and I don't know where that file is for pure-Cabal projects. I quickly built Hint as a Stack project with

$ stack init && stack build

And the header is at $(stack path --dist-dir)/build/autogen/cabal_macros.h. After this, running Argon with --cabal-macros $(stack path --dist-dir)/build/autogen/cabal_macros.h solves the problem.

There are still some weird errors, like

../hint/src/Hint/Configuration.hs
    error: 59:20 Not a data constructor: ‘forall’
Perhaps you intended to use ExistentialQuantification

which I don't understand because the mentioned extension is specified in the Cabal file and loaded by Argon (I confirmed this). So that's still a mystery. Feel free to open a new issue so that we can investigate that.

meditans commented 8 years ago

Hi, awesome work! Unfortunately, even after git pull && stack install, I still get the errors of the form

 error: 1:2 lexical error at character 'i'

even when using the --cabal-macros option (which shouldn't be needed for those errors). Do you have any idea why?

rubik commented 8 years ago

Are you using the --cabal-file option? Currently there is no way to pass language extensions names on the command line, so either they're in the code or in the Cabal file you pass to Argon.

meditans commented 8 years ago

Yeah, I wasn't using the option --cabal-file; using that I remain with two errors, one of which you mentioned before (about forall), and the other which probably shares the same cause.

So, why is --cabal-file necessary here? The default behaviour is reading only the file in question?

rubik commented 8 years ago

That option is necessary because Argon does not read the Cabal file automatically and if you don't specify the language extensions in the code then Argon needs the Cabal file to know which extensions to load. No extensions are loaded automatically.