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

Fix warning parsing emitted by newer version of GHC #99

Closed
wants to merge 1 commit into from

Conversation

EduardSergeev
Copy link
Contributor

@EduardSergeev EduardSergeev commented May 11, 2024

Fixes #98

@@ -13,7 +13,7 @@ const regex = {
// 7-10: variant 3: (line, col)
message_base: /^(.+):(?:(\d+):(\d+)|(\d+):(\d+)-(\d+)|\((\d+),(\d+)\)-\((\d+),(\d+)\)): (.+)$/,
single_line_error: /^error: (?:\[.+\] )?([^\[].*)$/,
single_line_warning: /^warning: \[(.+)\] (.+)$/,
single_line_warning: /^warning: \[(.+)\] ([^\[].*)$/,
Copy link
Owner

Choose a reason for hiding this comment

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

Unfortunately this breaks the error_warnings handling below, IIUC

Maybe something like (?:\[GHC-.*\] )?\[(-W.+)\]?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This should only affect warning handling. Could you provide a simple example of a code which it breaks?

Copy link
Owner

Choose a reason for hiding this comment

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

okay sorry i misunderstood, this only matches warnings printed in a single line

Copy link
Contributor Author

@EduardSergeev EduardSergeev May 11, 2024

Choose a reason for hiding this comment

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

Yes, but newer GHC versions emit multi-line warnings which in version 0.2.3 are captured as single_line_warning (cutting the rest of the warning off, see #98 for example).
Example of a warning which would be captured by the previous version of single_line_warning:

warning: [GHC-38417] [-Wmissing-signatures]
    Top-level binding with no type signature: test :: p -> p

With this change it is correctly handled as a multi-line warning.

Copy link
Owner

Choose a reason for hiding this comment

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

My concern is mainly that these warnings (in the error_warnings list below) should be shown as errors:

                '-Wdeferred-type-errors',
                '-Wdeferred-out-of-scope-variables',
                '-Wtyped-holes'

It would be fine as long as none of these could be emitted in a single line in GHC 9.6+. However if any of these could then the regex would match the [GHC-*] instead of the [-W*] part, and it would no longer match the list.

Copy link
Collaborator

@Hexadecimaaal Hexadecimaaal May 11, 2024

Choose a reason for hiding this comment

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

hi! i'm poking around on the pr branch with ghc 9.6.4, and what @dramforever mentioned above does happen:

image

seems like it only happens if we have ghc output like this:

ghci | .../test.hs:1:1-4: warning: [GHC-38417] [-Wmissing-signatures]
ghci |     Top-level binding with no type signature: mlem :: Num a => p -> a

with the [GHC-nnnnn] part. the defined but not used warning does not have this part:

ghci | .../test.hs:1:6-9: warning: [-Wunused-matches]
ghci |     Defined but not used: ‘blep’

ok i think mine was not rebuilt completely, i can confirm the fix does work for me. i'm looking into some minor issues, as the typed holes and deferred type errors should be severity error.

Copy link
Collaborator

Choose a reason for hiding this comment

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

one issue here is that if i enable -fdefer-typed-holes (as is by default), the behavior should be upgrading a hole to a error severity in our diagnosis. this requires us to actually match for the \[(-W.+)\] part: (two lines below)

    warning: /^warning:(?: \[GHC-.*\])? \[(-W.+)\]$/

this work as intended on my setup.

for the single line warning we do rely on the group (error_warnings.indexOf(res_sl_warning[1]) >= 0 ?). however i could not really get any single line warning in the first place, perhaps we need older ghc? or do you have some examples for a single line warning that does hit this regex?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

i could not really get any single line warning in the first place, perhaps we need older ghc? or do you have some examples for a single line warning that does hit this regex?

Personally I cannot recall seeing single-line warning from GHC myself. Maybe the author of this line can provide an example of the code which emit such warning in some version of GHC?

Copy link
Owner

Choose a reason for hiding this comment

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

As an example, GHC 8.10 prints -Wunrecognised-pragmas as a single line warning

{-# what #-}
module What where
[1 of 1] Compiling What             ( test.hs, test.o )

test.hs:1:1: warning: [-Wunrecognised-pragmas] Unrecognised pragma
  |
1 | {-# what #-}
  | ^^^

Copy link
Owner

@dramforever dramforever May 12, 2024

Choose a reason for hiding this comment

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

9.6.4 does print it as a multiline, so ithink it's changed so that it's not possible to get a single-line error/warning with [GHC-*] (edit i was wrong), although as shown earlier 9.6 also has unnumbered warnings

[1 of 1] Compiling What             ( test.hs, test.o )

test.hs:1:1: warning: [GHC-42044] [-Wunrecognised-pragmas]
    Unrecognised pragma: WHAT
  |
1 | {-# what #-}
  | ^^^^^^^^

edit no it looks like single line is still possible if we turn up the available columns for the pretty printer -dppr-cols=150, which is imo fair because like, i have that many columns on the terminal! it's also probably a good idea to handle this to cover all bases

test.hs:1:1: warning: [GHC-42044] [-Wunrecognised-pragmas] Unrecognised pragma: WHAT
  |
1 | {-# what #-}
  | ^^^^^^^^

@dramforever
Copy link
Owner

Thanks for the help. I don't think i'll be able to try it out and figure out how to integrate it atm.

I'll look into the ci fails myself later.

@EduardSergeev
Copy link
Contributor Author

EduardSergeev commented May 11, 2024

I'll look into the ci fails myself later.

The build is already fixed. With this and this commits.
Fixed by a previous commit so both commits from this PR were no longer needed and have been removed by rebasing on the latest master.

@EduardSergeev EduardSergeev force-pushed the fix/ghc-9.6 branch 2 times, most recently from c7a9900 to b17af8f Compare May 11, 2024 07:37
package.json Outdated Show resolved Hide resolved
Handle warning format of the latest (>= 9.6.4) versions of GHC
@Hexadecimaaal
Copy link
Collaborator

after some experimentation and discussions, we've decided to go with the /^warning:(?: \[GHC-.*\])? \[(-W.+)\]$/, as it preserves the original behavior. i'm closing this in favor of #100. thank you for your help!

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.

Diagnostics parsing is broken for GHC versions after 9.6.*
3 participants