-
Notifications
You must be signed in to change notification settings - Fork 1.2k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Share types (i.e., allow PathTypeHandlers) for properties with non-standard attributes (non-writable, etc.). #4118
Conversation
85de59f
to
5752595
Compare
lib/Runtime/Types/PathTypeHandler.h
Outdated
@@ -6,16 +6,62 @@ | |||
|
|||
namespace Js | |||
{ | |||
typedef uint8 SetterSlotIndex; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe explicit naming this PathTypeHandlerSettingSlotIndex since this is only for PathTypeHandlers?
@@ -30,6 +76,9 @@ namespace Js | |||
virtual BOOL IsLockable() const override { return true; } | |||
virtual BOOL IsSharable() const override { return true; } | |||
|
|||
static PropertyAttributes ObjectSlotAttributesToPropertyAttributes(const ObjectSlotAttributes attr) { return attr & ObjectSlotAttr_PropertyAttributesMask; } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ObjectSlotAttributesToPropertyAttributes [](start = 34, length = 40)
Could we add static asserts to ensure property attributes have the same corresponding bits in PropertyAttributes and ObjectSlotAttributes?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, there are CompileAssert's in the cpp file.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lib/Runtime/Types/PathTypeHandler.h
Outdated
DynamicType * GetPredecessorType() const { return predecessorType; } | ||
PathTypeHandlerBase* GetRootPathTypeHandler(); | ||
|
||
virtual ObjectSlotAttributes * GetAttributeArray() const { return nullptr; } | ||
virtual ObjectSlotAttributes GetAttributeArray(const PropertyIndex index) const { return ObjectSlotAttr_Default; } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
GetAttributeArray [](start = 37, length = 17)
This is returning the attributes for a particular property index, right? If so, can we rename it to GetAttributes?
Proposing similar change for GetSetterSlots below.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh, right. That's what I had originally; didn't mean to change it. I'll change it back.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
45a8241
to
d28fde3
Compare
|
||
if (instance->HasObjectArray()) | ||
{ | ||
requestContext->GetOrAddPropertyRecord(propertyName, propertyNameLength, &propertyRecord); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
use propertyNameString
directly? #3875 adds PropertyString
and LiteralStringWithPropertyStringPtr
check.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks. This is actually a copy of some very old logic. Should we change it everywhere?
LGTM |
…andard attributes (non-writable, etc.). This change draws on Kount Veluri's work on behalf of native fields. It adds a wrinkle to the type path mechanism, replacing the PropertyId successor map key with a key that munges PropertyId and attributes. The way is paved for sharing types with getter/setters, deleted properties, and native values. When a property's attributes are changed/set, the type path is branched starting with the type that precedes the one that introduced the property.
…properties with non-standard attributes (non-writable, etc.). Merge pull request #4118 from pleath:sharetypes-m This change draws on Kount Veluri's work on behalf of native fields. It adds a wrinkle to the type path mechanism, replacing the PropertyId successor map key with a key that munges PropertyId and attributes. The way is paved for sharing types with getter/setters, deleted properties, and native values. When a property's attributes are changed/set, the type path is branched starting with the type that precedes the one that introduced the property.
…te non-config props in strict mode)
… slots Rename ThrowCantDelete and fix regression from chakra-core#4118 (delete non-config props in strict mode)
This change draws on Kount Veluri's work on behalf of native fields. It adds a wrinkle to the type path mechanism, replacing the PropertyId successor map key with a key that munges PropertyId and attributes. The way is paved for sharing types with getter/setters, deleted properties, and native values. When a property's attributes are changed/set, the type path is branched starting with the type that precedes the one that introduced the property.