Closed Bug 770092 Opened 9 years ago Closed 9 years ago

JSScript::fullyInitFromEmitter() should only init JSScript

Categories

(Core :: JavaScript Engine, defect)

defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla16

People

(Reporter: n.nethercote, Assigned: n.nethercote)

References

Details

(Whiteboard: [js:t])

Attachments

(4 files)

JSScript::fullyInitFromEmitter() sets fields in JSScript, unsurprisingly.  But it also sets some fields in the corresponding JSFunction, and calls the new script notifier functions.

The aim of this bug is to move the non-initialization work elsewhere,
because it is non-intuitive to have it in fullyInitFromEmitter(), and it
gets in the way of lazy bytecode generation.
This patch moves the new script notification code out of
fullyInitFromEmitter().
Attachment #638274 - Flags: review?(bhackett1024)
This patch avoids setting JSScript::function_ in
JSScript::typeSetFunction(), which is a prerequisite for the next patch.
Attachment #638275 - Flags: review?(bhackett1024)
This patch moves JSScript::typeSetFunction() into JSFunction, which makes
sense, because it's now all about setting fields in JSFunction.  (I also
wonder if the function should be renamed?)
Attachment #638276 - Flags: review?(bhackett1024)
This patch moves the JSFunction initialization code out of
JSScript::fullyInitFromEmitter.
Attachment #638277 - Flags: review?(bhackett1024)
Attachment #638274 - Flags: review?(bhackett1024) → review+
Attachment #638275 - Flags: review?(bhackett1024) → review+
Comment on attachment 638276 [details] [diff] [review]
(part 3) - Move typeSetFunction() from JSScript to JSFunction.

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

::: js/src/jsfun.h
@@ +178,5 @@
>      inline void setExtendedSlot(size_t which, const js::Value &val);
>      inline const js::Value &getExtendedSlot(size_t which) const;
>  
> +    /* Constructs a new type for the function if necessary. */
> +    bool typeSetFunction(JSContext *cx, bool singleton = false);

Maybe setTypeForScriptedFunction?

::: js/src/jsinfer.cpp
@@ +5417,5 @@
>  
>  bool
> +JSFunction::typeSetFunction(JSContext *cx, bool singleton)
> +{
> +    JS_ASSERT(script());

This should also assert that script->function() == this
Attachment #638276 - Flags: review?(bhackett1024) → review+
Attachment #638277 - Flags: review?(bhackett1024) → review+
You need to log in before you can comment on or make changes to this bug.