Skip to content

Commit

Permalink
cmd/compile/internal/noder: fix implicit conversion position
Browse files Browse the repository at this point in the history
In go.dev/cl/413396, I implemented implicit conversions by setting the
conversion's position to the enclosing statement that necessitated the
conversion. However, users actually want the position information to
be at the expression itself, and this seems sensible anyway.

This was noticed because x/tools had a test for:

	fmt.Println(42)

and it was checking where the escape analysis diagnostic for
`42` (really `any(42)`) was reported.

Historically, we reported the column of the `4`; but CL 413396 caused
unified IR to instead report the column of the `(` instead (the
position associated with the call expression, which forced `42` to be
implicitly converted from `int` to `any`).

I chalk this mistake up to being accustomed to working with ir, where
we can't reliably use n.Pos() because of how ONAME positions work, so
I was trying to avoid relying on the implicitly converted expression's
own position.

Change-Id: I762076af6f65ebe6d444d64630722a5016dc2698
Reviewed-on: https://go-review.googlesource.com/c/go/+/422976
Reviewed-by: David Chase <[email protected]>
Run-TryBot: Matthew Dempsky <[email protected]>
TryBot-Result: Gopher Robot <[email protected]>
  • Loading branch information
mdempsky committed Aug 11, 2022
1 parent 8743198 commit db84f53
Showing 1 changed file with 16 additions and 16 deletions.
32 changes: 16 additions & 16 deletions src/cmd/compile/internal/noder/writer.go
Original file line number Diff line number Diff line change
Expand Up @@ -1060,7 +1060,7 @@ func (w *writer) stmt1(stmt syntax.Stmt) {
if stmt.Op != syntax.Shl && stmt.Op != syntax.Shr {
typ = w.p.typeOf(stmt.Lhs)
}
w.implicitConvExpr(stmt, typ, stmt.Rhs)
w.implicitConvExpr(typ, stmt.Rhs)

default:
w.assignStmt(stmt, stmt.Lhs, stmt.Rhs)
Expand Down Expand Up @@ -1125,7 +1125,7 @@ func (w *writer) stmt1(stmt syntax.Stmt) {
w.Code(stmtSend)
w.pos(stmt)
w.expr(stmt.Chan)
w.implicitConvExpr(stmt, chanType.Elem(), stmt.Value)
w.implicitConvExpr(chanType.Elem(), stmt.Value)

case *syntax.SwitchStmt:
w.Code(stmtSwitch)
Expand Down Expand Up @@ -1376,7 +1376,7 @@ func (w *writer) switchStmt(stmt *syntax.SwitchStmt) {
}

if w.Bool(tag != nil) {
w.implicitConvExpr(tag, tagType, tag)
w.implicitConvExpr(tagType, tag)
}
}

Expand Down Expand Up @@ -1406,7 +1406,7 @@ func (w *writer) switchStmt(stmt *syntax.SwitchStmt) {
w.Sync(pkgbits.SyncExprs)
w.Len(len(cases))
for _, cas := range cases {
w.implicitConvExpr(cas, tagType, cas)
w.implicitConvExpr(tagType, cas)
}
}

Expand Down Expand Up @@ -1569,7 +1569,7 @@ func (w *writer) expr(expr syntax.Expr) {
w.Code(exprIndex)
w.expr(expr.X)
w.pos(expr)
w.implicitConvExpr(expr, keyType, expr.Index)
w.implicitConvExpr(keyType, expr.Index)
if keyType != nil {
w.rtype(xtyp)
}
Expand Down Expand Up @@ -1619,9 +1619,9 @@ func (w *writer) expr(expr syntax.Expr) {

w.Code(exprBinaryOp)
w.op(binOps[expr.Op])
w.implicitConvExpr(expr, commonType, expr.X)
w.implicitConvExpr(commonType, expr.X)
w.pos(expr)
w.implicitConvExpr(expr, commonType, expr.Y)
w.implicitConvExpr(commonType, expr.Y)

case *syntax.CallExpr:
tv, ok := w.p.info.Types[expr.Fun]
Expand Down Expand Up @@ -1814,23 +1814,23 @@ func (w *writer) multiExpr(pos poser, dstType func(int) types2.Type, exprs []syn
w.Bool(false) // N:N assignment
w.Len(len(exprs))
for i, expr := range exprs {
w.implicitConvExpr(pos, dstType(i), expr)
w.implicitConvExpr(dstType(i), expr)
}
}

// implicitConvExpr is like expr, but if dst is non-nil and different from
// expr's type, then an implicit conversion operation is inserted at
// pos.
func (w *writer) implicitConvExpr(pos poser, dst types2.Type, expr syntax.Expr) {
// implicitConvExpr is like expr, but if dst is non-nil and different
// from expr's type, then an implicit conversion operation is inserted
// at expr's position.
func (w *writer) implicitConvExpr(dst types2.Type, expr syntax.Expr) {
src := w.p.typeOf(expr)
if dst != nil && !types2.Identical(src, dst) {
if !types2.AssignableTo(src, dst) {
w.p.fatalf(pos, "%v is not assignable to %v", src, dst)
w.p.fatalf(expr.Pos(), "%v is not assignable to %v", src, dst)
}
w.Code(exprConvert)
w.Bool(true) // implicit
w.typ(dst)
w.pos(pos)
w.pos(expr)
w.convRTTI(src, dst)
w.Bool(isTypeParam(dst))
// fallthrough
Expand Down Expand Up @@ -1882,12 +1882,12 @@ func (w *writer) compLit(lit *syntax.CompositeLit) {
if kv, ok := elem.(*syntax.KeyValueExpr); w.Bool(ok) {
// use position of expr.Key rather than of elem (which has position of ':')
w.pos(kv.Key)
w.implicitConvExpr(kv.Key, keyType, kv.Key)
w.implicitConvExpr(keyType, kv.Key)
elem = kv.Value
}
}
w.pos(elem)
w.implicitConvExpr(elem, elemType, elem)
w.implicitConvExpr(elemType, elem)
}
}

Expand Down

0 comments on commit db84f53

Please sign in to comment.