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

Haddock combinator 'many' is applied to a parser that accepts an empty string. #179

Open
Bodigrim opened this issue Sep 16, 2023 · 5 comments

Comments

@Bodigrim
Copy link
Contributor

I tried to bump parsec submodule in GHC subtree to HEAD, but got

# /usr/bin/makeindex
| Run Makeindex: Haddock.idx => /tmp/extra-dir-74820312079216
This is makeindex, version 2.15 [TeX Live 2019/dev] (kpathsea + Thai support).
Scanning style file ./python.ist.......done (7 attributes redefined, 0 ignored).
Scanning input file Haddock.idx....done (136 entries accepted, 0 rejected).
Sorting entries....done (974 comparisons).
Generating output file Haddock.ind....done (217 lines written, 0 warnings).
Output written in Haddock.ind.
Transcript written in Haddock.ilg.
| Run Xelatex: Haddock.tex => /tmp/extra-dir-74820312079216
| Run Xelatex: Haddock.tex => /tmp/extra-dir-74820312079216
[13 of 14] Compiling GHC.Debug        ( libraries/ghc-prim/GHC/Debug.hs, nothing )
[14 of 14] Compiling GHC.Classes      ( libraries/ghc-prim/GHC/Classes.hs, nothing )
| Copy file (untracked): /tmp/extra-dir-74820312079216/Haddock.pdf => _build/doc/Haddock.pdf
haddock: internal error: Text.ParserCombinators.Parsec.Prim.many: combinator 'many' is applied to a parser that accepts an empty string.
CallStack (from HasCallStack):
  error, called at libraries/parsec/src/Text/Parsec/Prim.hs:755:11 in parsec-3.1.17.0-inplace:Text.Parsec.Prim
Error when running Shake build system:
  at want, called at src/Main.hs:124:44 in main:Main
* Depends on: binary-dist
  at need, called at src/Rules/BinaryDist.hs:301:13 in main:Rules.BinaryDist
* Depends on: binary-dist-dir
  at need, called at src/Rules/BinaryDist.hs:232:24 in main:Rules.BinaryDist
* Depends on: docs
  at need, called at src/Rules/Documentation.hs:138:9 in main:Rules.Documentation
* Depends on: _build/doc/archives/libraries.html.tar.xz
  at need, called at src/Rules/Documentation.hs:386:9 in main:Rules.Documentation
* Depends on: _build/doc/html/libraries/index.html
  at need, called at src/Rules/Documentation.hs:240:9 in main:Rules.Documentation
* Depends on: _build/doc/html/libraries/ghc-prim-0.10.0-inplace/ghc-prim.haddock
  at cmd, called at src/Builder.hs:403:5 in main:Builder
* Raised the exception:
Development.Shake.cmd, system command failed
Command line: _build/stage1/bin/haddock --verbosity=0 -B_build/stage1/lib --lib=_build/stage1/lib --odir=_build/doc/html/libraries/ghc-prim-0.10.0-inplace --dump-interface=_build/doc/html/libraries/ghc-prim-0.10.0-inplace/ghc-prim.haddock --html '--title=ghc-prim-0.10.0: GHC primitives' --prologue=_build/doc/html/libraries/ghc-prim-0.10.0-inplace/haddock-prologue.txt --optghc=-D__HADDOCK_VERSION__=2280 --optghc=-hisuf --optghc=dyn_hi --optghc=-osuf --optghc=dyn_o --optghc=-hcsuf --optghc=dyn_hc --optghc=-fPIC --optghc=-dynamic --optghc=-hide-all-packages --optghc=-no-user-package-db '--optghc=-package-env -' '--optghc=-this-unit-id ghc-prim-0.10.0-inplace' '--optghc=-package-id rts-1.0.2' --optghc=-i --optghc=-i/builds/ghc/ghc/_build/stage1/libraries/ghc-prim/build --optghc=-i/builds/ghc/ghc/_build/stage1/libraries/ghc-prim/build/autogen --optghc=-i/builds/ghc/ghc/libraries/ghc-prim --optghc=-Irts/include --optghc=-I_build/stage1/libraries/ghc-prim/build --optghc=-I/builds/ghc/ghc/rts/include --optghc=-I/builds/ghc/ghc/_build/stage1/rts/build/include --optghc=-optP-include --optghc=-optP_build/stage1/libraries/ghc-prim/build/autogen/cabal_macros.h --optghc=-outputdir --optghc=_build/stage1/libraries/ghc-prim/build --optghc=-this-unit-id --optghc=ghc-prim --optghc=-XHaskell2010 --optghc=-no-global-package-db --optghc=-package-db=/builds/ghc/ghc/_build/stage1/inplace/package.conf.d --optghc=-ghcversion-file=rts/include/ghcversion.h --optghc=-ghcversion-file=rts/include/ghcversion.h --optghc=-this-unit-id --optghc=ghc-prim --optghc=-XHaskell2010 --optghc=-no-global-package-db --optghc=-package-db=/builds/ghc/ghc/_build/stage1/inplace/package.conf.d --optghc=-ghcversion-file=rts/include/ghcversion.h --optghc=-ghcversion-file=rts/include/ghcversion.h --optghc=-Wno-deprecated-flags --optghc=-Wno-trustworthy-safe +RTS -t_build/stage1/haddock-timing-files/ghc-prim.t --machine-readable -RTS --hyperlinked-source --hoogle --quickjump @/tmp/extra-file-74820312079218-6692-6324

(See https://gitlab.haskell.org/ghc/ghc/-/jobs/1665458#L5015 for the full log)

Is it related to #176?

@phadej
Copy link
Collaborator

phadej commented Sep 16, 2023

Probably related to #176 yes, but if many is applied to a parser that accepts empty string, did it - the parser- work to begin with?

@Bodigrim
Copy link
Contributor Author

It very well might be Haddock doing something fishy, but I'm out of my depth here.

My impression from the changelog was that #176 was not behaviour-changing, mere code deduplication. It might be worth to clarify that it fixed some bugs / infelicities if any.

@phadej
Copy link
Collaborator

phadej commented Sep 18, 2023

I'll revert #176 for no to unblock GHC updates, and investigate what's wrong happening with haddock later with better time. The many change isn't super important to get in.

@phadej
Copy link
Collaborator

phadej commented Sep 18, 2023

Undone. @Bodigrim could you verify that GHC can upgrade to the recent master i.e. 647c570

EDIT: I tried to build ghc's docs target before that commit and after. Before failed, after succeeded.

@phadej
Copy link
Collaborator

phadej commented Sep 18, 2023

The offending code looks like:

 emptyLines = void $ many (try (skipHorizontalSpace *> "\n"))

where

skipHorizontalSpace = Parsec.skipMany (Parsec.satisfy horizontalSpace)

I have to remember the intricacies of try, but generally it should be avoided.

In skipping emtpy lines (probably related to indentation handling?) the idea is to do an arbitrary long look-ahead, so something like try cannot be avoided. I have to think about this, many (try p) seems like a common pattern to occur.

The "check" in parsec is probably not completely correct. As try (p ... *> "\n") cannot succeed with empty string. It may fail not consuming anything, but that should be fine for many.

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

2 participants