Remove comparison operator and helper function from AstRawString interface

These comparisons are only meant to be done by AstValueFactory itself (in
its string_table_ operations), so make the Compare() function a private
member of AstValueFactory.

All other clients of AstRawStrings should compare them by pointer value.
There were only two clients which failed to abide by this rule, one
recently-added (in ModuleDescriptor) and the other in Literal::Match
(in ast.cc, added in https://code.google.com/p/v8/source/detail?r=24396).

Review URL: https://codereview.chromium.org/1069423003

Cr-Commit-Position: refs/heads/master@{#27715}

src/ast-value-factory.cc

@@ -114,19 +114,6 @@ bool AstRawString::IsOneByteEqualTo(const char* data) const {
 }
 
 
-bool AstRawString::Compare(void* a, void* b) {
-  return *static_cast<AstRawString*>(a) == *static_cast<AstRawString*>(b);
-}
-
-bool AstRawString::operator==(const AstRawString& rhs) const {
-  if (is_one_byte_ != rhs.is_one_byte_) return false;
-  if (hash_ != rhs.hash_) return false;
-  int len = literal_bytes_.length();
-  if (rhs.literal_bytes_.length() != len) return false;
-  return memcmp(literal_bytes_.start(), rhs.literal_bytes_.start(), len) == 0;
-}
-
-
 void AstConsString::Internalize(Isolate* isolate) {
   // AstRawStrings are internalized before AstConsStrings so left and right are
   // already internalized.
@@ -382,4 +369,13 @@ AstRawString* AstValueFactory::GetString(uint32_t hash, bool is_one_byte,
 }
 
 
+bool AstValueFactory::AstRawStringCompare(void* a, void* b) {
+  const AstRawString* lhs = static_cast<AstRawString*>(a);
+  const AstRawString* rhs = static_cast<AstRawString*>(b);
+  if (lhs->is_one_byte() != rhs->is_one_byte()) return false;
+  if (lhs->hash() != rhs->hash()) return false;
+  int len = lhs->byte_length();
+  if (rhs->byte_length() != len) return false;
+  return memcmp(lhs->raw_data(), rhs->raw_data(), len) == 0;
+}
 } }  // namespace v8::internal

src/ast-value-factory.h

@@ -70,6 +70,8 @@ class AstRawString : public AstString {
     return literal_bytes_.length() / 2;
   }
 
+  int byte_length() const { return literal_bytes_.length(); }
+
   void Internalize(Isolate* isolate) OVERRIDE;
 
   bool AsArrayIndex(uint32_t* index) const;
@@ -92,9 +94,6 @@ class AstRawString : public AstString {
   uint32_t hash() const {
     return hash_;
   }
-  static bool Compare(void* a, void* b);
-
-  bool operator==(const AstRawString& rhs) const;
 
  private:
   friend class AstValueFactory;
@@ -281,7 +280,7 @@ class AstValue : public ZoneObject {
 class AstValueFactory {
  public:
   AstValueFactory(Zone* zone, uint32_t hash_seed)
-      : string_table_(AstRawString::Compare),
+      : string_table_(AstRawStringCompare),
         zone_(zone),
         isolate_(NULL),
         hash_seed_(hash_seed) {
@@ -344,6 +343,8 @@ class AstValueFactory {
   AstRawString* GetString(uint32_t hash, bool is_one_byte,
                           Vector<const byte> literal_bytes);
 
+  static bool AstRawStringCompare(void* a, void* b);
+
   // All strings are copied here, one after another (no NULLs inbetween).
   HashMap string_table_;
   // For keeping track of all AstValues and AstRawStrings we've created (so that

src/ast.cc

@@ -1004,7 +1004,7 @@ uint32_t Literal::Hash() {
 bool Literal::Match(void* literal1, void* literal2) {
   const AstValue* x = static_cast<Literal*>(literal1)->raw_value();
   const AstValue* y = static_cast<Literal*>(literal2)->raw_value();
-  return (x->IsString() && y->IsString() && *x->AsString() == *y->AsString()) ||
+  return (x->IsString() && y->IsString() && x->AsString() == y->AsString()) ||
          (x->IsNumber() && y->IsNumber() && x->AsNumber() == y->AsNumber());
 }
 

src/modules.cc

@@ -20,8 +20,9 @@ void ModuleDescriptor::AddLocalExport(const AstRawString* export_name,
   ZoneAllocationPolicy allocator(zone);
 
   if (exports_ == nullptr) {
-    exports_ = new (zone->New(sizeof(ZoneHashMap))) ZoneHashMap(
-        AstRawString::Compare, ZoneHashMap::kDefaultHashMapCapacity, allocator);
+    exports_ = new (zone->New(sizeof(ZoneHashMap)))
+        ZoneHashMap(ZoneHashMap::PointersMatch,
+                    ZoneHashMap::kDefaultHashMapCapacity, allocator);
   }
 
   ZoneHashMap::Entry* p =

test/cctest/test-parsing.cc

@@ -5450,13 +5450,11 @@ TEST(ModuleParsingInternals) {
   i::Handle<i::Script> script = factory->NewScript(source);
   i::Zone zone;
   i::ParseInfo info(&zone, script);
-  i::AstValueFactory avf(&zone, isolate->heap()->HashSeed());
   i::Parser parser(&info);
   parser.set_allow_harmony_modules(true);
   info.set_module();
   CHECK(parser.Parse(&info));
   CHECK(i::Compiler::Analyze(&info));
-
   i::FunctionLiteral* func = info.function();
   i::Scope* module_scope = func->scope();
   i::Scope* outer_scope = module_scope->outer_scope();
@@ -5466,11 +5464,11 @@ TEST(ModuleParsingInternals) {
   CHECK(module_scope->is_module_scope());
   CHECK_NOT_NULL(module_scope->module_var());
   CHECK_EQ(i::INTERNAL, module_scope->module_var()->mode());
-
   i::ModuleDescriptor* descriptor = module_scope->module();
   CHECK_NOT_NULL(descriptor);
   CHECK_EQ(1, descriptor->Length());
-  const i::AstRawString* export_name = avf.GetOneByteString("y");
+  const i::AstRawString* export_name =
+      info.ast_value_factory()->GetOneByteString("y");
   const i::AstRawString* local_name =
       descriptor->LookupLocalExport(export_name, &zone);
   CHECK_NOT_NULL(local_name);