Cleanup ScopeInfo and SerializedScopeInfo.

src/objects.h

 // Copyright 2011 the V8 project authors. All rights reserved.
 // Redistribution and use in source and binary forms, with or without
 // modification, are permitted provided that the following conditions are
 // met:
 //
 //     * Redistributions of source code must retain the above copyright
 //       notice, this list of conditions and the following disclaimer.
 //     * Redistributions in binary form must reproduce the above
 //       copyright notice, this list of conditions and the following
 //       disclaimer in the documentation and/or other materials provided
@@ skipping 60 matching lines @@
 //         - FixedArray
 //           - DescriptorArray
 //           - HashTable
 //             - Dictionary
 //             - SymbolTable
 //             - CompilationCacheTable
 //             - CodeCacheHashTable
 //             - MapCache
 //           - Context
 //           - JSFunctionResultCache
-//           - SerializedScopeInfo
+//           - ScopeInfo
 //         - FixedDoubleArray
 //         - ExternalArray
 //           - ExternalPixelArray
 //           - ExternalByteArray
 //           - ExternalUnsignedByteArray
 //           - ExternalShortArray
 //           - ExternalUnsignedShortArray
 //           - ExternalIntArray
 //           - ExternalUnsignedIntArray
 //           - ExternalFloatArray
@@ skipping 715 matching lines @@
   V(JSObject)                                  \
   V(JSContextExtensionObject)                  \
   V(Map)                                       \
   V(DescriptorArray)                           \
   V(DeoptimizationInputData)                   \
   V(DeoptimizationOutputData)                  \
   V(FixedArray)                                \
   V(FixedDoubleArray)                          \
   V(Context)                                   \
   V(GlobalContext)                             \
-  V(SerializedScopeInfo)                       \
+  V(ScopeInfo)                                 \
   V(JSFunction)                                \
   V(Code)                                      \
   V(Oddball)                                   \
   V(SharedFunctionInfo)                        \
   V(JSValue)                                   \
   V(JSMessageObject)                           \
   V(StringWrapper)                             \
   V(Foreign)                                   \
   V(Boolean)                                   \
   V(JSArray)                                   \
@@ skipping 2263 matching lines @@
 
   // Casting
   static inline JSFunctionResultCache* cast(Object* obj);
 
 #ifdef DEBUG
   void JSFunctionResultCacheVerify();
 #endif
 };
 
 
+// ScopeInfo represents information about different scopes of a source
+// program  and the allocation of the scope's variables. Scope information
+// is stored in a compressed form in ScopeInfo objects and is used
+// at runtime (stack dumps, deoptimization, etc.).
+
 // This object provides quick access to scope info details for runtime
