]> Cypherpunks repositories - gostls13.git/commitdiff
cmd/compile: eliminate OXFALL
authorMatthew Dempsky <mdempsky@google.com>
Fri, 1 Sep 2017 21:55:15 +0000 (14:55 -0700)
committerMatthew Dempsky <mdempsky@google.com>
Tue, 19 Sep 2017 18:08:50 +0000 (18:08 +0000)
Previously, we used OXFALL vs OFALL to distinguish fallthrough
statements that had been validated. Because in the Node AST we flatten
statement blocks, OXCASE and OXFALL needed to keep track of their
block scopes for this purpose.

Now that we have an AST that keeps these separate, we can just perform
the validation earlier.

Passes toolstash-check.

Fixes #14540.

Change-Id: I8421eaba16c2b3b72c9c5483b5cf20b14261385e
Reviewed-on: https://go-review.googlesource.com/61130
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Robert Griesemer <gri@golang.org>
12 files changed:
src/cmd/compile/internal/gc/bexport.go
src/cmd/compile/internal/gc/bimport.go
src/cmd/compile/internal/gc/fmt.go
src/cmd/compile/internal/gc/noder.go
src/cmd/compile/internal/gc/opnames.go
src/cmd/compile/internal/gc/order.go
src/cmd/compile/internal/gc/racewalk.go
src/cmd/compile/internal/gc/swt.go
src/cmd/compile/internal/gc/syntax.go
src/cmd/compile/internal/gc/typecheck.go
src/cmd/compile/internal/gc/walk.go
test/fixedbugs/issue14540.go [new file with mode: 0644]

