[es6] Handle conflicts for sloppy let

test/mjsunit/harmony/block-conflicts-sloppy.js

 // Copyright 2011 the V8 project authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 // Test for conflicting variable bindings.
 
-"use strict";
+// Flags: --no-legacy-const --harmony-sloppy
 
 function CheckException(e) {
   var string = e.toString();
   assertTrue(string.indexOf("has already been declared") >= 0 ||
              string.indexOf("redeclaration") >= 0);
   return 'Conflict';
 }
 
 
 function TestGlobal(s,e) {
@@ skipping 19 matching lines @@
     return eval("(function(){ {" + s + "} return " + e + "})")();
   } catch (x) {
     return CheckException(x);
   }
 }
 
 function TestAll(expected,s,opt_e) {
   var e = "";
   var msg = s;
   if (opt_e) { e = opt_e; msg += opt_e; }
-  assertEquals(expected === 'LocalConflict' ? 'NoConflict' : expected,
-      TestGlobal(s,e), "global:'" + msg + "'");
+  // TODO(littledan): https://code.google.com/p/v8/issues/detail?id=4288
+  // It is also not clear whether these tests makes sense in sloppy mode.
+  // assertEquals(expected === 'LocalConflict' ? 'NoConflict' : expected,
+  //     TestGlobal(s,e), "global:'" + msg + "'");
   assertEquals(expected === 'LocalConflict' ? 'NoConflict' : expected,
       TestFunction(s,e), "function:'" + msg + "'");
   assertEquals(expected === 'LocalConflict' ? 'Conflict' : expected,
       TestBlock(s,e), "block:'" + msg + "'");
 }
 
 
 function TestConflict(s) {
   TestAll('Conflict', s);
-  TestAll('Conflict', 'eval("' + s + '");');
+  // TODO(littledan): https://code.google.com/p/v8/issues/detail?id=4288
+  // It is also not clear whether these tests makes sense in sloppy mode.
+  // TestAll('Conflict', 'eval("' + s + '");');

[Dan Ehrenberg, 2015/07/08 23:42:13]

I don't see why these tests wouldn't make sense. Wouldn't there be a conflict in
the new eval-driven block scope, even in sloppy mode?

[arv (Not doing code reviews), 2015/07/09 14:30:53]

See the bug.

  eval('let x; var x;')

is more similar to

  eval('{ let x; var x; }')

we are not creating the lexical block correctly for sloppy evals so these tests
are not passing or the tests are not correct for sloppy mode.

[rossberg, 2015/07/10 12:13:17]

I suppose

let x; eval('var x');

is an error in sloppy mode, but not in strict mode. That's a fairly unfortunate
semantic deviation, actually. But why does this difference only pop up in the
global scope tests?

These tests should probably be changed to use Realm.eval in TestGlobal, so that
we test the actual global scope, and there should be a separate TestEval
(similarly for strict mode). Can you add a TODO for that?

[arv (Not doing code reviews), 2015/07/10 13:00:48]

The other tests wrap the string in a function or a block so the implicit lexical
scope is not relevant.
I'll add a todo (and/or add a comment to the bug)

 }
 
 function TestNoConflict(s) {
   TestAll('NoConflict', s, "'NoConflict'");
-  TestAll('NoConflict', 'eval("' + s + '");', "'NoConflict'");
+  // TODO(littledan): https://code.google.com/p/v8/issues/detail?id=4288
+  // TestAll('NoConflict', 'eval("' + s + '");', "'NoConflict'");
 }
 
 function TestLocalConflict(s) {
   TestAll('LocalConflict', s, "'NoConflict'");
-  TestAll('NoConflict', 'eval("' + s + '");', "'NoConflict'");
+  // TODO(littledan): https://code.google.com/p/v8/issues/detail?id=4288
+  // It is also not clear whether these tests makes sense in sloppy mode.
+  // TestAll('NoConflict', 'eval("' + s + '");', "'NoConflict'");
 }
 
 var letbinds = [ "let x;",
                  "let x = 0;",
                  "let x = undefined;",
                  "let x = function() {};",
                  "let x, y;",
                  "let y, x;",
                  "const x = 0;",
                  "const x = undefined;",
@@ skipping 77 matching lines @@
 // Test conflicting parameter/var bindings.
 for (var v = 0; v < varbinds.length; ++v) {
   TestNoConflict('(function (x) {' + varbinds[v] + '})();');
 }
 
 // Test conflicting catch/function bindings.
 TestNoConflict('try {} catch(x) {' + funbind + '}');
 
 // Test conflicting parameter/function bindings.
 TestNoConflict('(function (x) {' + funbind + '})();');