Backed out 6 changesets (bug 1655052) for xpcshell failures at test_protocol_children.js. CLOSED TREE

Backed out changeset 0a841c531e0d (bug 1655052)
Backed out changeset b539cd841c63 (bug 1655052)
Backed out changeset fad29724ccd6 (bug 1655052)
Backed out changeset fd6f6cc20a90 (bug 1655052)
Backed out changeset c3dcadf05381 (bug 1655052)
Backed out changeset 374aebc92cc2 (bug 1655052)

js/src/frontend/AbstractScopePtr.cpp

@@ -61,6 +61,52 @@ bool AbstractScopePtr::isArrow() const {
   return scope()->as<FunctionScope>().canonicalFunction()->isArrow();
 }
 
+uint32_t AbstractScopePtr::nextFrameSlot() const {
+  if (isScopeStencil()) {
+    return scopeData().nextFrameSlot();
+  }
+
+  switch (kind()) {
+    case ScopeKind::Function:
+      return scope()->as<FunctionScope>().nextFrameSlot();
+    case ScopeKind::FunctionBodyVar:
+      return scope()->as<VarScope>().nextFrameSlot();
+    case ScopeKind::Lexical:
+    case ScopeKind::SimpleCatch:
+    case ScopeKind::Catch:
+    case ScopeKind::FunctionLexical:
+    case ScopeKind::ClassBody:
+      return scope()->as<LexicalScope>().nextFrameSlot();
+    case ScopeKind::NamedLambda:
+    case ScopeKind::StrictNamedLambda:
+      // Named lambda scopes cannot have frame slots.
+      return 0;
+    case ScopeKind::Eval:
+    case ScopeKind::StrictEval:
+      return scope()->as<EvalScope>().nextFrameSlot();
+    case ScopeKind::Global:
+    case ScopeKind::NonSyntactic:
+      return 0;
+    case ScopeKind::Module:
+      return scope()->as<ModuleScope>().nextFrameSlot();
+    case ScopeKind::WasmInstance:
+      MOZ_CRASH("WasmInstanceScope doesn't have nextFrameSlot()");
+      return 0;
+    case ScopeKind::WasmFunction:
+      MOZ_CRASH("WasmFunctionScope doesn't have nextFrameSlot()");
+      return 0;
+    case ScopeKind::With:
+      MOZ_CRASH("With Scopes don't get nextFrameSlot()");
+      return 0;
+  }
+  MOZ_CRASH("Not an enclosing intra-frame scope");
+}
+
 void AbstractScopePtr::trace(JSTracer* trc) {
   JS::GCPolicy<ScopeType>::trace(trc, &scope_, "AbstractScopePtr");
 }
+
+bool AbstractScopePtrIter::hasSyntacticEnvironment() const {
+  return abstractScopePtr().hasEnvironment() &&
+         abstractScopePtr().kind() != ScopeKind::NonSyntactic;
+}

js/src/frontend/AbstractScopePtr.h