index 596b8f7e833eb05356cd20910929a5e15603a117..e65e7f62806d1b8d64ddc0cbedcb1eb192eb1476 100644 (file)
@@ -1486,8 +1486,8 @@ func (p *exporter) stmt(n *Node) {
                p.stmtList(n.List)
                p.stmtList(n.Nbody)
 
-       case OFALL, OXFALL:
-               p.op(OXFALL)
+       case OFALL:
+               p.op(OFALL)
                p.pos(n)
 
        case OBREAK, OCONTINUE:
index 7cd155c4cb09354e32afbee08e3bf9944619da03..b676cd2054d47af3b2e0329ce8cbdb56130f0390 100644 (file)
@@ -1176,7 +1176,6 @@ func (p *importer) node() *Node {
        case OXCASE:
                types.Markdcl()
                n := nodl(p.pos(), OXCASE, nil, nil)
-               n.Xoffset = int64(types.Block)
                n.List.Set(p.exprList())
                // TODO(gri) eventually we must declare variables for type switch
                // statements (type switch statements are not yet exported)
@@ -1187,9 +1186,8 @@ func (p *importer) node() *Node {
        // case OFALL:
        //      unreachable - mapped to OXFALL case below by exporter
 
-       case OXFALL:
-               n := nodl(p.pos(), OXFALL, nil, nil)
-               n.Xoffset = int64(types.Block)
+       case OFALL:
+               n := nodl(p.pos(), OFALL, nil, nil)
                return n
 
        case OBREAK, OCONTINUE:
index 21f5e9d9b965557459635fd795e1f9a0052819ef..0ba1ba3d80b4e3bd1aea6b6b8098d7bec27eadae 100644 (file)
@@ -204,7 +204,6 @@ var goopnames = []string{
        OSUB:      "-",
        OSWITCH:   "switch",
        OXOR:      "^",
-       OXFALL:    "fallthrough",
 }
 
 func (o Op) String() string {
@@ -1080,11 +1079,7 @@ func (n *Node) stmtfmt(s fmt.State, mode fmtMode) {
                }
                mode.Fprintf(s, ": %v", n.Nbody)
 
-       case OBREAK,
-               OCONTINUE,
-               OGOTO,
-               OFALL,
-               OXFALL:
+       case OBREAK, OCONTINUE, OGOTO, OFALL:
                if n.Left != nil {
                        mode.Fprintf(s, "%#v %v", n.Op, n.Left)
                } else {
@@ -1219,7 +1214,6 @@ var opprec = []int{
        OSELECT:     -1,
        OSWITCH:     -1,
        OXCASE:      -1,
-       OXFALL:      -1,
 
        OEND: 0,
 }
index 1ab7a033bc8f832f655eb033d281d4a69b6d296a..851f8723a3708b26460c0bc76fae885b7a2a0bb6 100644 (file)
@@ -710,9 +710,13 @@ func (p *noder) embedded(typ syntax.Expr) *Node {
 }
 
 func (p *noder) stmts(stmts []syntax.Stmt) []*Node {
+       return p.stmtsFall(stmts, false)
+}
+
+func (p *noder) stmtsFall(stmts []syntax.Stmt, fallOK bool) []*Node {
        var nodes []*Node
-       for _, stmt := range stmts {
-               s := p.stmt(stmt)
+       for i, stmt := range stmts {
+               s := p.stmtFall(stmt, fallOK && i+1 == len(stmts))
                if s == nil {
                } else if s.Op == OBLOCK && s.Ninit.Len() == 0 {
                        nodes = append(nodes, s.List.Slice()...)
@@ -724,12 +728,16 @@ func (p *noder) stmts(stmts []syntax.Stmt) []*Node {
 }
 
 func (p *noder) stmt(stmt syntax.Stmt) *Node {
+       return p.stmtFall(stmt, false)
+}
+
+func (p *noder) stmtFall(stmt syntax.Stmt, fallOK bool) *Node {
        p.lineno(stmt)
        switch stmt := stmt.(type) {
        case *syntax.EmptyStmt:
                return nil
        case *syntax.LabeledStmt:
-               return p.labeledStmt(stmt)
+               return p.labeledStmt(stmt, fallOK)
        case *syntax.BlockStmt:
                l := p.blockStmt(stmt)
                if len(l) == 0 {
@@ -780,7 +788,10 @@ func (p *noder) stmt(stmt syntax.Stmt) *Node {
                case syntax.Continue:
                        op = OCONTINUE
                case syntax.Fallthrough:
-                       op = OXFALL
+                       if !fallOK {
+                               yyerror("fallthrough statement out of place")
+                       }
+                       op = OFALL
                case syntax.Goto:
                        op = OGOTO
                default:
@@ -790,9 +801,6 @@ func (p *noder) stmt(stmt syntax.Stmt) *Node {
                if stmt.Label != nil {
                        n.Left = p.newname(stmt.Label)
                }
-               if op == OXFALL {
-                       n.Xoffset = int64(types.Block)
-               }
                return n
        case *syntax.CallStmt:
                var op Op
@@ -912,7 +920,7 @@ func (p *noder) switchStmt(stmt *syntax.SwitchStmt) *Node {
        }
 
        tswitch := n.Left
-       if tswitch != nil && (tswitch.Op != OTYPESW || tswitch.Left == nil) {
+       if tswitch != nil && tswitch.Op != OTYPESW {
                tswitch = nil
        }
        n.List.Set(p.caseClauses(stmt.Body, tswitch, stmt.Rbrace))
@@ -934,15 +942,35 @@ func (p *noder) caseClauses(clauses []*syntax.CaseClause, tswitch *Node, rbrace
                if clause.Cases != nil {
                        n.List.Set(p.exprList(clause.Cases))
                }
-               if tswitch != nil {
+               if tswitch != nil && tswitch.Left != nil {
                        nn := newname(tswitch.Left.Sym)
                        declare(nn, dclcontext)
                        n.Rlist.Set1(nn)
                        // keep track of the instances for reporting unused
                        nn.Name.Defn = tswitch
                }
-               n.Xoffset = int64(types.Block)
-               n.Nbody.Set(p.stmts(clause.Body))
+
+               // Trim trailing empty statements. We omit them from
+               // the Node AST anyway, and it's easier to identify
+               // out-of-place fallthrough statements without them.
+               body := clause.Body
+               for len(body) > 0 {
+                       if _, ok := body[len(body)-1].(*syntax.EmptyStmt); !ok {
+                               break
+                       }
+                       body = body[:len(body)-1]
+               }
+
+               n.Nbody.Set(p.stmtsFall(body, true))
+               if l := n.Nbody.Len(); l > 0 && n.Nbody.Index(l-1).Op == OFALL {
+                       if tswitch != nil {
+                               yyerror("cannot fallthrough in type switch")
+                       }
+                       if i+1 == len(clauses) {
+                               yyerror("cannot fallthrough final case in switch")
+                       }
+               }
+
                nodes = append(nodes, n)
        }
        if len(clauses) > 0 {
@@ -980,12 +1008,12 @@ func (p *noder) commClauses(clauses []*syntax.CommClause, rbrace src.Pos) []*Nod
        return nodes
 }
 
-func (p *noder) labeledStmt(label *syntax.LabeledStmt) *Node {
+func (p *noder) labeledStmt(label *syntax.LabeledStmt, fallOK bool) *Node {
        lhs := p.nod(label, OLABEL, p.newname(label.Label), nil)
 
        var ls *Node
        if label.Stmt != nil { // TODO(mdempsky): Should always be present.
-               ls = p.stmt(label.Stmt)
+               ls = p.stmtFall(label.Stmt, fallOK)
        }
 
        lhs.Name.Defn = ls
index 09442b595f4b0926723958dff4630d41e8735413..47089aad0b8c7be217adebeade71ad70ab5bb918 100644 (file)
@@ -122,7 +122,6 @@ var opnames = []string{
        ODEFER:           "DEFER",
        OEMPTY:           "EMPTY",
        OFALL:            "FALL",
-       OXFALL:           "XFALL",
        OFOR:             "FOR",
        OFORUNTIL:        "FORUNTIL",
        OGOTO:            "GOTO",
index 27c3272cab371280fdd88124ce8d02a2c2323201..c555a5eabcd9aa470114ca655fbb7136b28e420d 100644 (file)
@@ -620,7 +620,6 @@ func orderstmt(n *Node, order *Order) {
                ODCLCONST,
                ODCLTYPE,
                OFALL,
-               OXFALL,
                OGOTO,
                OLABEL,
                ORETJMP:
index 0740abbbe83bf2fa590528116eaafb0e64a0d6dc..1771e03b866bbed1f7de5b67003ceb90eca3cef4 100644 (file)
@@ -332,7 +332,6 @@ func instrumentnode(np **Node, init *Nodes, wr int, skip int) {
                OCLOSE,
                ONEW,
                OXCASE,
-               OXFALL,
                OCASE,
                OPANIC,
                ORECOVER,
index 3051341b6a6edca97550bca048c17c8c93ddcf82..469af86aa63a5e36d76e07c11efbb376b929a7f2 100644 (file)
@@ -380,7 +380,7 @@ func casebody(sw *Node, typeswvar *Node) {
        var def *Node    // defaults
        br := nod(OBREAK, nil, nil)
 
-       for i, n := range sw.List.Slice() {
+       for _, n := range sw.List.Slice() {
                setlineno(n)
                if n.Op != OXCASE {
                        Fatalf("casebody %v", n.Op)
@@ -474,21 +474,7 @@ func casebody(sw *Node, typeswvar *Node) {
                        fallIndex--
                }
                last := stat[fallIndex]
-
-               // botch - shouldn't fall through declaration
-               if last.Xoffset == n.Xoffset && last.Op == OXFALL {
-                       if typeswvar != nil {
-                               setlineno(last)
-                               yyerror("cannot fallthrough in type switch")
-                       }
-
-                       if i+1 >= sw.List.Len() {
-                               setlineno(last)
-                               yyerror("cannot fallthrough final case in switch")
-                       }
-
-                       last.Op = OFALL
-               } else {
+               if last.Op != OFALL {
                        stat = append(stat, br)
                }
        }
index 32ae6f2f28ee4f96b7b922c5427c1f7ec3d62eeb..ee0666a9462b37582150af231688bb082f306d81 100644 (file)
@@ -45,7 +45,6 @@ type Node struct {
        // - ONAME nodes that refer to local variables use it to identify their stack frame position.
        // - ODOT, ODOTPTR, and OINDREGSP use it to indicate offset relative to their base address.
        // - OSTRUCTKEY uses it to store the named field's offset.
-       // - OXCASE and OXFALL use it to validate the use of fallthrough.
        // - Named OLITERALs use it to to store their ambient iota value.
        // Possibly still more uses. If you find any, document them.
        Xoffset int64
@@ -564,8 +563,8 @@ const (
        OCONTINUE // continue
        ODEFER    // defer Left (Left must be call)
        OEMPTY    // no-op (empty statement)
-       OFALL     // fallthrough (after processing)
-       OXFALL    // fallthrough (before processing)
+       _         // placeholder to appease toolstash
+       OFALL     // fallthrough
        OFOR      // for Ninit; Left; Right { Nbody }
        OFORUNTIL // for Ninit; Left; Right { Nbody } ; test applied after executing body, not before
        OGOTO     // goto Left
index 59dea3a2e1a80f7d2007c0a76dabd29914fdf87a..23fdb3486e2bb1b369305877d002932a29e29114 100644 (file)
@@ -2012,7 +2012,7 @@ func typecheck1(n *Node, top int) *Node {
                ODCL,
                OEMPTY,
                OGOTO,
-               OXFALL,
+               OFALL,
                OVARKILL,
                OVARLIVE:
                ok |= Etop
@@ -3898,7 +3898,7 @@ func (n *Node) isterminating() bool {
        case OBLOCK:
                return n.List.isterminating()
 
-       case OGOTO, ORETURN, ORETJMP, OPANIC, OXFALL:
+       case OGOTO, ORETURN, ORETJMP, OPANIC, OFALL:
                return true
 
        case OFOR, OFORUNTIL:
index fba037b164e67e2e24666aac81e06078c9ba3bc6..b0c98eea8389a6badf33b0c28a2b097262c0647c 100644 (file)
@@ -348,10 +348,6 @@ func walkstmt(n *Node) *Node {
 
        case ORANGE:
                n = walkrange(n)
-
-       case OXFALL:
-               yyerror("fallthrough statement out of place")
-               n.Op = OFALL
        }
 
        if n.Op == ONAME {
diff --git a/test/fixedbugs/issue14540.go b/test/fixedbugs/issue14540.go
new file mode 100644 (file)
index 0000000..62b17a0
--- /dev/null
@@ -0,0 +1,20 @@
+// errorcheck
+
+// Copyright 2017 The Go Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style
+// license that can be found in the LICENSE file.
+
+package p
+
+func f(x int) {
+       switch x {
+       case 0:
+               fallthrough
+               ; // ok
+       case 1:
+               fallthrough // ERROR "fallthrough statement out of place"
+               {}
+       case 2:
+               fallthrough // ERROR "cannot fallthrough"
+       }
+}