Closed Bug 1142311 Opened 7 years ago Closed 7 years ago

Stop parenting self-hosted functions to the intrinsics holder

Categories

(Core :: JavaScript Engine, defect)

x86
macOS
defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla39
Tracking Status
firefox39 --- fixed

People

(Reporter: bzbarsky, Assigned: bzbarsky)

References

Details

Attachments

(2 files)

I don't think we need this for anything.
With these changes, I have a green try push with this assert in js::NewFunctionWithProto:

  MOZ_ASSERT(!realParent || realParent == cx->global());

Yay!
Attachment #8576387 - Flags: review?(jwalden+bmo) → review+
Comment on attachment 8576388 [details] [diff] [review]
part 2.  Rename the parent arg of NewScriptedFunction to dynamicScope, and make it optional

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

You renamed the arg to |enclosingScope|, so retitle the patch before landing.

::: js/src/jsfun.h
@@ +525,3 @@
>  
> +// If proto is nullptr, Function.prototype is used instead.  If enclosingScope
> +// is null, the function will use that null as the enclosing scope.

"use that null" by which you mean "use the global" or something?
Attachment #8576388 - Flags: review?(jwalden+bmo) → review+
> You renamed the arg to |enclosingScope|, so retitle the patch before landing.

Will do.

> "use that null" by which you mean "use the global" or something?

No, I mean it will use null.  It will be a scripted function for which environment() returns null.  I'll make that clearer in the comment; this is actually very important (as in, we have code elsewhere that barfs if this is not done).
> You renamed the arg to |enclosingScope|, so retitle the patch before landing.

Changed to enclosingDynamicScope throughout, for greater clarity.

https://hg.mozilla.org/integration/mozilla-inbound/rev/16dff51f1bb6
https://hg.mozilla.org/integration/mozilla-inbound/rev/6da864042bbf
https://hg.mozilla.org/mozilla-central/rev/16dff51f1bb6
https://hg.mozilla.org/mozilla-central/rev/6da864042bbf
Status: NEW → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla39
You need to log in before you can comment on or make changes to this bug.