ext/intl: Refactor error handling (#19196)

This is a comprehensive refactoring of the error mechanism of the Intl extension.

By moving the prefixing of the current method/function being executed to actual error message creation by accessing the execution context, we get the following benefits:
- Accurate error messages indicating *what* call caused the error
  - As we *always* "copy" the message, the `copyMsg` arg becomes unused, meaning we can reduce the size of the `intl_error` struct by 4 bytes.
  - Saving it as a zend_string means we know the length of the message
- Remove the need to pass around a "function name" `char*` across multiple calls
- Use Intl's exception mechanism to generate exceptions for constructor call
  - This removes the need for replacing the error handler
  - Which didn't do anything anyway in silent mode, which required throwing non-descriptive exceptions

Author: Girgias

ext/intl/breakiterator/breakiterator_iterators.cpp

@@ -43,7 +43,7 @@ inline BreakIterator *_breakiter_prolog(zend_object_iterator *iter)
 	if (bio->biter == NULL) {
 		intl_errors_set(BREAKITER_ERROR_P(bio), U_INVALID_STATE_ERROR,
 			"The BreakIterator object backing the PHP iterator is not "
-			"properly constructed", 0);
+			"properly constructed");
 	}
 	return bio->biter;
 }

ext/intl/breakiterator/breakiterator_methods.cpp

@@ -41,14 +41,13 @@ U_CFUNC PHP_METHOD(IntlBreakIterator, __construct)
 		0 );
 }
 
