'DynFlag'-free version of 'mkParserFlags'
ClosedPublic

Authored by harpocrates on Oct 26 2018, 11:46 PM.

Details

Summary

Obtaining a DynFlags is difficult, making using the lexer/parser
for pure parsing/lexing unreasonably difficult, even with mkPStatePure.
This is despite the fact that we only really need

  • language extension flags
  • warning flags
  • a handful of boolean options

The new mkParserFlags' function makes is easier to directly construct a
ParserFlags. Furthermore, since pExtsBitmap is just a footgun, I've gone
ahead and made ParserFlags an abstract type.

Diff Detail

Repository
rGHC Glasgow Haskell Compiler
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.
harpocrates created this revision.Oct 26 2018, 11:46 PM

Exposing pExtsBitmap really is a footgun - I shot myself while trying to work out a detail of D5067.

sjakobi added inline comments.
compiler/parser/Lexer.x
2439

I probably wouldn't know whether to use True or False for the boolean parameters here.

Maybe you could add a short example, e.g. with the usage from D5067?

harpocrates added inline comments.Oct 27 2018, 11:54 AM
compiler/parser/Lexer.x
2439

I see how this might be confusing to anyone who doesn't understand why SccProfilingOn, for example, has to be a parser flag. But I'm not sure how an example would help... Do you mind writing out what you had in mind?

sjakobi added inline comments.Oct 28 2018, 6:14 AM
compiler/parser/Lexer.x
2439

I was thinking about something like

-- Sample parser flags for using 'parseIdentifier':
--
-- >>> mkParserFlags' EnumSet.empty (EnumSet.fromList [MagicHash]) (stringToUnitId "") False False False False False

It might also be helpful to reference the actual option constructors instead of a somewhat fuzzy "Haddock", so e.g.

-  -> Bool                       -- ^ Haddock
+  -> Bool                       -- ^ Is 'Opt_Haddock' set?
bgamari accepted this revision.Oct 28 2018, 10:14 AM

Looks quite sensible to me. Thanks!

compiler/parser/Lexer.x
2439

Indeed it surprised me that the parser cares about whether SCC profiling is enabled. I check and, if I'm not mistake, the SccProfilingOnBit flag is used precisely nowhere. I believe the same applies to Hpc.

Perhaps we can just remove these? Feel free to try in this diff or a new one.

This revision is now accepted and ready to land.Oct 28 2018, 10:14 AM
  • Removed HPC and SCC bits in ParserFlags
  • Uniform handling of extension flags in ParserFlags
sjakobi accepted this revision.Oct 29 2018, 11:19 AM

Cheers! :)

bgamari added inline comments.Nov 1 2018, 5:30 PM
compiler/parser/Lexer.x
2419

Instead of all of this boilerplate why not just use the EnumSet type, like is done in DynFlags? This is why it exists, afterall.

harpocrates added inline comments.Nov 1 2018, 7:06 PM
compiler/parser/Lexer.x
2419

I can't help but think that whoever decided to try to pack all of these flags into on Word64 must've had some performance related reason for doing it. Other than that, I agree that this boilerplate is ridiculous. At the very least we should just export ExtsBitmap(..) and a checkExt :: ExtsBitmap -> P Bool function.

I'll try it out.

@bgamari Do you mind merging these changes as is first? I'd prefer this patch remain primarily about mkParserFlags/mkParserFlags'.

I've got some other lexer refactorings in the pipeline and I think swapping the current bitmap for an EnumSet would fit better there.

@bgamari can you merge this? I'd like to use this patch in D5067.

This revision was automatically updated to reflect the committed changes.