Opened 5 years ago

Closed 5 years ago

Last modified 5 years ago

#9561 closed task (fixed)

Clean up mergeSATInfo

Reported by: dfeuer Owned by:
Priority: normal Milestone: 7.10.1
Component: Compiler Version: 7.9
Keywords: Cc:
Operating System: Unknown/Multiple Architecture: Unknown/Multiple
Type of failure: None/Unknown Test Case:
Blocked By: Blocking:
Related Tickets: Differential Rev(s):
Wiki Page:

Description

mergeSATInfo in SAT.hs is much longer and harder to read than it needs to be. I am attaching a patch to make it use zipWith instead of zipping manually.

Attachments (1)

mergeSATInfo-cleanup.diff (2.0 KB) - added by dfeuer 5 years ago.

Download all attachments as: .zip

Change History (7)

Changed 5 years ago by dfeuer

Attachment: mergeSATInfo-cleanup.diff added

comment:1 Changed 5 years ago by dfeuer

Status: newpatch

comment:2 Changed 5 years ago by simonpj

Let's do #9374 before implementing this patch. Right now SAT is only run when you say -fstatic-argument-transformation (ie almost never) so it would be hard to test this (doubtless excellent) refactoring thoroughly.

Simon

comment:3 in reply to:  2 Changed 5 years ago by dfeuer

Replying to simonpj:

Let's do #9374 before implementing this patch. Right now SAT is only run when you say -fstatic-argument-transformation (ie almost never) so it would be hard to test this (doubtless excellent) refactoring thoroughly.

Simon

I understand your concern, but if you look at it, this is a very limited change. I think two pairs of eyeballs and the type checker passing it should be pretty convincing.

comment:4 Changed 5 years ago by Joachim Breitner <mail@…>

In 93b8d0fd63cf8e00ca37c1ce76b93d4ee1fc56f8/ghc:

Simplify mergeSATInfo by using zipWith

Closes #9561.

comment:5 Changed 5 years ago by nomeata

Resolution: fixed
Status: patchclosed

I agree with dfeuer, this is an unproblematic refactoring; merged.

comment:6 Changed 5 years ago by thoughtpolice

Milestone: 7.8.47.10.1
Note: See TracTickets for help on using tickets.