Rename some mutable closure types for consistency
ClosedPublic

Authored by osa1 on Jun 4 2018, 2:39 AM.

Details

Summary

SMALL_MUT_ARR_PTRS_FROZEN0 -> SMALL_MUT_ARR_PTRS_FROZEN_DIRTY
SMALL_MUT_ARR_PTRS_FROZEN -> SMALL_MUT_ARR_PTRS_FROZEN_CLEAN
MUT_ARR_PTRS_FROZEN0 -> MUT_ARR_PTRS_FROZEN_DIRTY
MUT_ARR_PTRS_FROZEN -> MUT_ARR_PTRS_FROZEN_CLEAN

Naming is now consistent with other CLEAR/DIRTY objects (MVAR, MUT_VAR,
MUT_ARR_PTRS).

(alternatively we could rename MVAR_DIRTY/MVAR_CLEAN etc. to MVAR0/MVAR)

Removed a few comments in Scav.c about FROZEN0 being on the mut_list
because it's now clear from the closure 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.
osa1 created this revision.Jun 4 2018, 2:39 AM

Yes, this is a good idea, I never liked the 0-suffix naming terminology.

rts/PrimOps.cmm
277–278

This comment reads a bit strangely, I think it could do with being rewritten. e.g. "So that we can tell whether a MUT_ARR_PTRS_FROZEN_CLEAN is on the mutable list," doesn't make any sense, because a MUT_ARR_PTRS_FROZEN_CLEAN is never on the mutable list.

bgamari requested changes to this revision.Jun 4 2018, 9:24 AM

I agree that this renaming makes much more sense. Just fix the comment and I think this is ready.

This revision now requires changes to proceed.Jun 4 2018, 9:24 AM
osa1 updated this revision to Diff 16715.Jun 5 2018, 12:42 AM
  • Rebase
  • Reword a comment
osa1 marked an inline comment as done.Jun 5 2018, 12:43 AM
osa1 edited the summary of this revision. (Show Details)Jun 5 2018, 12:53 AM
osa1 updated this revision to Diff 16716.Jun 5 2018, 1:01 AM
  • Rename Haskell bindings of the symbols
osa1 added a comment.Jun 5 2018, 2:33 AM

This validates.

simonmar added inline comments.Jun 5 2018, 3:50 AM
rts/PrimOps.cmm
278–283

You probably meant MUT_ARR_PTRS_FROZEN here, not FROZEN_MUT_ARR_PTRS

simonmar accepted this revision.Jun 5 2018, 3:50 AM
osa1 updated this revision to Diff 16718.Jun 5 2018, 3:58 AM
  • Fix typo
This revision was not accepted when it landed; it landed in state Needs Review.Jun 5 2018, 9:30 AM
This revision was automatically updated to reflect the committed changes.
Harbormaster failed remote builds in B21061: Diff 16716!
Harbormaster failed remote builds in B21063: Diff 16718!
AndreasK added a subscriber: AndreasK.EditedSep 7 2018, 6:56 AM

This should probably be mentioned in the Changelog as it can break existing code. (For example it breaks the packman package).

I've also created a trac ticket for it so the change is easier to discover: Trac #15615