[intl] Switch to using declared accessors

This patch cleans up the Intl code by switching to using declared
accessors, rather than embedder fields, for holding references to
ICU objects. Additionally:
- Rename classes to be more similar to how other classes are named
- Make some unreachable paths into check-fails, rather than throwing
  JS exceptions
- Move some macros from objects-inl.h into object-macros.h, to allow
  the implementation here to not touch objects.h
- Some setup logic is moved from runtime-i18n.cc to i18n.cc.

This patch leaves type tags as they are; a future patch should move
from a special Intl type tagging system to object types as other system
objects use. Future patches should also move more logic to i18n.cc

BUG=v8:5402,v8:5751,v8:6057
CQ_INCLUDE_TRYBOTS=master.tryserver.v8:v8_linux_noi18n_rel_ng

Change-Id: Ia9cbb25cf8f52662e3deb15e64179d792c10842c
Reviewed-on: https://chromium-review.googlesource.com/479651
Commit-Queue: Daniel Ehrenberg <littledan@chromium.org>
Reviewed-by: Adam Klein <adamk@chromium.org>
Reviewed-by: Michael Starzinger <mstarzinger@chromium.org>
Cr-Commit-Position: refs/heads/master@{#44804}

BUILD.gn

@@ -1714,6 +1714,7 @@ v8_source_set("v8_base") {
     "src/objects/frame-array-inl.h",
     "src/objects/frame-array.h",
     "src/objects/hash-table.h",
+    "src/objects/intl-objects-inl.h",
     "src/objects/intl-objects.cc",
     "src/objects/intl-objects.h",
     "src/objects/literal-objects.cc",
@@ -2376,6 +2377,7 @@ v8_source_set("v8_base") {
       "src/builtins/builtins-intl.cc",
       "src/intl.cc",
       "src/intl.h",
+      "src/objects/intl-objects-inl.h",
       "src/objects/intl-objects.cc",
       "src/objects/intl-objects.h",
       "src/runtime/runtime-intl.cc",

src/bootstrapper.cc

@@ -2619,7 +2619,7 @@ void Genesis::InitializeGlobal(Handle<JSGlobalObject> global_object,
         factory->Object_string(),
         static_cast<PropertyAttributes>(DONT_ENUM | READ_ONLY));
     Handle<JSFunction> date_time_format_constructor = InstallFunction(
-        intl, "DateTimeFormat", JS_OBJECT_TYPE, DateFormat::kSize,
+        intl, "DateTimeFormat", JS_OBJECT_TYPE, JSIntlDateTimeFormat::kSize,
         date_time_format_prototype, Builtins::kIllegal);
     JSObject::AddProperty(date_time_format_prototype,
                           factory->constructor_string(),
@@ -2636,7 +2636,7 @@ void Genesis::InitializeGlobal(Handle<JSGlobalObject> global_object,
         factory->Object_string(),
         static_cast<PropertyAttributes>(DONT_ENUM | READ_ONLY));
     Handle<JSFunction> number_format_constructor = InstallFunction(
-        intl, "NumberFormat", JS_OBJECT_TYPE, NumberFormat::kSize,
+        intl, "NumberFormat", JS_OBJECT_TYPE, JSIntlNumberFormat::kSize,
         number_format_prototype, Builtins::kIllegal);
     JSObject::AddProperty(number_format_prototype,
                           factory->constructor_string(),
@@ -2653,7 +2653,7 @@ void Genesis::InitializeGlobal(Handle<JSGlobalObject> global_object,
         factory->Object_string(),
         static_cast<PropertyAttributes>(DONT_ENUM | READ_ONLY));
     Handle<JSFunction> collator_constructor =
-        InstallFunction(intl, "Collator", JS_OBJECT_TYPE, Collator::kSize,
+        InstallFunction(intl, "Collator", JS_OBJECT_TYPE, JSIntlCollator::kSize,
                         collator_prototype, Builtins::kIllegal);
     JSObject::AddProperty(collator_prototype, factory->constructor_string(),
                           collator_constructor, DONT_ENUM);
@@ -2668,7 +2668,7 @@ void Genesis::InitializeGlobal(Handle<JSGlobalObject> global_object,
         factory->Object_string(),
         static_cast<PropertyAttributes>(DONT_ENUM | READ_ONLY));
     Handle<JSFunction> v8_break_iterator_constructor = InstallFunction(
-        intl, "v8BreakIterator", JS_OBJECT_TYPE, V8BreakIterator::kSize,
+        intl, "v8BreakIterator", JS_OBJECT_TYPE, JSIntlV8BreakIterator::kSize,
         v8_break_iterator_prototype, Builtins::kIllegal);
     JSObject::AddProperty(v8_break_iterator_prototype,
                           factory->constructor_string(),

src/objects-inl.h

@@ -1284,15 +1284,6 @@ bool JSObject::PrototypeHasNoElements(Isolate* isolate, JSObject* object) {
   return true;
 }
 
-#define FIELD_ADDR(p, offset) \
-  (reinterpret_cast<byte*>(p) + offset - kHeapObjectTag)
-
-#define FIELD_ADDR_CONST(p, offset) \
-  (reinterpret_cast<const byte*>(p) + offset - kHeapObjectTag)
-
-#define READ_FIELD(p, offset) \
-  (*reinterpret_cast<Object* const*>(FIELD_ADDR_CONST(p, offset)))
-
 #define ACQUIRE_READ_FIELD(p, offset)           \
   reinterpret_cast<Object*>(base::Acquire_Load( \
       reinterpret_cast<const base::AtomicWord*>(FIELD_ADDR_CONST(p, offset))))
@@ -1301,9 +1292,6 @@ bool JSObject::PrototypeHasNoElements(Isolate* isolate, JSObject* object) {
   reinterpret_cast<Object*>(base::NoBarrier_Load( \
       reinterpret_cast<const base::AtomicWord*>(FIELD_ADDR_CONST(p, offset))))
 
-#define WRITE_FIELD(p, offset, value) \
-  (*reinterpret_cast<Object**>(FIELD_ADDR(p, offset)) = value)
-
 #define RELEASE_WRITE_FIELD(p, offset, value)                     \
   base::Release_Store(                                            \
       reinterpret_cast<base::AtomicWord*>(FIELD_ADDR(p, offset)), \
@@ -8343,11 +8331,7 @@ SMI_ACCESSORS(JSStringIterator, index, kNextIndexOffset)
 #undef NOBARRIER_SMI_ACCESSORS
 #undef BOOL_GETTER
 #undef BOOL_ACCESSORS
-#undef FIELD_ADDR
-#undef FIELD_ADDR_CONST
-#undef READ_FIELD
 #undef NOBARRIER_READ_FIELD
-#undef WRITE_FIELD
 #undef NOBARRIER_WRITE_FIELD
 #undef WRITE_BARRIER
 #undef CONDITIONAL_WRITE_BARRIER

src/objects/intl-objects-inl.h

+// Copyright 2017 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_INTL_SUPPORT
+#error Internationalization is expected to be enabled.
+#endif  // V8_INTL_SUPPORT
+
+#ifndef V8_OBJECTS_INTL_OBJECTS_INL_H_
+#define V8_OBJECTS_INTL_OBJECTS_INL_H_
+
+#include "src/objects/intl-objects.h"
+
+// Has to be the last include (doesn't have include guards):
+#include "src/objects/object-macros.h"
+
+#define PTR_ACCESSORS(holder, name, type, offset)   \
+  inline type* holder::name() const {               \
+    Object* obj = READ_FIELD(this, offset);         \
+    DCHECK(obj->IsSmi());                           \
+    return reinterpret_cast<type*>(obj);            \
+  }                                                 \
+  inline void holder::set_##name(type* value) {     \
+    Object* obj = reinterpret_cast<Object*>(value); \
+    DCHECK(obj->IsSmi());                           \
+    WRITE_FIELD(this, offset, obj);                 \
+  }
+
+namespace v8 {
+namespace internal {
+
+PTR_ACCESSORS(JSIntlDateTimeFormat, simple_date_format, icu::SimpleDateFormat,
+              kSimpleDateFormat)
+
+PTR_ACCESSORS(JSIntlNumberFormat, decimal_format, icu::DecimalFormat,
+              kDecimalFormat)
+
+PTR_ACCESSORS(JSIntlCollator, collator, icu::Collator, kCollator)
+
+PTR_ACCESSORS(JSIntlV8BreakIterator, break_iterator, icu::BreakIterator,
+              kBreakIterator)
+PTR_ACCESSORS(JSIntlV8BreakIterator, unicode_string, icu::UnicodeString,
+              kUnicodeString)
+
+}  // namespace internal
+}  // namespace v8
+
+#undef PTR_ACCESSORS
+
+#include "src/objects/object-macros-undef.h"
+
+#endif  // V8_OBJECTS_INTL_OBJECTS_INL_H_

src/objects/intl-objects.h

@@ -17,103 +17,98 @@ class BreakIterator;
 class Collator;
 class DecimalFormat;
 class SimpleDateFormat;
+class UnicodeString;
 }
 
 namespace v8 {
 namespace internal {
 
-template <typename T>
-class Handle;
+#define DECL_PTR_ACCESSORS(name, type) \
+  inline type* name() const;           \
+  inline void set_##name(type* value);
 
-class DateFormat {
+// Intl.DateTimeFormat
+// ECMA-402#datetimeformat-objects
+class JSIntlDateTimeFormat : public JSObject {
  public:
-  // Create a formatter for the specificied locale and options. Returns the
-  // resolved settings for the locale / options.
-  static icu::SimpleDateFormat* InitializeDateTimeFormat(
+  DECL_PTR_ACCESSORS(simple_date_format, icu::SimpleDateFormat)
+
+  // Constructor for Intl.DateTimeFormat(), based on the resolved locale
+  // and user options. Writes resolved options into resolved.
+  MUST_USE_RESULT static MaybeHandle<JSIntlDateTimeFormat> New(
       Isolate* isolate, Handle<String> locale, Handle<JSObject> options,
       Handle<JSObject> resolved);
 
-  // Unpacks date format object from corresponding JavaScript object.
-  static icu::SimpleDateFormat* UnpackDateFormat(Isolate* isolate,
-                                                 Handle<JSObject> obj);
-
-  // Release memory we allocated for the DateFormat once the JS object that
-  // holds the pointer gets garbage collected.
-  static void DeleteDateFormat(const v8::WeakCallbackInfo<void>& data);
-
   // Layout description.
   static const int kSimpleDateFormat = JSObject::kHeaderSize;
   static const int kSize = kSimpleDateFormat + kPointerSize;
 
  private:
-  DateFormat();
+  // Finalizer responsible for freeing the icu::SimpleDateFormat
+  static void Delete(const v8::WeakCallbackInfo<void>& data);
+
+  DISALLOW_IMPLICIT_CONSTRUCTORS(JSIntlDateTimeFormat);
 };
 
-class NumberFormat {
+// Intl.NumberFormat
+// ECMA-402#numberformat-objects
+class JSIntlNumberFormat : public JSObject {
  public:
-  // Create a formatter for the specificied locale and options. Returns the
-  // resolved settings for the locale / options.
-  static icu::DecimalFormat* InitializeNumberFormat(Isolate* isolate,
-                                                    Handle<String> locale,
-                                                    Handle<JSObject> options,
-                                                    Handle<JSObject> resolved);
-
-  // Unpacks number format object from corresponding JavaScript object.
-  static icu::DecimalFormat* UnpackNumberFormat(Isolate* isolate,
-                                                Handle<JSObject> obj);
+  DECL_PTR_ACCESSORS(decimal_format, icu::DecimalFormat)
 
-  // Release memory we allocated for the NumberFormat once the JS object that
-  // holds the pointer gets garbage collected.
-  static void DeleteNumberFormat(const v8::WeakCallbackInfo<void>& data);
+  // Constructor for Intl.NumberFormat(), based on the resolved locale
+  // and user options. Writes resolved options into resolved.
+  MUST_USE_RESULT static MaybeHandle<JSIntlNumberFormat> New(
+      Isolate* isolate, Handle<String> locale, Handle<JSObject> options,
+      Handle<JSObject> resolved);
 
   // Layout description.
   static const int kDecimalFormat = JSObject::kHeaderSize;
   static const int kSize = kDecimalFormat + kPointerSize;
 
  private:
-  NumberFormat();
+  // Finalizer responsible for freeing the icu::DecimalFormat
+  static void Delete(const v8::WeakCallbackInfo<void>& data);
+
+  DISALLOW_IMPLICIT_CONSTRUCTORS(JSIntlNumberFormat);
 };
 
-class Collator {
+// Intl.Collator
+// ECMA-402#collator-objects
+class JSIntlCollator : public JSObject {
  public:
-  // Create a collator for the specificied locale and options. Returns the
-  // resolved settings for the locale / options.
-  static icu::Collator* InitializeCollator(Isolate* isolate,
-                                           Handle<String> locale,
-                                           Handle<JSObject> options,
-                                           Handle<JSObject> resolved);
-
-  // Unpacks collator object from corresponding JavaScript object.
-  static icu::Collator* UnpackCollator(Isolate* isolate, Handle<JSObject> obj);
+  DECL_PTR_ACCESSORS(collator, icu::Collator)
 
-  // Release memory we allocated for the Collator once the JS object that holds
-  // the pointer gets garbage collected.
-  static void DeleteCollator(const v8::WeakCallbackInfo<void>& data);
+  // Constructor for Intl.Collator(), based on the resolved locale
+  // and user options. Writes resolved options into resolved.
+  MUST_USE_RESULT static MaybeHandle<JSIntlCollator> New(
+      Isolate* isolate, Handle<String> locale, Handle<JSObject> options,
+      Handle<JSObject> resolved);
 
   // Layout description.
   static const int kCollator = JSObject::kHeaderSize;
   static const int kSize = kCollator + kPointerSize;
 
  private:
-  Collator();
+  // Finalizer responsible for freeing the icu::Collator
+  static void Delete(const v8::WeakCallbackInfo<void>& data);
+
+  DISALLOW_IMPLICIT_CONSTRUCTORS(JSIntlCollator);
 };
 
-class V8BreakIterator {
+// Intl.v8BreakIterator, Custom non-standard V8 word break binding
+// TODO(littledan,jwolfe): Specify, implement and ship Intl.Segmenter,
+// allowing this interface to be deprecasted and removed.
+class JSIntlV8BreakIterator : public JSObject {
  public:
-  // Create a BreakIterator for the specificied locale and options. Returns the
-  // resolved settings for the locale / options.
-  static icu::BreakIterator* InitializeBreakIterator(Isolate* isolate,
-                                                     Handle<String> locale,
-                                                     Handle<JSObject> options,
-                                                     Handle<JSObject> resolved);
-
-  // Unpacks break iterator object from corresponding JavaScript object.
-  static icu::BreakIterator* UnpackBreakIterator(Isolate* isolate,
-                                                 Handle<JSObject> obj);
+  DECL_PTR_ACCESSORS(break_iterator, icu::BreakIterator)
+  DECL_PTR_ACCESSORS(unicode_string, icu::UnicodeString)
 
-  // Release memory we allocated for the BreakIterator once the JS object that
-  // holds the pointer gets garbage collected.
-  static void DeleteBreakIterator(const v8::WeakCallbackInfo<void>& data);
+  // Constructor for Intl.v8BreakIterator(), based on the resolved locale
+  // and user options. Writes resolved options into resolved.
+  MUST_USE_RESULT static MaybeHandle<JSIntlV8BreakIterator> New(
+      Isolate* isolate, Handle<String> locale, Handle<JSObject> options,
+      Handle<JSObject> resolved);
 
   // Layout description.
   static const int kBreakIterator = JSObject::kHeaderSize;
@@ -121,9 +116,15 @@ class V8BreakIterator {
   static const int kSize = kUnicodeString + kPointerSize;
 
  private:
-  V8BreakIterator();
+  // Finalizer responsible for freeing the icu::BreakIterator
+  // and icu::UnicodeString
+  static void Delete(const v8::WeakCallbackInfo<void>& data);
+
+  DISALLOW_IMPLICIT_CONSTRUCTORS(JSIntlV8BreakIterator);
 };
 
+#undef DECL_PTR_ACCESSORS
+
 }  // namespace internal
 }  // namespace v8
 

src/objects/object-macros-undef.h

@@ -8,3 +8,7 @@
 #undef DECL_ACCESSORS
 #undef DECLARE_CAST
 #undef DECLARE_VERIFIER
+#undef FIELD_ADDR
+#undef FIELD_ADDR_CONST
+#undef READ_FIELD
+#undef WRITE_FIELD

src/objects/object-macros.h

@@ -40,3 +40,15 @@
 #else
 #define DECLARE_VERIFIER(Name)
 #endif
+
+#define FIELD_ADDR(p, offset) \
+  (reinterpret_cast<byte*>(p) + offset - kHeapObjectTag)
+
+#define FIELD_ADDR_CONST(p, offset) \
+  (reinterpret_cast<const byte*>(p) + offset - kHeapObjectTag)
+
+#define READ_FIELD(p, offset) \
+  (*reinterpret_cast<Object* const*>(FIELD_ADDR_CONST(p, offset)))
+
+#define WRITE_FIELD(p, offset, value) \
+  (*reinterpret_cast<Object**>(FIELD_ADDR(p, offset)) = value)

src/v8.gyp

@@ -1124,6 +1124,7 @@
         'objects/frame-array-inl.h',
         'objects/hash-table.h',
         'objects/intl-objects.cc',
+        'objects/intl-objects-inl.h',
         'objects/intl-objects.h',
         'objects/literal-objects.cc',
         'objects/literal-objects.h',
@@ -1834,6 +1835,7 @@
             'intl.cc',
             'intl.h',
             'objects/intl-objects.cc',
+            'objects/intl-objects-inl.h',
             'objects/intl-objects.h',
             'runtime/runtime-intl.cc',
           ],