Open Bug 790676 Opened 12 years ago Updated 2 years ago

Use the same machinery to implement both weird scope chains and with-blocks

Categories

(Core :: JavaScript Engine, defect)

Other Branch
defect

Tracking

()

People

(Reporter: jorendorff, Unassigned)

Details

(Whiteboard: [js:t])

Attachments

(1 file)

Currently we allow all sorts of objects on the scope chain, and today bholley asked about using proxies on the scope chain.

That sounds a little crazy to me so I'm proposing a different way: use the same code to implement both weird-objects-on-the-scope-chain and with-blocks.

with-blocks have standard semantics, and I think they pretty much work right. So the answer here is, instead of letting API users parent functions to arbitrary objects, allow them to create WithObjects, and require that they parent only to those or other scope objects.

I have no idea if this will actually work but I'm tinkering now.
It all seems to just work. CC-ing luke and Waldo for a sanity check.

js> var fn = evaluate("(function () { return x + y; })", {compileAndGo: false});
js> fn
(function () { return x + y; })
js> var f2 = clone(fn, withScope({x: 1, y: 2}));
js> f2()
3
js> var f3 = clone(fn, withScope({z: 66}));
js> f3()
@evaluate:1:15 ReferenceError: x is not defined
js> var x = 'hello', y = 'world';
js> f3()
"helloworld"

Forever I've wanted to force WithObjects everywhere we currently use bare non-scope objects on the scope chain. Worth doing? Does it matter?

The patch I have right now exposes a JS_NewWithObject API, but that's not how I want to do it really. Better idea coming.
Attached patch WIP 1Splinter Review
Actually I changed my mind about the awesome new idea; it makes sense to let JSAPI users make their own WithObject chains.
Assignee: general → jorendorff
Something like With objects would be closer to the spec's declarative/object environment record dichotomy.  I'd name it closer to the spec terminology, but that's beside the interesting conceptual questions, of course.

But you should also note that the scope chain, with stuff like BindingsIter having happened now, is not all that far from ditching scope elements being JS objects at all.  Your patch entrenches objects as scope elements, which is a bad idea.  Rather, there should be a way to create a scope element *given* an object it should consult.  How that's implemented internally could be an object at first, something else later.  But we might be close enough to doing a scope object->scope element change that it'd be better to wait on that.  I think the only hurdle is Firebug and jsd users, but I'm not certain; Luke has the details closer to mind.
Blocks: 790732
(In reply to Jeff Walden [:Waldo] (remove +bmo to email) from comment #3)
> not all that far from ditching scope elements being JS objects at all.

Good. I've been in favor of that for a long time.

> Your patch entrenches objects as scope elements, which is a bad idea.

That is the exact opposite of the intent of the patch.

The idea is that with this patch, we can start banning non-scope-objects from the scope chain, a necessary prerequisite to changing the type of scopes to something other than JSObject *.

> Rather, there should be a way to create a scope element
> *given* an object it should consult.  How that's implemented internally
> could be an object at first, something else later.

That's exactly what this patch does. The type is currently JSObject, but it'll be something else later.

It is much, much easier to make breaking changes to new APIs than old ones.
Sounds great.  Two notes:

 - We already support proxies on the scope chain: DebugScopeObject.

 - Unlike real WithScopes, non-scope objects can only show up between the global object and the first ScopeObject (this is asserted in several places).  I'm not exactly sure how this helps...
No longer blocks: 790732
Whiteboard: [js:t]

The bug assignee didn't login in Bugzilla in the last 7 months, so the assignee is being reset.

Assignee: jorendorff → nobody
Severity: normal → S3
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: