Skip to content

Improve hlint cpp support using the preprocessed buffer #1281

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

Closed
wants to merge 6 commits into from

Conversation

jneira
Copy link
Member

@jneira jneira commented Jan 31, 2021

  • As suggested by @ndmitchell here
  • It enables hlint suggestions for files with cpp conditions and ghc < 8.10

@jneira jneira requested a review from ndmitchell January 31, 2021 21:57
Copy link
Collaborator

@ndmitchell ndmitchell left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Awesome - looks good to me.

@jneira jneira marked this pull request as draft February 4, 2021 08:54
@jneira
Copy link
Member Author

jneira commented Feb 4, 2021

After doing a manual test i've found that the code span of the hlint diagnostics using the preprocessed buffer doesnt match the real file. That was not tested and it should be cause i suspected it could be happen: #554 (comment)

@ndmitchell i understood from your comment that the code spans would be honoured in the preprocessed but it seems that if it is not the case afaics.

To make it work we should recover the original source spans from the preprocessed one to correct the diagnostic and not sure if it will worth (over other alternatives like extract cpp flags from the ghc session and pass them to hlint) 😟

@ndmitchell
Copy link
Collaborator

What should happen is the preprocessor should drop {-# LINE #-} pragmas, which GHC should honour when building the AST. Are the LINE pragmas being produced? And are they being honoured?

@jneira
Copy link
Member Author

jneira commented Feb 6, 2021

Mmm, i've kept the hspp intermediate file with ghc -keep-hscpp-file -tmpdir . -XCPP .\ApplyRefact2.hs

being .\ApplyRefact2.hs

module ApplyRefact2 where

#include "test.h"

#ifdef TEST
f = (1)
#else
f = 1
#endif

And the file in the line 215 of ApplyRefact2.hscpp:

# 8 "<command-line>" 2
# 1 "ApplyRefact2.hs"
module ApplyRefact2 where


# 1 "test.h" 1
# 4 "ApplyRefact2.hs" 2


f = (1)

Not sure if lines should be there

@ndmitchell
Copy link
Collaborator

OK, so GHC is dropping the # line number prefixes. That's a bit surprising, but I think is supported by HLint (or rather, by GHC). However, I think cpphs probably removes them. So you may need to pass NoCpp to the parseModuleEx function of HLint - which kind of makes sense, give you have already handled CPP. Does that help?

(And sorry for taking so long to respond, I'm miles deep in a pile of emails)

@jneira
Copy link
Member Author

jneira commented Feb 28, 2022

dont have time to continue working on this, feel free to reuse it

@jneira jneira closed this Feb 28, 2022
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 this pull request may close these issues.

2 participants