Cleanup obsoleted data and add missed class stub

pinepain · pinepain · commit 463b6186fb56 · 2017-02-25T23:55:52.000+02:00
diff --git a/src/php_v8_callbacks.cc b/src/php_v8_callbacks.cc
@@ -268,7 +268,7 @@ void php_v8_callback_call_from_bucket_with_zargs(size_t index, v8::Local<v8::Val
         ZVAL_ZVAL(retval, fci.retval, 1, 1);
     }
 
-    // TODO: what about exceptions? - we let user handle any case of exceptions for themselves
+    // We let user handle any case of exceptions for themselves
 
     /* Clean up our mess */
     zend_fcall_info_args_clear(&fci, 1);
diff --git a/src/php_v8_exceptions.cc b/src/php_v8_exceptions.cc
@@ -27,23 +27,9 @@ zend_class_entry* php_v8_time_limit_exception_class_entry;
 zend_class_entry* php_v8_memory_limit_exception_class_entry;
 
 zend_class_entry* php_v8_value_exception_class_entry;
-zend_class_entry* php_v8_script_exception_class_entry;
 
 void php_v8_create_try_catch_exception(zval *return_value, php_v8_isolate_t *php_v8_isolate, php_v8_context_t *php_v8_context, v8::TryCatch *try_catch);
 
