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 case change for warning breaks haskell-mode check #1513

Closed
sirlensalot opened this issue May 31, 2017 · 4 comments
Closed

GHC case change for warning breaks haskell-mode check #1513

sirlensalot opened this issue May 31, 2017 · 4 comments

Comments

@sirlensalot
Copy link
Contributor

As of 8.0.2, GHC is now issuing warnings with a lower-case 'w':

src/Pact/Compile.hs:49:1-40: warning: [-Wunused-imports] …
    The import of ‘Hash’ from module ‘Pact.Types.Crypto’ is redundant

As a result, tools are not recognizing this and flagging things as errors or worse. Here's relevant results from a grep:

./haskell-interactive-mode.el:1032:             (not (string-match "^\n<interactive>:[-0-9]+:[-0-9]+:[\n ]+Warning:" response)))
./haskell-load.el:204:        ((string-match "Warning: orphan instance: " msg)
./haskell-load.el:398:    ((string-prefix-p "Warning:\n    " haskell-msg)
./haskell-load.el:521:             (type (cond ((string-match "^Warning:" error-msg)  'warning)

For string-match, using [Ww]fixes the issue and is presumably backward-compatible. Ie for line 521:

             (type (cond ((string-match "^[Ww]arning:" error-msg)  'warning)

works with my GHC 8.0.2 environment. string-prefix-p would need a better fix. I can't comment on the other places string-match is in use so I would request someone more familiar with this codebase make the fix.

NB: Why in [deity]'s name did they change the case in 8.0.2????? Do the haskell gods just enjoy breaking tools, because the GHC8 upgrade wasn't painful enough???

@ivan-m
Copy link
Contributor

ivan-m commented May 31, 2017

This seems to be the commit they did it in (or at least some of their test-cases started to use the lower-case variant).

@gracjan
Copy link
Contributor

gracjan commented May 31, 2017

Gods are whimsical and erratic and we were almost prepared: #1168.

@sirlensalot
Copy link
Contributor Author

Deeper research using a grep for "arning" in *.el files. Relevant finds:

  1. ./haskell-compile.el:73: ":\\(?6:\n?[ \t]+[Ww]arning:\\)?") From Improve warning recognition. Clean up whitespace. #1477, which was aware of the case issue. not sure if it works with [-Wfoo] stuff that shows up though.
  2. ./haskell-interactive-mode.el:1032: (not (string-match "^\n<interactive>:[-0-9]+:[-0-9]+:[\n ]+Warning:" response))) This is a hard one to test as it's unclear why it's avoiding warnings; in my REPL errors are getting clobbered with weird types no matter what. Nonetheless, would seem to be safe to change to [Ww].
  3. ./haskell-load.el:204: ((string-match "Warning: orphan instance: " msg) This is broken irrespective of case change.
  4. ./haskell-load.el:398: ((string-prefix-p "Warning:\n " haskell-msg) Also broken irr. of case change
  5. ./haskell-load.el:521: (type (cond ((string-match "^Warning:" error-msg) 'warning) this is the main one for this issue, easily fixed with [Ww]
  6. ./inf-haskell.el:84: ("^\\(.+?\\):\\([0-9]+\\):\\(\\([0-9]+\\):\\)?\\( \\|\n *\\)\\([Ww]arning\\)?" case is fine, again not sure it handles [-Wfoo] stuff.

Proposal: I go ahead and introduce [Ww] for 2-5, but will only test the results for 5, and assume it doesn't break things further for the other cases. Please LMK if this is acceptable.

@gracjan
Copy link
Contributor

gracjan commented Jun 1, 2017

Yes, good enough! Go ahead!

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

3 participants