From 349a2876467cdae58a772424f044f882fd4e2f6b Mon Sep 17 00:00:00 2001 From: Alberto Donizetti Date: Mon, 12 Oct 2020 14:35:24 +0200 Subject: cmd/compile: use Bool accessor in place of Val.U.(bool) We have a Bool() accessor for the value in boolean nodes, that we use elsewhere for n.Val().U.(bool), use it here too. Noticed while reading the code. Change-Id: Ie42e014970099a05fe9f02f378af77b63e7e6b13 Reviewed-on: https://go-review.googlesource.com/c/go/+/261360 Trust: Alberto Donizetti Run-TryBot: Alberto Donizetti TryBot-Result: Go Bot Reviewed-by: Robert Griesemer --- src/cmd/compile/internal/gc/swt.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'src/cmd/compile/internal/gc/swt.go') diff --git a/src/cmd/compile/internal/gc/swt.go b/src/cmd/compile/internal/gc/swt.go index 138b0acc53..bf0410900f 100644 --- a/src/cmd/compile/internal/gc/swt.go +++ b/src/cmd/compile/internal/gc/swt.go @@ -440,7 +440,7 @@ func (c *exprClause) test(exprname *Node) *Node { // Optimize "switch true { ...}" and "switch false { ... }". if Isconst(exprname, CTBOOL) && !c.lo.Type.IsInterface() { - if exprname.Val().U.(bool) { + if exprname.Bool() { return c.lo } else { return nodl(c.pos, ONOT, c.lo, nil) -- cgit v1.2.1 From e2931612b04e2ea6be337872c6f4a31c7d7dec54 Mon Sep 17 00:00:00 2001 From: Alberto Donizetti Date: Mon, 12 Oct 2020 15:02:59 +0200 Subject: cmd/compile: rename strlit, Bool, and Int64 *Node accessors The Node type has shortcuts to access bool and int Values: func (n *Node) Int64() int64 for n.Val().U.(*Mpint).Int64() func (n *Node) Bool() bool for n.Val().U.(bool) I was convinced we didn't have one for string literal nodes, until I noticed that we do, it's just called strlit, it's not a method, and it's later in the file: func strlit(n *Node) string This change, for consistency: - Renames strlit to StringVal and makes it a *Node method - Renames Bool and Int64 to BoolVal and Int64Val - Moves StringVal near the other two Change-Id: I18e635384c35eb3a238fd52b1ccd322b1a74d733 Reviewed-on: https://go-review.googlesource.com/c/go/+/261361 Trust: Alberto Donizetti Reviewed-by: Matthew Dempsky --- src/cmd/compile/internal/gc/swt.go | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) (limited to 'src/cmd/compile/internal/gc/swt.go') diff --git a/src/cmd/compile/internal/gc/swt.go b/src/cmd/compile/internal/gc/swt.go index bf0410900f..bfbedb2aa5 100644 --- a/src/cmd/compile/internal/gc/swt.go +++ b/src/cmd/compile/internal/gc/swt.go @@ -358,8 +358,8 @@ func (s *exprSwitch) flush() { // all we need here is consistency. We respect this // sorting below. sort.Slice(cc, func(i, j int) bool { - si := strlit(cc[i].lo) - sj := strlit(cc[j].lo) + si := cc[i].lo.StringVal() + sj := cc[j].lo.StringVal() if len(si) != len(sj) { return len(si) < len(sj) } @@ -368,7 +368,7 @@ func (s *exprSwitch) flush() { // runLen returns the string length associated with a // particular run of exprClauses. - runLen := func(run []exprClause) int64 { return int64(len(strlit(run[0].lo))) } + runLen := func(run []exprClause) int64 { return int64(len(run[0].lo.StringVal())) } // Collapse runs of consecutive strings with the same length. var runs [][]exprClause @@ -405,7 +405,7 @@ func (s *exprSwitch) flush() { merged := cc[:1] for _, c := range cc[1:] { last := &merged[len(merged)-1] - if last.jmp == c.jmp && last.hi.Int64()+1 == c.lo.Int64() { + if last.jmp == c.jmp && last.hi.Int64Val()+1 == c.lo.Int64Val() { last.hi = c.lo } else { merged = append(merged, c) @@ -440,7 +440,7 @@ func (c *exprClause) test(exprname *Node) *Node { // Optimize "switch true { ...}" and "switch false { ... }". if Isconst(exprname, CTBOOL) && !c.lo.Type.IsInterface() { - if exprname.Bool() { + if exprname.BoolVal() { return c.lo } else { return nodl(c.pos, ONOT, c.lo, nil) -- cgit v1.2.1 From 8773d141641708574654c617b686a7fd687c3f70 Mon Sep 17 00:00:00 2001 From: Alberto Donizetti Date: Tue, 13 Oct 2020 15:58:10 +0200 Subject: cmd/compile: make assignop/convertop reason a return param On a negative answer, the assignop and convertop functions write the reason why to a string pointer passed as an argument, likely a C-ism leftover since the compiler's machine assisted translation to Go. This change makes why a return parameter. It also fixes a few places where the assignop/convertop result was compared to 0. While OXXX's value may be zero now, using the named constant is more robust. Change-Id: Id9147ed4c1b97d658d30a2f778f876b7867006b4 Reviewed-on: https://go-review.googlesource.com/c/go/+/261857 Reviewed-by: Matthew Dempsky Trust: Alberto Donizetti --- src/cmd/compile/internal/gc/swt.go | 19 +++++++++++-------- 1 file changed, 11 insertions(+), 8 deletions(-) (limited to 'src/cmd/compile/internal/gc/swt.go') diff --git a/src/cmd/compile/internal/gc/swt.go b/src/cmd/compile/internal/gc/swt.go index bfbedb2aa5..8d9fbe300e 100644 --- a/src/cmd/compile/internal/gc/swt.go +++ b/src/cmd/compile/internal/gc/swt.go @@ -189,16 +189,19 @@ func typecheckExprSwitch(n *Node) { continue } - switch { - case nilonly != "" && !n1.isNil(): + if nilonly != "" && !n1.isNil() { yyerrorl(ncase.Pos, "invalid case %v in switch (can only compare %s %v to nil)", n1, nilonly, n.Left) - case t.IsInterface() && !n1.Type.IsInterface() && !IsComparable(n1.Type): + } else if t.IsInterface() && !n1.Type.IsInterface() && !IsComparable(n1.Type) { yyerrorl(ncase.Pos, "invalid case %L in switch (incomparable type)", n1) - case assignop(n1.Type, t, nil) == 0 && assignop(t, n1.Type, nil) == 0: - if n.Left != nil { - yyerrorl(ncase.Pos, "invalid case %v in switch on %v (mismatched types %v and %v)", n1, n.Left, n1.Type, t) - } else { - yyerrorl(ncase.Pos, "invalid case %v in switch (mismatched types %v and bool)", n1, n1.Type) + } else { + op1, _ := assignop(n1.Type, t) + op2, _ := assignop(t, n1.Type) + if op1 == OXXX && op2 == OXXX { + if n.Left != nil { + yyerrorl(ncase.Pos, "invalid case %v in switch on %v (mismatched types %v and %v)", n1, n.Left, n1.Type, t) + } else { + yyerrorl(ncase.Pos, "invalid case %v in switch (mismatched types %v and bool)", n1, n1.Type) + } } } -- cgit v1.2.1