chakrashim: a few refactors for clang

Found a few minor issues while trying to compile with clang:
- `Global` implementation had `this.xx`. Should be `this->xx`.
- clang requires forward declaration of `ObjectData` for its usage. Rather
  than forwarding this private symbol, I refactored the code to hide it
  from public header.

Ran cpplint and fixed accordingly on the changed files.

PR-URL: #94
Reviewed-By: Kunal Pathak <Kunal.Pathak@microsoft.com>

deps/chakrashim/include/v8.h

@@ -291,7 +291,6 @@ class Local {
   friend class Number;
   friend class NumberObject;
   friend class Object;
-  friend class ObjectData;
   friend class ObjectTemplate;
   friend class Private;
   friend class Proxy;
@@ -462,7 +461,7 @@ V8_EXPORT void SetObjectWeakReferenceCallback(
 // A helper method for turning off the WeakReferenceCallback that was set using
 // the previous method
 V8_EXPORT void ClearObjectWeakReferenceCallback(JsValueRef object, bool revive);
-}
+}  // namespace chakrashim
 
 enum class WeakCallbackType { kParameter, kInternalFields };
 
@@ -620,7 +619,6 @@ class Persistent : public PersistentBase<T> {
 
  private:
   friend class Object;
-  friend class ObjectData;
   friend class ObjectTemplate;
   friend class ObjectTemplateData;
   friend class TemplateData;
@@ -669,7 +667,7 @@ class Global : public PersistentBase<T> {
   }
 
   V8_INLINE Global(Global&& other) : PersistentBase<T>(other.val_) {
-    this._weakWrapper = other._weakWrapper;
+    this->_weakWrapper = other._weakWrapper;
     other.val_ = nullptr;
     other._weakWrapper.reset();
   }
@@ -692,8 +690,8 @@ class Global : public PersistentBase<T> {
   Global Pass() { return static_cast<Global&&>(*this); }
 
  private:
-  Global(Global&) = delete;
-  void operator=(Global&) = delete;
+  Global(const Global&) = delete;
+  void operator=(const Global&) = delete;
 };
 
 
@@ -830,10 +828,10 @@ class V8_EXPORT ScriptCompiler {
       BufferOwned
     };
 
-   const uint8_t* data;
-   int length;
-   bool rejected = true;
-   BufferPolicy buffer_policy;
+    const uint8_t* data;
+    int length;
+    bool rejected = true;
+    BufferPolicy buffer_policy;
 
     CachedData();
     CachedData(const uint8_t* data, int length,
@@ -1044,13 +1042,13 @@ class V8_EXPORT Value : public Data {
 };
 
 class V8_EXPORT Private : public Data {
-public:
+ public:
   Local<Value> Name() const;
   static Local<Private> New(Isolate* isolate,
                             Local<String> name = Local<String>());
   static Local<Private> ForApi(Isolate* isolate, Local<String> name);
 
-private:
+ private:
   Private();
 };
 
@@ -1432,7 +1430,6 @@ class V8_EXPORT Object : public Value {
   static Object *Cast(Value *obj);
 
  private:
-  friend class ObjectData;
   friend class ObjectTemplate;
   friend class Utils;
 
@@ -1446,7 +1443,6 @@ class V8_EXPORT Object : public Value {
                           PropertyAttribute attribute,
                           Handle<AccessorSignature> signature);
 
-  JsErrorCode GetObjectData(ObjectData** objectData);
   ObjectTemplate* GetObjectTemplate();
 };
 
@@ -1537,6 +1533,7 @@ class ReturnValue {
   Isolate* GetIsolate() { return Isolate::GetCurrent(); }
 
   Value* Get() const { return *_value; }
+
  private:
   explicit ReturnValue(Value** value)
     : _value(value) {
@@ -1681,7 +1678,7 @@ class V8_EXPORT Promise : public Object {
 };
 
 class V8_EXPORT Proxy : public Object {
-public:
+ public:
   Local<Object> GetTarget();
   Local<Value> GetHandler();
   bool IsRevoked();
@@ -1693,7 +1690,7 @@ class V8_EXPORT Proxy : public Object {
 
   V8_INLINE static Proxy* Cast(Value* obj);
 
-private:
+ private:
   Proxy();
   static void CheckCast(Value* obj);
 };
@@ -1736,6 +1733,7 @@ class V8_EXPORT ArrayBuffer : public Object {
   Contents GetContents();
 
   static ArrayBuffer* Cast(Value* obj);
+
  private:
   ArrayBuffer();
 };
@@ -2126,15 +2124,15 @@ class V8_EXPORT HeapStatistics {
 };
 
 class V8_EXPORT HeapSpaceStatistics {
-public:
+ public:
   HeapSpaceStatistics() {}
   const char* space_name() { return ""; }
   size_t space_size() { return 0; }
   size_t space_used_size() { return 0; }
   size_t space_available_size() { return 0; }
   size_t physical_space_size() { return 0; }
 
-private:
+ private:
   const char* space_name_;
   size_t space_size_;
   size_t space_used_size_;

deps/chakrashim/src/v8chakra.h

@@ -45,7 +45,7 @@ class ExternalData {
   const ExternalDataTypes type;
 
  public:
-  ExternalData(ExternalDataTypes type) : type(type) {}
+  explicit ExternalData(ExternalDataTypes type) : type(type) {}
   virtual ~ExternalData() {}
 
   ExternalDataTypes GetType() const { return type; }
@@ -218,6 +218,14 @@ class Utils {
     unsigned short argumentCount,
     void *callbackState);
 
+  // Create a Local<T> internally (use private constructor)
+  template <class T>
+  static Local<T> ToLocal(T* that) {
+    return Local<T>(that);
+  }
+
+  static JsErrorCode GetObjectData(Object* object, ObjectData** objectData);
+
   static bool IsInstanceOf(Object* obj, ObjectTemplate* objectTemplate) {
     return obj->GetObjectTemplate() == objectTemplate;
   }
@@ -239,12 +247,12 @@ class Utils {
                                 size_t byte_offset, size_t length);
 
   static ObjectTemplate* EnsureObjectTemplate(
-      Persistent<ObjectTemplate>& objectTemplate) {
-    if (objectTemplate.IsEmpty()) {
+      Persistent<ObjectTemplate>* objectTemplate) {
+    if (objectTemplate->IsEmpty()) {
       // Create ObjectTemplate lazily
-      objectTemplate = ObjectTemplate::New(nullptr);
+      *objectTemplate = ObjectTemplate::New(nullptr);
     }
-    return *objectTemplate;
+    return **objectTemplate;
   }
 };
 

deps/chakrashim/src/v8functiontemplate.cc

@@ -28,8 +28,8 @@ using jsrt::ContextShim;
 
 class FunctionCallbackData : public ExternalData {
  public:
-   static const ExternalDataTypes ExternalDataType =
-      ExternalDataTypes::FunctionCallbackData;
+  static const ExternalDataTypes ExternalDataType =
+    ExternalDataTypes::FunctionCallbackData;
 
  private:
   FunctionCallback callback;
@@ -71,7 +71,7 @@ class FunctionCallbackData : public ExternalData {
   }
 
   Local<Object> NewInstance() {
-    Utils::EnsureObjectTemplate(instanceTemplate);
+    Utils::EnsureObjectTemplate(&instanceTemplate);
     return !instanceTemplate.IsEmpty() ?
       instanceTemplate->NewInstance(prototype) : Local<Object>();
   }
@@ -102,7 +102,7 @@ class FunctionCallbackData : public ExternalData {
 
     FunctionCallbackData* callbackData = nullptr;
     if (!ExternalData::TryGet(JsValueRef(callbackState), &callbackData)) {
-      CHAKRA_ASSERT(false); // This should never happen
+      CHAKRA_ASSERT(false);  // This should never happen
       return JS_INVALID_REFERENCE;
     }
 
@@ -206,11 +206,11 @@ class FunctionTemplateData : public TemplateData {
   }
 
   ObjectTemplate* EnsurePrototypeTemplate() {
-    return Utils::EnsureObjectTemplate(prototypeTemplate);
+    return Utils::EnsureObjectTemplate(&prototypeTemplate);
   }
 
   ObjectTemplate* EnsureInstanceTemplate() {
-    return Utils::EnsureObjectTemplate(instanceTemplate);
+    return Utils::EnsureObjectTemplate(&instanceTemplate);
   }
 
   Function* EnsureFunction() {
@@ -319,7 +319,7 @@ Local<FunctionTemplate> FunctionTemplate::New(Isolate* isolate,
 MaybeLocal<Function> FunctionTemplate::GetFunction(Local<Context> context) {
   FunctionTemplateData* functionTemplateData = nullptr;
   if (!ExternalData::TryGet(this, &functionTemplateData)) {
-    CHAKRA_ASSERT(false); // This should never happen
+    CHAKRA_ASSERT(false);  // This should never happen
     return Local<Function>();
   }
 
@@ -333,7 +333,7 @@ Local<Function> FunctionTemplate::GetFunction() {
 Local<ObjectTemplate> FunctionTemplate::InstanceTemplate() {
   FunctionTemplateData* functionTemplateData = nullptr;
   if (!ExternalData::TryGet(this, &functionTemplateData)) {
-    CHAKRA_ASSERT(false); // This should never happen
+    CHAKRA_ASSERT(false);  // This should never happen
     return Local<ObjectTemplate>();
   }
 
@@ -343,7 +343,7 @@ Local<ObjectTemplate> FunctionTemplate::InstanceTemplate() {
 Local<ObjectTemplate> FunctionTemplate::PrototypeTemplate() {
   FunctionTemplateData* functionTemplateData = nullptr;
   if (!ExternalData::TryGet(this, &functionTemplateData)) {
-    CHAKRA_ASSERT(false); // This should never happen
+    CHAKRA_ASSERT(false);  // This should never happen
     return Local<ObjectTemplate>();
   }
 

deps/chakrashim/src/v8object.cc

@@ -663,25 +663,26 @@ Maybe<bool> Object::SetPrivate(Local<Context> context, Local<Private> key,
 
 ObjectTemplate* Object::GetObjectTemplate() {
   ObjectData *objectData = nullptr;
-  return GetObjectData(&objectData) == JsNoError && objectData != nullptr ?
+  return Utils::GetObjectData(this, &objectData) == JsNoError
+          && objectData != nullptr ?
     *objectData->objectTemplate : nullptr;
 }
 
-JsErrorCode Object::GetObjectData(ObjectData** objectData) {
+JsErrorCode Utils::GetObjectData(Object* object, ObjectData** objectData) {
   *objectData = nullptr;
 
-  if (this->IsUndefined()) {
+  if (object->IsUndefined()) {
     return JsNoError;
   }
 
   JsErrorCode error;
-  JsValueRef self = this;
+  JsValueRef self = object;
   {
     JsPropertyIdRef selfSymbolIdRef =
       jsrt::IsolateShim::GetCurrent()->GetSelfSymbolPropertyIdRef();
     if (selfSymbolIdRef != JS_INVALID_REFERENCE) {
       JsValueRef result;
-      error = JsGetProperty(this, selfSymbolIdRef, &result);
+      error = JsGetProperty(object, selfSymbolIdRef, &result);
       if (error != JsNoError) {
         return error;
       }
@@ -697,7 +698,7 @@ JsErrorCode Object::GetObjectData(ObjectData** objectData) {
 
 int Object::InternalFieldCount() {
   ObjectData* objectData;
-  if (GetObjectData(&objectData) != JsNoError || !objectData) {
+  if (Utils::GetObjectData(this, &objectData) != JsNoError || !objectData) {
     return 0;
   }