Change comparsion for checking if number is negative to include 128

[dfireBird]

The last byte in Little-Endian representation of negative integers start at 128 (Ox80) till 255 (OxFF). The comparison before the fix didn't check for 128 which made is_negative variable as false.

Potentially fixes #15096

[dfireBird]

++ @HKalbasi I created a PR for the fix.

[HKalbasi]

Can you also add some tests in hir_ty::const_eval::tests to make sure that it won't regress in future?

[dfireBird]

Should I create a new test function or should I add the tests in hir_ty::const_eval::tests::consts test function?

[HKalbasi]

Both would work, do it as you see fit.

[dfireBird]

I added the tests and it passed with fix. I temporarily reverted the see if the test fails or not but the test passed.
Then I checked the implementation of the check_number and saw it doesn't use the pad16 function where the bug was present.

Should I change my tests to test the HirDisplay trait instead of using check_number? Can you help me on this?

[HKalbasi]

I think using as casts like the original example in #15096 and comparing large positive and negative numbers with themself and with zero would trigger the bug.

For checking the HirDisplay you can also add a test in ide::hover::tests, but keep the as cast and compare tests in hir-ty.

[dfireBird]

I wrote more tests in hir_ty::const_eval::tests::consts and using as cast like in original example triggered the bug and made the test fail. I also added some tests to the ide::hover::tests module as well.

P.S: Should I amend the two test commits into one, so they are organized and reduce the commit clutter?

[HKalbasi]

P.S: Should I amend the two test commits into one, so they are organized and reduce the commit clutter?

Yes please, and then we can merge this I think.

[HKalbasi]

Thanks!
@bors r+

[bors]

📌 Commit 410ede9 has been approved by HKalbasi

It is now in the queue for this repository.

[bors]

⌛ Testing commit 410ede9 with merge 6c73f67...

[bors]

☀️ Test successful - checks-actions
Approved by: HKalbasi
Pushing 6c73f67 to master...