Fix instrumentation of types based on bool

The predefined type `bool` is not interchangeable with custom-defined
boolean types.

The result of `==` is a `bool` type compatible with both the predefined
`bool` and derived boolean types, while `!` preserves the derived type,
so use the `==` operator to convert between the boolean types. While
this adds some bloat to the instrumented code, it still allows gobco to
work without resolving the actual types. Working with the actual types
may be more elegant but is left for later.

Fixes #35.

instrumenter.go

@@ -25,10 +25,11 @@ type cond struct {
 
 // exprSubst prepares to later replace '*ref' with 'expr'.
 type exprSubst struct {
-	ref  *ast.Expr
-	expr ast.Expr
-	pos  token.Pos
-	text string
+	ref     *ast.Expr
+	expr    ast.Expr
+	pos     token.Pos
+	text    string
+	convert bool
 }
 
 // instrumenter rewrites the code of a go package
@@ -248,7 +249,7 @@ func (i *instrumenter) findRefsField(field reflect.Value) {
 			delete(i.marked, expr)
 			ref := field.Addr().Interface().(*ast.Expr)
 			i.exprSubst[expr] = &exprSubst{
-				ref, expr, expr.Pos(), i.str(expr),
+				ref, expr, expr.Pos(), i.str(expr), true,
 			}
 		}
 
@@ -257,7 +258,7 @@ func (i *instrumenter) findRefsField(field reflect.Value) {
 			if i.marked[expr] {
 				delete(i.marked, expr)
 				i.exprSubst[expr] = &exprSubst{
-					&val[ei], expr, expr.Pos(), i.str(expr),
+					&val[ei], expr, expr.Pos(), i.str(expr), true,
 				}
 			}
 		}
@@ -292,6 +293,13 @@ func (i *instrumenter) prepareStmts(n ast.Node) bool {
 
 func (i *instrumenter) prepareSwitchStmt(n *ast.SwitchStmt) {
 	if n.Tag == nil {
+		for _, clause := range n.Body.List {
+			for _, expr := range clause.(*ast.CaseClause).List {
+				if s := i.exprSubst[expr]; s != nil {
+					s.convert = false
+				}
+			}
+		}
 		return // Already handled in instrumenter.markConds.
 	}
 
@@ -318,6 +326,7 @@ func (i *instrumenter) prepareSwitchStmt(n *ast.SwitchStmt) {
 				gen.eql(tagExprName, expr),
 				expr.Pos(),
 				i.strEql(n.Tag, expr),
+				false,
 			}
 			tagExprUsed = true
 		}
@@ -421,7 +430,7 @@ func (i *instrumenter) prepareTypeSwitchStmt(ts *ast.TypeSwitchStmt) {
 
 			gen := codeGenerator{test.pos}
 			ident := gen.ident(test.varname)
-			wrapped := i.callCover(ident, test.pos, test.code)
+			wrapped := i.callCover(ident, test.pos, test.code, false)
 			newList = append(newList, wrapped)
 		}
 
@@ -458,7 +467,7 @@ func (i *instrumenter) replace(n ast.Node) bool {
 
 	case ast.Expr:
 		if s := i.exprSubst[n]; s != nil {
-			*s.ref = i.callCover(s.expr, s.pos, s.text)
+			*s.ref = i.callCover(s.expr, s.pos, s.text, s.convert)
 		}
 
 	case ast.Stmt:
@@ -478,7 +487,7 @@ func (i *instrumenter) replace(n ast.Node) bool {
 // that is most closely related to the instrumented condition.
 // Especially for switch statements,
 // the position may differ from the expression that is wrapped.
-func (i *instrumenter) callCover(expr ast.Expr, pos token.Pos, code string) ast.Expr {
+func (i *instrumenter) callCover(expr ast.Expr, pos token.Pos, code string, convert bool) ast.Expr {
 	assert(pos.IsValid(), "pos must refer to the code from before instrumentation")
 
 	start := i.fset.Position(pos)
@@ -491,7 +500,7 @@ func (i *instrumenter) callCover(expr ast.Expr, pos token.Pos, code string) ast.
 	idx := len(i.conds) - 1
 
 	gen := codeGenerator{pos}
-	return gen.callGobcoCover(idx, expr)
+	return gen.callGobcoCover(idx, expr, convert)
 }
 
 // strEql returns the string representation of (lhs == rhs).
@@ -712,8 +721,8 @@ func (gen codeGenerator) callFinish(arg ast.Expr) ast.Expr {
 	}
 }
 
-func (gen codeGenerator) callGobcoCover(idx int, cond ast.Expr) ast.Expr {
-	return &ast.CallExpr{
+func (gen codeGenerator) callGobcoCover(idx int, cond ast.Expr, convert bool) ast.Expr {
+	call := &ast.CallExpr{
 		Fun:    gen.ident("GobcoCover"),
 		Lparen: gen.pos,
 		Args: []ast.Expr{
@@ -722,10 +731,32 @@ func (gen codeGenerator) callGobcoCover(idx int, cond ast.Expr) ast.Expr {
 				Kind:     token.INT,
 				Value:    fmt.Sprint(idx),
 			},
-			cond,
+			gen.toBool(cond),
 		},
 		Rparen: gen.pos,
 	}
+	if !convert {
+		return call
+	}
+	return gen.toCustomBool(call)
+}
+
+func (gen codeGenerator) toBool(cond ast.Expr) ast.Expr {
+	switch cond.(type) {
+	case *ast.UnaryExpr, *ast.BinaryExpr:
+		return cond
+	default:
+		return gen.toCustomBool(cond)
+	}
+}
+
+func (gen codeGenerator) toCustomBool(cond ast.Expr) ast.Expr {
+	return &ast.BinaryExpr{
+		X:     cond,
+		OpPos: gen.pos,
+		Op:    token.EQL,
+		Y:     gen.ident("true"),
+	}
 }
 
 func (gen codeGenerator) define(lhs string, rhs ast.Expr) *ast.AssignStmt {

testdata/instrumenter/AssignStmt.cond

@@ -17,20 +17,20 @@ func assignStmt() {
 	assertEquals(i, 3)
 
 	// Most assignments assign to a single variable.
-	mm[GobcoCover(0, i > 0)] = GobcoCover(1, i > 3)
+	mm[GobcoCover(0, i > 0) == true] = GobcoCover(1, i > 3) == true
 
 	// An assignment can assign multiple variables simultaneously.
-	b1, m[GobcoCover(2, i > 0)], b2 = b2, m[GobcoCover(3, i > 1)], b1
-	mm[GobcoCover(4, i > 10)], mm[GobcoCover(5, i > 11)], mm[GobcoCover(6, i > 12)] =
-		GobcoCover(7, i > 13), GobcoCover(8, i > 14), GobcoCover(9, i > 15)
+	b1, m[GobcoCover(2, i > 0) == true], b2 = b2, m[GobcoCover(3, i > 1) == true], b1
+	mm[GobcoCover(4, i > 10) == true], mm[GobcoCover(5, i > 11) == true], mm[GobcoCover(6, i > 12) == true] =
+		GobcoCover(7, i > 13) == true, GobcoCover(8, i > 14) == true, GobcoCover(9, i > 15) == true
 
 	// Assignments from a function call can assign to multiple variables.
-	mm[GobcoCover(10, i > 0)], mm[GobcoCover(11, i > 1)], mm[GobcoCover(12, i > 2)] =
+	mm[GobcoCover(10, i > 0) == true], mm[GobcoCover(11, i > 1) == true], mm[GobcoCover(12, i > 2) == true] =
 		func() (bool, bool, bool) { return false, false, false }()
 
 	// Operands may be parenthesized.
-	(m[GobcoCover(13, i > 21)]), (m[GobcoCover(14, i > 22)]) =
-		(m[GobcoCover(15, i > 23)]), (m[GobcoCover(16, i > 24)])
+	(m[GobcoCover(13, i > 21) == true]), (m[GobcoCover(14, i > 22) == true]) =
+		(m[GobcoCover(15, i > 23) == true]), (m[GobcoCover(16, i > 24) == true])
 
 	// Since the left-hand side in an assignment must be a variable,
 	// and since only those expressions are instrumented that are

testdata/instrumenter/BinaryExpr.branch

@@ -71,10 +71,10 @@ func binaryExpr(i int, a bool, b bool, c bool) {
 	// if statements; in branch coverage mode, only instrument the main
 	// condition.
 	mi := map[bool]int{}
-	if GobcoCover(0, i == mi[i > 51]) {
+	if GobcoCover(0, i == mi[i > 51]) == true {
 		_ = i == mi[i > 52]
 	}
-	for GobcoCover(1, i == mi[i > 61]) {
+	for GobcoCover(1, i == mi[i > 61]) == true {
 		_ = i == mi[i > 62]
 	}
 }

testdata/instrumenter/BinaryExpr.cond

@@ -17,8 +17,8 @@ package instrumenter
 func binaryExpr(i int, a bool, b bool, c bool) {
 	// Comparison expressions have return type boolean and are
 	// therefore instrumented.
-	_ = GobcoCover(0, i > 0)
-	pos := GobcoCover(1, i > 0)
+	_ = GobcoCover(0, i > 0) == true
+	pos := GobcoCover(1, i > 0) == true
 
 	// Expressions consisting of a single identifier do not look like boolean
 	// expressions, therefore they are not instrumented.
@@ -32,50 +32,50 @@ func binaryExpr(i int, a bool, b bool, c bool) {
 	//
 	// Also, gobco only looks at the parse tree without any type resolution.
 	// Therefore it cannot decide whether a variable is boolean or not.
-	both := GobcoCover(2, a) && GobcoCover(3, b)
-	either := GobcoCover(4, a) || GobcoCover(5, b)
+	both := GobcoCover(2, a == true) == true && GobcoCover(3, b == true) == true
+	either := GobcoCover(4, a == true) == true || GobcoCover(5, b == true) == true
 	_, _ = both, either
 
 	// When a long chain of '&&' or '||' is parsed, it is split into
 	// the rightmost operand and the rest, instrumenting both these
 	// parts.
-	_ = GobcoCover(6, i == 11) ||
-		GobcoCover(7, i == 12) ||
-		GobcoCover(8, i == 13) ||
-		GobcoCover(9, i == 14) ||
-		GobcoCover(10, i == 15)
-	_ = GobcoCover(11, i != 21) &&
-		GobcoCover(12, i != 22) &&
-		GobcoCover(13, i != 23) &&
-		GobcoCover(14, i != 24) &&
-		GobcoCover(15, i != 25)
+	_ = GobcoCover(6, i == 11) == true ||
+		GobcoCover(7, i == 12) == true ||
+		GobcoCover(8, i == 13) == true ||
+		GobcoCover(9, i == 14) == true ||
+		GobcoCover(10, i == 15) == true
+	_ = GobcoCover(11, i != 21) == true &&
+		GobcoCover(12, i != 22) == true &&
+		GobcoCover(13, i != 23) == true &&
+		GobcoCover(14, i != 24) == true &&
+		GobcoCover(15, i != 25) == true
 
 	// The operators '&&' and '||' can be mixed as well.
-	_ = GobcoCover(16, i == 31) ||
-		GobcoCover(17, i >= 32) && GobcoCover(18, i <= 33) ||
-		GobcoCover(19, i >= 34) && GobcoCover(20, i <= 35)
+	_ = GobcoCover(16, i == 31) == true ||
+		GobcoCover(17, i >= 32) == true && GobcoCover(18, i <= 33) == true ||
+		GobcoCover(19, i >= 34) == true && GobcoCover(20, i <= 35) == true
 
 	m := map[bool]int{}
-	_ = GobcoCover(21, m[GobcoCover(22, i == 41)] == m[GobcoCover(23, i == 42)])
+	_ = GobcoCover(21, m[GobcoCover(22, i == 41) == true] == m[GobcoCover(23, i == 42) == true]) == true
 
 	// In condition coverage mode, do not instrument complex conditions
 	// but instead their terminal conditions, in this case 'a', 'b' and
 	// 'c', to avoid large and redundant conditions in the output.
 	f := func(args ...bool) {}
-	f(GobcoCover(24, a) && GobcoCover(25, b))
-	f(GobcoCover(26, a) && GobcoCover(27, b) && GobcoCover(28, c))
-	f(!GobcoCover(29, a))
-	f(!GobcoCover(30, a) && !GobcoCover(31, b) && !GobcoCover(32, c))
+	f(GobcoCover(24, a == true) == true && GobcoCover(25, b == true) == true)
+	f(GobcoCover(26, a == true) == true && GobcoCover(27, b == true) == true && GobcoCover(28, c == true) == true)
+	f(!(GobcoCover(29, a == true) == true))
+	f(!(GobcoCover(30, a == true) == true) && !(GobcoCover(31, b == true) == true) && !(GobcoCover(32, c == true) == true))
 
 	// In condition coverage mode, instrument deeply nested conditions in
 	// if statements; in branch coverage mode, only instrument the main
 	// condition.
 	mi := map[bool]int{}
-	if GobcoCover(33, i == mi[GobcoCover(34, i > 51)]) {
-		_ = GobcoCover(35, i == mi[GobcoCover(36, i > 52)])
+	if GobcoCover(33, i == mi[GobcoCover(34, i > 51) == true]) == true {
+		_ = GobcoCover(35, i == mi[GobcoCover(36, i > 52) == true]) == true
 	}
-	for GobcoCover(37, i == mi[GobcoCover(38, i > 61)]) {
-		_ = GobcoCover(39, i == mi[GobcoCover(40, i > 62)])
+	for GobcoCover(37, i == mi[GobcoCover(38, i > 61) == true]) == true {
+		_ = GobcoCover(39, i == mi[GobcoCover(40, i > 62) == true]) == true
 	}
 }
 

testdata/instrumenter/CallExpr.branch

@@ -14,7 +14,7 @@ func callExpr(a bool, b string) bool {
 	// not wrapped since, as of January 2023, gobco does not use type
 	// resolution.
 
-	if GobcoCover(0, len(b) > 0) {
+	if GobcoCover(0, len(b) > 0) == true {
 		return callExpr(len(b)%2 == 0, b[1:])
 	}
 
@@ -29,7 +29,20 @@ func callExpr(a bool, b string) bool {
 	type myBool bool
 	_ = myBool(3 > 0)
 
+	// There may be custom types based on bool,
+	// and these types cannot be directly assigned to bool.
+	// https://github.com/rillig/gobco/issues/35
+	toMyBool := func(i int) myBool { return i%2 == 0 }
+	if GobcoCover(1, toMyBool(4) == true) == true {
+	}
+
+	fromMyBool := func(m myBool) bool { return bool(m) }
+	if GobcoCover(2, fromMyBool(toMyBool(3) && toMyBool(6)) == true) == true {
+	}
+
 	return false
 }
 
 // :17:5: "len(b) > 0"
+// :36:5: "toMyBool(4)"
+// :40:5: "fromMyBool(toMyBool(3) && toMyBool(6))"

testdata/instrumenter/CallExpr.cond

@@ -14,20 +14,31 @@ func callExpr(a bool, b string) bool {
 	// not wrapped since, as of January 2023, gobco does not use type
 	// resolution.
 
-	if GobcoCover(0, len(b) > 0) {
-		return callExpr(GobcoCover(1, len(b)%2 == 0), b[1:])
+	if GobcoCover(0, len(b) > 0) == true {
+		return callExpr(GobcoCover(1, len(b)%2 == 0) == true, b[1:])
 	}
 
 	// A CallExpr without identifier is also covered.
-	(func(a bool) {})(GobcoCover(2, 1 != 2))
+	(func(a bool) {})(GobcoCover(2, 1 != 2) == true)
 
 	// The function expression can contain conditions as well.
 	m := map[bool]func(){}
-	m[GobcoCover(3, 3 != 0)]()
+	m[GobcoCover(3, 3 != 0) == true]()
 
 	// Type conversions end up as CallExpr as well.
 	type myBool bool
-	_ = myBool(GobcoCover(4, 3 > 0))
+	_ = myBool(GobcoCover(4, 3 > 0) == true)
+
+	// There may be custom types based on bool,
+	// and these types cannot be directly assigned to bool.
+	// https://github.com/rillig/gobco/issues/35
+	toMyBool := func(i int) myBool { return GobcoCover(5, i%2 == 0) == true }
+	if GobcoCover(6, toMyBool(4) == true) == true {
+	}
+
+	fromMyBool := func(m myBool) bool { return bool(m) }
+	if GobcoCover(7, fromMyBool(GobcoCover(8, toMyBool(3) == true) == true && GobcoCover(9, toMyBool(6) == true) == true) == true) == true {
+	}
 
 	return false
 }
@@ -37,3 +48,8 @@ func callExpr(a bool, b string) bool {
 // :22:20: "1 != 2"
 // :26:4: "3 != 0"
 // :30:13: "3 > 0"
+// :35:42: "i%2 == 0"
+// :36:5: "toMyBool(4)"
+// :40:5: "fromMyBool(toMyBool(3) && toMyBool(6))"
+// :40:16: "toMyBool(3)"
+// :40:31: "toMyBool(6)"

testdata/instrumenter/CallExpr.go

@@ -29,5 +29,16 @@ func callExpr(a bool, b string) bool {
 	type myBool bool
 	_ = myBool(3 > 0)
 
+	// There may be custom types based on bool,
+	// and these types cannot be directly assigned to bool.
+	// https://github.com/rillig/gobco/issues/35
+	toMyBool := func(i int) myBool { return i%2 == 0 }
+	if toMyBool(4) {
+	}
+
+	fromMyBool := func(m myBool) bool { return bool(m) }
+	if fromMyBool(toMyBool(3) && toMyBool(6)) {
+	}
+
 	return false
 }

testdata/instrumenter/Comment.branch

@@ -33,23 +33,23 @@ func comment() {
 		_, gobco4 := gobco0.([][]int)
 		_, gobco5 := gobco0.([]int)
 		switch {
-		case GobcoCover(0, gobco1):
+		case GobcoCover(0, gobco1 == true):
 			// begin int
 			_ = 1
 			// end int
-		case GobcoCover(1, gobco2):
+		case GobcoCover(1, gobco2 == true):
 			// begin int-4D
 			_ = 1
 			// end int-4D
-		case GobcoCover(2, gobco3):
+		case GobcoCover(2, gobco3 == true):
 			// begin int-3D
 			_ = 1
 			// end int-3D
-		case GobcoCover(3, gobco4):
+		case GobcoCover(3, gobco4 == true):
 			// begin int-2D
 			_ = 1
 			// end int-2D
-		case GobcoCover(4, gobco5):
+		case GobcoCover(4, gobco5 == true):
 			// begin int-1D
 			_ = 1
 		}