678074 - Implement js::PropertyName to represent property names which are never store characters for an index

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect)

Description

[Jeff Walden [:Waldo]]

Attachment: Patch

PropertyName will eventually replace jsid internally, so that no one internally need ever consider (except when fed an unknown value for a property) whether to look in element storage or property storage for an object property.  Externally, jsid will stick around for simplicity, probably for some time.

Comment 1

[Luke Wagner [:luke]]

Comment on attachment 552239 [details] [diff] [review]
Patch

Review of attachment 552239 [details] [diff] [review]:
-----------------------------------------------------------------

::: js/src/jsapi-tests/testPropertyName.cpp
@@ +1,1 @@
> +/* -*- Mode: C++; tab-width: 8; indent-tabs-mode: nil; c-basic-offset: 4 -*-

Have you considered merging this into that other jsapi-test .cpp you added recently?  Its nice to sorta minimize the number or jsapi-test .cpps (not tests!) since I have to build them all the time when I make a shell.

::: js/src/vm/String.cpp
@@ +356,5 @@
> +
> +    if (cp != end)
> +        return false;
> +
> +    if (oldIndex < UINT32_MAX / 10 || (oldIndex == UINT32_MAX / 10 && c <= (UINT32_MAX % 10))) {

Comment to the effect of "strlen("9999999999") == UINT32_CHAR_BUFFER_LENGTH, but greater than UINT32_MAX."

::: js/src/vm/String.h
@@ +814,5 @@
>      return &asFixed();
>  }
>  
> +inline js::PropertyName *
> +JSAtom::asPropertyName()

Can you put this in String-inl.h?  The few inlines in String.h are ones that are used super-commonly where its nice to avoid requiring String-inl.h.  Or do you forsee this being used everywhere?

Comment 2

[Jeff Walden [:Waldo]]

http://hg.mozilla.org/integration/mozilla-inbound/rev/137325e8319c

(In reply to Luke Wagner [:luke] from comment #1)
> Have you considered merging this into that other jsapi-test .cpp you added
> recently?  Its nice to sorta minimize the number or jsapi-test .cpps (not
> tests!) since I have to build them all the time when I make a shell.

Done.  I generally haven't thought about keeping the number minimal -- I think that's a good idea for shell tests, to minimize interdependencies -- but I guess extra compile time is more notable than extra shell startup/shutdown time.

> > +inline js::PropertyName *
> > +JSAtom::asPropertyName()
> 
> Can you put this in String-inl.h?  The few inlines in String.h are ones that
> are used super-commonly where its nice to avoid requiring String-inl.h.  Or
> do you forsee this being used everywhere?

No idea.  I can put it in the inlines file, I've just always thought of them as the "put it here because it has cyclic dependencies" workaround, and not as a good place to put things as a general matter of course.

Comment 3

[Kyle Huey (Exited; not receiving bugmail, old account, do not use)]

http://hg.mozilla.org/mozilla-central/rev/137325e8319c