Cache the number of data cons in DataTyCon and SumTyCon
ClosedPublic

Authored by niteria on Oct 27 2017, 10:03 PM.

Details

Summary

This is a follow-up after faf60e85 - Make tagForCon non-linear.
On the mailing list @simonpj suggested to solve the
linear behavior by caching the sizes.

Test Plan

./validate

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.
niteria created this revision.Oct 27 2017, 10:03 PM
simonpj requested changes to this revision.Oct 30 2017, 6:17 AM

Looks good. One small change.

compiler/iface/BuildTyCl.hs
43–44

Put this in TyCon: it uses dataConFulSig, but TyCon already SOURCE-imports DataCon and DataCon.hs-boot already exposes dataDonFullSig.

We don't want two identically named functions unless we have to!

This revision now requires changes to proceed.Oct 30 2017, 6:17 AM
austin resigned from this revision.Nov 9 2017, 5:42 PM
niteria updated this revision to Diff 15013.EditedJan 3 2018, 5:37 PM

I ended up with both mkDataTyConRhs and mkDataTyConRhsKnownEnum
because pcTyCon already knows is_enum.

I could instead:

  1. make everything use mkDataTyConRhs and remove is_enum argument from pcTyCon
  2. make mkDataTyConRhs take Maybe Bool
  3. ??

I'm not sure how to make that trade-off, so I leave it to a reviewer.

niteria marked an inline comment as done.Jan 3 2018, 5:39 PM
simonpj accepted this revision.Jan 4 2018, 2:42 AM

make everything use mkDataTyConRhs and remove is_enum argument from pcTyCon

Yes please do that. It's simpler, more robust, less code -- and no one is going to notice the efficiency change.

Once done, looks good. Thanks!

Simon

This revision is now accepted and ready to land.Jan 4 2018, 2:42 AM
niteria updated this revision to Diff 15017.Jan 4 2018, 7:16 AM

use only mkDataTyConRhs

This revision was automatically updated to reflect the committed changes.