Make sure variables dynamically bound in parameter scope conflict with parameter bindings. See #305.

compiler_expr.go

@@ -891,7 +891,6 @@ func (e *compiledFunctionLiteral) emitGetter(putOnStack bool) {
 				firstDupIdx = offset
 			}
 			b.isArg = true
-			b.isVar = true
 		case ast.Pattern:
 			b := e.c.scope.addBinding(int(item.Idx0()) - 1)
 			b.isArg = true
@@ -943,6 +942,10 @@ func (e *compiledFunctionLiteral) emitGetter(putOnStack bool) {
 	preambleLen := 4 // enter, boxThis, createArgs, set
 	e.c.p.code = make([]instruction, preambleLen, 8)
 
+	emitArgsRestMark := -1
+	firstForwardRef := -1
+	enterFunc2Mark := -1
+
 	if hasPatterns || hasInits {
 		if e.isExpr && e.expr.Name != nil {
 			if b, created := s.bindNameLexical(e.expr.Name.Name, false, 0); created {
@@ -954,13 +957,6 @@ func (e *compiledFunctionLiteral) emitGetter(putOnStack bool) {
 			e.c.emit(loadCallee)
 			calleeBinding.emitInit()
 		}
-	}
-
-	emitArgsRestMark := -1
-	firstForwardRef := -1
-	enterFunc2Mark := -1
-
-	if hasPatterns || hasInits {
 		for i, item := range e.expr.ParameterList.List {
 			if pattern, ok := item.Target.(ast.Pattern); ok {
 				i := i
@@ -1035,6 +1031,11 @@ func (e *compiledFunctionLiteral) emitGetter(putOnStack bool) {
 			}
 		}
 	} else {
+		// To avoid triggering variable conflict when binding from non-strict direct eval().
+		// Parameters are supposed to be in a parent scope, hence no conflict.
+		for _, b := range s.bindings[:paramsCount] {
+			b.isVar = true
+		}
 		e.c.compileDeclList(e.expr.DeclarationList, true)
 		e.c.createFunctionBindings(funcs)
 		e.c.compileLexicalDeclarations(body, true)
@@ -1074,22 +1075,26 @@ func (e *compiledFunctionLiteral) emitGetter(putOnStack bool) {
 	}
 
 	if s.argsNeeded {
-		pos := preambleLen - 2
-		delta += 2
-		if s.strict || hasPatterns || hasInits {
-			code[pos] = createArgsUnmapped(paramsCount)
+		b, created := s.bindNameLexical("arguments", false, 0)
+		if !created && !b.isVar {
+			s.argsNeeded = false
 		} else {
-			code[pos] = createArgsMapped(paramsCount)
-		}
-		pos++
-		b, _ := s.bindNameLexical("arguments", false, 0)
-		if s.strict {
-			b.isConst = true
-		} else {
-			b.isVar = true
+			if s.strict {
+				b.isConst = true
+			} else {
+				b.isVar = true
+			}
+			pos := preambleLen - 2
+			delta += 2
+			if s.strict || hasPatterns || hasInits {
+				code[pos] = createArgsUnmapped(paramsCount)
+			} else {
+				code[pos] = createArgsMapped(paramsCount)
+			}
+			pos++
+			b.markAccessPointAtScope(s, pos)
+			code[pos] = storeStashP(0)
 		}
-		b.markAccessPointAtScope(s, pos)
-		code[pos] = storeStashP(0)
 	}
 
 	stashSize, stackSize := s.finaliseVarAlloc(0)

compiler_stmt.go

@@ -726,7 +726,7 @@ func (c *compiler) compileReturnStatement(v *ast.ReturnStatement) {
 
 func (c *compiler) checkVarConflict(name unistring.String, offset int) {
 	for sc := c.scope; sc != nil; sc = sc.outer {
-		if b, exists := sc.boundNames[name]; exists && !b.isVar {
+		if b, exists := sc.boundNames[name]; exists && !b.isVar && !(b.isArg && sc != c.scope) {
 			c.throwSyntaxError(offset, "Identifier '%s' has already been declared", name)
 		}
 		if sc.function {

compiler_test.go

@@ -1679,6 +1679,45 @@ func TestArgumentsInEval(t *testing.T) {
 	testScript1(SCRIPT, intToValue(1), t)
 }
 
+func TestArgumentsRedeclareInEval(t *testing.T) {
+	const SCRIPT = `
+	assert.sameValue("arguments" in this, false, "No global 'arguments' binding");
+
+	function f(p = eval("var arguments = 'param'"), arguments) {}
+	assert.throws(SyntaxError, f);
+
+	assert.sameValue("arguments" in this, false, "No global 'arguments' binding");
+	`
+
+	testScript1(TESTLIB+SCRIPT, _undefined, t)
+}
+
+func TestEvalParamWithDef(t *testing.T) {
+	const SCRIPT = `
+	function f(param = 0) {
+		eval("var param = 1");
+		return param;
+	}
+	f();
+	`
+
+	testScript1(SCRIPT, valueInt(1), t)
+}
+
+func TestArgumentsRedefinedAsLetDyn(t *testing.T) {
+	const SCRIPT = `
+	function f() {
+		let arguments;
+		eval(""); // force dynamic scope
+		return arguments;
+	}
+	
+	f(1,2);
+	`
+
+	testScript1(SCRIPT, _undefined, t)
+}
+
 func TestWith(t *testing.T) {
 	const SCRIPT = `
 	var b = 1;

tc39_test.go

@@ -408,6 +408,7 @@ var (
 		"sec-function-definitions-static-semantics-early-errors",
 		"sec-functiondeclarationinstantiation",
 		"sec-functiondeclarations-in-ifstatement-statement-clauses",
+		"sec-evaldeclarationinstantiation",
 	}
 )