Add an Eq instance for UniqSet
ClosedPublic

Authored by dfeuer on May 1 2017, 10:27 AM.

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.
dfeuer created this revision.May 1 2017, 10:27 AM
rwbarton added inline comments.May 1 2017, 10:47 AM
compiler/utils/UniqFM.hs
354

Is this really better than just keys m1 == keys m2?

compiler/utils/UniqSet.hs
135

It's not clear to me that this is better/less surprising than the instance UniqSet a == UniqSet b = a == b.

A plugin relying on an instance that is not used anywhere in GHC sounds like a recipe for the instance later being removed, or having its semantics changed. It seems to me that the plugin author would be better off directly using a descriptively-named function like equalKeysUFM.

dfeuer updated this revision to Diff 12333.May 1 2017, 10:48 AM

Fix stage1 build

dfeuer updated this revision to Diff 12334.May 1 2017, 10:52 AM

Streamline

dfeuer added inline comments.May 1 2017, 10:55 AM
compiler/utils/UniqFM.hs
354

Good question. Even if it is, it may not be enough to matter.

compiler/utils/UniqSet.hs
135

It should be faster, because it doesn't look at the values. Can two things with the same type and the same unique have different values?

dfeuer updated this revision to Diff 12338.May 1 2017, 12:47 PM

Qualify name.

christiaanb added inline comments.May 2 2017, 2:39 AM
compiler/utils/UniqSet.hs
135

As the author of the referred to plugin, I could live with a equalKeysUS instead of an Eq instance. I mostly want a way to test two UniqSets for equality that doesn't involve converting to a list first.

bgamari accepted this revision.May 2 2017, 10:20 PM

I am fine with providing an Eq instance.

This revision is now accepted and ready to land.May 2 2017, 10:20 PM
rwbarton added inline comments.May 2 2017, 10:56 PM
compiler/utils/UniqSet.hs
135

Well it's certainly common for the compiler to manipulate different values with the same Unique, like the Var for a particular variable before and after some analysis pass. That's one common purpose of UniqSet really, to substitute new Vars for old ones.

Maybe any type with a unique has an Eq instance that depends only on the unique, though. I'm not sure, but it is true for Var at least.

This revision was automatically updated to reflect the committed changes.