Skip to content

ext/intl: W.I.P.: Refactor error handling #19196

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft
wants to merge 2 commits into
base: master
Choose a base branch
from
+1,026 −914
Draft

ext/intl: W.I.P.: Refactor error handling #19196

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
a95bd7e
ext/intl: W.I.P.: Refactor error handling
Girgias Jul 20, 2025
d825e0d
Fix more tests
Girgias Jul 26, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
32 changes: 13 additions & 19 deletions ext/intl/breakiterator/breakiterator_methods.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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);

Expand All @@ -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", false);
RETURN_NULL();
}

Expand All @@ -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);
}
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -305,16 +300,15 @@ U_CFUNC PHP_METHOD(IntlBreakIterator, getLocale)
/* 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", 0);
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());
}
Expand Down
6 changes: 3 additions & 3 deletions ext/intl/breakiterator/rulebasedbreakiterator_methods.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -105,7 +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",
"Error converting result to UTF-8 string",
0);
RETURN_FALSE;
}
Expand Down Expand Up @@ -144,7 +144,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",
"failed obtaining the status values",
0);
RETURN_FALSE;
}
Expand All @@ -169,7 +169,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",
"the rules are too large",
0);
RETURN_FALSE;
}
Expand Down
63 changes: 27 additions & 36 deletions ext/intl/calendar/calendar_methods.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -85,8 +85,7 @@ U_CFUNC PHP_FUNCTION(intlcal_create_instance)
Z_PARAM_STRING_OR_NULL(locale_str, locale_len)
ZEND_PARSE_PARAMETERS_END();

timeZone = timezone_process_timezone_argument(zv_timezone, NULL,
"intlcal_create_instance");
timeZone = timezone_process_timezone_argument(zv_timezone, NULL);
if (timeZone == NULL) {
RETURN_NULL();
}
Expand Down Expand Up @@ -179,7 +178,7 @@ U_CFUNC PHP_FUNCTION(intlcal_get_keyword_values_for_locale)
Locale::createFromName(locale), (UBool)commonly_used,
status);
if (se == NULL) {
intl_error_set(NULL, status, "intlcal_get_keyword_values_for_locale: "
intl_error_set(NULL, status,
"error calling underlying method", 0);
RETURN_FALSE;
}
Expand Down Expand Up @@ -252,8 +251,7 @@ U_CFUNC PHP_FUNCTION(intlcal_get_time)
CALENDAR_METHOD_FETCH_OBJECT;

UDate result = co->ucal->getTime(CALENDAR_ERROR_CODE(co));
INTL_METHOD_CHECK_STATUS(co,
"intlcal_get_time: error calling ICU Calendar::getTime");
INTL_METHOD_CHECK_STATUS(co, "error calling ICU Calendar::getTime");

RETURN_DOUBLE((double)result);
}
Expand Down Expand Up @@ -293,7 +291,7 @@ U_CFUNC PHP_FUNCTION(intlcal_add)
CALENDAR_METHOD_FETCH_OBJECT;

co->ucal->add((UCalendarDateFields)field, (int32_t)amount, CALENDAR_ERROR_CODE(co));
INTL_METHOD_CHECK_STATUS(co, "intlcal_add: Call to underlying method failed");
INTL_METHOD_CHECK_STATUS(co, "Call to underlying method failed");

RETURN_TRUE;
}
Expand All @@ -316,7 +314,7 @@ U_CFUNC PHP_FUNCTION(intlcal_set_time_zone)
}

timeZone = timezone_process_timezone_argument(zv_timezone,
CALENDAR_ERROR_P(co), "intlcal_set_time_zone");
CALENDAR_ERROR_P(co));
if (timeZone == NULL) {
RETURN_FALSE;
}
Expand Down Expand Up @@ -350,7 +348,7 @@ static void _php_intlcal_before_after(
}

UBool res = (co->ucal->*func)(*when_co->ucal, CALENDAR_ERROR_CODE(co));
INTL_METHOD_CHECK_STATUS(co, "intlcal_before/after: Error calling ICU method");
INTL_METHOD_CHECK_STATUS(co, "Error calling ICU method");

RETURN_BOOL((int)res);
}
Expand Down Expand Up @@ -490,7 +488,7 @@ U_CFUNC PHP_FUNCTION(intlcal_roll)

co->ucal->roll((UCalendarDateFields)field, (int32_t)value, CALENDAR_ERROR_CODE(co));

INTL_METHOD_CHECK_STATUS(co, "intlcal_roll: Error calling ICU Calendar::roll");
INTL_METHOD_CHECK_STATUS(co, "Error calling ICU Calendar::roll");

RETURN_TRUE;
}
Expand Down Expand Up @@ -536,8 +534,7 @@ U_CFUNC PHP_FUNCTION(intlcal_field_difference)

int32_t result = co->ucal->fieldDifference((UDate)when,
(UCalendarDateFields)field, CALENDAR_ERROR_CODE(co));
INTL_METHOD_CHECK_STATUS(co,
"intlcal_field_difference: Call to ICU method has failed");
INTL_METHOD_CHECK_STATUS(co, "Call to ICU method has failed");

RETURN_LONG((zend_long)result);
}
Expand Down Expand Up @@ -570,8 +567,7 @@ U_CFUNC PHP_FUNCTION(intlcal_get_day_of_week_type)

int32_t result = co->ucal->getDayOfWeekType(
(UCalendarDaysOfWeek)dow, CALENDAR_ERROR_CODE(co));
INTL_METHOD_CHECK_STATUS(co,
"intlcal_get_day_of_week_type: Call to ICU method has failed");
INTL_METHOD_CHECK_STATUS(co, "Call to ICU method has failed");

RETURN_LONG((zend_long)result);
}
Expand All @@ -588,8 +584,7 @@ U_CFUNC PHP_FUNCTION(intlcal_get_first_day_of_week)
CALENDAR_METHOD_FETCH_OBJECT;

int32_t result = co->ucal->getFirstDayOfWeek(CALENDAR_ERROR_CODE(co));
INTL_METHOD_CHECK_STATUS(co,
"intlcal_get_first_day_of_week: Call to ICU method has failed");
INTL_METHOD_CHECK_STATUS(co, "Call to ICU method has failed");

RETURN_LONG((zend_long)result);
}
Expand Down Expand Up @@ -647,8 +642,7 @@ U_CFUNC PHP_FUNCTION(intlcal_get_locale)

Locale locale = co->ucal->getLocale((ULocDataLocaleType)locale_type,
CALENDAR_ERROR_CODE(co));
INTL_METHOD_CHECK_STATUS(co,
"intlcal_get_locale: Call to ICU method has failed");
INTL_METHOD_CHECK_STATUS(co, "Call to ICU method has failed");

RETURN_STRING(locale.getName());
}
Expand All @@ -671,8 +665,8 @@ U_CFUNC PHP_FUNCTION(intlcal_get_minimal_days_in_first_week)
CALENDAR_METHOD_FETCH_OBJECT;

uint8_t result = co->ucal->getMinimalDaysInFirstWeek();
INTL_METHOD_CHECK_STATUS(co,
"intlcal_get_first_day_of_week: Call to ICU method has failed"); /* TODO Is it really a failure? */
/* TODO Is it really a failure? */
INTL_METHOD_CHECK_STATUS(co, "Call to ICU method has failed");