-// routines w/o the need to explicitly create a ScopeInfo object.
-class SerializedScopeInfo : public FixedArray {
+// routines.
+class ScopeInfo : public FixedArray {
  public :
-  static SerializedScopeInfo* cast(Object* object) {
-    ASSERT(object->IsSerializedScopeInfo());
-    return reinterpret_cast<SerializedScopeInfo*>(object);
+

[Kevin Millikin (Chromium), 2011/11/02 16:31:19]

Presubmit check is not going to like this extra blank line.

[Steven, 2011/11/02 18:52:59]

tools/presubmit.py didn't complain. Maybe it should?
On 2011/11/02 16:31:19, Kevin Millikin wrote:

[Kevin Millikin (Chromium), 2011/11/03 09:18:37]

I'm pretty sure the one from Chromium's depot_tools will complain.  That's the
one that's run on the buildbots.

+  static ScopeInfo* cast(Object* object) {

[Kevin Millikin (Chromium), 2011/11/02 16:31:19]

We normally do this this way:

in objects.h:

static inline ScopeInfo* cast(Object* object);

and in objects-inl.h:

CAST_ACCESSOR(ScopeInfo)

(I actually would support moving CAST_ACCESSOR into objects.h and just using it
in the class body, but that's a separate change.)

[Steven, 2011/11/02 18:52:59]

Done.

+    ASSERT(object->IsScopeInfo());
+    return reinterpret_cast<ScopeInfo*>(object);
   }
 
   // Return the type of this scope.
   ScopeType Type();
 
   // Does this scope call eval?
   bool CallsEval();
 
   // Is this scope a strict mode scope?
   bool IsStrictMode();
 
-  // Return the number of stack slots for code.
+  // Does this scope make a non-strict eval call?
+  bool CallsNonStrictEval() {
+    return CallsEval() && !IsStrictMode();
+  }
+
+  // Return the total number of locals allocated on the stack and in the
+  // context. This includes the parameters that are allocated in the context.
+  int NumberOfLocals();
+
+  // Return the number of stack slots for code. This number consists of two
+  // parts:
+  //  1. One stack slot per stack allocated local.
+  //  2. One stack slot for the function name if it is stack allocated.
   int NumberOfStackSlots();
 
-  // Return the number of context slots for code.
+  // Return the number of context slots for code if a context is allocated. This
+  // number consists of three parts:
+  //  1. Size of fixed header for every context: Context::MIN_CONTEXT_SLOTS
+  //  2. One context slot per context allocated local.
+  //  3. One context slot for the function name if it is context allocated.
+  // Parameters allocated in the context count as context allocated locals. If
+  // no contexts are allocated for this scope NumberOfContextSlots returns 0.
   int NumberOfContextSlots();

[Kevin Millikin (Chromium), 2011/11/02 16:31:19]

I've always found it a bit confusing to count the context header as "slots". 
Perhaps we should make this so it does not count the size of the header; or else
count the header size and rename it to something like ContextLength.

[Steven, 2011/11/02 18:52:59]

Renamed it to ContextLength.
On 2011/11/02 16:31:19, Kevin Millikin wrote:

 
-  // Return if this has context slots besides MIN_CONTEXT_SLOTS;
+  // Is this scope the scope of a named function expression?
+  bool HasFunctionName();
+
+  // Return if this has context allocated locals.
   bool HasHeapAllocatedLocals();
 
   // Return if contexts are allocated for this scope.
   bool HasContext();
 
+  // Return the function_name if present.
+  Handle<String> function_name();

[Kevin Millikin (Chromium), 2011/11/02 16:31:19]

There's a strange mix of hacker_style and CamelCase function names in here.  I
can't figure out why, that's distracting, so maybe we should adopt one or the
other (hopefully CamelCase).

[Steven, 2011/11/02 18:52:59]

Done.

+
+  // Return the name of the given parameter.
+  Handle<String> parameter_name(int var);
+
+  // Return the name of the given local.
+  Handle<String> local_name(int var);
+
+  // Return the name of the given stack local.
+  Handle<String> stack_local_name(int var);
+
+  // Return the name of the given context local.
+  Handle<String> context_local_name(int var);
+
+  // Return the mode of the given context local.
+  VariableMode context_local_mode(int var);
+
   // Lookup support for serialized scope info. Returns the
   // the stack slot index for a given slot name if the slot is
   // present; otherwise returns a value < 0. The name must be a symbol
   // (canonicalized).
   int StackSlotIndex(String* name);
 
   // Lookup support for serialized scope info. Returns the
   // context slot index for a given slot name if the slot is present; otherwise
   // returns a value < 0. The name must be a symbol (canonicalized).
   // If the slot is present and mode != NULL, sets *mode to the corresponding
   // mode for that variable.
-  int ContextSlotIndex(String* name, VariableMode* mode);
+  int ContextSlotIndex(String* name, VariableMode* mode = NULL);

[Kevin Millikin (Chromium), 2011/11/02 16:31:19]

I'm not thrilled with the default parameter.  It does seem to fit the allowable
exception (being used to simulate variable length argument lists), but it also
seems unnecessary.

If you make the caller always pass it, it's only inconvenient at a handful of
sites, and then the lookup functions don't have to check for mode != NULL.

[Steven, 2011/11/02 18:52:59]

Done.

 
   // Lookup support for serialized scope info. Returns the
   // parameter index for a given parameter name if the parameter is present;
   // otherwise returns a value < 0. The name must be a symbol (canonicalized).
   int ParameterIndex(String* name);
 
   // Lookup support for serialized scope info. Returns the
   // function context slot index if the function name is present (named
   // function expressions, only), otherwise returns a value < 0. The name
   // must be a symbol (canonicalized).
   int FunctionContextSlotIndex(String* name, VariableMode* mode);
 
-  static Handle<SerializedScopeInfo> Create(Scope* scope);
+  static Handle<ScopeInfo> Create(Scope* scope);
 
   // Serializes empty scope info.
-  static SerializedScopeInfo* Empty();
+  static ScopeInfo* Empty();
 
- private:
-  Object** ContextEntriesAddr();
+#ifdef DEBUG
+  void Print();
+#endif
 
-  Object** ParameterEntriesAddr();
+#define SCOPE_INFO_NUMERIC_FIELDS(V)                 \

[Kevin Millikin (Chromium), 2011/11/02 16:31:19]

OK to use a macro to generate these, but there's no need for the higher-order
macro (which isn't undef'd here, kind of scary).  It isn't much worse to have

#define FIELD_ACCESSORS(index, name) \
...
FIELD_ACCESSORS(FLAGS, flags)
FIELD_ACCESSORS(NUM_PARAMETERS, num_parameters)
...
#undef FIELD_ACCESSORS

Where I've taken advantage of the default being 0 for all of them and of the
ability to generate the _INDEX part of the name in the macro.

[Steven, 2011/11/02 18:52:59]

Kept the higher order macro but now it generates the Index part of the name and
the default value 0. It's also undef'd now :)

On 2011/11/02 16:31:19, Kevin Millikin wrote:

+  V(FLAGS_INDEX, flags, 0)                           \
+  V(NUM_PARAMETERS_INDEX, num_parameters, 0)         \
+  V(NUM_STACK_LOCALS_INDEX, num_stack_locals, 0)     \
+  V(NUM_CONTEXT_LOCALS_INDEX, num_context_locals, 0)
 
-  Object** StackSlotEntriesAddr();
+#define SCOPE_INFO_NUMERIC_FIELD_ACCESSORS(index, name, default)  \
+  void set_##name(int value) {                                    \
+    set(index, Smi::FromInt(value));                              \
+  }                                                               \
+  int name() {                                                    \
+    if (length() > 0) {                                           \
+      return Smi::cast(get(index))->value();                      \
+    } else {                                                      \
+      return default;                                             \
+    }                                                             \
+  }
+  SCOPE_INFO_NUMERIC_FIELDS(SCOPE_INFO_NUMERIC_FIELD_ACCESSORS)
+#undef SCOPE_INFO_NUMERIC_FIELD_ACCESSORS
+
+  // Layout description of the fixed part of a ScopeInfo.
+  enum {

[Kevin Millikin (Chromium), 2011/11/02 16:31:19]

Or, you could keep the higher-order macro, and use it here.  I'd rename it and
also rename the enum tags to fit the current style guide:

enum {
#define DECL_INDEX(index, name) k##index,
  FOR_EACH_NUMERIC_FIELD(DECL_INDEX)
#undef DECL_INDEX
  kVariablePartIndex
};

And you might as well make this enum private (since it's not named it can't
really be part of an signature anyway), and also FunctionVariableInfo, and all
the classes defining the bit fields.

[Steven, 2011/11/02 18:52:59]

Done.

+    FLAGS_INDEX,
+    // The number of parameters for the function.
+    NUM_PARAMETERS_INDEX,
+    // The number of non-parameter variables allocated on the stack.
+    NUM_STACK_LOCALS_INDEX,
+    // The number of non-parameter and parameter variables
+    // allocated in the context.
+    NUM_CONTEXT_LOCALS_INDEX,
+    // Beginning of variable sized part of ScopeInfo.
+    VARIABLE_PART_INDEX
+  };
+
+  // The layout of the variable part of a ScopeInfo is as follows:
+  // 1. ParameterEntries:
+  //    This part stores the names of the parameters for function scopes. One
+  //    slot is used per parameter, so in total this part occupies
+  //    num_parameters() slots in the array. For other scopes than function
+  //    scopes num_parameters() is 0.
+  // 2. StackLocalEntries:
+  //    Contains the names of local variables that are allocated on the stack,
+  //    in increasing order of the stack slot index. One slot is used per
+  //    stack local, so in total this part occupies num_stack_locals() slots
+  //    in the array.
+  // 3. ContextLocalNameEntries:
+  //    Contains the names of local variables and parameters that are allocated
+  //    in the context. They are stored in increasing order of the context slot
+  //    index starting with Context::MIN_CONTEXT_SLOTS. One slot is used per
+  //    context local, so in total this part occupies num_context_locals()
+  //    slots in the array.
+  // 4. ContextLocalModeEntries:
+  //    Contains the variable modes corresponding to the context locals in
+  //    ContextLocalNameEntries. One slot is used per context local, so in total
+  //    this part occupies num_context_locals() slots in the array.
+  // 5. FunctionNameEntryIndex:
+  //    If the scope belongs to a named function expression this part contains
+  //    information about the function variable. It always occupies two array
+  //    slots:  a. The name of the function variable.
+  //            b. The context or stack slot index for the variable.
+  int ParameterEntriesIndex();
+  int StackLocalEntriesIndex();
+  int ContextLocalNameEntriesIndex();
+  int ContextLocalModeEntriesIndex();
+  int FunctionNameEntryIndex();
+
+  // Location of the function variable for named function expressions.
+  enum FunctionVariableInfo {
+    NONE,     // No function name present.
+    STACK,    // Function
+    CONTEXT,
+    UNUSED
+  };
+
+  // Properties of scopes.
+  class TypeField:             public BitField<ScopeType,            0, 3> {};
+  class CallsEvalField:        public BitField<bool,                 3, 1> {};
+  class StrictModeField:       public BitField<bool,                 4, 1> {};
+  class FunctionVariableField: public BitField<FunctionVariableInfo, 5, 2> {};
+  class FunctionVariableMode:  public BitField<VariableMode,         7, 3> {};
 };
 
 
 // The cache for maps used by normalized (dictionary mode) objects.
 // Such maps do not have property descriptors, so a typical program
 // needs very limited number of distinct normalized maps.
 class NormalizedMapCache: public FixedArray {
  public:
   static const int kEntries = 64;
 
@@ skipping 1502 matching lines @@
 // shared by multiple instances of the function.
 class SharedFunctionInfo: public HeapObject {
  public:
   // [name]: Function name.
   DECL_ACCESSORS(name, Object)
 
   // [code]: Function code.
   DECL_ACCESSORS(code, Code)
 
   // [scope_info]: Scope info.
-  DECL_ACCESSORS(scope_info, SerializedScopeInfo)
+  DECL_ACCESSORS(scope_info, ScopeInfo)
 
   // [construct stub]: Code stub for constructing instances of this function.
   DECL_ACCESSORS(construct_stub, Code)
 
   inline Code* unchecked_code();
 
   // Returns if this function has been compiled to native code yet.
   inline bool is_compiled();
 
   // [length]: The function length - usually the number of declared parameters.
@@ skipping 3107 matching lines @@
     } else {
       value &= ~(1 << bit_position);
     }
     return value;
   }
 };
 
 } }  // namespace v8::internal
 
 #endif  // V8_OBJECTS_H_