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 (and ghc-lib-parser-9.10.1.20240511) no longer builds with happy 2.0 #303

Closed
Tritlo opened this issue Sep 19, 2024 · 8 comments · Fixed by #304
Closed

ghc (and ghc-lib-parser-9.10.1.20240511) no longer builds with happy 2.0 #303

Tritlo opened this issue Sep 19, 2024 · 8 comments · Fixed by #304

Comments

@Tritlo
Copy link

Tritlo commented Sep 19, 2024

I know this is a downstream change, but since it is used across the ecosystem, I figured I would report it here (maybe it's a "happy" bug?). When installing fourmolu, which depends on ghc-lib-parser, I get the following error:

[416 of 447] Compiling GHC.Parser       ( dist/build/GHC/Parser.hs, dist/build/GHC/Parser.o, dist/build/GHC/Parser.dyn_o )

dist/build/GHC/Parser.hs:5975:24: error: [GHC-58481]
    parse error on input ‘<-’
    Suggested fix: Possibly caused by a missing 'do'?
     |
5975 |                   ; mg <- mkPatSynMatchGroup name happy_var_5
     |                        ^^

Maybe the version bound is not tight enough in ghc-lib-parser? Anyway, seems like libraries that depend on happy are breaking 😓

@phadej
Copy link
Contributor

phadej commented Sep 19, 2024

ghc-lib-parser doesn't have upper bounds on happy, it specifies:

build-tool-depends: alex:alex >= 3.1, happy:happy > 1.20

I couldn't test whether GHC proper is buildable with happy-2.0 (https://gitlab.haskell.org/ghc/ghc/-/issues/25276); I'd expect that it should be, as happy-2.0 doesn't mention any changes to the format/grammar/generated code. If GHC isn't buildable (i.e needs changes), I'd consider it a bug in happy-2.0.

@phadej
Copy link
Contributor

phadej commented Sep 19, 2024

you'll see this bug!

I meant that if GHC proper fails to build with happy-2.0, then arguably a bug is in happy-2.0 (at least in changelog as "everything is broken"); if GHC proper however builds fine, then the issue is in ghc-lib-parser.

But GHC proper is not buildable, I see

_build_master/stage0/compiler/build/GHC/Parser.hs:6113:24: error: [GHC-58481]
    parse error on input ‘<-’
    Suggested fix: Possibly caused by a missing 'do'?
     |
6113 |                   ; mg <- mkPatSynMatchGroup name happy_var_5
     |                        ^^
Command failed

... so the something wrong with happy-2.0


For the reference the rule looks like

-- Glasgow extension: pattern synonyms
pattern_synonym_decl :: { LHsDecl GhcPs }
        : 'pattern' pattern_synonym_lhs '=' pat_syn_pat
         {%      let (name, args, as ) = $2 in
                 amsA' (sLL $1 $> . ValD noExtField $ mkPatSynBind name args $4
                                                    ImplicitBidirectional
                      (as ++ [mj AnnPattern $1, mj AnnEqual $3])) }

        | 'pattern' pattern_synonym_lhs '<-' pat_syn_pat
         {%    let (name, args, as) = $2 in
               amsA' (sLL $1 $> . ValD noExtField $ mkPatSynBind name args $4 Unidirectional
                       (as ++ [mj AnnPattern $1,mu AnnLarrow $3])) }

        | 'pattern' pattern_synonym_lhs '<-' pat_syn_pat where_decls
            {% do { let (name, args, as) = $2
                  ; mg <- mkPatSynMatchGroup name $5
                  ; amsA' (sLL $1 $> . ValD noExtField $
                           mkPatSynBind name args $4 (ExplicitBidirectional mg)
                            (as ++ [mj AnnPattern $1,mu AnnLarrow $3]))
                   }}

but the generated code is

  ( do { let (name, args, as) = happy_var_2
                  ; mg <- mkPatSynMatchGroup name happy_var_5
                  ; amsA' (sLL happy_var_1 happy_var_5 . ValD noExtField $
                           mkPatSynBind name args happy_var_4 (ExplicitBidirectional mg)
                            (as ++ [mj AnnPattern happy_var_1,mu AnnLarrow happy_var_3]))
                   })}}}}})
  ) (\r -> happyReturn (happyIn118 r))

@Ericson2314 Ericson2314 changed the title ghc-lib-parser-9.10.1.20240511 no longer builds ghc (and ghc-lib-parser-9.10.1.20240511) no longer builds with happy 2.0 Sep 19, 2024
@phadej
Copy link
Contributor

phadej commented Sep 19, 2024

This looks like a breaking change caused by (undocumented in changelog) tab-to-spaces change.

EDIT: one can fix that occurrence in GHC by not using layout:

         | 'pattern' pattern_synonym_lhs '<-' pat_syn_pat where_decls
-            {% do { let (name, args, as) = $2
+            {% do { let { (name, args, as) = $2 }
                   ; mg <- mkPatSynMatchGroup name $5

but there are more errors to fix. Arguably, using layout in happy rules is not a great idea, and GHC parser seems to use some.

@int-index int-index removed the bug label Sep 19, 2024
@int-index
Copy link
Collaborator

int-index commented Sep 19, 2024

Alright, so it's not a bug in a strict sense. happy offers no guarantees about the layout of the code it emits. Still, we should avoid unnecessary breakage, so I suggest emitting 8 spaces instead of tabs (fbc1896 uses 2 spaces)

@Tritlo
Copy link
Author

Tritlo commented Sep 19, 2024

Alright, so it's not a bug in a strict sense. happy offers no guarantees about the layout of the code it emits. Still, we should avoid unnecessary breakage, so I suggest emitting 8 spaces instead of tabs (fbc1896 uses 2 spaces)

I mean, happy should at least work for GHC, right?

@sgraf812
Copy link
Collaborator

sgraf812 commented Sep 20, 2024

It's generally a bad idea to use layout in happy-generated parsers. For example, happy will not attempt to strip whitespace to keep the alignment with the start of the {% line (but perhaps it should?). It's IMO shear coincidence that GHC builds with the code as-is. I have debugged such errors myself multiple times while working on GHC's parsers. In #305 I outline a promising proposal which should be fairly insensitive to the indent used by happy-generated code.

That said, it's an unforced change and I'm unsure if increasing to 8 spaces will fix the problem. Although actually perhaps it will, given that 8 characters is the size of a tab stop prescribed by Haskell2010.

sgraf812 added a commit that referenced this issue Sep 20, 2024
2 spaces forced a breaking change on GHC. Fixes #303.
sgraf812 added a commit that referenced this issue Sep 20, 2024
2 spaces forced a breaking change on GHC. Fixes #303.
sgraf812 added a commit that referenced this issue Sep 20, 2024
2 spaces forced a breaking change on GHC. Fixes #303.
sgraf812 added a commit that referenced this issue Sep 20, 2024
2 spaces forced a breaking change on GHC. Fixes #303.
@sgraf812
Copy link
Collaborator

Version 2.0 has been deprecated and 2.0.1 with the fix has been uploaded. @Tritlo, could you try again?

@Tritlo
Copy link
Author

Tritlo commented Sep 20, 2024

Version 2.0 has been deprecated and 2.0.1 with the fix has been uploaded. @Tritlo, could you try again?

Yes, it's working again. Thanks for the quick response!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants