-
Notifications
You must be signed in to change notification settings - Fork 6
Specialiser refactoring #301
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: mbenke/renamecore
Are you sure you want to change the base?
Conversation
7250f05 to
cf8be5d
Compare
src/Solcore/Backend/TVSubst.hs
Outdated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It seems this file is a dead file.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Indeed. I tried to make it a separate module but failed due to cyclic imports. Removed.
| instance Semigroup TVRenaming where | ||
| r1 <> r2 = TVR (filter (uncurry (/=)) [ (u, renameTV r1 v) | (u, v) <- unTVR r2]) | ||
|
|
||
| instance Monoid TVRenaming where | ||
| mempty = TVR mempty |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It seems <> implementation doesn't satisfy the monoid law here. The right identity law is violated,
e.g., TVR [(a, b)] <> TVR [] should be equal to TVR [(a, b)], but actually returns TVR [].
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good catch! I have focused so much on removing redundant pairs, I forgot about this. Fixed.
Y-Nak
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great!
No description provided.