[crankshaft] Do not optimize ClassConstructor calls and apply.

LOG=N
BUG=v8:4428

Review URL: https://codereview.chromium.org/1425293007

Cr-Commit-Position: refs/heads/master@{#31839}

src/crankshaft/hydrogen.cc

@@ -8357,6 +8357,11 @@ bool HOptimizedGraphBuilder::TryInline(Handle<JSFunction> target,
 
   CompilationInfo target_info(&parse_info);
   Handle<SharedFunctionInfo> target_shared(target->shared());
+
+  if (IsClassConstructor(target_shared->kind())) {
+    TraceInline(target, caller, "target is classConstructor");
+    return false;
+  }
   if (target_shared->HasDebugInfo()) {
     TraceInline(target, caller, "target is being debugged");
     return false;
@@ -9353,6 +9358,7 @@ bool HOptimizedGraphBuilder::TryIndirectCall(Call* expr) {
 }
 
 
+// f.apply(...)
 void HOptimizedGraphBuilder::BuildFunctionApply(Call* expr) {
   ZoneList<Expression*>* args = expr->arguments();
   CHECK_ALIVE(VisitForValue(args->at(0)));
@@ -9751,7 +9757,8 @@ void HOptimizedGraphBuilder::VisitCall(Call* expr) {
     Push(graph()->GetConstantUndefined());
     CHECK_ALIVE(VisitExpressions(expr->arguments()));
 
-    if (expr->IsMonomorphic()) {
+    if (expr->IsMonomorphic() &&
+        !IsClassConstructor(expr->target()->shared()->kind())) {
       Add<HCheckValue>(function, expr->target());
 
       // Patch the global object on the stack by the expected receiver.

src/parser.cc

@@ -359,7 +359,6 @@ FunctionLiteral* Parser::DefaultConstructor(bool call_super, Scope* scope,
                                  kind, &function_factory);
 
     body = new (zone()) ZoneList<Statement*>(call_super ? 2 : 1, zone());
-    AddAssertIsConstruct(body, pos);
     if (call_super) {
       // %_DefaultConstructorCallSuper(new.target, %GetPrototype(<this-fun>))
       ZoneList<Expression*>* args =
@@ -4536,21 +4535,6 @@ void Parser::SkipLazyFunctionBody(int* materialized_literal_count,
 }
 
 
-void Parser::AddAssertIsConstruct(ZoneList<Statement*>* body, int pos) {
-  ZoneList<Expression*>* arguments =
-      new (zone()) ZoneList<Expression*>(0, zone());
-  CallRuntime* construct_check = factory()->NewCallRuntime(
-      Runtime::kInlineIsConstructCall, arguments, pos);
-  CallRuntime* non_callable_error = factory()->NewCallRuntime(
-      Runtime::kThrowConstructorNonCallableError, arguments, pos);
-  IfStatement* if_statement = factory()->NewIfStatement(
-      factory()->NewUnaryOperation(Token::NOT, construct_check, pos),
-      factory()->NewReturnStatement(non_callable_error, pos),
-      factory()->NewEmptyStatement(pos), pos);
-  body->Add(if_statement, zone());
-}
-
-
 Statement* Parser::BuildAssertIsCoercible(Variable* var) {
   // if (var === null || var === undefined)
   //     throw /* type error kNonCoercible) */;
@@ -4744,12 +4728,6 @@ ZoneList<Statement*>* Parser::ParseEagerFunctionBody(
     result->Add(NULL, zone());
   }
 
-  // For concise constructors, check that they are constructed,
-  // not called.
-  if (IsClassConstructor(kind)) {
-    AddAssertIsConstruct(result, pos);
-  }
-
   ZoneList<Statement*>* body = result;
   Scope* inner_scope = scope_;
   Block* inner_block = nullptr;

src/parser.h

@@ -1154,7 +1154,6 @@ class Parser : public ParserBase<ParserTraits> {
   BreakableStatement* LookupBreakTarget(const AstRawString* label, bool* ok);
   IterationStatement* LookupContinueTarget(const AstRawString* label, bool* ok);
 
-  void AddAssertIsConstruct(ZoneList<Statement*>* body, int pos);
   Statement* BuildAssertIsCoercible(Variable* var);
 
   // Factory methods.

test/mjsunit/es6/classes.js

@@ -2,7 +2,7 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
-// Flags: --harmony-sloppy
+// Flags: --harmony-sloppy --allow-natives-syntax
 
 (function TestBasics() {
   var C = class C {}
@@ -625,6 +625,7 @@ function assertAccessorDescriptor(object, name) {
   assertTrue(new C(1) instanceof C);
 })();
 
+
 (function TestConstructorCall(){
   var realmIndex = Realm.create();
   var otherTypeError = Realm.eval(realmIndex, "TypeError");
@@ -651,6 +652,30 @@ function assertAccessorDescriptor(object, name) {
   assertThrows(function() { A.call() }, otherTypeError);
 })();
 
+
+(function TestConstructorCallOptimized() {
+  class A { };
+
+  function invoke_constructor() { A() }
+  function call_constructor() { A.call() }
+  function apply_constructor() { A.apply() }
+
+  for (var i=0; i<3; i++) {
+    assertThrows(invoke_constructor);
+    assertThrows(call_constructor);
+    assertThrows(apply_constructor);
+  }
+  // Make sure we still check for class constructors when calling optimized
+  // code.
+  %OptimizeFunctionOnNextCall(invoke_constructor);
+  assertThrows(invoke_constructor);
+  %OptimizeFunctionOnNextCall(call_constructor);
+  assertThrows(call_constructor);
+  %OptimizeFunctionOnNextCall(apply_constructor);
+  assertThrows(apply_constructor);
+})();
+
+
 (function TestDefaultConstructor() {
   var calls = 0;
   class Base {

test/mjsunit/es6/debug-break-default-constructor.js

@@ -24,19 +24,19 @@ function listener(event, execState, eventData, data) {
 }
 
 class Base {
-  constructor() {  // 2.
-    var x = 1;     // 3.
-  }                // 4.
+  constructor() {
+    var x = 1;     // 2.
+  }                // 3.
 }
 
-class Derived extends Base {}  // 1. // 5.
+class Derived extends Base {}  // 1. // 4.
 
 Debug.setListener(listener);
 var bp = Debug.setBreakPoint(Derived, 0);
 
 new Derived();
 
-Debug.setListener(null);  // 6.
+Debug.setListener(null);  // 5.
 
 assertNull(exception);
-assertEquals(6, step_count);
+assertEquals(5, step_count);

test/mjsunit/es6/debug-step-into-constructor.js

@@ -14,7 +14,6 @@ function listener(event, execState, eventData, data) {
     if (!done) {
       execState.prepareStep(Debug.StepAction.StepInto);
       var s = execState.frame().sourceLineText();
-      print(s);
       assertTrue(s.indexOf('// ' + stepCount + '.') !== -1);
       stepCount++;
     }
@@ -41,7 +40,7 @@ class Derived extends Base {}
   var bp = Debug.setBreakPoint(Base, 0);
 
   new Base();
-  assertEquals(5, stepCount);
+  assertEquals(1, stepCount);
 
   Debug.clearBreakPoint(bp);
 })();
@@ -53,7 +52,7 @@ class Derived extends Base {}
 
   var bp = Debug.setBreakPoint(Base, 0);
   new Derived();
-  assertEquals(5, stepCount);
+  assertEquals(1, stepCount);
 
   Debug.clearBreakPoint(bp);
 })();
@@ -69,7 +68,7 @@ class Derived extends Base {}
 
   var bp = Debug.setBreakPoint(f, 0);
   f();
-  assertEquals(5, stepCount);
+  assertEquals(1, stepCount);
 
   Debug.clearBreakPoint(bp);
 })();
@@ -87,7 +86,7 @@ class Derived extends Base {}
 
   var bp = Debug.setBreakPoint(f, 0);
   f();
-  assertEquals(5, stepCount);
+  assertEquals(1, stepCount);
 
   Debug.clearBreakPoint(bp);
 })();
@@ -105,7 +104,7 @@ class Derived extends Base {}
 
   var bp = Debug.setBreakPoint(f, 0);
   f();
-  assertEquals(5, stepCount);
+  assertEquals(1, stepCount);
 
   Debug.clearBreakPoint(bp);
 })();

test/mjsunit/mjsunit.status

@@ -76,8 +76,6 @@
   'whitespaces': [PASS, NO_VARIANTS],
   'compiler/osr-assert': [PASS, NO_VARIANTS],
   'es6/string-fromcodepoint': [PASS, NO_VARIANTS],
-  # TODO(cbruni): enable once we fix classConstructor calls in optimized code
-  'es6/classes': [PASS, NO_VARIANTS],
   'regress/regress-2185-2': [PASS, NO_VARIANTS],
   'regress/regress-2612': [PASS, NO_VARIANTS],