@@ -103,6 +103,7 @@ class AbstractScopePtr {
   ScopeKind kind() const;
   AbstractScopePtr enclosing() const;
   bool hasEnvironment() const;
+  uint32_t nextFrameSlot() const;
   // Valid iff is<FunctionScope>
   bool isArrow() const;
 
@@ -131,6 +132,41 @@ inline bool AbstractScopePtr::is<EvalScope>() const {
          (kind() == ScopeKind::Eval || kind() == ScopeKind::StrictEval);
 }
 
+// Iterate over abstract scopes rather than scopes.
+class AbstractScopePtrIter {
+  AbstractScopePtr scope_;
+
+ public:
+  explicit AbstractScopePtrIter(const AbstractScopePtr& f) : scope_(f) {}
+  explicit operator bool() const { return !done(); }
+
+  bool done() const { return !scope_; }
+
+  ScopeKind kind() const {
+    MOZ_ASSERT(!done());
+    MOZ_ASSERT(scope_);
+    return scope_.kind();
+  }
+
+  AbstractScopePtr abstractScopePtr() const { return scope_; }
+
+  void operator++(int) {
+    MOZ_ASSERT(!done());
+    scope_ = scope_.enclosing();
+  }
+
+  // Returns whether this scope has a syntactic environment (i.e., an
+  // Environment that isn't a non-syntactic With or NonSyntacticVariables)
+  // on the environment chain.
+  bool hasSyntacticEnvironment() const;
+
+  void trace(JSTracer* trc) {
+    if (scope_) {
+      scope_.trace(trc);
+    }
+  };
+};
+
 }  // namespace js
 
 namespace JS {

js/src/frontend/BytecodeCompiler.cpp

@@ -102,11 +102,10 @@ class MOZ_STACK_CLASS frontend::SourceAwareCompiler {
                                const JS::ReadOnlyCompileOptions& options,
                                CompilationInfo& compilationInfo,
                                SourceText<Unit>& sourceBuffer,
-                               InheritThis inheritThis = InheritThis::No,
                                js::Scope* enclosingScope = nullptr,
                                JSObject* enclosingEnv = nullptr)
       : sourceBuffer_(sourceBuffer),
-        compilationState_(cx, allocScope, options, compilationInfo, inheritThis,
+        compilationState_(cx, allocScope, options, compilationInfo,
                           enclosingScope, enclosingEnv) {
     MOZ_ASSERT(sourceBuffer_.get() != nullptr);
   }
@@ -164,11 +163,10 @@ class MOZ_STACK_CLASS frontend::ScriptCompiler
                           const JS::ReadOnlyCompileOptions& options,
                           CompilationInfo& compilationInfo,
                           SourceText<Unit>& sourceBuffer,
-                          InheritThis inheritThis = InheritThis::No,
                           js::Scope* enclosingScope = nullptr,
                           JSObject* enclosingEnv = nullptr)
       : Base(cx, allocScope, options, compilationInfo, sourceBuffer,
-             inheritThis, enclosingScope, enclosingEnv) {}
+             enclosingScope, enclosingEnv) {}
 
   using Base::createSourceAndParser;
 
@@ -455,8 +453,7 @@ static JSScript* CompileEvalScriptImpl(
 
   frontend::ScriptCompiler<Unit> compiler(
       cx, allocScope, compilationInfo.get().input.options,
-      compilationInfo.get(), srcBuf, InheritThis::Yes, enclosingScope,
-      enclosingEnv);
+      compilationInfo.get(), srcBuf, enclosingScope, enclosingEnv);
   if (!compiler.createSourceAndParser(cx, compilationInfo.get())) {
     return nullptr;
   }
@@ -508,7 +505,7 @@ class MOZ_STACK_CLASS frontend::ModuleCompiler final
                           js::Scope* enclosingScope = nullptr,
                           JSObject* enclosingEnv = nullptr)
       : Base(cx, allocScope, options, compilationInfo, sourceBuffer,
-             InheritThis::No, enclosingScope, enclosingEnv) {}
+             enclosingScope, enclosingEnv) {}
 
   bool compile(JSContext* cx, CompilationInfo& compilationInfo);
 };
@@ -533,11 +530,10 @@ class MOZ_STACK_CLASS frontend::StandaloneFunctionCompiler final
                                       const JS::ReadOnlyCompileOptions& options,
                                       CompilationInfo& compilationInfo,
                                       SourceText<Unit>& sourceBuffer,
-                                      InheritThis inheritThis = InheritThis::No,
                                       js::Scope* enclosingScope = nullptr,
                                       JSObject* enclosingEnv = nullptr)
       : Base(cx, allocScope, options, compilationInfo, sourceBuffer,
-             inheritThis, enclosingScope, enclosingEnv) {}
+             enclosingScope, enclosingEnv) {}
 
   using Base::createSourceAndParser;
 