-void php_v8_try_catch_throw_exception(v8::TryCatch *try_catch, const char* message, zend_class_entry *ce) {
-    if (try_catch->Exception()->IsNull() && try_catch->Message().IsEmpty() && !try_catch->CanContinue() && try_catch->HasTerminated()) {
-        // TODO: output termination exception somehow
-        return;
-    }
-
-    v8::String::Utf8Value exception(try_catch->Exception());
-
-    PHP_V8_CONVERT_UTF8VALUE_TO_STRING_WITH_CHECK(exception, exception_message);
-
-    PHP_V8_THROW_EXCEPTION_CE(exception_message, ce);
-}
-
 void php_v8_throw_try_catch_exception(php_v8_isolate_t *php_v8_isolate, php_v8_context_t *php_v8_context, v8::TryCatch *try_catch) {
     zval exception_zv;
 
@@ -256,9 +242,5 @@ PHP_MINIT_FUNCTION(php_v8_exceptions) {
     INIT_NS_CLASS_ENTRY(ce, "V8\\Exceptions", "ValueException", php_v8_value_exception_methods);
     php_v8_value_exception_class_entry = zend_register_internal_class_ex(&ce, php_v8_generic_exception_class_entry);
 
-    // TODO: completely replace ScriptException with TryCatchException
-    INIT_NS_CLASS_ENTRY(ce, "V8\\Exceptions", "ScriptException", php_v8_script_exception_methods);
-    php_v8_script_exception_class_entry = zend_register_internal_class_ex(&ce, php_v8_generic_exception_class_entry);
-
     return SUCCESS;
 }
diff --git a/src/php_v8_exceptions.h b/src/php_v8_exceptions.h
@@ -35,9 +35,7 @@ extern zend_class_entry* php_v8_time_limit_exception_class_entry;
 extern zend_class_entry* php_v8_memory_limit_exception_class_entry;
 
 extern zend_class_entry* php_v8_value_exception_class_entry;
-extern zend_class_entry* php_v8_script_exception_class_entry;
 
-extern void php_v8_try_catch_throw_exception(v8::TryCatch *try_catch, const char* message, zend_class_entry *ce);
 extern void php_v8_throw_try_catch_exception(php_v8_isolate_t *php_v8_isolate, php_v8_context_t *php_v8_context, v8::TryCatch *try_catch);
 extern void php_v8_throw_try_catch_exception(php_v8_context_t *php_v8_context, v8::TryCatch *try_catch);
 
diff --git a/src/php_v8_function.cc b/src/php_v8_function.cc
@@ -75,9 +75,9 @@ bool php_v8_function_unpack_args(zval *arguments_zv, int arg_position, v8::Isola
 
         php_v8_tmp_data = PHP_V8_VALUE_FETCH(pzval);
 
-        // NOTE: check for emptiness may be considered redundant while we may catch the fact that value was not properly
-        //       constructed by checking isolates mismatch, but this check serves for user-friendly purposes to throw
-        //       less confusing exception message
+        // Check for emptiness may be considered redundant while we may catch the fact that value was not properly
+        // constructed by checking isolates mismatch, but this check serves for user-friendly purposes to throw
+        // less confusing exception message
         if (NULL == php_v8_tmp_data->persistent || php_v8_tmp_data->persistent->IsEmpty()) {
             spprintf(&exception_message, 0, PHP_V8_EMPTY_VALUE_MSG ": argument %d passed to %s::%s() at %d offset",
                      arg_position, ZSTR_VAL(ce_name), get_active_function_name(), i);
@@ -164,9 +164,9 @@ bool php_v8_function_unpack_string_args(zval* arguments_zv, int arg_position, v8
 
                 php_v8_tmp_data = PHP_V8_VALUE_FETCH(pzval);
 
-                // NOTE: check for emptiness may be considered redundant while we may catch the fact that value was not properly
-                //       constructed by checking isolates mismatch, but this check serves for user-friendly purposes to throw
-                //       less confusing exception message
+                // Check for emptiness may be considered redundant while we may catch the fact that value was not properly
+                // constructed by checking isolates mismatch, but this check serves for user-friendly purposes to throw
+                // less confusing exception message
                 if (NULL == php_v8_tmp_data->persistent || php_v8_tmp_data->persistent->IsEmpty()) {
                     spprintf(&exception_message, 0, PHP_V8_EMPTY_VALUE_MSG ": argument %d passed to %s::%s() at %d offset",
                              arg_position, ZSTR_VAL(ce_name), get_active_function_name(), i);
@@ -253,9 +253,9 @@ bool php_v8_function_unpack_object_args(zval* arguments_zv, int arg_position, v8
 
                 php_v8_tmp_data = PHP_V8_VALUE_FETCH(pzval);
 
-                // NOTE: check for emptiness may be considered redundant while we may catch the fact that value was not properly
-                //       constructed by checking isolates mismatch, but this check serves for user-friendly purposes to throw
-                //       less confusing exception message
+                // Check for emptiness may be considered redundant while we may catch the fact that value was not properly
+                // constructed by checking isolates mismatch, but this check serves for user-friendly purposes to throw
+                // less confusing exception message
                 if (NULL == php_v8_tmp_data->persistent || php_v8_tmp_data->persistent->IsEmpty()) {
                     spprintf(&exception_message, 0, PHP_V8_EMPTY_VALUE_MSG ": argument %d passed to %s::%s() at %d offset",
                              arg_position, ZSTR_VAL(ce_name), get_active_function_name(), i);
@@ -333,14 +333,7 @@ static PHP_METHOD(V8Function, __construct) {
         callback = php_v8_callback_function;
     }
 
-    // TODO: check length range (PHP uses long, while V8 uses int
-
-    v8::MaybeLocal<v8::Function> maybe_local_function = v8::Function::New(
-            context,
-            callback,
-            data,
-            (int) length
-    );
+    v8::MaybeLocal<v8::Function> maybe_local_function = v8::Function::New(context, callback, data, static_cast<int>(length));
 
     if (maybe_local_function.IsEmpty()) {
         PHP_V8_THROW_EXCEPTION("Failed to create Function value");
diff --git a/src/php_v8_function_template.cc b/src/php_v8_function_template.cc
@@ -77,8 +77,7 @@ static void php_v8_function_template_free(zend_object *object) {
      * persistent handler and cleanup callbacks. Alternatively, we can detect in weak callback that object is live and
      * unmark it as weak and do all that cleanings in free handler. What about if object will be reused after being
      * unmarked as week? Note, that the only action on weak handler callback is Reset()ing persistent handler.
-     *
-     * */
+     */
     if (zend_is_executing() && !CG(unclean_shutdown) && php_v8_function_template->persistent_data && !php_v8_function_template->persistent_data->empty()) {
         php_v8_function_template_make_weak(php_v8_function_template);
     }
@@ -161,13 +160,7 @@ static PHP_METHOD(V8FunctionTemplate, __construct) {
         callback = php_v8_callback_function;
     }
 
-    v8::Local<v8::FunctionTemplate> local_template = v8::FunctionTemplate::New(
-            isolate,
-            callback,
-            data,
-            signature,
-            static_cast<int>(length)
-    );
+    v8::Local<v8::FunctionTemplate> local_template = v8::FunctionTemplate::New(isolate, callback, data, signature, static_cast<int>(length));
 
     PHP_V8_THROW_VALUE_EXCEPTION_WHEN_EMPTY(local_template, "Failed to create FunctionTemplate value");
 
diff --git a/src/php_v8_indexed_property_handler_configuration.cc b/src/php_v8_indexed_property_handler_configuration.cc
@@ -119,8 +119,7 @@ static PHP_METHOD (V8IndexedPropertyHandlerConfiguration, __construct) {
         php_v8_handlers->enumerator = php_v8_callback_indexed_property_enumerator;
     }
 
-    // TODO: php_v8_handlers->flags is long, while flag is zend_long, unify types or cast ???
-    php_v8_handlers->flags = flags ? flags & PHP_V8_PROPERTY_HANDLER_FLAGS : flags;
+    php_v8_handlers->flags = static_cast<long>(flags ? flags & PHP_V8_PROPERTY_HANDLER_FLAGS : flags);
 }
 
 
diff --git a/src/php_v8_indexed_property_handler_configuration.h b/src/php_v8_indexed_property_handler_configuration.h
@@ -38,7 +38,7 @@ inline php_v8_indexed_property_handler_configuration_t * php_v8_indexed_property
 
 #define PHP_V8_INDEXED_PROPERTY_HANDLER_FETCH_WITH_CHECK(pzval, into) \
     PHP_V8_INDEXED_PROPERTY_HANDLER_FETCH_INTO(pzval, into); \
-    PHP_V8_CHECK_EMPTY_INDEXED_PROPERTY_HANDLER(into);// TODO: fetch with check
+    PHP_V8_CHECK_EMPTY_INDEXED_PROPERTY_HANDLER(into);
 
 #define PHP_V8_INDEXED_PROPERTY_HANDLER_STORE_ISOLATE(to_zval, from_isolate_zv) zend_update_property(php_v8_indexed_property_handler_configuration_class_entry, (to_zval), ZEND_STRL("isolate"), (from_isolate_zv));
 #define PHP_V8_INDEXED_PROPERTY_HANDLER_READ_ISOLATE(from_zval) zend_read_property(php_v8_indexed_property_handler_configuration_class_entry, (from_zval), ZEND_STRL("isolate"), 0, &rv)
diff --git a/src/php_v8_isolate.cc b/src/php_v8_isolate.cc
@@ -164,8 +164,6 @@ static void php_v8_fatal_error_handler(const char *location, const char *message
     v8::Isolate *isolate = v8::Isolate::GetCurrent();
     assert(isolate != NULL); // as we set fatal error handler per-isolate, we should always have at least any of them as current one
 
-    // TODO: if there are no entered isolates = fatal error
-
     php_v8_isolate_t *php_v8_isolate = PHP_V8_ISOLATE_FETCH_REFERENCE(isolate);
 
     assert(NULL != php_v8_isolate);
diff --git a/src/php_v8_isolate.h b/src/php_v8_isolate.h
@@ -57,7 +57,6 @@ inline v8::Local<v8::Private> php_v8_isolate_get_key_local(php_v8_isolate_t *php
     PHP_V8_CHECK_EMPTY_ISOLATE_HANDLER(into);
 
 
-// TODO: cover cases when first and/or second are NULLs
 #define PHP_V8_ISOLATES_CHECK(first, second) if ((first)->isolate != (second)->isolate) { PHP_V8_THROW_EXCEPTION(PHP_V8_ISOLATES_MISMATCH_MSG); return; }
 #define PHP_V8_ISOLATES_CHECK_USING_ISOLATE(first, using_isolate) if ((first)->isolate != (using_isolate)) { PHP_V8_THROW_EXCEPTION(PHP_V8_ISOLATES_MISMATCH_MSG); return; }
 
diff --git a/src/php_v8_object.cc b/src/php_v8_object.cc
@@ -1189,10 +1189,6 @@ static PHP_METHOD(V8Object, CallAsConstructor) {
     php_v8_get_or_create_value(return_value, local_res, php_v8_value->php_v8_isolate);
 }
 
-// Not supported yet
-//static PHP_METHOD(V8Object, Cast) {
-//}
-
 /* Non-standard, implementations of AdjustableExternalMemoryInterface::AdjustExternalAllocatedMemory */
 static PHP_METHOD(V8Object, AdjustExternalAllocatedMemory) {
     php_v8_ext_mem_interface_value_AdjustExternalAllocatedMemory(INTERNAL_FUNCTION_PARAM_PASSTHRU);
@@ -1428,9 +1424,6 @@ static const zend_function_entry php_v8_object_methods[] = {
         PHP_ME(V8Object, CallAsFunction, arginfo_v8_object_CallAsFunction, ZEND_ACC_PUBLIC)
         PHP_ME(V8Object, CallAsConstructor, arginfo_v8_object_CallAsConstructor, ZEND_ACC_PUBLIC)
 
-        // NOTE: Not supported yet
-        //PHP_ME(V8Object, Cast, arginfo_v8_object_Cast, ZEND_ACC_PUBLIC | ZEND_ACC_STATIC)
-
         PHP_ME(V8Object, AdjustExternalAllocatedMemory, arginfo_v8_object_AdjustExternalAllocatedMemory, ZEND_ACC_PUBLIC)
         PHP_ME(V8Object, GetExternalAllocatedMemory, arginfo_v8_object_GetExternalAllocatedMemory, ZEND_ACC_PUBLIC)
 
diff --git a/src/php_v8_script.cc b/src/php_v8_script.cc
@@ -48,7 +48,6 @@ static void php_v8_script_free(zend_object *object)
 {
     php_v8_script_t *php_v8_script = php_v8_script_fetch_object(object);
 
-    // TODO: think about making script weak, it probably may still in use by returned functions from it, isn't it?
     if (php_v8_script->persistent) {
         if (PHP_V8_ISOLATE_HAS_VALID_HANDLE(php_v8_script)) {
             php_v8_script->persistent->Reset();
diff --git a/src/php_v8_startup_data.cc b/src/php_v8_startup_data.cc
@@ -115,8 +115,8 @@ static PHP_METHOD(V8StartupData, CreateFromSource) {
     if (zend_parse_parameters(ZEND_NUM_ARGS(), "S", &blob) == FAILURE) {
         return;
     }
+
     /* we can't try-catch here while we have no isolate yet */
-    // TODO: test to create blob from invalid source
 
     const char *source = ZSTR_VAL(blob);
     php_v8_init();
diff --git a/src/php_v8_value.cc b/src/php_v8_value.cc
@@ -106,8 +106,8 @@ static void php_v8_value_free(zend_object *object) {
         if (local_value->IsObject()) {
             // TODO: at this point we SHOULD drop link to complete object and replace it with link to persistent handler and callbacks
 
-            /* NOTE: here we lose reference to persistent handler and callbacks. While in most cases this should be
-             *       rare case, it may lead to allocated memory bloating, so it may be a good idea to store proper reference
+            /* Here we lose reference to persistent handler and callbacks. While in most cases this should be
+             * rare case, it may lead to allocated memory bloating, so it may be a good idea to store proper reference
              */
             php_v8_object_delete_self_ptr(php_v8_value, v8::Local<v8::Object>::Cast(local_value));
         }
@@ -427,7 +427,6 @@ static PHP_V8_VALUE_IS_METHOD(V8Value, IsSharedArrayBuffer)
 static PHP_V8_VALUE_IS_METHOD(V8Value, IsProxy)
 //static PHP_V8_VALUE_IS_METHOD(V8Value, IsWebAssemblyCompiledModule) // Experimental
 
-// TODO: bind other methods that matters
 
 /* -----------------------------------------------------------------------
           Converters from v8::Value to high-level v8::Value's children
diff --git a/src/php_v8_value.h b/src/php_v8_value.h
@@ -70,7 +70,6 @@ extern php_v8_value_t *php_v8_get_or_create_value(zval *return_value, v8::Local<
     PHP_V8_ENTER_STORED_ISOLATE((value_v8)); \
     PHP_V8_ENTER_STORED_CONTEXT((value_v8)); \
 
-// TODO: move to better place or is it fine here too?
 #define PHP_V8_CONVERT_UTF8VALUE_TO_STRING(from, to) const char *(to) = (const char*) *(from);
 #define PHP_V8_CONVERT_UTF8VALUE_TO_STRING_NODECL(from, to) (to) = (const char*) *(from);
 
@@ -97,7 +96,6 @@ extern php_v8_value_t *php_v8_get_or_create_value(zval *return_value, v8::Local<
     PHP_V8_CONVERT_UTF8VALUE_TO_STRING_WITH_CHECK_NODECL(_v8_utf8_str_##cstr, cstr); \
 }
 
-// TODO: string length check?
 #define PHP_V8_SET_ZVAL_STRING_FROM_V8_STRING(zval_to, v8_local_string_from) { \
     v8::String::Utf8Value _v8_utf8_str((v8_local_string_from)); \
     PHP_V8_CONVERT_UTF8VALUE_TO_STRING_WITH_CHECK(_v8_utf8_str, _v8_utf8_cstr); \
diff --git a/stubs/src/Exceptions/ValueException.php b/stubs/src/Exceptions/ValueException.php
@@ -0,0 +1,20 @@
+<?php
+
+/**
+ * This file is part of the pinepain/php-v8 PHP extension.
+ *
+ * Copyright (c) 2015-2017 Bogdan Padalko <pinepain@gmail.com>
+ *
+ * Licensed under the MIT license: http://opensource.org/licenses/MIT
+ *
+ * For the full copyright and license information, please view the
+ * LICENSE file that was distributed with this source or visit
+ * http://opensource.org/licenses/MIT
+ */
+
+namespace V8\Exceptions;
+
+
+class ValueException extends GenericException
+{
+}
diff --git a/tests/V8StartupData_CreateFromSource.phpt b/tests/V8StartupData_CreateFromSource.phpt
@@ -27,6 +27,16 @@ $data = null;
 $context = new \V8\Context($isolate);
 
 $helper->assert('Context global is affected by snapshot blob', $context->GlobalObject()->Get($context, new \V8\StringValue($isolate, 'test_snapshot'))->IsFunction());
+
+
+try {
+    V8\StartupData::CreateFromSource(') bad +/^\\');
+    $helper->assert('Unable to create startup data from bad source', false);
+} catch (Exception $e) {
+    $helper->space();
+    $helper->exception_export($e);
+}
+
 ?>
 --EXPECT--
 Object representation:
@@ -39,3 +49,6 @@ Snapshot blob is large binary string: ok
 Snapshot raw_size is the same as binary_string length: ok
 Snapshot raw_size is the same as binary_string length: ok
 Context global is affected by snapshot blob: ok
+
+
+V8\Exceptions\GenericException: Failed to create startup blob

Original file line number	Diff line number	Diff line change
`@@ -268,7 +268,7 @@ void php_v8_callback_call_from_bucket_with_zargs(size_t index, v8::Local<v8::Val`
`268`	`268`	`ZVAL_ZVAL(retval, fci.retval, 1, 1);`
`269`	`269`	`}`
`270`	`270`
`271`		`- // TODO: what about exceptions? - we let user handle any case of exceptions for themselves`
	`271`	`+ // We let user handle any case of exceptions for themselves`
`272`	`272`
`273`	`273`	`/* Clean up our mess */`
`274`	`274`	`zend_fcall_info_args_clear(&fci, 1);`
Original file line number	Diff line number	Diff line change
`@@ -119,8 +119,7 @@ static PHP_METHOD (V8IndexedPropertyHandlerConfiguration, __construct) {`
`119`	`119`	`php_v8_handlers->enumerator = php_v8_callback_indexed_property_enumerator;`
`120`	`120`	`}`
`121`	`121`
`122`		`- // TODO: php_v8_handlers->flags is long, while flag is zend_long, unify types or cast ???`
`123`		`- php_v8_handlers->flags = flags ? flags & PHP_V8_PROPERTY_HANDLER_FLAGS : flags;`
	`122`	`+ php_v8_handlers->flags = static_cast<long>(flags ? flags & PHP_V8_PROPERTY_HANDLER_FLAGS : flags);`
`124`	`123`	`}`
`125`	`124`
`126`	`125`