Repository: goplus/gop
Pull request #488 (2020-07-23 07:40:16)

Var scope issues
try to fix var scope bug at #303 and #304 

codecov-commenter: # [Codecov](https://codecov.io/gh/goplus/gop/pull/488?src=pr&el=h1) Report
> Merging [#488](https://codecov.io/gh/goplus/gop/pull/488?src=pr&el=desc) into [master](https://codecov.io/gh/goplus/gop/commit/693b1694e7d97f056a034adc6407eaccff980d0a&el=desc) will **increase** coverage by `0.05%`.
> The diff coverage is `91.75%`.

[![Impacted file tree graph](https://codecov.io/gh/goplus/gop/pull/488/graphs/tree.svg?width=650&height=150&src=pr&token=4qln9Wk3Pn)](https://codecov.io/gh/goplus/gop/pull/488?src=pr&el=tree)

```diff
@@            Coverage Diff             @@
##           master     #488      +/-   ##
==========================================
+ Coverage   88.92%   88.97%   +0.05%     
==========================================
  Files          38       39       +1     
  Lines        9605     9773     +168     
==========================================
+ Hits         8541     8696     +155     
- Misses        785      794       +9     
- Partials      279      283       +4     
```


| [Impacted Files](https://codecov.io/gh/goplus/gop/pull/488?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [exec/bytecode/code.go](https://codecov.io/gh/goplus/gop/pull/488/diff?src=pr&el=tree#diff-ZXhlYy9ieXRlY29kZS9jb2RlLmdv) | `100.00% <ø> (ø)` | |
| [exec/golang/var.go](https://codecov.io/gh/goplus/gop/pull/488/diff?src=pr&el=tree#diff-ZXhlYy9nb2xhbmcvdmFyLmdv) | `82.40% <ø> (-1.86%)` | :arrow_down: |
| [cl/context.go](https://codecov.io/gh/goplus/gop/pull/488/diff?src=pr&el=tree#diff-Y2wvY29udGV4dC5nbw==) | `81.60% <85.71%> (+0.14%)` | :arrow_up: |
| [cl/stmt.go](https://codecov.io/gh/goplus/gop/pull/488/diff?src=pr&el=tree#diff-Y2wvc3RtdC5nbw==) | `92.06% <87.95%> (-1.13%)` | :arrow_down: |
| [cl/expr.go](https://codecov.io/gh/goplus/gop/pull/488/diff?src=pr&el=tree#diff-Y2wvZXhwci5nbw==) | `83.59% <88.88%> (+0.08%)` | :arrow_up: |
| [exec/golang/stmt.go](https://codecov.io/gh/goplus/gop/pull/488/diff?src=pr&el=tree#diff-ZXhlYy9nb2xhbmcvc3RtdC5nbw==) | `95.70% <88.88%> (-0.81%)` | :arrow_down: |
| [cl/ctx\_check.go](https://codecov.io/gh/goplus/gop/pull/488/diff?src=pr&el=tree#diff-Y2wvY3R4X2NoZWNrLmdv) | `75.10% <100.00%> (+1.02%)` | :arrow_up: |
| [exec/bytecode/const.go](https://codecov.io/gh/goplus/gop/pull/488/diff?src=pr&el=tree#diff-ZXhlYy9ieXRlY29kZS9jb25zdC5nbw==) | `90.32% <100.00%> (+0.21%)` | :arrow_up: |
| [exec/bytecode/context.go](https://codecov.io/gh/goplus/gop/pull/488/diff?src=pr&el=tree#diff-ZXhlYy9ieXRlY29kZS9jb250ZXh0Lmdv) | `91.59% <100.00%> (+0.21%)` | :arrow_up: |
| [exec/bytecode/defer.go](https://codecov.io/gh/goplus/gop/pull/488/diff?src=pr&el=tree#diff-ZXhlYy9ieXRlY29kZS9kZWZlci5nbw==) | `100.00% <100.00%> (ø)` | |
| ... and [8 more](https://codecov.io/gh/goplus/gop/pull/488/diff?src=pr&el=tree-more) | |

------

[Continue to review full report at Codecov](https://codecov.io/gh/goplus/gop/pull/488?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/goplus/gop/pull/488?src=pr&el=footer). Last update [28dff1c...c1a72c3](https://codecov.io/gh/goplus/gop/pull/488?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).


Patch:
diff --git a/cl/context.go b/cl/context.go
index 2654941c..98dbfdee 100644
--- a/cl/context.go
+++ b/cl/context.go
@@ -323,6 +323,18 @@ func (p *blockCtx) findFunc(name string) (addr *funcDecl, err error) {
 	return nil, ErrSymbolNotFunc
 }
 
+// getCtxVar finds a var in currentCtx only
+func (p *blockCtx) getCtxVar(name string) (addr iVar, err error) {
+	v, ok := p.syms[name]
+	if !ok {
+		return nil, ErrNotFound
+	}
+	if addr, ok = v.(iVar); ok {
+		return
+	}
+	return nil, ErrSymbolNotVariable
+}
+
 func (p *blockCtx) findVar(name string) (addr iVar, err error) {
 	v, ok := p.find(name)
 	if !ok {
diff --git a/cl/ctx_check.go b/cl/ctx_check.go
index c029f796..6e7120dd 100644
--- a/cl/ctx_check.go
+++ b/cl/ctx_check.go
@@ -345,7 +345,7 @@ func isNoExecCtxAssignStmt(ctx *blockCtx, expr *ast.AssignStmt) bool {
 	return true
 }
 
-func isNoExecCtxExprLHS(ctx *blockCtx, expr ast.Expr, mode compleMode) bool {
+func isNoExecCtxExprLHS(ctx *blockCtx, expr ast.Expr, mode compileMode) bool {
 	switch v := expr.(type) {
 	case *ast.Ident:
 		return isNoExecCtxIdentLHS(ctx, v.Name, mode)
@@ -359,21 +359,21 @@ func isNoExecCtxExprLHS(ctx *blockCtx, expr ast.Expr, mode compleMode) bool {
 	return true
 }
 
-func isNoExecCtxIndexExprLHS(ctx *blockCtx, v *ast.IndexExpr, mode compleMode) bool {
+func isNoExecCtxIndexExprLHS(ctx *blockCtx, v *ast.IndexExpr, mode compileMode) bool {
 	if noExecCtx := isNoExecCtxExpr(ctx, v.X); !noExecCtx {
 		return false
 	}
 	return isNoExecCtxExpr(ctx, v.Index)
 }
 
-func isNoExecCtxIdentLHS(ctx *blockCtx, name string, mode compleMode) bool {
+func isNoExecCtxIdentLHS(ctx *blockCtx, name string, mode compileMode) bool {
 	if mode == lhsDefine && !ctx.exists(name) {
 		ctx.insertVar(name, exec.TyEmptyInterface, true)
 	}
 	return true
 }
 
-func isNoExecCtxSelectorExprLHS(ctx *blockCtx, v *ast.SelectorExpr, mode compleMode) bool {
+func isNoExecCtxSelectorExprLHS(ctx *blockCtx, v *ast.SelectorExpr, mode compileMode) bool {
 	if noExecCtx := isNoExecCtxExpr(ctx, v.X); !noExecCtx {
 		return false
 	}
diff --git a/cl/expr.go b/cl/expr.go
index 2a3548d8..3f849120 100644
--- a/cl/expr.go
+++ b/cl/expr.go
@@ -19,7 +19,6 @@ package cl
 import (
 	"reflect"
 	"strings"
-	"syscall"
 
 	"github.com/goplus/gop/ast"
 	"github.com/goplus/gop/ast/astutil"
@@ -32,16 +31,16 @@ import (
 
 // -----------------------------------------------------------------------------
 
-type compleMode = token.Token
+type compileMode = token.Token
 
 const (
-	lhsAssign compleMode = token.ASSIGN // leftHandSide = ...
-	lhsDefine compleMode = token.DEFINE // leftHandSide := ...
+	lhsAssign compileMode = token.ASSIGN // leftHandSide = ...
+	lhsDefine compileMode = token.DEFINE // leftHandSide := ...
 )
 
 // -----------------------------------------------------------------------------
 
-func compileExprLHS(ctx *blockCtx, expr ast.Expr, mode compleMode) {
+func compileExprLHS(ctx *blockCtx, expr ast.Expr, mode compileMode) {
 	switch v := expr.(type) {
 	case *ast.Ident:
 		compileIdentLHS(ctx, v.Name, mode)
@@ -98,14 +97,20 @@ func compileExpr(ctx *blockCtx, expr ast.Expr) func() {
 	}
 }
 
-func compileIdentLHS(ctx *blockCtx, name string, mode compleMode) {
+func compileIdentLHS(ctx *blockCtx, name string, mode compileMode) {
 	in := ctx.infer.Get(-1)
 	addr, err := ctx.findVar(name)
+	if mode == lhsDefine {
+		addr, err = ctx.getCtxVar(name)
+		if addr != nil {
+			log.Panicf("compileIdentLHS failed: %s redeclared in this block\n", name)
+		}
+	}
 	if err == nil {
 		if mode == lhsDefine && !addr.inCurrentCtx(ctx) {
 			log.Warn("requireVar: variable is shadowed -", name)
 		}
-	} else if mode == lhsAssign || err != syscall.ENOENT {
+	} else if mode == lhsAssign || err != ErrNotFound {
 		log.Panicln("compileIdentLHS failed:", err, "-", name)
 	} else {
 		typ := boundType(in.(iValue))
@@ -688,7 +693,7 @@ func compileCallExprCall(ctx *blockCtx, exprFun func(), v *ast.CallExpr) func()
 	return nil
 }
 
-func compileIndexExprLHS(ctx *blockCtx, v *ast.IndexExpr, mode compleMode) {
+func compileIndexExprLHS(ctx *blockCtx, v *ast.IndexExpr, mode compileMode) {
 	if mode == lhsDefine {
 		log.Panicln("compileIndexExprLHS: `:=` can't be used for index expression")
 	}
@@ -893,7 +898,7 @@ func getFuncInfo(fun exec.FuncInfo) (name string, narg int) {
 	return "main", 0
 }
 
-func compileSelectorExprLHS(ctx *blockCtx, v *ast.SelectorExpr, mode compleMode) {
+func compileSelectorExprLHS(ctx *blockCtx, v *ast.SelectorExpr, mode compileMode) {
 	if mode == lhsDefine {
 		log.Panicln("compileSelectorExprLHS: `:=` can't be used for index expression")
 	}
diff --git a/cl/stmt.go b/cl/stmt.go
index 3015994e..4a2c112d 100644
--- a/cl/stmt.go
+++ b/cl/stmt.go
@@ -37,6 +37,12 @@ func compileBlockStmtWithout(ctx *blockCtx, body *ast.BlockStmt) {
 	}
 }
 
+func compileNewBlock(ctx *blockCtx, block *ast.BlockStmt) {
+	ctx.out.DefineBlock()
+	compileBlockStmtWith(ctx, block)
+	ctx.out.EndBlock()
+}
+
 func compileBodyWith(ctx *blockCtx, body []ast.Stmt) {
 	ctxWith := newNormBlockCtx(ctx)
 	for _, stmt := range body {
@@ -62,7 +68,7 @@ func compileStmt(ctx *blockCtx, stmt ast.Stmt) {
 	case *ast.ForStmt:
 		compileForStmt(ctx, v)
 	case *ast.BlockStmt:
-		compileBlockStmtWith(ctx, v)
+		compileNewBlock(ctx, v)
 	case *ast.ReturnStmt:
 		compileReturnStmt(ctx, v)
 	case *ast.IncDecStmt:
@@ -147,9 +153,11 @@ func toIdent(e ast.Expr) *ast.Ident {
 }
 
 func compileForStmt(ctx *blockCtx, v *ast.ForStmt) {
-	if v.Init != nil {
-		ctx = newNormBlockCtx(ctx)
-		compileStmt(ctx, v.Init)
+	if init := v.Init; init != nil {
+		v.Init = nil
+		block := &ast.BlockStmt{List: []ast.Stmt{init, v}}
+		compileNewBlock(ctx, block)
+		return
 	}
 	out := ctx.out
 	start := ctx.NewLabel("")
@@ -477,7 +485,11 @@ func compileIfStmt(ctx *blockCtx, v *ast.IfStmt) {
 	}
 	out.Label(label)
 	if hasElse {
-		compileStmt(ctxIf, v.Else)
+		if ve, ok := v.Else.(*ast.BlockStmt); ok {
+			compileBlockStmtWithout(ctx, ve)
+		} else {
+			compileStmt(ctxIf, v.Else)
+		}
 		out.Label(done)
 	}
 }
diff --git a/exec.spec/interface.go b/exec.spec/interface.go
index 83293713..f26485ef 100644
--- a/exec.spec/interface.go
+++ b/exec.spec/interface.go
@@ -363,6 +363,12 @@ type Builder interface {
 
 	// Defer instr
 	Defer(start, end Label) Instr
+
+	// DefineBlock instr
+	DefineBlock() Builder
+
+	// EndBlock instr
+	EndBlock() Builder
 }
 
 // Package represents a Go+ package.
diff --git a/exec/bytecode/interface.go b/exec/bytecode/interface.go
index ba7f5542..66252c4b 100644
--- a/exec/bytecode/interface.go
+++ b/exec/bytecode/interface.go
@@ -437,4 +437,14 @@ func (p *iBuilder) Resolve() exec.Code {
 	return ((*Builder)(p)).Resolve()
 }
 
+// DefineBlock
+func (p *iBuilder) DefineBlock() exec.Builder {
+	return p
+}
+
+// EndBlock
+func (p *iBuilder) EndBlock() exec.Builder {
+	return p
+}
+
 // -----------------------------------------------------------------------------
diff --git a/exec/golang/interface.go b/exec/golang/interface.go
index 38891c7d..7353f261 100644
--- a/exec/golang/interface.go
+++ b/exec/golang/interface.go
@@ -453,4 +453,16 @@ func (p *iBuilder) Resolve() exec.Code {
 	return ((*Builder)(p)).Resolve()
 }
 
+// DefineBlock
+func (p *iBuilder) DefineBlock() exec.Builder {
+	((*Builder)(p)).DefineBlock()
+	return p
+}
+
+// EndBlock
+func (p *iBuilder) EndBlock() exec.Builder {
+	((*Builder)(p)).EndBlock()
+	return p
+}
+
 // -----------------------------------------------------------------------------
diff --git a/exec/golang/stmt.go b/exec/golang/stmt.go
index 539253ae..b774cfde 100644
--- a/exec/golang/stmt.go
+++ b/exec/golang/stmt.go
@@ -397,4 +397,44 @@ func (p *Builder) Defer(start, end *Label) exec.Instr {
 	panic("The method defer under the builder of golang is not yet supported")
 }
 
+// DefineBlock starts a new block
+func (p *Builder) DefineBlock() *Builder {
+	p.scopeCtx = &scopeCtx{parentCtx: p.scopeCtx}
+	p.initStmts()
+	return p
+}
+
+//  EndBlock ends a  block
+func (p *Builder) EndBlock() *Builder {
+	p.endBlockStmt(0)
+	blockStmt := getBlockStmts(p)
+	if p.parentCtx == nil {
+		p.rhs.Push(blockStmt)
+	} else {
+		p.parentCtx.stmts = append(p.parentCtx.stmts, blockStmt)
+		p.scopeCtx = p.parentCtx
+	}
+	return p
+}
+
+// getBlockStmts will check whether the first stmt in block stmts is labeledStmt.
+// if true ,the whole block should be labeled
+func getBlockStmts(p *Builder) ast.Stmt {
+	body := &ast.BlockStmt{List: p.getStmts(p)}
+	if len(body.List) > 0 {
+		for i := 0; i < len(body.List); i++ {
+			if _, ok := body.List[i].(*ast.DeclStmt); ok {
+				continue
+			}
+			if v, ok := body.List[i].(*ast.LabeledStmt); ok {
+				body.List[i] = v.Stmt
+				v.Stmt = body
+				return v
+			}
+			break
+		}
+	}
+	return body
+}
+
 // ----------------------------------------------------------------------------
diff --git a/exec/golang/var.go b/exec/golang/var.go
index b71c486a..7dc0c940 100644
--- a/exec/golang/var.go
+++ b/exec/golang/var.go
@@ -69,8 +69,9 @@ func (p *Var) setScope(where *scopeCtx) {
 // -----------------------------------------------------------------------------
 
 type scopeCtx struct {
-	vlist []exec.Var
-	stmts []ast.Stmt
+	parentCtx *scopeCtx
+	vlist     []exec.Var
+	stmts     []ast.Stmt
 }
 
 func (p *scopeCtx) addVar(vars ...exec.Var) {


Test patch:
diff --git a/cl/stmt_test.go b/cl/stmt_test.go
index afdc9468..34dbadde 100644
--- a/cl/stmt_test.go
+++ b/cl/stmt_test.go
@@ -1305,3 +1305,28 @@ var testDeferClauses = map[string]testData{
 func TestDeferStmt(t *testing.T) {
 	testScripts(t, "TestDeferStmt", testDeferClauses)
 }
+
+// -----------------------------------------------------------------------------
+
+var testVarScopeClauses = map[string]testData{
+	"variable_redefinition_#issue304": {`
+						a := []float64{1, 2, 3.4}
+						println(a)
+						
+						a := []float64{1, 2, 3.4}
+						println(a)
+					`, "", true},
+	"variable_scope_bug_#issue303": {`
+						i:=0
+						{
+						    i:=0
+						    i++
+						    println("inner is",i)
+						}
+						println("outer is",i)
+					`, "inner is 1\nouter is 0\n", false},
+}
+
+func TestVarScopeStmt(t *testing.T) {
+	testScripts(t, "TestVarScopeStmt", testVarScopeClauses)
+}
diff --git a/exec/golang/testdata/01.const/bar.gop b/exec/golang/testdata/01.const/bar.gop
deleted file mode 100644
index c46de522..00000000
--- a/exec/golang/testdata/01.const/bar.gop
+++ /dev/null
@@ -1,1 +0,0 @@
-println("Hello "+"qiniu:", 123, 4.5, 7i)
diff --git a/exec/golang/testdata/01.var-const/var_const.gop b/exec/golang/testdata/01.var-const/var_const.gop
new file mode 100644
index 00000000..d9be9099
--- /dev/null
+++ b/exec/golang/testdata/01.var-const/var_const.gop
@@ -0,0 +1,26 @@
+println("Hello "+"qiniu:", 123, 4.5, 7i)
+
+i := 0
+{
+	i := 0
+	i++
+	println("inner is", i)
+}
+println("outer is", i)
+println("start")
+{
+	i := 0
+	{
+		j := 2
+		{
+			c := 3
+			println(i, j, c)
+		}
+		j++
+		i++
+		println(i, j)
+	}
+	i++
+	println(i)
+}
+println("end")