821789 - Move Object builtins to their own file

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect)

Description

[Tom S [:evilpie]]

Attachment: v1

jsobj.cpp has so much crap, let's at least move the functions to their own file.

Comment 1

[Tom S [:evilpie]]

Attachment: v1 with hg add :)

Comment 2

[Jeff Walden [:Waldo]]

Comment on attachment 692349 [details] [diff] [review]
v1 with hg add :)

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

Oops, I forgot to review this after getting a smidgen of help from you earlier.  Sorry for forgetting!

::: js/src/Makefile.in
@@ +153,5 @@
>  		Verifier.cpp \
>  		StringBuffer.cpp \
>  		Unicode.cpp \
>  		Xdr.cpp \
> +		Object.cpp \

Alphabetize this in with all the other vm/ .cpp files.

::: js/src/builtin/Object.cpp
@@ +3,5 @@
> + *
> + * This Source Code Form is subject to the terms of the Mozilla Public
> + * License, v. 2.0. If a copy of the MPL was not distributed with this
> + * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
> +

Add a #include "mozilla/Util.h" up here, since you're using ArrayLength later.

@@ +40,5 @@
> +}
> +
> +
> +JSBool
> +js::js_Object(JSContext *cx, unsigned argc, Value *vp)

js::js_?  DO NOT WANT.  :-)  Make this js::ObjectFun or something like that, just please get rid of the double-namespacing!  :-)

@@ +44,5 @@
> +js::js_Object(JSContext *cx, unsigned argc, Value *vp)
> +{
> +    CallArgs args = CallArgsFromVp(argc, vp);
> +
> +    RootedObject obj(cx);

Make this (cx, NULL) to be explicit.  Then you can make the if-else below an |if (args.length() > 0)| for a little more concision.

@@ +95,5 @@
> +
> +    /*
> +     * ECMA spec botch: return false unless hasOwnProperty. Leaving "own" out
> +     * of propertyIsEnumerable's name was a mistake.
> +     */

Remove this comment, too -- spec says it, everything else is immaterial now.

@@ +417,5 @@
> +
> +    JSBool dummy;
> +    if (!js_DefineOwnProperty(cx, thisObj, id, ObjectValue(*descObj), &dummy)) {
> +        return false;
> +    }

Remove the braces.

@@ +571,5 @@
> +obj_watch(JSContext *cx, unsigned argc, Value *vp)
> +{
> +    CallArgs args = CallArgsFromVp(argc, vp);
> +
> +    if (argc <= 1) {

args.length()

@@ +624,5 @@
> +
> +/*
> + * Prototype and property query methods, to complement the 'in' and
> + * 'instanceof' operators.
> + */

This comment might have made sense in the past, but now these methods are long-standing spec methods; any rationale along these lines is negligible.  Remove this?

::: js/src/builtin/Object.h
@@ +10,5 @@
> +
> +#include "jsobj.h"
> +
> +extern JSFunctionSpec object_methods[];
> +extern JSFunctionSpec object_static_methods[];

Move these into namespace js, please.

::: js/src/jscntxtinlines.h
@@ +17,5 @@
>  #include "jsgc.h"
>  
>  #include "frontend/ParseMaps.h"
>  #include "vm/RegExpObject.h"
> +#include "builtin/Object.h" // For js_Object

This should go before the frontend/ParseMaps.h include.

::: js/src/vm/GlobalObject.cpp
@@ +18,5 @@
>  #include "builtin/Eval.h"
>  #include "builtin/Intl.h"
>  #include "builtin/MapObject.h"
>  #include "builtin/RegExp.h"
> +#include "builtin/Object.h"

M, then O, then R, so this should be before RegExp.h.

Comment 3

[Tom S [:evilpie]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/c38f055cd3b1

Comment 4

[Tom S [:evilpie]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/5589176b4f58

Comment 5

[Ed Morley [:emorley]]

https://hg.mozilla.org/mozilla-central/rev/c38f055cd3b1