1143794 - assert the structure of the scope chain

Status: NEW

(Core :: JavaScript Engine, defect, P3)

Description

[Luke Wagner [:luke]]

With all the recent refactorings, we have a pretty clean scope chain structure now:
 zero or more syntactic scope objects followed by
 zero or more non-syntactic with objects followed by 
 zero or one isUnqualifiedVarObj() plain objects
 the global object

(That isUnqualifiedVarObj() plain object is an impl detail of ExecuteInGlobalAndReturnScope() and could possibly be replaced with something more structured if necessary.)

This structure may be useful in the future (perhaps for optimizing scope representation), so it'd be nice to pin it down with a global AssertScopeChainStructure() that we call everywhere.  Giving the isUnqualifiedVarObj() vanilla object its own new class could add further specificity to the assert.

Comment 1

[Boris Zbarsky [:bzbarsky]]

Actually, I forgot one other thing that can live on the scope chain before the global: a DebugScopeObject.  Though I'm not actually 100% sure it must live before the global; it might be able to live in between syntactic scope objects?

Comment 2

[Luke Wagner [:luke]]

Oh, right.  IIUC, this just changes the list to:
 ((zero or more syntactic scope objects) OR (zero or more DebugScopeProxies)) followed by
 zero or more non-syntactic with objects followed by 
 zero or one isUnqualifiedVarObj() plain objects
 the global object

Comment 3

[Luke Wagner [:luke]]

Oops, I forgot that, once running, debugger code can add it's own syntactic scope objects, so we really have:
 zero or more syntactic scope objects followed by
 zero or more DebugScopeProxies followed by
 zero or more non-syntactic with objects followed by 
 zero or one isUnqualifiedVarObj() plain objects
 the global object
(Right?)

Comment 4

[Luke Wagner [:luke]]

Updating from IRC: it seems like you can't have both non-syntactic with AND the isUnqualifiedVarObj():
 zero or more syntactic scope objects followed by
 zero or more DebugScopeProxies followed by
 ((zero or more non-syntactic with objects) OR
  (zero or one isUnqualifiedVarObj() plain object)) followed by
 the global object

Additionally, we'd want to assert:
 - that the final "return global()" case in JSObject::enclosingScope() only hit for the isUnqualifiedVarObj()
 - that the dynamic-scope chain Clone/Execute never clone with an isUnqualifiedVarObj()

Comment 5

[Matthew Gaudet (he/him) [:mgaudet]]

Arai: I think we have a fair amount of this these days, so I'm inclined to close this bug, but given your recent adventures here I thought I should double check with you.

Comment 6

[Tooru Fujisawa [:arai]]

Adding assertions sounds reasonable. that way we don't overlook any other use cases (bug 1938994 was the overlooked case when the document was initially added).