hlint and CPP

[alanz]

There is an attempt to add a .hlint.yaml config file to the GHC source tree. This has uncovered a problem when running against files with CPP in them. See https://gitlab.haskell.org/ghc/ghc/-/issues/18424#note_309956 for details.

[jneira]

But does the problem arise using hlint standalone or only through the hlint plugin and hls?

[ndmitchell]

It will arise in HLint standalone too, since standalone HLint uses cpphs, which has an API that fails rather than returns error values. But, HLS should be able to do a lot better - you can grab the code after CPP has been expanded, and pass that source direct to HLint, and disable CPP in HLint itself.

[jneira]

But, HLS should be able to do a lot better - you can grab the code after CPP has been expanded, and pass that source direct to HLint, and disable CPP in HLint itself.

Interesting, thanks for the tip. However we have two code paths, one for latest ghc and the other one when hlint is using ghc-lib.

• For the first one i have the parsed module from ghcide, afaiu after cpp expanse
• For the second one we are parsing the module using hlint parseModuleEx and not sure if could apply the fix in that case

@alanz have you tried the plugin over ghc using hls compiled with ghc-8.10.2?

[ndmitchell]

When you pass in the parsed module, you won't be going near CPP, as that only runs on the source. No need to do anything with that code path.

When you take the source to give to parseModuleEx, if you get it after calling preprocessor, then GHC will have already applied the CPP preprocessor to it. I think you can get that by asking for the GetModSummary rule on the file and pulling out the ms_hspp_buf field. You then disable CPP on the HLint side and it should all work out. In theory. I don't guarantee that HLint gets some aspect of this code wrong, but it is meant to work. Bonus is that it also applies other non-CPP preprocessors too.

[jneira]

I wonder if code spans would be the same between the original file and ms_hspp_buf (afaiu they could be different) and hovers and refactors will be correct or if we will need some ugly workaround to make it work. 😟

Anyways, will it be a temporary workaround in hls, until hlint itself is able to handle cpp correctly? Is there some hlint issue about?

EDIT:

• not sure if the main issue would be: Support preprocessors ndmitchell/hlint#501
• but other cpp issues has been closed with the solutions like use -XNoCpp, --cpp-simple or give hlint the needed cpp info it needs to parse the file

[ndmitchell]

CPP drops out {# LINE #} droppings which GHC parses, so yes, the locations should be identical. Refactors with CPP have never worked, and certainly won't start working.

HLint doesn't have full CPP information. HLS does. I think we should fix this in HLS, because we can do a better job there. The HLint solution of a bunch of half-solutions is as good as we can get. You shouldn't use --cpp-simple because that misparses some files - its as good as HLint can do, but HLS can do better.

[jneira]

We have added a bunch of tests over hlint, some of them cpp related. The results are:

• using ghc -D argument only works for ghc-8.10, as expected and anticipatedby @ndmitchell
• defining the variable in a header file and include it in the haskell module works for all versions

So if you use ghc-8.10 to build ghc HEAD, the hlint plugin should work, as far hie-bios is passing the -D arguments correctly.
Not sure if that workaround could be feasible.

[jneira]

When you take the source to give to parseModuleEx, if you get it after calling preprocessor, then GHC will have already applied the CPP preprocessor to it. I think you can get that by asking for the GetModSummary rule on the file and pulling out the ms_hspp_buf field. You then disable CPP on the HLint side and it should all work out. In theory. I don't guarantee that HLint gets some aspect of this code wrong, but it is meant to work. Bonus is that it also applies other non-CPP preprocessors too.

hi @ndmitchell, i was thinking in another alternative, not sure if it could be better though: we could extract the -D arguments from the underlying ghc info provided and pass them to hlint, keeping cpp enabled.

I've tested that hlint supports header file included in the source file:

haskell-language-server/test/functional/FunctionalCodeAction.hs

Lines 108 to 110 in 0c7e9a0

	, testCase "hlint diagnostics works with CPP via -XCPP argument and flag via #include header (#554)" $ runHlintSession "cpp" $ do
	doc <- openDoc "ApplyRefact2.hs" "haskell"
	testHlintDiagnostics doc

haskell-language-server/test/testdata/hlint/cpp/ApplyRefact2.hs

Lines 1 to 9 in 0c7e9a0

	module ApplyRefact2 where
	#include "test.h"
	#ifdef TEST
	f = (1)
	#else
	f = 1
	#endif

Are there other ways to add cpp info to the source file?

[ndmitchell]

I put a lot of work into CPP on the Ghcide side, since Ghcide requires CPP to be perfect. There's no way I'll ever put as much effort into the HLint side, so I think using the Ghcide CPP work will always give better results.

Passing the -D arguments etc from GHC to HLint will do better than not doing that, but still I doubt it will be as good as leaving Ghcide to preprocess it, and even if it was, only running CPP once is always going to be (very fractionally) faster.

[jneira]

Fair enough, i will try to use ms_hspp_buf as first option.