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.

Sign up for GitHub

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

Jump to bottom

Fix warning parsing emitted by newer version of GHC #99

Closed
wants to merge 1 commit into from
Closed

Fix warning parsing emitted by newer version of GHC #99

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion src/features/diagnostics.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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 •
edited
Loading

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 •
edited
Loading

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 •
edited
Loading

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 #-}
  | ^^^^^^^^

error: /^error:(?: \[.*\])?$/,
warning: /^warning:(?: \[(.+)\])?$/
};
Expand Down
Loading