@@ -1024,12 +1020,10 @@ static bool CompileLazyFunctionToStencilImpl(JSContext* cx,
 
   Rooted<JSFunction*> fun(cx, lazy->function());
 
-  InheritThis inheritThis = fun->isArrow() ? InheritThis::Yes : InheritThis::No;
-
   LifoAllocScope allocScope(&cx->tempLifoAlloc());
   frontend::CompilationState compilationState(
       cx, allocScope, compilationInfo.input.options, compilationInfo,
-      inheritThis, fun->enclosingScope());
+      fun->enclosingScope());
 
   Parser<FullParseHandler, Unit> parser(
       cx, compilationInfo.input.options, units, length,
@@ -1127,12 +1121,9 @@ static JSFunction* CompileStandaloneFunction(
   }
 
   LifoAllocScope allocScope(&cx->tempLifoAlloc());
-  InheritThis inheritThis = (syntaxKind == FunctionSyntaxKind::Arrow)
-                                ? InheritThis::Yes
-                                : InheritThis::No;
   StandaloneFunctionCompiler<char16_t> compiler(
       cx, allocScope, compilationInfo.get().input.options,
-      compilationInfo.get(), srcBuf, inheritThis, enclosingScope);
+      compilationInfo.get(), srcBuf, enclosingScope);
   if (!compiler.createSourceAndParser(cx, compilationInfo.get())) {
     return nullptr;
   }

js/src/frontend/BytecodeEmitter.cpp

