[scopes] Clean up and centralize mapped/unmapped arguments logic

Also update comments that'd gotten unnecessarily verbose over
ten years of language development.

Bug: v8:8015
Change-Id: I6688ce22e4aa92f66f937159d890b9922f109d43
Reviewed-on: https://chromium-review.googlesource.com/1180357Reviewed-by: Ross McIlroy <rmcilroy@chromium.org>
Commit-Queue: Adam Klein <adamk@chromium.org>
Cr-Commit-Position: refs/heads/master@{#55288}

src/ast/scopes.cc

@@ -2178,25 +2178,16 @@ void Scope::AllocateHeapSlot(Variable* var) {
 void DeclarationScope::AllocateParameterLocals() {
   DCHECK(is_function_scope());
 
-  bool uses_sloppy_arguments = false;
-
+  bool has_mapped_arguments = false;
   if (arguments_ != nullptr) {
     DCHECK(!is_arrow_scope());
-    // 'arguments' is used. Unless there is also a parameter called
-    // 'arguments', we must be conservative and allocate all parameters to
-    // the context assuming they will be captured by the arguments object.
-    // If we have a parameter named 'arguments', a (new) value is always
-    // assigned to it via the function invocation. Then 'arguments' denotes
-    // that specific parameter value and cannot be used to access the
-    // parameters, which is why we don't need to allocate an arguments
-    // object in that case.
     if (MustAllocate(arguments_) && !has_arguments_parameter_) {
-      // In strict mode 'arguments' does not alias formal parameters.
-      // Therefore in strict mode we allocate parameters as if 'arguments'
-      // were not used.
-      // If the parameter list is not simple, arguments isn't sloppy either.
-      uses_sloppy_arguments =
-          is_sloppy(language_mode()) && has_simple_parameters();
+      // 'arguments' is used and does not refer to a function
+      // parameter of the same name. If the arguments object
+      // aliases formal parameters, we conservatively allocate
+      // them specially in the loop below.
+      has_mapped_arguments =
+          GetArgumentsType() == CreateArgumentsType::kMappedArguments;
     } else {
       // 'arguments' is unused. Tell the code generator that it does not need to
       // allocate the arguments object by nulling out arguments_.
@@ -2212,7 +2203,7 @@ void DeclarationScope::AllocateParameterLocals() {
     Variable* var = params_[i];
     DCHECK(!has_rest_ || var != rest_parameter());
     DCHECK_EQ(this, var->scope());
-    if (uses_sloppy_arguments) {
+    if (has_mapped_arguments) {
       var->set_is_used();
       var->set_maybe_assigned();
       var->ForceContextAllocation();

src/ast/scopes.h

@@ -804,6 +804,16 @@ class V8_EXPORT_PRIVATE DeclarationScope : public Scope {
     has_simple_parameters_ = false;
   }
 
+  // Returns whether the arguments object aliases formal parameters.
+  CreateArgumentsType GetArgumentsType() const {
+    DCHECK(is_function_scope());
+    DCHECK(!is_arrow_scope());
+    DCHECK_NOT_NULL(arguments_);
+    return is_sloppy(language_mode()) && has_simple_parameters()
+               ? CreateArgumentsType::kMappedArguments
+               : CreateArgumentsType::kUnmappedArguments;
+  }
+
   // The local variable 'arguments' if we need to allocate it; nullptr
   // otherwise.
   Variable* arguments() const {

src/interpreter/bytecode-generator.cc

@@ -4811,11 +4811,7 @@ void BytecodeGenerator::VisitArgumentsObject(Variable* variable) {
 
   // Allocate and initialize a new arguments object and assign to the
   // {arguments} variable.
-  CreateArgumentsType type =
-      is_strict(language_mode()) || !info()->has_simple_parameters()
-          ? CreateArgumentsType::kUnmappedArguments
-          : CreateArgumentsType::kMappedArguments;
-  builder()->CreateArguments(type);
+  builder()->CreateArguments(closure_scope()->GetArgumentsType());
   BuildVariableAssignment(variable, Token::ASSIGN, HoleCheckMode::kElided);
 }
 

src/unoptimized-compilation-info.cc

@@ -49,10 +49,6 @@ int UnoptimizedCompilationInfo::num_parameters_including_this() const {
   return scope()->num_parameters() + 1;
 }
 
-bool UnoptimizedCompilationInfo::has_simple_parameters() {
-  return scope()->has_simple_parameters();
-}
-
 SourcePositionTableBuilder::RecordingMode
 UnoptimizedCompilationInfo::SourcePositionRecordingMode() const {
   return is_native() ? SourcePositionTableBuilder::OMIT_SOURCE_POSITIONS

src/unoptimized-compilation-info.h

@@ -55,8 +55,6 @@ class V8_EXPORT_PRIVATE UnoptimizedCompilationInfo final {
 
   DeclarationScope* scope() const;
 
-  bool has_simple_parameters();
-
   int num_parameters() const;
   int num_parameters_including_this() const;