Skip to content

Commit a3b3284

Browse files
committed
cmd/compile: prevent untyped types from reaching walk
We already require expressions to have already been typechecked before reaching walk. Moreover, all untyped expressions should have been converted to their default type by walk. However, in practice, we've been somewhat sloppy and inconsistent about ensuring this. In particular, a lot of AST rewrites ended up leaving untyped bool expressions scattered around. These likely aren't harmful in practice, but it seems worth cleaning up. The two most common cases addressed by this CL are: 1) When generating OIF and OFOR nodes, we would often typecheck the conditional expression, but not apply defaultlit to force it to the expression's default type. 2) When rewriting string comparisons into more fundamental primitives, we were simply overwriting r.Type with the desired type, which didn't propagate the type to nested subexpressions. These are fixed by utilizing finishcompare, which correctly handles this (and is already used by other comparison lowering rewrites). Lastly, walkexpr is extended to assert that it's not called on untyped expressions. Fixes #23834. Change-Id: Icbd29648a293555e4015d3b06a95a24ccbd3f790 Reviewed-on: https://go-review.googlesource.com/98337 Reviewed-by: Robert Griesemer <[email protected]>
1 parent ed8b7a7 commit a3b3284

File tree

4 files changed

+16
-24
lines changed

4 files changed

+16
-24
lines changed

