[webkit-reviews] review granted: [Bug 20981] Store per-class info in StructureID : [Attachment 23639] add per-class TypeInfo to the StructureID (right now only used for JSType)

[bugzilla-daemon at webkit.org]

Darin Adler <darin at apple.com> has granted Maciej Stachowiak <mjs at apple.com>'s
request for review:
Bug 20981: Store per-class info in StructureID
https://bugs.webkit.org/show_bug.cgi?id=20981

Attachment 23639: add per-class TypeInfo to the StructureID (right now only
used for JSType)
https://bugs.webkit.org/attachment.cgi?id=23639&action=edit

------- Additional Comments from Darin Adler <darin at apple.com>
I see this:

    OBJECT_OFFSET(StructureID, m_typeInfo) + OBJECT_OFFSET(TypeInfo, m_type)

Sometimes I've written that as:

    OBJECT_OFFSET(StructureID, m_typeInfo.m_type)

Not sure the pros and cons.

+#include "JSCallbackConstructor.h"
+#include "JSCallbackFunction.h"
+#include "JSCallbackObject.h"

What's this change to AllInOneFile.cpp about? Doesn't seem right to me. Maybe
you pasted here by accident meaning to paste into JSGlobalObject.cpp at one
point?

-	 return m_structureID->type() == ObjectType;
+	 return m_structureID->typeInfo().type() == ObjectType;

You could have sugared this by retaining a StructureID;:type() function. Why
did you chose not to?

+#include "JSCallbackConstructor.h"
+#include "JSCallbackFunction.h"
+#include "JSCallbackObject.h"
 #include "ArrayConstructor.h"

Why aren't these sorted alphabetically in JSGlobalObject.cpp?

+++ JavaScriptCore/kjs/RegExpMatchesArray.h	(revision 0)
@@ -0,0 +1,47 @@
+/*
+ *  Copyright (C) 1999-2000 Harri Porten (porten at kde.org)
+ *  Copyright (C) 2003, 2007, 2008 Apple Inc. All Rights Reserved.

This class is new this year and doesn't need the pre-2008 copyrights on its
source file.

+	 TypeInfo(JSType type) : m_type(type) {}

We normally put spaces in those braces.

+    JSC::StructureID* createDOMStructure(JSC::ExecState*,
CreateStructureIDFunction, const JSC::ClassInfo*, JSC::JSObject* prototype);

It's not helpful to pass the function for creating the structure instead of
calling it. There's really no reason to do it that way. This should take a
PassRefPtr<StructureID> instead, not take a prototype at all, and be renamed to
cacheDOMStructure.

r=me