Zig: comptime_int bitwise operators with negative values give incorrect results

[mdsteele]

This is with the stage1 compiler.

std.debug.warn("{}\n", i64(3 & -1)); // Prints 1, but should be 3
std.debug.warn("{}\n", i64(3 | -1)); // Prints 140338432934211, but should be -1
std.debug.warn("{}\n", i64(3 ^ -1)); // Prints 140678271664562, but should be -4

[mdsteele]

Hmm, digging into src/bigint.cpp, in each of bigint_and(), bigint_or(), and bigint_xor(), I see this code:

if (op1->is_negative || op2->is_negative) {
    // TODO this code path is untested
    ...etc...
}

I guess that explains it. (-:

[mtn]

I think the problem is in to_twos_complement's handling of negative numbers. I looked at the bigint source recently, so I'm going to spend a bit of time on this.

[kristate]

@mtn thanks! I was thinking about writing a patch, too -- I wish we could assign people to each issue :-)

[mtn]

@kristate Sounds good -- I'm just at the point of familiarizing myself with the compiler slowly, so it could take me a little bit. I'll hopefully come back with good news shortly :)

[kristate]

@mtn if you get stuck or have questions, feel free to follow-up here!

[kristate]

@mtn take a look at #1389 for some hints

[mtn]

@kristate Thanks for the help. More confused than I expected :-)

I'll try and convey what I currently understand by walking through what I understand to be the implementation of bigint_and. There will probably be a number of things wrong, so I'd appreciate corrections.

So our bigint type stores the data for the digits (always positive), and a sign externally (as opposed to storing the twos complement form like llvm's APInt does). Thus, to do bitwise operations like & on negative numbers, we need to first get their data in twos complement form (by negating them so they're the actual negative numbers) so we can then & each digit.

Assuming that's correct from a high level, then my confusion is just with the implementation of to_twos_complement.

Also, please don't hesitate to fix the issue yourself -- I don't want to artificially slow things down and can learn more about what's going on either way.

[kristate]

@mtn yeah, I think that the bigint library is working too hard / trying to do too much.
I have an hour or two into a patch. hang tight!