Skip to content

Update orphan instance check to use covering sets#2522

Merged
paf31 merged 1 commit intopurescript:masterfrom
LiamGoodacre:update/orphan-detection
Jan 2, 2017
Merged

Update orphan instance check to use covering sets#2522
paf31 merged 1 commit intopurescript:masterfrom
LiamGoodacre:update/orphan-detection

Conversation

@LiamGoodacre
Copy link
Copy Markdown
Member

For #2394
Let me know if you think of any more tests, want me to comment more with explanations, etc.

I think the error message for OrphanInstance also wants updating - I'll have a look at that shortly.

Copy link
Copy Markdown
Contributor

@paf31 paf31 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks great, thanks!

Just ms -> S.fromList (toList ms)
Nothing -> internalError "Unknown type index in checkOrphanInstance"

nonOrphanModules :: S.Set ModuleName
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you please add a short comment explaining why we take the intersection here?

@paf31
Copy link
Copy Markdown
Contributor

paf31 commented Jan 2, 2017

I'd like to merge this in to 0.10.4, so I'm going to merge this and tag. We can add the comment later.

@paf31 paf31 closed this Jan 2, 2017
@paf31 paf31 reopened this Jan 2, 2017
@paf31 paf31 merged commit 488f8c2 into purescript:master Jan 2, 2017
@LiamGoodacre
Copy link
Copy Markdown
Member Author

(I already added a comment; perhaps I should have made a new commit instead of amending, so it was more obvious there was a change.)

@LiamGoodacre LiamGoodacre deleted the update/orphan-detection branch January 2, 2017 11:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants