unit64's legal range should be 0000 0000 0000 0000 ~~ ffff ffff ffff ffff

Do I misunderstand something here?

The constant referenced in the error (-4609115386278082961) is given by the first hex literal minus 0xff...ff:

0xc00921f9f01b866e - (2^64 - 1)         = 
0xc00921f9f01b866e - 0xffffffffffffffff = 
  -4609115386278082961

and any literal in the second comparison triggers the error, for example:

if 0xc00921f9f01b866e < c && c < 2 {

works too (but you need a second comparison: just the first doesn't error).

Some kind of comparison optimization gone wrong? (but the issue is still there even with -N).

Simplified version.

This works:

package main
func main() {
	var c uint64
	const x, y uint64 = 0xc00921f9f01b866e, 0xc00921ff2e48e8a7
	_ = c < y
	_ = c > x
	_ = c < y || c > x
	_ = c < y && c < x
	_ = c > y && c > x
	_ = c > y && c < x
	_ = c < y && c >= x
	_ = c <= y && c >= x
}

This doesn't (at least since 1.8):

package main
func main() {
	var c uint64
	const x, y uint64 = 0xc00921f9f01b866e, 0xc00921ff2e48e8a7
	_ = c < y && c > x // constant -4609115386278082961 overflows uint64
	_ = c <= y && c > x // same as above
}

First one works, the second errors:

_ = 0x7ffffffffffffff < c && c < 0x7ffffffffffffff+1
_ = 0x8000000000000000 < c && c < 0x8000000000000000+1

When C >= 2**63 (like in the second line) the front-end rewrites the boolean expression into a single LT:

.   .   LT u(2) l(6) tc(1) bool
.   .   .   CONVNOP u(2) l(6) tc(1) uint64
.   .   .   .   SUB u(2) l(6) tc(1) uint64
.   .   .   .   .   NAME-p.c u(1) a(true) g(1) l(4) x(0+0) class(PAUTO) f(1) tc(1) assigned used(true) uint64
.   .   .   .   .   LITERAL--9223372036854775807 u(1) a(true) l(6) tc(1) uint64
.   .   .   LITERAL-0 u(1) a(true) l(6) tc(1) uint64

The rewrite introduces the bad const.

I suspect this may be an issue with walkinrange (CL 27652).

cc @josharian

Change https://golang.org/cl/130735 mentions this issue: cmd/compile: prevent overflow in walkinrange

walkinrange assumes in several places that a.Int64( ) (where a is a CTINT Node) will succeed, but if a contains a const bigger than 2**63 the result is actually undefined (in practise, it'll return a negative number).

One nuclear option would be to just disable the range check optimization when Int64( ) cannot be used reliably (CL 130735). Another option is to review and fix all Int64 uses in walkinrange.

I'm on the fence about whether this fix for this should be backported. Opinions?

It's the compiler rejecting valid code... even if it's not common to write expressions with integers over 2**63. IMO this should be considered for backporting, at least on 1.11.1. The fix is very simple and it just disables an incorrect optimization, it should be safe to backport.

Certainly seems to qualify for a backport.

Certainly seems to qualify for a backport.

Yeah. My hesitation was only that this appears to have existed for many releases, and the symptom is a spurious compiler error (not bad code generation). Nevertheless, I agree.

@gopherbot please open backport tracking issues.

Backport issue(s) opened: #27246 (for 1.11), #27247 (for 1.10).

Remember to create the cherry-pick CL(s) as soon as the patch is submitted to master, according to https://golang.org/wiki/MinorReleases.