src/cmd/compile/internal/gc/range.go

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -434,6 +434,7 @@ func walkrange(n *Node) *Node {
434434
typecheckslice(n.Left.Ninit.Slice(), Etop)
435435

436436
n.Left = typecheck(n.Left, Erv)
437+
n.Left = defaultlit(n.Left, nil)
437438
n.Right = typecheck(n.Right, Etop)
438439
typecheckslice(body, Etop)
439440
n.Nbody.Prepend(body...)
@@ -529,6 +530,7 @@ func memclrrange(n, v1, v2, a *Node) bool {
529530
n.Nbody.Append(v1)
530531

531532
n.Left = typecheck(n.Left, Erv)
533+
n.Left = defaultlit(n.Left, nil)
532534
typecheckslice(n.Nbody.Slice(), Etop)
533535
n = walkstmt(n)
534536
return true

src/cmd/compile/internal/gc/select.go

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -308,6 +308,7 @@ func walkselectcases(cases *Nodes) []*Node {
308308

309309
cond := nod(OEQ, chosen, nodintconst(int64(i)))
310310
cond = typecheck(cond, Erv)
311+
cond = defaultlit(cond, nil)
311312

312313
r = nod(OIF, cond, nil)
313314
r.Nbody.AppendNodes(&cas.Nbody)

src/cmd/compile/internal/gc/swt.go

Lines changed: 6 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -217,6 +217,7 @@ func walkswitch(sw *Node) {
217217
if sw.Left == nil {
218218
sw.Left = nodbool(true)
219219
sw.Left = typecheck(sw.Left, Erv)
220+
sw.Left = defaultlit(sw.Left, nil)
220221
}
221222

222223
if sw.Left.Op == OTYPESW {
@@ -314,21 +315,16 @@ func (s *exprSwitch) walkCases(cc []caseClause) *Node {
314315
low := nod(OGE, s.exprname, rng[0])
315316
high := nod(OLE, s.exprname, rng[1])
316317
a.Left = nod(OANDAND, low, high)
317-
a.Left = typecheck(a.Left, Erv)
318-
a.Left = defaultlit(a.Left, nil)
319-
a.Left = walkexpr(a.Left, nil) // give walk the opportunity to optimize the range check
320318
} else if (s.kind != switchKindTrue && s.kind != switchKindFalse) || assignop(n.Left.Type, s.exprname.Type, nil) == OCONVIFACE || assignop(s.exprname.Type, n.Left.Type, nil) == OCONVIFACE {
321319
a.Left = nod(OEQ, s.exprname, n.Left) // if name == val
322-
a.Left = typecheck(a.Left, Erv)
323-
a.Left = defaultlit(a.Left, nil)
324320
} else if s.kind == switchKindTrue {
325321
a.Left = n.Left // if val
326322
} else {
327323
// s.kind == switchKindFalse
328324
a.Left = nod(ONOT, n.Left, nil) // if !val
329-
a.Left = typecheck(a.Left, Erv)
330-
a.Left = defaultlit(a.Left, nil)
331325
}
326+
a.Left = typecheck(a.Left, Erv)
327+
a.Left = defaultlit(a.Left, nil)
332328
a.Nbody.Set1(n.Right) // goto l
333329

334330
cas = append(cas, a)
@@ -750,6 +746,7 @@ func (s *typeSwitch) walk(sw *Node) {
750746
def = blk
751747
}
752748
i.Left = typecheck(i.Left, Erv)
749+
i.Left = defaultlit(i.Left, nil)
753750
cas = append(cas, i)
754751

755752
// Load hash from type or itab.
@@ -869,6 +866,7 @@ func (s *typeSwitch) walkCases(cc []caseClause) *Node {
869866
a := nod(OIF, nil, nil)
870867
a.Left = nod(OEQ, s.hashname, nodintconst(int64(c.hash)))
871868
a.Left = typecheck(a.Left, Erv)
869+
a.Left = defaultlit(a.Left, nil)
872870
a.Nbody.Set1(n.Right)
873871
cas = append(cas, a)
874872
}
@@ -880,6 +878,7 @@ func (s *typeSwitch) walkCases(cc []caseClause) *Node {
880878
a := nod(OIF, nil, nil)
881879
a.Left = nod(OLE, s.hashname, nodintconst(int64(cc[half-1].hash)))
882880
a.Left = typecheck(a.Left, Erv)
881+
a.Left = defaultlit(a.Left, nil)
883882
a.Nbody.Set1(s.walkCases(cc[:half]))
884883
a.Rlist.Set1(s.walkCases(cc[half:]))
885884
return a

src/cmd/compile/internal/gc/walk.go

Lines changed: 7 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -476,6 +476,10 @@ func walkexpr(n *Node, init *Nodes) *Node {
476476
Fatalf("missed typecheck: %+v", n)
477477
}
478478

479+
if n.Type.IsUntyped() {
480+
Fatalf("expression has untyped type: %+v", n)
481+
}
482+
479483
if n.Op == ONAME && n.Class() == PAUTOHEAP {
480484
nn := nod(OIND, n.Name.Param.Heapaddr, nil)
481485
nn = typecheck(nn, Erv)
@@ -1234,10 +1238,7 @@ opswitch:
12341238
if (Op(n.Etype) == OEQ || Op(n.Etype) == ONE) && Isconst(n.Right, CTSTR) && n.Left.Op == OADDSTR && n.Left.List.Len() == 2 && Isconst(n.Left.List.Second(), CTSTR) && strlit(n.Right) == strlit(n.Left.List.Second()) {
12351239
// TODO(marvin): Fix Node.EType type union.
12361240
r := nod(Op(n.Etype), nod(OLEN, n.Left.List.First(), nil), nodintconst(0))
1237-
r = typecheck(r, Erv)
1238-
r = walkexpr(r, init)
1239-
r.Type = n.Type
1240-
n = r
1241+
n = finishcompare(n, r, init)
12411242
break
12421243
}
12431244

@@ -1337,10 +1338,7 @@ opswitch:
13371338
remains -= step
13381339
i += step
13391340
}
1340-
r = typecheck(r, Erv)
1341-
r = walkexpr(r, init)
1342-
r.Type = n.Type
1343-
n = r
1341+
n = finishcompare(n, r, init)
13441342
break
13451343
}
13461344
}
@@ -1374,22 +1372,14 @@ opswitch:
13741372
r = nod(ONOT, r, nil)
13751373
r = nod(OOROR, nod(ONE, llen, rlen), r)
13761374
}
1377-
1378-
r = typecheck(r, Erv)
1379-
r = walkexpr(r, nil)
13801375
} else {
13811376
// sys_cmpstring(s1, s2) :: 0
13821377
r = mkcall("cmpstring", types.Types[TINT], init, conv(n.Left, types.Types[TSTRING]), conv(n.Right, types.Types[TSTRING]))
13831378
// TODO(marvin): Fix Node.EType type union.
13841379
r = nod(Op(n.Etype), r, nodintconst(0))
13851380
}
13861381

1387-
r = typecheck(r, Erv)
1388-
if !n.Type.IsBoolean() {
1389-
Fatalf("cmp %v", n.Type)
1390-
}
1391-
r.Type = n.Type
1392-
n = r
1382+
n = finishcompare(n, r, init)
13931383

13941384
case OADDSTR:
13951385
n = addstr(n, init)

0 commit comments

Comments
 (0)