Refactor ScriptData class for cached compile data.

src/parser.h

 // Copyright 2012 the V8 project authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #ifndef V8_PARSER_H_
 #define V8_PARSER_H_
 
 #include "src/allocation.h"
 #include "src/ast.h"
 #include "src/compiler.h"  // For CachedDataMode
@@ skipping 41 matching lines @@
     return static_cast<StrictMode>(backing_[kStrictModeIndex]);
   }
 
   bool is_valid() { return !backing_.is_empty(); }
 
  private:
   Vector<unsigned> backing_;
 };
 
 
-class ScriptData {
+// Wrapper around ScriptData to provide parser-specific functionality,

[vogelheim, 2014/07/09 17:23:36]

super nitpick: '.' instead of ','.

[Yang, 2014/07/10 08:28:46]

Done.

+class ParseData {
  public:
-  explicit ScriptData(Vector<unsigned> store)
-      : store_(store),
-        owns_store_(true) { }
-
-  ScriptData(Vector<unsigned> store, bool owns_store)
-      : store_(store),
-        owns_store_(owns_store) { }
-
-  // The created ScriptData won't take ownership of the data. If the alignment
-  // is not correct, this will copy the data (and the created ScriptData will
-  // take ownership of the copy).
-  static ScriptData* New(const char* data, int length, bool owns_store = false);
-
-  virtual ~ScriptData();
-  virtual int Length();
-  virtual const char* Data();
-  virtual bool HasError();
-
+  explicit ParseData(ScriptData* script_data) : script_data_(script_data) {
+    CHECK(IsAligned(script_data->length(), sizeof(unsigned)));
+    CHECK(Sanity());
+  }
   void Initialize();
-  void ReadNextSymbolPosition();
-
   FunctionEntry GetFunctionEntry(int start);
   int GetSymbolIdentifier();
-  bool SanityCheck();
+  int FunctionCount();
 
   Scanner::Location MessageLocation() const;
   bool IsReferenceError() const;
-  const char* BuildMessage() const;
-  const char* BuildArg() const;
+  const char* BuildMessage();
+  const char* BuildArg();
+  bool HasError() const;
 
-  int function_count() {
-    int functions_size =
-        static_cast<int>(store_[PreparseDataConstants::kFunctionsSizeOffset]);
-    if (functions_size < 0) return 0;
-    if (functions_size % FunctionEntry::kSize != 0) return 0;
-    return functions_size / FunctionEntry::kSize;
+  unsigned* Data() const {  // Writable data as unsigned int array.
+    return reinterpret_cast<unsigned*>(const_cast<byte*>(script_data_->data()));

[vogelheim, 2014/07/09 17:23:36]

Why is this a const method?

(I guess one can define const-ness this way, in that the actual object isn't
changed. But I don't think const-access to a pointer that allows to manipulate
the dependent state is a super readable design. If you really want to do that,
maybe declare script_data_ mutable.)

[Yang, 2014/07/10 08:28:46]

Done.

   }
-  // The following functions should only be called if SanityCheck has
-  // returned true.
-  bool has_error() { return store_[PreparseDataConstants::kHasErrorOffset]; }
-  unsigned magic() { return store_[PreparseDataConstants::kMagicOffset]; }
-  unsigned version() { return store_[PreparseDataConstants::kVersionOffset]; }
 
  private:
-  // Disable copying and assigning; because of owns_store they won't be correct.
-  ScriptData(const ScriptData&);
-  ScriptData& operator=(const ScriptData&);
-
-  friend class v8::ScriptCompiler;
-  Vector<unsigned> store_;
-  unsigned char* symbol_data_;
-  unsigned char* symbol_data_end_;
-  int function_index_;
-  bool owns_store_;
-
+  bool Sanity();

[vogelheim, 2014/07/09 17:23:36]

nitpick: IsSane() ?

(I can see how CHECK(Sanity()) is attractive, but I think IsSane() would be the
more consistent naming. Either way, you pick...)

[Yang, 2014/07/10 08:28:46]

Done.

+  unsigned Magic() const;
+  unsigned Version() const;
+  int FunctionsSize() const;
   unsigned Read(int position) const;
   unsigned* ReadAddress(int position) const;
-  // Reads a number from the current symbols
   int ReadNumber(byte** source);
+  const char* ReadString(unsigned* start, int* chars);
+  int Length() const { return script_data_->length() / sizeof(unsigned); }

[vogelheim, 2014/07/09 17:23:36]

nitpick: round up?

[Yang, 2014/07/10 08:28:46]

No need. Sanity check makes sure the length is a multiple of pointer size. Added
comment.

 
-  // Read strings written by ParserRecorder::WriteString.
-  static const char* ReadString(unsigned* start, int* chars);
+  ScriptData* script_data_;
+  byte* symbol_data_;
+  byte* symbol_data_end_;
+  int function_index_;
+
+  DISALLOW_COPY_AND_ASSIGN(ParseData);
 };
 
-
 // ----------------------------------------------------------------------------
 // REGEXP PARSING
 
 // A BufferedZoneList is an automatically growing list, just like (and backed
 // by) a ZoneList, that is optimized for the case of adding and removing
 // a single element. The last element added is stored outside the backing list,
 // and if no more than one element is ever added, the ZoneList isn't even
 // allocated.
 // Elements must not be NULL pointers.
 template <typename T, int initial_size>
@@ skipping 500 matching lines @@
   FunctionLiteral* ParseLazy();
   FunctionLiteral* ParseLazy(Utf16CharacterStream* source);
 
   Isolate* isolate() { return isolate_; }
   CompilationInfo* info() const { return info_; }
 
   // Called by ParseProgram after setting up the scanner.
   FunctionLiteral* DoParseProgram(CompilationInfo* info,
                                   Handle<String> source);
 
-  // Report syntax error
-  void ReportInvalidCachedData(const AstRawString* name, bool* ok);
-
-  void SetCachedData(ScriptData** data,
-                     CachedDataMode cached_data_mode) {
-    cached_data_mode_ = cached_data_mode;
-    if (cached_data_mode == NO_CACHED_DATA) {
-      cached_data_ = NULL;
-    } else {
-      ASSERT(data != NULL);
-      cached_data_ = data;
-    }
-  }
+  void SetCachedData();
 
   bool inside_with() const { return scope_->inside_with(); }
-  ScriptData** cached_data() const { return cached_data_; }
-  CachedDataMode cached_data_mode() const { return cached_data_mode_; }
+  CachedDataMode cached_data_mode() const { return info_->cached_data_mode(); }
   Scope* DeclarationScope(VariableMode mode) {
     return IsLexicalVariableMode(mode)
         ? scope_ : scope_->DeclarationScope();
   }
 
   // All ParseXXX functions take as the last argument an *ok parameter
   // which is set to false if parsing failed; it is unchanged otherwise.
   // By making the 'exception handling' explicit, we are forced to check
   // for failure at the call sites.
   void* ParseSourceElements(ZoneList<Statement*>* processor, int end_token,
@@ skipping 126 matching lines @@
 
   void InternalizeUseCounts();
 
   Isolate* isolate_;
 
   Handle<Script> script_;
   Scanner scanner_;
   PreParser* reusable_preparser_;
   Scope* original_scope_;  // for ES5 function declarations in sloppy eval
   Target* target_stack_;  // for break, continue statements
-  ScriptData** cached_data_;
+  ParseData* cached_parse_data_;
   CachedDataMode cached_data_mode_;
   AstValueFactory* ast_value_factory_;
 
   CompilationInfo* info_;
 
   // Pending errors.
   bool has_pending_error_;
   Scanner::Location pending_error_location_;
   const char* pending_error_message_;
   const AstRawString* pending_error_arg_;
@@ skipping 28 matching lines @@
  private:
   static const int kLiteralTypeSlot = 0;
   static const int kElementsSlot = 1;
 
   DISALLOW_IMPLICIT_CONSTRUCTORS(CompileTimeValue);
 };
 
 } }  // namespace v8::internal
 
 #endif  // V8_PARSER_H_