Skip to content

Conversation

MrDurion
Copy link
Contributor

@MrDurion MrDurion commented Sep 4, 2025

This PR corrects an error where the negate of the Num Bit instance used the complement instead of the identity function.
The negate is the additive inverse, and for xor as the addition operator, the inverse should be the identity, not the complement.

Fixes: #2999

Still TODO:

  • Write a changelog entry (see changelog/README.md)
  • Added regression test

@DigitalBrains1
Copy link
Member

@kloonbot run_ci d913fd7

@christiaanb
Copy link
Member

@MrDurion Could you add a changelog entry? thanks!

martijnbastiaan added a commit to MrDurion/clash-compiler that referenced this pull request Sep 20, 2025
Copy link
Member

@martijnbastiaan martijnbastiaan left a comment

Choose a reason for hiding this comment

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

I've added a regression test and a change log entry. This PR now LGTM.

@martijnbastiaan
Copy link
Member

@kloonbot run_ci 1f31ba9

Comment on lines 41 to 44
, testAdditiveInverse "Index 1" (genIndex @1)
-- TODO:
-- , testAdditiveInverse "Index 2" (genIndex @2)
-- , testAdditiveInverse "Index 128" (genIndex @128)
Copy link
Member

Choose a reason for hiding this comment

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

The additive inverse, and thus negate, is not defined for Index because the definition requires closure, which Index does not have for addition (maxBound + 1 => errorX). So you can remove these tests (and perhaps we should give thought to removing negate for Index)

Copy link
Member

Choose a reason for hiding this comment

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

Agreed, could you add your thoughts under #3015?

@martijnbastiaan
Copy link
Member

@kloonbot run_ci 63ae22e

@martijnbastiaan martijnbastiaan merged commit 5ec5edc into clash-lang:master Sep 22, 2025
16 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Incorrect negate implementation for the Num Bit instance
5 participants