Remove StructureIDBlob
Created attachment 445608 [details] Patch
Created attachment 445609 [details] Patch
Comment on attachment 445609 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=445609&action=review r=me > Source/JavaScriptCore/runtime/Structure.h:861 > + // part of the object. And need to match the order of the equivalent properties in > + // JSCell. > + IndexingType m_indexingModeIncludingHistory; > + JSType m_type; > + TypeInfo::InlineTypeFlags m_inlineTypeFlags; > + CellState m_defaultCellState { CellState::DefinitelyWhite }; I think we should rename them to explicitly state that they are information of JSCell allocated with this Structure. Since Structure itself is a JSCell, it has Structure::m_type etc. in JSCell parent class. So using the same name here is confusing. e.g. m_instanceType, m_instanceIndexingModeIncludingHistory etc.
Comment on attachment 445609 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=445609&action=review > Source/JavaScriptCore/ChangeLog:11 > + can be a single load platforms that allow (and don't penalize) misaligned loads. /load platforms/load on platforms/ >> Source/JavaScriptCore/runtime/Structure.h:861 >> + IndexingType m_indexingModeIncludingHistory; >> + JSType m_type; >> + TypeInfo::InlineTypeFlags m_inlineTypeFlags; >> + CellState m_defaultCellState { CellState::DefinitelyWhite }; > > I think we should rename them to explicitly state that they are information of JSCell allocated with this Structure. > Since Structure itself is a JSCell, it has Structure::m_type etc. in JSCell parent class. So using the same name here is confusing. > e.g. m_instanceType, m_instanceIndexingModeIncludingHistory etc. I suggest adding static_asserts to guarantee that the order of these fields match those in JSCell to ensure that anyone messing this up will not go unnoticed. Alternatively, define a JSCellHeader macro / struct and use that both here and in JSCell to ensure that we never mess up. That would be a much stronger guarantee than the comment above. I think the static_asserts would probably be least intrusive for now.
Comment on attachment 445609 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=445609&action=review >>> Source/JavaScriptCore/runtime/Structure.h:861 >>> + CellState m_defaultCellState { CellState::DefinitelyWhite }; >> >> I think we should rename them to explicitly state that they are information of JSCell allocated with this Structure. >> Since Structure itself is a JSCell, it has Structure::m_type etc. in JSCell parent class. So using the same name here is confusing. >> e.g. m_instanceType, m_instanceIndexingModeIncludingHistory etc. > > I suggest adding static_asserts to guarantee that the order of these fields match those in JSCell to ensure that anyone messing this up will not go unnoticed. Alternatively, define a JSCellHeader macro / struct and use that both here and in JSCell to ensure that we never mess up. That would be a much stronger guarantee than the comment above. I think the static_asserts would probably be least intrusive for now. In response to Yusuke's comment, one alternative id to to put these in an embedded struct here e.g. struct { IndexingType m_indexingModeIncludingHistory; JSType m_type; TypeInfo::InlineTypeFlags m_inlineTypeFlags; CellState m_defaultCellState { CellState::DefinitelyWhite }; } m_cellHeader;
Comment on attachment 445609 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=445609&action=review Nice > Source/JavaScriptCore/runtime/Structure.h:168 > + int32_t objectInitializationBlob() const { return *reinterpret_cast_ptr<const uint32_t*>(&m_indexingModeIncludingHistory); } nit: bitwise_cast
Comment on attachment 445609 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=445609&action=review >> Source/JavaScriptCore/runtime/Structure.h:168 >> + int32_t objectInitializationBlob() const { return *reinterpret_cast_ptr<const uint32_t*>(&m_indexingModeIncludingHistory); } > > nit: bitwise_cast Why bitwise_cast? It's casting a pointer to a different pointer, I usually only use bitwise_cast for integral->integral.
Created attachment 445867 [details] Patch for landing
Committed r286502 (244841@main): <https://commits.webkit.org/244841@main> All reviewed patches have been landed. Closing bug and clearing flags on attachment 445867 [details].
<rdar://problem/86024479>
Re-opened since this is blocked by bug 233930