[scopes] Drop dead hidden catch scope code

verwaest · V8 LUCI CQ · commit 6054ba94db09 · 2024-11-11T08:21:52.000Z
Change-Id: If38698e5c20dabdbebe5c03aed88452a86588fe8 Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/6010587 Reviewed-by: Leszek Swirski <leszeks@chromium.org> Commit-Queue: Toon Verwaest <verwaest@chromium.org> Auto-Submit: Toon Verwaest <verwaest@chromium.org> Cr-Commit-Position: refs/heads/main@{#97085}
diff --git a/src/ast/scopes.cc b/src/ast/scopes.cc
@@ -2574,7 +2574,6 @@ void ModuleScope::AllocateModuleVariables() {
 // Needs to be kept in sync with ScopeInfo::UniqueIdInScript and
 // SharedFunctionInfo::UniqueIdInScript.
 int Scope::UniqueIdInScript() const {
-  DCHECK(!is_hidden_catch_scope());
   // Script scopes start "before" the script to avoid clashing with a scope that
   // starts on character 0.
   if (is_script_scope() || scope_type() == EVAL_SCOPE ||
@@ -2652,17 +2651,14 @@ void Scope::AllocateScopeInfosRecursively(
   DCHECK(scope_info_.is_null());
   MaybeHandle<ScopeInfo> next_outer_scope = outer_scope;
 
-  auto it = is_hidden_catch_scope()
-                ? scope_infos_to_reuse.end()
-                : scope_infos_to_reuse.find(UniqueIdInScript());
+  auto it = scope_infos_to_reuse.find(UniqueIdInScript());
   if (it != scope_infos_to_reuse.end()) {
     scope_info_ = it->second;
     CHECK(NeedsContext());
     // The ScopeInfo chain mirrors the context chain, so we only link to the
     // next outer scope that needs a context.
     next_outer_scope = scope_info_;
     DCHECK(!scope_info_.is_null());
-    DCHECK(!is_hidden_catch_scope());
     CHECK_EQ(scope_info_->scope_type(), scope_type_);
     CHECK_EQ(scope_info_->ContextLength(), num_heap_slots_);
 #ifdef DEBUG
@@ -2672,9 +2668,8 @@ void Scope::AllocateScopeInfosRecursively(
   } else if (NeedsScopeInfo()) {
     scope_info_ = ScopeInfo::Create(isolate, zone(), this, outer_scope);
 #ifdef DEBUG
-    // Mark this ID as being used. Skip hidden scopes because they are
-    // synthetic, unreusable, but hard to make unique.
-    if (v8_flags.reuse_scope_infos && !is_hidden_catch_scope()) {
+    // Mark this ID as being used.
+    if (v8_flags.reuse_scope_infos) {
       scope_infos_to_reuse[UniqueIdInScript()] = {};
       DCHECK_EQ(UniqueIdInScript(), scope_info_->UniqueIdInScript());
     }
@@ -2687,12 +2682,9 @@ void Scope::AllocateScopeInfosRecursively(
   // Allocate ScopeInfos for inner scopes.
   for (Scope* scope = inner_scope_; scope != nullptr; scope = scope->sibling_) {
 #ifdef DEBUG
-    if (!scope->is_hidden_catch_scope()) {
-      DCHECK_GT(scope->UniqueIdInScript(), UniqueIdInScript());
-      DCHECK_IMPLIES(
-          scope->sibling_ && !scope->sibling_->is_hidden_catch_scope(),
-          scope->sibling_->UniqueIdInScript() != scope->UniqueIdInScript());
-    }
+    DCHECK_GT(scope->UniqueIdInScript(), UniqueIdInScript());
+    DCHECK_IMPLIES(scope->sibling_, scope->sibling_->UniqueIdInScript() !=
+                                        scope->UniqueIdInScript());
 #endif
     if (!scope->is_function_scope() ||
         scope->AsDeclarationScope()->ShouldEagerCompile()) {
diff --git a/src/ast/scopes.h b/src/ast/scopes.h
@@ -342,10 +342,6 @@ class V8_EXPORT_PRIVATE Scope : public NON_EXPORTED_BASE(ZoneObject) {
   bool is_hidden() const { return is_hidden_; }
   void set_is_hidden() { is_hidden_ = true; }
 
-  bool is_hidden_catch_scope() const {
-    return is_hidden() && scope_type() == CATCH_SCOPE;
-  }
-
   void ForceContextAllocationForParameters() {
     DCHECK(!already_resolved_);
     force_context_allocation_for_parameters_ = true;
diff --git a/src/codegen/compiler.cc b/src/codegen/compiler.cc
@@ -2205,7 +2205,6 @@ class ConstantPoolPointerForwarder {
   template <typename TArray>
   void VisitScopeInfo(Tagged<TArray> constant_pool, int i,
                       Tagged<ScopeInfo> scope_info) {
-    if (scope_info->IsHiddenCatchScope()) return;
     auto it = scope_infos_to_update_.find(scope_info->UniqueIdInScript());
     // Try to replace the scope info itself with an already existing version.
     if (it != scope_infos_to_update_.end()) {
diff --git a/src/objects/scope-info.cc b/src/objects/scope-info.cc
@@ -695,7 +695,6 @@ int ScopeInfo::ContextLength() const {
 // Needs to be kept in sync with Scope::UniqueIdInScript and
 // SharedFunctionInfo::UniqueIdInScript.
 int ScopeInfo::UniqueIdInScript() const {
-  DCHECK(!IsHiddenCatchScope());
   // Script scopes start "before" the script to avoid clashing with a scope that
   // starts on character 0.
   if (is_script_scope() || scope_type() == EVAL_SCOPE ||
@@ -806,10 +805,6 @@ bool ScopeInfo::IsReplModeScope() const {
   return scope_type() == REPL_MODE_SCOPE;
 }
 
-bool ScopeInfo::IsHiddenCatchScope() const {
-  return IsHiddenBit::decode(Flags()) && scope_type() == CATCH_SCOPE;
-}
-
 bool ScopeInfo::IsWrappedFunctionScope() const {
   DCHECK_IMPLIES(IsWrappedFunctionBit::decode(Flags()),
                  scope_type() == FUNCTION_SCOPE);
diff --git a/src/objects/scope-info.h b/src/objects/scope-info.h
@@ -114,8 +114,6 @@ class ScopeInfo : public TorqueGeneratedScopeInfo<ScopeInfo, HeapObject> {
   // Does this scope belong to a function?
   bool HasPositionInfo() const;
 
-  bool IsHiddenCatchScope() const;
-
   bool IsWrappedFunctionScope() const;
 
   // Return if contexts are allocated for this scope.
diff --git a/src/parsing/parser.cc b/src/parsing/parser.cc
@@ -2987,21 +2987,6 @@ Block* Parser::BuildParameterInitializationBlock(
   return factory()->NewParameterInitializationBlock(init_statements);
 }
 
-// TODO(verwaest): Consider building these try/catches in the bytecode generator
-// without hidden scopes.
-Scope* Parser::NewHiddenCatchScope() {
-  DCHECK(scope()->is_declaration_scope());
-  Scope* catch_scope = NewScopeWithParent(scope(), CATCH_SCOPE);
-  catch_scope->set_start_position(position());
-  catch_scope->set_end_position(end_position());
-  bool was_added;
-  catch_scope->DeclareLocal(ast_value_factory()->dot_catch_string(),
-                            VariableMode::kVar, NORMAL_VARIABLE, &was_added);
-  DCHECK(was_added);
-  catch_scope->set_is_hidden();
-  return catch_scope;
-}
-
 Expression* Parser::BuildInitialYield(int pos, FunctionKind kind) {
   Expression* yield_result = factory()->NewVariableProxy(
       function_state_->scope()->generator_object_var());
diff --git a/src/parsing/parser.h b/src/parsing/parser.h
@@ -380,8 +380,6 @@ class V8_EXPORT_PRIVATE Parser : public NON_EXPORTED_BASE(ParserBase<Parser>) {
 
   Block* IgnoreCompletion(Statement* statement);
 
-  Scope* NewHiddenCatchScope();
-
   bool HasCheckedSyntax() {
     return scope()->GetDeclarationScope()->has_checked_syntax();
   }

Original file line number	Diff line number	Diff line change
`@@ -380,8 +380,6 @@ class V8_EXPORT_PRIVATE Parser : public NON_EXPORTED_BASE(ParserBase<Parser>) {`
`380`	`380`
`381`	`381`	`Block* IgnoreCompletion(Statement* statement);`
`382`	`382`
`383`		`- Scope* NewHiddenCatchScope();`
`384`		`-`
`385`	`383`	`bool HasCheckedSyntax() {`
`386`	`384`	`return scope()->GetDeclarationScope()->has_checked_syntax();`
`387`	`385`	`}`