@@ -1576,27 +1576,17 @@ bool BytecodeEmitter::emitThisEnvironmentCallee() {
 
   // We have to load the callee from the environment chain.
   unsigned numHops = 0;
-  EmitterScope* es = innermostEmitterScope();
-  for (; es; es = es->enclosingInFrame()) {
-    if (es->scope(this).is<FunctionScope>()) {
-      if (!es->scope(this).isArrow()) {
-        // The Parser is responsible for marking the environment as either
-        // closed-over or used-by-eval which ensure that is must exist.
-        MOZ_ASSERT(es->scope(this).hasEnvironment());
+  for (AbstractScopePtrIter si(innermostScope()); si; si++) {
+    if (si.hasSyntacticEnvironment() &&
+        si.abstractScopePtr().is<FunctionScope>()) {
+      if (!si.abstractScopePtr().isArrow()) {
         break;
       }
     }
-    if (es->scope(this).hasEnvironment()) {
+    if (si.abstractScopePtr().hasEnvironment()) {
       numHops++;
     }
   }
-  if (!es) {
-    // The "this" environment exists outside of the compilation, but the
-    // `ScopeContext` recorded the number of additional hops needed, so add
-    // those in now.
-    MOZ_ASSERT(sc->allowSuperProperty());
-    numHops += compilationState.scopeContext.enclosingThisEnvironmentHops;
-  }
 
   static_assert(
       ENVCOORD_HOPS_LIMIT - 1 <= UINT8_MAX,

js/src/frontend/CompilationInfo.h

@@ -43,59 +43,53 @@ namespace frontend {
 // ScopeContext hold information derivied from the scope and environment chains
 // to try to avoid the parser needing to traverse VM structures directly.
 struct ScopeContext {
-  // If this eval is in response to Debugger.Frame.eval, we may have an
-  // incomplete scope chain. In order to provide a better debugging experience,
-  // we inspect the (optional) environment chain to determine it's enclosing
-  // FunctionScope if there is one. If there is no such scope, we use the
-  // orignal scope provided.
-  //
-  // NOTE: This is used to compute the ThisBinding kind and to allow access to
-  //       private fields, while other contextual information only uses the
-  //       actual scope passed to the compile.
-  JS::Rooted<Scope*> effectiveScope;
-
-  // The type of binding required for `this` of the top level context, as
-  // indicated by the enclosing scopes of this parse.
-  //
-  // NOTE: This is computed based on the effective scope (defined above).
-  ThisBinding thisBinding = ThisBinding::Global;
-
-  // Eval and arrow scripts inherit certain syntax allowances from their
-  // enclosing scripts.
+  // Whether the enclosing scope allows certain syntax. Eval and arrow scripts
+  // inherit this from their enclosing scipt so we track it here.
   bool allowNewTarget = false;
   bool allowSuperProperty = false;
   bool allowSuperCall = false;
   bool allowArguments = true;
 
-  // Eval and arrow scripts also inherit the "this" environment -- used by
-  // `super` expressions -- from their enclosing script. We count the number of
-  // environment hops needed to get from enclosing scope to the nearest
-  // appropriate environment. This value is undefined if the script we are
-  // compiling is not an eval or arrow-function.
-  uint32_t enclosingThisEnvironmentHops = 0;
+  // The type of binding required for `this` of the top level context, as
+  // indicated by the enclosing scopes of this parse.
+  ThisBinding thisBinding = ThisBinding::Global;
+
+  // Somewhere on the scope chain this parse is embedded within is a 'With'
+  // scope.
+  bool inWith = false;
+
+  // Somewhere on the scope chain this parse is embedded within a class scope.
+  bool inClass = false;
 
   // Class field initializer info if we are nested within a class constructor.
   // We may be an combination of arrow and eval context within the constructor.
   mozilla::Maybe<MemberInitializers> memberInitializers = {};
 
-  // Indicates there is a 'class' or 'with' scope on enclosing scope chain.
-  bool inClass = false;
-  bool inWith = false;
+  // If this eval is in response to Debugger.Frame.eval, we may have an
+  // incomplete scope chain. In order to determine a better 'this' binding, as
+  // well as to ensure we can provide better static error semantics for private
+  // names, we use the environment chain to attempt to find a more effective
+  // scope than the enclosing scope.
+  // If there is no more effective scope, this will just be the scope given in
+  // the constructor.
+  JS::Rooted<Scope*> effectiveScope;
 
-  explicit ScopeContext(JSContext* cx, InheritThis inheritThis, Scope* scope,
+  explicit ScopeContext(JSContext* cx, Scope* scope,
                         JSObject* enclosingEnv = nullptr)
       : effectiveScope(cx, determineEffectiveScope(scope, enclosingEnv)) {
-    if (inheritThis == InheritThis::Yes) {
-      computeThisBinding(effectiveScope);
-      computeThisEnvironment(scope);
-    }
-    computeInScope(scope);
+    computeAllowSyntax(scope);
+    computeThisBinding(effectiveScope);
+    computeInWith(scope);
+    computeExternalInitializers(scope);
+    computeInClass(scope);
   }
 
  private:
+  void computeAllowSyntax(Scope* scope);
   void computeThisBinding(Scope* scope);
-  void computeThisEnvironment(Scope* scope);
-  void computeInScope(Scope* scope);
+  void computeInWith(Scope* scope);
+  void computeExternalInitializers(Scope* scope);
+  void computeInClass(Scope* scope);
 
   static Scope* determineEffectiveScope(Scope* scope, JSObject* environment);
 };
@@ -243,7 +237,6 @@ struct MOZ_RAII CompilationState {
   CompilationState(JSContext* cx, LifoAllocScope& frontendAllocScope,
                    const JS::ReadOnlyCompileOptions& options,
                    CompilationInfo& compilationInfo,
-                   InheritThis inheritThis = InheritThis::No,
                    Scope* enclosingScope = nullptr,
                    JSObject* enclosingEnv = nullptr);
 };

js/src/frontend/SharedContext.cpp

@@ -70,43 +70,29 @@ SharedContext::SharedContext(JSContext* cx, Kind kind,
   setFlag(ImmutableFlags::Strict, directives.strict());
 }
 
-void ScopeContext::computeThisEnvironment(Scope* scope) {
-  uint32_t envCount = 0;
+void ScopeContext::computeAllowSyntax(Scope* scope) {
   for (ScopeIter si(scope); si; si++) {
     if (si.kind() == ScopeKind::Function) {
-      JSFunction* fun = si.scope()->as<FunctionScope>().canonicalFunction();
+      FunctionScope* funScope = &si.scope()->as<FunctionScope>();
+      JSFunction* fun = funScope->canonicalFunction();
 
-      // Arrow function inherit the "this" environment of the enclosing script,
-      // so continue ignore them.
-      if (!fun->isArrow()) {
-        allowNewTarget = true;
-
-        if (fun->allowSuperProperty()) {
-          allowSuperProperty = true;
-          enclosingThisEnvironmentHops = envCount;
-        }
-
-        if (fun->isClassConstructor()) {
-          memberInitializers =
-              mozilla::Some(fun->baseScript()->getMemberInitializers());
-          MOZ_ASSERT(memberInitializers->valid);
-        }
-
-        if (fun->isDerivedClassConstructor()) {
-          allowSuperCall = true;
-        }
-
-        if (fun->isFieldInitializer()) {
-          allowArguments = false;
-        }
-
-        // Found the effective "this" environment, so stop.
-        return;
+      // Arrow function inherit syntax restrictions of enclosing scope.
+      if (fun->isArrow()) {
+        continue;
       }
-    }
 
-    if (si.scope()->hasEnvironment()) {
-      envCount++;
+      allowNewTarget = true;
+      allowSuperProperty = fun->allowSuperProperty();
+
+      if (fun->isDerivedClassConstructor()) {
+        allowSuperCall = true;
+      }
+
+      if (fun->isFieldInitializer()) {
+        allowArguments = false;
+      }
+
+      return;
     }
   }
 }
@@ -143,14 +129,42 @@ void ScopeContext::computeThisBinding(Scope* scope) {
   thisBinding = ThisBinding::Global;
 }
 
-void ScopeContext::computeInScope(Scope* scope) {
+void ScopeContext::computeInWith(Scope* scope) {
+  for (ScopeIter si(scope); si; si++) {
+    if (si.kind() == ScopeKind::With) {
+      inWith = true;
+      break;
+    }
+  }
+}
+
+void ScopeContext::computeInClass(Scope* scope) {
   for (ScopeIter si(scope); si; si++) {
     if (si.kind() == ScopeKind::ClassBody) {
       inClass = true;
+      break;
     }
+  }
+}
 
-    if (si.kind() == ScopeKind::With) {
-      inWith = true;
+void ScopeContext::computeExternalInitializers(Scope* scope) {
+  for (ScopeIter si(scope); si; si++) {
+    if (si.scope()->is<FunctionScope>()) {
+      FunctionScope& funcScope = si.scope()->as<FunctionScope>();
+      JSFunction* fun = funcScope.canonicalFunction();
+
+      // Arrows can call `super()` on behalf on parent so keep searching.
+      if (fun->isArrow()) {
+        continue;
+      }
+
+      if (fun->isClassConstructor()) {
+        memberInitializers =
+            mozilla::Some(fun->baseScript()->getMemberInitializers());
+        MOZ_ASSERT(memberInitializers->valid);
+      }
+
+      break;
     }
   }
 }

js/src/frontend/SharedContext.h

@@ -105,10 +105,6 @@ enum class ThisBinding : uint8_t {
   DerivedConstructor
 };
 
-// If Yes, the script inherits it's "this" environment and binding from the
-// enclosing script. This is true for arrow-functions and eval scripts.
-enum class InheritThis { No, Yes };
-
 class GlobalSharedContext;
 class EvalSharedContext;
 class ModuleSharedContext;

js/src/frontend/Stencil.cpp

@@ -912,14 +912,14 @@ bool CompilationInfoVector::deserializeStencils(JSContext* cx,
   return true;
 }
 
-CompilationState::CompilationState(
-    JSContext* cx, LifoAllocScope& frontendAllocScope,
-    const JS::ReadOnlyCompileOptions& options, CompilationInfo& compilationInfo,
-    InheritThis inheritThis /* = InheritThis::No */,
-    Scope* enclosingScope /* = nullptr */,
-    JSObject* enclosingEnv /* = nullptr */)
+CompilationState::CompilationState(JSContext* cx,
+                                   LifoAllocScope& frontendAllocScope,
+                                   const JS::ReadOnlyCompileOptions& options,
+                                   CompilationInfo& compilationInfo,
+                                   Scope* enclosingScope /* = nullptr */,
+                                   JSObject* enclosingEnv /* = nullptr */)
     : directives(options.forceStrictMode()),
-      scopeContext(cx, inheritThis, enclosingScope, enclosingEnv),
+      scopeContext(cx, enclosingScope, enclosingEnv),
       usedNames(cx),
       allocScope(frontendAllocScope),
       parserAtoms(cx->runtime(), compilationInfo.alloc,

js/src/vm/Scope.cpp

@@ -510,7 +510,7 @@ uint32_t Scope::firstFrameSlot() const {
     case ScopeKind::ClassBody:
       // For intra-frame scopes, find the enclosing scope's next frame slot.
       MOZ_ASSERT(is<LexicalScope>());
-      return LexicalScope::nextFrameSlot(enclosing());
+      return LexicalScope::nextFrameSlot(AbstractScopePtr(enclosing()));
 
     case ScopeKind::NamedLambda:
     case ScopeKind::StrictNamedLambda:
@@ -762,45 +762,28 @@ bool Scope::dumpForDisassemble(JSContext* cx, JS::Handle<Scope*> scope,
 #endif /* defined(DEBUG) || defined(JS_JITSPEW) */
 
 /* static */
-uint32_t LexicalScope::nextFrameSlot(Scope* scope) {
-  for (ScopeIter si(scope); si; si++) {
+uint32_t LexicalScope::nextFrameSlot(const AbstractScopePtr& scope) {
+  for (AbstractScopePtrIter si(scope); si; si++) {
     switch (si.kind()) {
       case ScopeKind::With:
         continue;
-
       case ScopeKind::Function:
-        return si.scope()->as<FunctionScope>().nextFrameSlot();
-
       case ScopeKind::FunctionBodyVar:
-        return si.scope()->as<VarScope>().nextFrameSlot();
-
       case ScopeKind::Lexical:
       case ScopeKind::SimpleCatch:
       case ScopeKind::Catch:
       case ScopeKind::FunctionLexical:
       case ScopeKind::ClassBody:
-        return si.scope()->as<LexicalScope>().nextFrameSlot();
-
       case ScopeKind::NamedLambda:
       case ScopeKind::StrictNamedLambda:
-        // Named lambda scopes cannot have frame slots.
-        return 0;
-
       case ScopeKind::Eval:
       case ScopeKind::StrictEval:
-        return si.scope()->as<EvalScope>().nextFrameSlot();
-
       case ScopeKind::Global:
       case ScopeKind::NonSyntactic:
-        return 0;
-
       case ScopeKind::Module:
-        return si.scope()->as<ModuleScope>().nextFrameSlot();
-
       case ScopeKind::WasmInstance:
       case ScopeKind::WasmFunction:
-        // Invalid; MOZ_CRASH below.
-        break;
+        return si.abstractScopePtr().nextFrameSlot();
     }
   }
   MOZ_CRASH("Not an enclosing intra-frame Scope");

js/src/vm/Scope.h

@@ -586,7 +586,7 @@ class LexicalScope : public Scope {
     return *static_cast<const RuntimeData*>(rawData());
   }
 
-  static uint32_t nextFrameSlot(Scope* scope);
+  static uint32_t nextFrameSlot(const AbstractScopePtr& scope);
 
  public:
   uint32_t nextFrameSlot() const { return data().slotInfo.nextFrameSlot; }