RETURN_LONG((zend_long)result);
}
Expand All @@ -697,7 +691,7 @@ U_CFUNC PHP_FUNCTION(intlcal_get_time_zone)
TimeZone *tz = co->ucal->getTimeZone().clone();
if (UNEXPECTED(tz == NULL)) {
intl_errors_set(CALENDAR_ERROR_P(co), U_MEMORY_ALLOCATION_ERROR,
"intlcal_get_time_zone: could not clone TimeZone", 0);
"could not clone TimeZone", 0);
RETURN_FALSE;
}

Expand Down Expand Up @@ -734,8 +728,7 @@ U_CFUNC PHP_FUNCTION(intlcal_get_weekend_transition)

int32_t res = co->ucal->getWeekendTransition((UCalendarDaysOfWeek)dow,
CALENDAR_ERROR_CODE(co));
INTL_METHOD_CHECK_STATUS(co, "intlcal_get_weekend_transition: "
"Error calling ICU method");
INTL_METHOD_CHECK_STATUS(co, "Error calling ICU method");

RETURN_LONG((zend_long)res);
}
Expand All @@ -752,8 +745,7 @@ U_CFUNC PHP_FUNCTION(intlcal_in_daylight_time)
CALENDAR_METHOD_FETCH_OBJECT;

UBool ret = co->ucal->inDaylightTime(CALENDAR_ERROR_CODE(co));
INTL_METHOD_CHECK_STATUS(co, "intlcal_in_daylight_time: "
"Error calling ICU method");
INTL_METHOD_CHECK_STATUS(co, "Error calling ICU method");

