Modified: trunk/Source/_javascript_Core/ChangeLog (236017 => 236018)
--- trunk/Source/_javascript_Core/ChangeLog 2018-09-14 20:36:45 UTC (rev 236017)
+++ trunk/Source/_javascript_Core/ChangeLog 2018-09-14 21:00:21 UTC (rev 236018)
@@ -1,3 +1,40 @@
+2018-09-14 Mark Lam <mark....@apple.com>
+
+ Refactor some ForInContext code for better encapsulation.
+ https://bugs.webkit.org/show_bug.cgi?id=189626
+ <rdar://problem/44466415>
+
+ Reviewed by Keith Miller.
+
+ 1. Add a ForInContext::m_type field to store the context type. This does not
+ increase the class size, but eliminates the need for a virtual call to get the
+ type.
+
+ Note: we still need a virtual destructor because we'll be mingling
+ IndexedForInContexts and StructureForInContexts in the BytecodeGenerator::m_forInContextStack.
+
+ 2. Add ForInContext::isIndexedForInContext() and ForInContext::isStructureForInContext()
+ convenience methods.
+
+ 3. Add ForInContext::asIndexedForInContext() and ForInContext::asStructureForInContext()
+ to do the casting to the subclass types. This ensures that we'll properly
+ assert that the casting is legal.
+
+ * bytecompiler/BytecodeGenerator.cpp:
+ (JSC::BytecodeGenerator::emitGetByVal):
+ (JSC::BytecodeGenerator::popIndexedForInScope):
+ (JSC::BytecodeGenerator::popStructureForInScope):
+ * bytecompiler/BytecodeGenerator.h:
+ (JSC::ForInContext::type const):
+ (JSC::ForInContext::isIndexedForInContext const):
+ (JSC::ForInContext::isStructureForInContext const):
+ (JSC::ForInContext::asIndexedForInContext):
+ (JSC::ForInContext::asStructureForInContext):
+ (JSC::ForInContext::ForInContext):
+ (JSC::StructureForInContext::StructureForInContext):
+ (JSC::IndexedForInContext::IndexedForInContext):
+ (JSC::ForInContext::~ForInContext): Deleted.
+
2018-09-14 Devin Rousso <web...@devinrousso.com>
Web Inspector: Record actions performed on ImageBitmapRenderingContext
Modified: trunk/Source/_javascript_Core/bytecompiler/BytecodeGenerator.cpp (236017 => 236018)
--- trunk/Source/_javascript_Core/bytecompiler/BytecodeGenerator.cpp 2018-09-14 20:36:45 UTC (rev 236017)
+++ trunk/Source/_javascript_Core/bytecompiler/BytecodeGenerator.cpp 2018-09-14 21:00:21 UTC (rev 236018)
@@ -2912,14 +2912,14 @@
unsigned instIndex = instructions().size();
- if (context.type() == ForInContext::IndexedForInContextType) {
- static_cast<IndexedForInContext&>(context).addGetInst(instIndex, property->index());
- property = static_cast<IndexedForInContext&>(context).index();
+ if (context.isIndexedForInContext()) {
+ auto& indexedContext = context.asIndexedForInContext();
+ indexedContext.addGetInst(instIndex, property->index());
+ property = indexedContext.index();
break;
}
- ASSERT(context.type() == ForInContext::StructureForInContextType);
- StructureForInContext& structureContext = static_cast<StructureForInContext&>(context);
+ StructureForInContext& structureContext = context.asStructureForInContext();
UnlinkedValueProfile profile = ""
instructions().append(kill(dst));
instructions().append(base->index());
@@ -4631,9 +4631,7 @@
{
if (!localRegister)
return;
-
- ASSERT(m_forInContextStack.last()->type() == ForInContext::IndexedForInContextType);
- static_cast<IndexedForInContext&>(m_forInContextStack.last().get()).finalize(*this);
+ m_forInContextStack.last()->asIndexedForInContext().finalize(*this);
m_forInContextStack.removeLast();
}
@@ -4743,8 +4741,7 @@
{
if (!localRegister)
return;
- ASSERT(m_forInContextStack.last()->type() == ForInContext::StructureForInContextType);
- static_cast<StructureForInContext&>(m_forInContextStack.last().get()).finalize(*this);
+ m_forInContextStack.last()->asStructureForInContext().finalize(*this);
m_forInContextStack.removeLast();
}
Modified: trunk/Source/_javascript_Core/bytecompiler/BytecodeGenerator.h (236017 => 236018)
--- trunk/Source/_javascript_Core/bytecompiler/BytecodeGenerator.h 2018-09-14 20:36:45 UTC (rev 236017)
+++ trunk/Source/_javascript_Core/bytecompiler/BytecodeGenerator.h 2018-09-14 21:00:21 UTC (rev 236018)
@@ -56,6 +56,8 @@
class JSImmutableButterfly;
class Identifier;
+ class IndexedForInContext;
+ class StructureForInContext;
enum ExpectedFunction {
NoExpectedFunction,
@@ -180,30 +182,44 @@
WTF_MAKE_FAST_ALLOCATED;
WTF_MAKE_NONCOPYABLE(ForInContext);
public:
- ForInContext(RegisterID* localRegister)
- : m_localRegister(localRegister)
- , m_isValid(true)
+ virtual ~ForInContext() = default;
+
+ bool isValid() const { return m_isValid; }
+ void invalidate() { m_isValid = false; }
+
+ enum class Type : uint8_t {
+ IndexedForIn,
+ StructureForIn
+ };
+
+ Type type() const { return m_type; }
+ bool isIndexedForInContext() const { return m_type == Type::IndexedForIn; }
+ bool isStructureForInContext() const { return m_type == Type::StructureForIn; }
+
+ IndexedForInContext& asIndexedForInContext()
{
+ ASSERT(isIndexedForInContext());
+ return *reinterpret_cast<IndexedForInContext*>(this);
}
- virtual ~ForInContext()
+ StructureForInContext& asStructureForInContext()
{
+ ASSERT(isStructureForInContext());
+ return *reinterpret_cast<StructureForInContext*>(this);
}
- bool isValid() const { return m_isValid; }
- void invalidate() { m_isValid = false; }
+ RegisterID* local() const { return m_localRegister.get(); }
- enum ForInContextType {
- StructureForInContextType,
- IndexedForInContextType
- };
- virtual ForInContextType type() const = 0;
+ protected:
+ ForInContext(RegisterID* localRegister, Type type)
+ : m_localRegister(localRegister)
+ , m_type(type)
+ { }
- RegisterID* local() const { return m_localRegister.get(); }
-
private:
RefPtr<RegisterID> m_localRegister;
- bool m_isValid;
+ bool m_isValid { true };
+ Type m_type;
};
class StructureForInContext : public ForInContext {
@@ -211,7 +227,7 @@
using GetInst = std::tuple<unsigned, int, UnlinkedValueProfile>;
StructureForInContext(RegisterID* localRegister, RegisterID* indexRegister, RegisterID* propertyRegister, RegisterID* enumeratorRegister)
- : ForInContext(localRegister)
+ : ForInContext(localRegister, Type::StructureForIn)
, m_indexRegister(indexRegister)
, m_propertyRegister(propertyRegister)
, m_enumeratorRegister(enumeratorRegister)
@@ -218,11 +234,6 @@
{
}
- ForInContextType type() const override
- {
- return StructureForInContextType;
- }
-
RegisterID* index() const { return m_indexRegister.get(); }
RegisterID* property() const { return m_propertyRegister.get(); }
RegisterID* enumerator() const { return m_enumeratorRegister.get(); }
@@ -244,16 +255,11 @@
class IndexedForInContext : public ForInContext {
public:
IndexedForInContext(RegisterID* localRegister, RegisterID* indexRegister)
- : ForInContext(localRegister)
+ : ForInContext(localRegister, Type::IndexedForIn)
, m_indexRegister(indexRegister)
{
}
- ForInContextType type() const override
- {
- return IndexedForInContextType;
- }
-
RegisterID* index() const { return m_indexRegister.get(); }
void finalize(BytecodeGenerator&);