Give min and max the kind of their untyped arguments, cover the evaluator

max(5, 4.0) has an untyped float type even though the integer wins, so
max(5, 4.0) / 2 * 10 is 25 and not 20. A right shift count is bounded by a
constant instead of the width of the value, which reaches the same result
without measuring it.

Tests cover every branch of the evaluator, including the error paths of each
operand position, and the package is back to full statement coverage.

Author: paskal

internal/generator/constexpr.go

@@ -5,14 +5,17 @@ import (
 	"go/ast"
 	"go/constant"
 	"go/token"
-	"math/big"
 	"strconv"
 	"unicode/utf8"
 )
 
-// maxShiftLeft caps a left shift so a malformed source can't ask for an enormous allocation. a right
-// shift needs no cap, shifting past the width of a value settles at 0 or -1.
-const maxShiftLeft = 512
+const (
+	// maxShiftLeft caps a left shift so a malformed source can't ask for an enormous allocation
+	maxShiftLeft = 512
+	// maxShiftRight bounds a right shift count, which only has to reach past the width of the value;
+	// no constant comes near it, and it keeps the count in range of uint
+	maxShiftRight = 1 << 20
+)
 
 // intTypeInfo describes the width and signedness of a builtin integer type
 type intTypeInfo struct {
@@ -357,11 +360,9 @@ func (r *constResolver) evalShift(e *ast.BinaryExpr, x typedValue, iotaVal int64
 	switch {
 	case e.Op == token.SHL && (!exact || count > maxShiftLeft):
 		return typedValue{}, fmt.Errorf("shift count %s is too large", y.ExactString())
-	case e.Op == token.SHR:
-		// a shift wider than the value itself keeps its result, clamp it to stay in range of uint
-		if width := bitLen(xv) + 1; !exact || count > width {
-			count = width
-		}
+	case e.Op == token.SHR && (!exact || count > maxShiftRight):
+		// shifting further than the value is wide keeps giving the same result
+		count = maxShiftRight
 	}
 	return typedValue{value: constant.Shift(xv, e.Op, uint(count)), typ: x.typ}, nil
 }
@@ -431,7 +432,7 @@ func (r *constResolver) evalMinMax(e *ast.CallExpr, name string, iotaVal int64)
 	}
 
 	var best typedValue