RETURN_BOOL((int)ret);
}
Expand Down Expand Up @@ -829,8 +821,7 @@ U_CFUNC PHP_FUNCTION(intlcal_is_weekend)
RETURN_BOOL((int)co->ucal->isWeekend());
} else {
UBool ret = co->ucal->isWeekend((UDate)date, CALENDAR_ERROR_CODE(co));
INTL_METHOD_CHECK_STATUS(co, "intlcal_is_weekend: "
"Error calling ICU method");
INTL_METHOD_CHECK_STATUS(co, "Error calling ICU method");
RETURN_BOOL((int)ret);
}
}
Expand Down Expand Up @@ -915,7 +906,7 @@ U_CFUNC PHP_FUNCTION(intlcal_equals)
}

UBool result = co->ucal->equals(*other_co->ucal, CALENDAR_ERROR_CODE(co));
INTL_METHOD_CHECK_STATUS(co, "intlcal_equals: error calling ICU Calendar::equals");
INTL_METHOD_CHECK_STATUS(co, "error calling ICU Calendar::equals");

RETURN_BOOL((int)result);
}
Expand Down Expand Up @@ -1028,15 +1019,15 @@ U_CFUNC PHP_FUNCTION(intlcal_from_date_time)
datetime = php_date_obj_from_obj(date_obj);
if (!datetime->time) {
intl_error_set(NULL, U_ILLEGAL_ARGUMENT_ERROR,
"intlcal_from_date_time: DateTime object is unconstructed",
"DateTime object is unconstructed",
0);
goto error;
}

zend_call_method_with_0_params(date_obj, php_date_get_date_ce(), NULL, "gettimestamp", &zv_timestamp);
if (Z_TYPE(zv_timestamp) != IS_LONG) {
intl_error_set(NULL, U_ILLEGAL_ARGUMENT_ERROR,
"intlcal_from_date_time: bad DateTime; call to "
"bad DateTime; call to "
"DateTime::getTimestamp() failed", 0);
zval_ptr_dtor(&zv_timestamp);
goto error;
Expand All @@ -1046,7 +1037,7 @@ U_CFUNC PHP_FUNCTION(intlcal_from_date_time)
timeZone = TimeZone::getGMT()->clone();
} else {
timeZone = timezone_convert_datetimezone(datetime->time->zone_type,
datetime, 1, NULL, "intlcal_from_date_time");
datetime, 1, NULL);
if (timeZone == NULL) {
goto error;
}
Expand All @@ -1060,15 +1051,15 @@ U_CFUNC PHP_FUNCTION(intlcal_from_date_time)
Locale::createFromName(locale_str), status);
if (UNEXPECTED(cal == NULL)) {
delete timeZone;
intl_error_set(NULL, status, "intlcal_from_date_time: "
"error creating ICU Calendar object", 0);
intl_error_set(NULL, status,
"error creating ICU Calendar object", 0);
goto error;
}
cal->setTime(((UDate)Z_LVAL(zv_timestamp)) * 1000., status);
if (U_FAILURE(status)) {
/* time zone was adopted by cal; should not be deleted here */
delete cal;
intl_error_set(NULL, status, "intlcal_from_date_time: "
intl_error_set(NULL, status,
"error creating ICU Calendar::setTime()", 0);
goto error;
}
Expand Down Expand Up @@ -1105,7 +1096,7 @@ U_CFUNC PHP_FUNCTION(intlcal_to_date_time)

if (UNEXPECTED(date > (double)U_INT64_MAX || date < (double)U_INT64_MIN)) {
intl_errors_set(CALENDAR_ERROR_P(co), U_ILLEGAL_ARGUMENT_ERROR,
"intlcal_to_date_time: The calendar date is out of the "
"The calendar date is out of the "
"range for a 64-bit integer", 0);
RETURN_FALSE;
}
Expand All @@ -1119,7 +1110,7 @@ U_CFUNC PHP_FUNCTION(intlcal_to_date_time)
/* Now get the time zone */
const TimeZone& tz = co->ucal->getTimeZone();
zval *timezone_zval = timezone_convert_to_datetimezone(
&tz, CALENDAR_ERROR_P(co), "intlcal_to_date_time", &tmp);
&tz, CALENDAR_ERROR_P(co), &tmp);
if (timezone_zval == NULL) {
zval_ptr_dtor(&ts_zval);
RETURN_FALSE;
Expand All @@ -1146,7 +1137,7 @@ U_CFUNC PHP_FUNCTION(intlcal_to_date_time)
&retval, timezone_zval);
if (Z_ISUNDEF(retval) || Z_TYPE(retval) == IS_FALSE) {
intl_errors_set(CALENDAR_ERROR_P(co), U_ILLEGAL_ARGUMENT_ERROR,
"intlcal_to_date_time: call to DateTime::setTimeZone has failed",
"call to DateTime::setTimeZone has failed",
1);
zval_ptr_dtor(return_value);
RETVAL_FALSE;
Expand Down
Loading