-static void _breakiter_factory(const char *func_name,
-							   BreakIterator *(*func)(const Locale&, UErrorCode&),
-							   INTERNAL_FUNCTION_PARAMETERS)
+static void _breakiter_factory(
+	BreakIterator *(*func)(const Locale&, UErrorCode&),
+	INTERNAL_FUNCTION_PARAMETERS)
 {
 	BreakIterator	*biter;
 	char		                *locale_str = NULL;
 	size_t				dummy;
-	char			*msg;
 	UErrorCode		status = UErrorCode();
 	intl_error_reset(NULL);
 
@@ -64,10 +63,7 @@ static void _breakiter_factory(const char *func_name,
 	biter = func(Locale::createFromName(locale_str), status);
 	intl_error_set_code(NULL, status);
 	if (U_FAILURE(status)) {
-		spprintf(&msg, 0, "%s: error creating BreakIterator",
-				func_name);
-		intl_error_set_custom_msg(NULL, msg, 1);
-		efree(msg);
+		intl_error_set_custom_msg(NULL, "error creating BreakIterator");
 		RETURN_NULL();
 	}
 
@@ -76,35 +72,35 @@ static void _breakiter_factory(const char *func_name,
 
 U_CFUNC PHP_METHOD(IntlBreakIterator, createWordInstance)
 {
-	_breakiter_factory("breakiter_create_word_instance",
+	_breakiter_factory(
 			&BreakIterator::createWordInstance,
 			INTERNAL_FUNCTION_PARAM_PASSTHRU);
 }
 
 U_CFUNC PHP_METHOD(IntlBreakIterator, createLineInstance)
 {
-	_breakiter_factory("breakiter_create_line_instance",
+	_breakiter_factory(
 			&BreakIterator::createLineInstance,
 			INTERNAL_FUNCTION_PARAM_PASSTHRU);
 }
 
 U_CFUNC PHP_METHOD(IntlBreakIterator, createCharacterInstance)
 {
-	_breakiter_factory("breakiter_create_character_instance",
+	_breakiter_factory(
 			&BreakIterator::createCharacterInstance,
 			INTERNAL_FUNCTION_PARAM_PASSTHRU);
 }
 
 U_CFUNC PHP_METHOD(IntlBreakIterator, createSentenceInstance)
 {
-	_breakiter_factory("breakiter_create_sentence_instance",
+	_breakiter_factory(
 			&BreakIterator::createSentenceInstance,
 			INTERNAL_FUNCTION_PARAM_PASSTHRU);
 }
 
 U_CFUNC PHP_METHOD(IntlBreakIterator, createTitleInstance)
 {
-	_breakiter_factory("breakiter_create_title_instance",
+	_breakiter_factory(
 			&BreakIterator::createTitleInstance,
 			INTERNAL_FUNCTION_PARAM_PASSTHRU);
 }
@@ -149,12 +145,11 @@ U_CFUNC PHP_METHOD(IntlBreakIterator, setText)
 	BREAKITER_METHOD_FETCH_OBJECT;
 
 	ut = utext_openUTF8(ut, ZSTR_VAL(text), ZSTR_LEN(text), BREAKITER_ERROR_CODE_P(bio));
-	INTL_METHOD_CHECK_STATUS(bio, "breakiter_set_text: error opening UText");
+	INTL_METHOD_CHECK_STATUS(bio, "error opening UText");
 
 	bio->biter->setText(ut, BREAKITER_ERROR_CODE(bio));
 	utext_close(ut); /* ICU shallow clones the UText */
-	INTL_METHOD_CHECK_STATUS(bio, "breakiter_set_text: error calling "
-		"BreakIterator::setText()");
+	INTL_METHOD_CHECK_STATUS(bio, "error calling BreakIterator::setText()");
 
 	/* When ICU clones the UText, it does not copy the buffer, so we have to
 	 * keep the string buffer around by holding a reference to its zval. This
@@ -302,19 +297,18 @@ U_CFUNC PHP_METHOD(IntlBreakIterator, getLocale)
 		Z_PARAM_LONG(locale_type)
 	ZEND_PARSE_PARAMETERS_END();
 
-	/* Change to ValueError? */
+	/* TODO: Change to ValueError? */
 	if (locale_type != ULOC_ACTUAL_LOCALE && locale_type != ULOC_VALID_LOCALE) {
 		intl_error_set(NULL, U_ILLEGAL_ARGUMENT_ERROR,
-			"breakiter_get_locale: invalid locale type", 0);
+			"invalid locale type");
 		RETURN_FALSE;
 	}
 
 	BREAKITER_METHOD_FETCH_OBJECT;
 
 	Locale locale = bio->biter->getLocale((ULocDataLocaleType)locale_type,
 		BREAKITER_ERROR_CODE(bio));
-	INTL_METHOD_CHECK_STATUS(bio,
-		"breakiter_get_locale: Call to ICU method has failed");
+	INTL_METHOD_CHECK_STATUS(bio, "Call to ICU method has failed");
 
 	RETURN_STRING(locale.getName());
 }

ext/intl/breakiterator/rulebasedbreakiterator_methods.cpp

@@ -105,8 +105,7 @@ U_CFUNC PHP_METHOD(IntlRuleBasedBreakIterator, getRules)
 	if (!u8str)
 	{
 		intl_errors_set(BREAKITER_ERROR_P(bio), BREAKITER_ERROR_CODE(bio),
-				"rbbi_hash_code: Error converting result to UTF-8 string",
-				0);
+				"Error converting result to UTF-8 string");
 		RETURN_FALSE;
 	}
 	RETVAL_STR(u8str);
@@ -144,8 +143,7 @@ U_CFUNC PHP_METHOD(IntlRuleBasedBreakIterator, getRuleStatusVec)
 			BREAKITER_ERROR_CODE(bio));
 	if (U_FAILURE(BREAKITER_ERROR_CODE(bio))) {
 		intl_errors_set(BREAKITER_ERROR_P(bio), BREAKITER_ERROR_CODE(bio),
-				"rbbi_get_rule_status_vec: failed obtaining the status values",
-				0);
+				"failed obtaining the status values");
 		RETURN_FALSE;
 	}
 
@@ -169,8 +167,7 @@ U_CFUNC PHP_METHOD(IntlRuleBasedBreakIterator, getBinaryRules)
 
 	if (rules_len > INT_MAX - 1) {
 		intl_errors_set(BREAKITER_ERROR_P(bio), BREAKITER_ERROR_CODE(bio),
-				"rbbi_get_binary_rules: the rules are too large",
-				0);
+				"the rules are too large");
 		RETURN_FALSE;
 	}