Assign unique IDs to FunctionLiterals

src/parsing/preparser.h

Index: src/parsing/preparser.h
diff --git a/src/parsing/preparser.h b/src/parsing/preparser.h
index 0bac1277b048ff62fcf17febeecd0f7d7e77d32a..331c91f203784a2ccdea5c6d3d0220dcbe6ad1f3 100644
--- a/src/parsing/preparser.h
+++ b/src/parsing/preparser.h
@@ -662,7 +662,7 @@ class PreParserFactory {
       FunctionLiteral::ParameterFlag has_duplicate_parameters,
       FunctionLiteral::FunctionType function_type,
       FunctionLiteral::EagerCompileHint eager_compile_hint, int position,
-      bool has_braces) {
+      bool has_braces, int function_literal_id) {
     return PreParserExpression::Default();
   }
 
@@ -1080,10 +1080,33 @@ class PreParser : public ParserBase<PreParser> {
                                       int class_token_pos, bool* ok) {}
   V8_INLINE void DeclareClassProperty(PreParserIdentifier class_name,
                                       PreParserExpression property,
-                                      ClassInfo* class_info, bool* ok) {}
+                                      ClassLiteralProperty::Kind kind,
+                                      bool is_static, ClassInfo* class_info,
+                                      bool* ok) {
+    if (class_info->has_seen_constructor &&

[marja, 2016/11/25 13:14:56]

This function could still use a comment... as far as I understand it now, it
sets up class_info in such a way that RewriteClassLiteral can do its thing. And
the thing cannot be done here since it's too early.

RewriteClassLiteral checks instance_field_initializers, so here we add stuff to
it...

But I still don't get why we need to set class_info->constructor. What part
checks it? RewriteClassLiteral doesn't. ELI5?

[jochen (gone - plz use gerrit), 2016/11/28 08:12:07]

I guess the contract of DeclareClassProperty is just too confusing.. from the
invocation on where the encountered property is the constructor,
has_seen_constructor is true. To be able to tell the single constructor apart
from other class literals that might get added to instance_field_initializers,
we need a second bit of information that tells us that we observed the switch of
has_seen_constructor from false to true.

Anyway, I've now added an explicit "is_constructor" parameter to clarify this.

+        !class_info->constructor.IsEmpty()) {
+      // We use Empty to differentiate an existing constructor from Default
+      // which encodes the absence of a constructor.
+      class_info->constructor = PreParserExpression::Empty();
+      return;
+    }
+    if (kind == ClassLiteralProperty::FIELD && !is_static) {
+      class_info->instance_field_initializers->Add(
+          PreParserExpression::Default(), zone());
+    }
+  }
   V8_INLINE PreParserExpression RewriteClassLiteral(PreParserIdentifier name,
                                                     ClassInfo* class_info,
                                                     int pos, bool* ok) {
+    bool has_default_constructor = !class_info->has_seen_constructor;
+    bool has_instance_fields =
+        class_info->instance_field_initializers->length() > 0;
+    // Account for the default constructor.
+    if (has_default_constructor) GetNextFunctionLiteralId();

[Toon Verwaest, 2016/11/28 11:12:33]

Do we really need an ID for the default constructor? THe default constructor
doesn't *really* live in the script. You can't break in it or anything ...

[jochen (gone - plz use gerrit), 2016/11/28 11:26:50]

they get lazily compiled, so they end up in the SFI cache etc.. the alternative
to giving them function IDs is adding special cases for them all over the place
(like DoParseFunction already has)

+    if (allow_harmony_class_fields() && has_instance_fields) {
+      // Account for initializer function.

[Toon Verwaest, 2016/11/28 11:12:33]

Can we e.g., put breakpoints in the initializer by putting it on the field
initializer expression?

+      GetNextFunctionLiteralId();
+    }
     return PreParserExpression::Default();
   }