-	typ := ""
+	typ, anyFloat := "", false
 	for i, arg := range e.Args {
 		v, err := r.eval(arg, iotaVal)
 		if err != nil {
@@ -443,14 +444,19 @@ func (r *constResolver) evalMinMax(e *ast.CallExpr, name string, iotaVal int64)
 		if typ == "" {
 			typ = v.typ // a typed argument gives the result its type
 		}
+		anyFloat = anyFloat || v.value.Kind() == constant.Float
 		if i == 0 || constant.Compare(v.value, op, best.value) {
 			best = v
 		}
 	}
-	if typ == "" {
-		return best, nil
+	if typ != "" {
+		return r.convert(best, typ)
+	}
+	if anyFloat {
+		// an untyped float argument makes the result untyped float, whichever argument won
+		return typedValue{value: constant.ToFloat(best.value)}, nil
 	}
-	return r.convert(best, typ)
+	return best, nil
 }
 
 // roundFloat drops the precision a float type cannot hold, the compiler stores a typed float
@@ -464,18 +470,6 @@ func roundFloat(v constant.Value, typ string) constant.Value {
 	return constant.MakeFloat64(f)
 }
 
-// bitLen is the number of bits an integer value occupies
-func bitLen(v constant.Value) uint64 {
-	i, ok := constant.Val(v).(*big.Int)
-	if !ok {
-		return 64 // anything go/constant keeps as an int64
-	}
-	if n := i.BitLen(); n > 0 {
-		return uint64(n)
-	}
-	return 0
-}
-
 // unparen strips the parentheses around an expression
 func unparen(expr ast.Expr) ast.Expr {
 	for {
@@ -524,13 +518,7 @@ func (r *constResolver) convert(v typedValue, typ string) (typedValue, error) {
 // the language allows
 func literalValue(lit *ast.BasicLit) (constant.Value, error) {
 	switch lit.Kind {
-	case token.INT, token.FLOAT:
-		v := constant.MakeFromLiteral(lit.Value, lit.Kind, 0)
-		if v.Kind() == constant.Unknown {
-			return nil, fmt.Errorf("invalid literal %s", lit.Value)
-		}
-		return v, nil
-	case token.STRING:
+	case token.INT, token.FLOAT, token.STRING:
 		v := constant.MakeFromLiteral(lit.Value, lit.Kind, 0)
 		if v.Kind() == constant.Unknown {
 			return nil, fmt.Errorf("invalid literal %s", lit.Value)

internal/generator/constexpr_test.go

@@ -98,6 +98,13 @@ func TestConstResolverValues(t *testing.T) {
 		{"string of a code point", `len(string(0x100))`, "2"},
 		{"min of a typed argument", "^max(2, uint8(1))", "253"},
 		{"max of a typed argument", "min(uint8(7), 9) + 1", "8"},
+		{"max with an untyped float", "max(5, 4.0) / 2 * 10", "25"},
+		{"min with an untyped float", "min(5, 6.0) / 2 * 10", "25"},
+		{"max of integers only", "max(5, 4) / 2 * 10", "20"},
+		{"string of a value out of range", "len(string(-1))", "3"},
+		{"shift right of a wide value", "1<<70 >> 2000", "0"},
+		// the compiler rejects a shift count this large outright, the result is still 0
+		{"shift right past the bound", "1 >> 2000000", "0"},
 	}
 
 	for _, tt := range tests {
@@ -136,6 +143,29 @@ func TestConstResolverErrors(t *testing.T) {
 		{"comparison operator", "const x = 1 < 2", "unsupported binary operator"},
 		{"self reference", "const x = x + 1", "refers to itself"},
 		{"reference cycle", "const (\n\tx = y\n\ty = x\n)", "refers to itself"},
+		{"failing operand of a negation", "const x = -missing", "unknown constant missing"},
+		{"complement of a string", `const x = ^"str"`, "not an integer"},
+		{"failing right operand", "const x = 1 + missing", "unknown constant missing"},
+		{"operand out of range for the type", "const x = uint8(1) + 300", "overflows uint8"},
+		{"string added to a number", `const x = 1 + "s"`, "not a number"},
+		{"number added to a string", `const x = "s" + 1`, "not a number"},
+		{"fractional remainder", "const x = 1.5 % 2", "not an integer"},
+		{"remainder by a fraction", "const x = 2 % 1.5", "not an integer"},
+		{"shift of a string", `const x = "s" << 1`, "not an integer"},
+		{"failing shift count", "const x = 1 << missing", "unknown constant missing"},
+		{"fractional shift count", "const x = 1 << 1.5", "not an integer"},
+		{"conversion with two arguments", "const x = uint8(1, 2)", "unsupported call expression"},
+		{"min without arguments", "const x = min()", "at least one argument"},
+		{"failing min argument", "const x = min(missing, 1)", "unknown constant missing"},
+		{"min of a string", `const x = min("a", 1)`, "not a number"},
+		{"failing len argument", "const x = len(missing)", "unknown constant missing"},
+		{"failing conversion argument", "const x = uint8(missing)", "unknown constant missing"},
+		{"fractional conversion", "const x = uint8(1.5)", "not an integer"},
+		{"float conversion of a string", `const x = float64("s")`, "not a number"},
+		{"string conversion of a fraction", "const (\n\tx = str(1.5)\n)\ntype str string", "not a string"},
+		{"call of an expression", "const x = (1 + 1)(2)", "unsupported call expression"},
+		{"failing left operand of a typed sum", "const x = 300 + uint8(1)", "overflows uint8"},
+		{"alias cycle", "const x = ^a(0)\ntype a = b\ntype b = a", "unsupported call to a"},
 	}
 
 	for _, tt := range tests {
@@ -322,6 +352,27 @@ func TestCheckIntRange(t *testing.T) {
 	}
 }
 
+func TestConstResolverDeclarations(t *testing.T) {
+	// a name declared twice keeps the first declaration, which is what a compiling package has
+	v, err := resolveSrc(t, "package p\nconst x = 1\nconst x = 2\n", "x")
+	require.NoError(t, err)
+	assert.Equal(t, "1", v.ExactString())
+
+	// specs that are not value or type declarations are skipped
+	r := newConstResolver()
+	r.addFile(&ast.File{
+		Name: &ast.Ident{Name: "p"},
+		Decls: []ast.Decl{
+			&ast.GenDecl{Tok: token.TYPE, Specs: []ast.Spec{&ast.ImportSpec{}}},
+			&ast.GenDecl{Tok: token.CONST, Specs: []ast.Spec{&ast.ImportSpec{}}},
+			&ast.GenDecl{Tok: token.IMPORT, Specs: []ast.Spec{&ast.ImportSpec{}}},
+			&ast.FuncDecl{Name: &ast.Ident{Name: "f"}},
+		},
+	})
+	assert.Empty(t, r.decls)
+	assert.Empty(t, r.types)
+}
+
 func TestConstResolverUnsupportedNodes(t *testing.T) {
 	r := newConstResolver()
 
@@ -345,6 +396,18 @@ func TestConstResolverUnsupportedNodes(t *testing.T) {
 	require.Error(t, err)
 	assert.Contains(t, err.Error(), "not a number")
 
+	for _, lit := range []*ast.BasicLit{
+		{Kind: token.INT, Value: "12abc"},
+		{Kind: token.FLOAT, Value: "1.2.3"},
+		{Kind: token.CHAR, Value: "'"},
+		{Kind: token.CHAR, Value: "abc"},
+		{Kind: token.CHAR, Value: `'\q'`},
+	} {
+		_, err = literalValue(lit)
+		require.Error(t, err, lit.Value)
+		assert.Contains(t, err.Error(), "invalid literal", lit.Value)
+	}
+
 	_, err = r.resolve("nothing")
 	require.Error(t, err)
 	assert.Contains(t, err.Error(), "unknown constant nothing")
@@ -472,6 +535,32 @@ const (
 	assert.Contains(t, err.Error(), "const codeB: value -1 is negative but the type is uint8")
 }
 
+func TestParseUntypedValueOutOfRange(t *testing.T) {
+	// a constant without a type of its own still has to fit the underlying type of the enum
+	src := `package test
+type small int8
+const (
+	smallA small = 100
+	smallB       = 200
+)
+`
+	_, err := parseSrc(t, "small", src)
+	require.Error(t, err)
+	assert.Contains(t, err.Error(), "const smallB: value 200 overflows int8")
+}
+
+func TestParseConstWithoutValue(t *testing.T) {
+	src := `package test
+type status int
+const (
+	statusA
+)
+`
+	_, err := parseSrc(t, "status", src)
+	require.Error(t, err)
+	assert.Contains(t, err.Error(), "no value for const statusA")
+}
+
 func TestParseValueOutOfRange(t *testing.T) {
 	src := `package test
 type small int8

internal/generator/generator_test.go

@@ -1449,6 +1449,7 @@ func TestParseAliasComment(t *testing.T) {
 		{"multiple aliases", "// enum:alias=rw,read-write", []string{"rw", "read-write"}},
 		{"with whitespace", "// enum:alias= rw , read-write ", []string{"rw", "read-write"}},
 		{"empty value", "// enum:alias=", nil},
+		{"only separators", "// enum:alias=,,", nil},
 		{"empty between commas", "// enum:alias=a,,b", []string{"a", "b"}},
 		{"no alias directive", "// some comment", nil},
 		{"nil comment", "", nil},