hadrian: eliminate most of the remaining big rule enumerations
ClosedPublic

Authored by alpmestan on Dec 7 2018, 5:18 AM.

Details

Summary

Following what was done to Rules.Library some time ago and to Rules.Compile
recently (D5412), this patch moves more rules away from the "enumerate
a lot of contexts and generate one rule for each" style and instead uses
the "parse data from file path to recover context" approach. In fact, the
only rules left to convert seem to be the ones from Rules.Generate.

This effectively decreases the pauses described in Trac #15938 further as well as
the amount of allocations and GC that we do, unsurprisingly. Nowhere as
drastically as D5412, though.

Test Plan

perform full build and generate docs

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.
alpmestan created this revision.Dec 7 2018, 5:18 AM
alpmestan updated this revision to Diff 19043.Dec 7 2018, 6:37 AM
  • oops, remove leftover
snowleopard accepted this revision.Dec 7 2018, 8:16 AM

@alpmestan Awesome, thank you!

I've only got a nitpick about inconsistent naming of rules. My suggestion would be to get rid of the three instances of adding the suffix Rules: they all can be avoided without much pain, but having an inconsistent naming may be rather confusing.

hadrian/src/Rules/Documentation.hs
143

Was this a bug? I guess it is not related to the current refactoring.

hadrian/src/Rules/Program.hs
19

The change of the name buildProgram -> buildProgramRules is inconsistent with others build rules (they do not have the suffix Rules). What is the reason behind it? I think we should just revert back to buildProgram (or alternatively add suffix Rules to all rules, which seems like an overkill).

58

Aha, I guess this is the reason. Perhaps, this one could be called buildProgramInContext to avoid the conflict with the exported name.

hadrian/src/Rules/Register.hs
1

Same as above. I don't mind adding the suffix Rules if it makes it convenient, but let's do this consistently across all Hadrian rules.

40

I'd say this function should just be inlined (it's just two lines and not called from anywhere else) instead of occupying a useful name and adding an indirection.

This revision is now accepted and ready to land.Dec 7 2018, 8:16 AM
This revision was automatically updated to reflect the committed changes.