Skip to content

Refactor GregorianCalendar constructor #7605

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

Closed
wants to merge 1 commit into from
Closed

Refactor GregorianCalendar constructor #7605

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
7 changes: 1 addition & 6 deletions ext/intl/calendar/calendar.stub.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -284,13 +284,8 @@ class IntlGregorianCalendar extends IntlCalendar
{
/**
* @param DateTimeZone|IntlTimeZone|string|int|null $timezoneOrYear
* @param string|int|null $localeOrMonth
* @param int $day
* @param int $hour
* @param int $minute
* @param int $second
*/
public function __construct($timezoneOrYear = UNKNOWN, $localeOrMonth = UNKNOWN, $day = UNKNOWN, $hour = UNKNOWN, $minute = UNKNOWN, $second = UNKNOWN) {}
public function __construct($timezoneOrYear = null, string|int|null $localeOrMonth = null, ?int $day = null, int $hour = 0, int $minute = 0, int $second = 0) {}

/**
* @tentative-return-type
Expand Down
14 changes: 7 additions & 7 deletions ext/intl/calendar/calendar_arginfo.h
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/* This is a generated file, edit the .stub.php file instead.
* Stub hash: 2265c2a4f478d6ccd576ce09a19a158df38a2bdb */
* Stub hash: 556e84d52ad03a5eab6007d45d80ea9301fefd75 */

ZEND_BEGIN_ARG_INFO_EX(arginfo_class_IntlCalendar___construct, 0, 0, 0)
ZEND_END_ARG_INFO()
Expand Down Expand Up @@ -157,12 +157,12 @@ ZEND_BEGIN_ARG_WITH_TENTATIVE_RETURN_OBJ_TYPE_MASK_EX(arginfo_class_IntlCalendar
ZEND_END_ARG_INFO()

ZEND_BEGIN_ARG_INFO_EX(arginfo_class_IntlGregorianCalendar___construct, 0, 0, 0)
ZEND_ARG_INFO(0, timezoneOrYear)
ZEND_ARG_INFO(0, localeOrMonth)
ZEND_ARG_INFO(0, day)
ZEND_ARG_INFO(0, hour)
ZEND_ARG_INFO(0, minute)
ZEND_ARG_INFO(0, second)
ZEND_ARG_INFO_WITH_DEFAULT_VALUE(0, timezoneOrYear, "null")
ZEND_ARG_TYPE_MASK(0, localeOrMonth, MAY_BE_STRING|MAY_BE_LONG|MAY_BE_NULL, "null")
ZEND_ARG_TYPE_INFO_WITH_DEFAULT_VALUE(0, day, IS_LONG, 1, "null")
ZEND_ARG_TYPE_INFO_WITH_DEFAULT_VALUE(0, hour, IS_LONG, 0, "0")
ZEND_ARG_TYPE_INFO_WITH_DEFAULT_VALUE(0, minute, IS_LONG, 0, "0")
ZEND_ARG_TYPE_INFO_WITH_DEFAULT_VALUE(0, second, IS_LONG, 0, "0")
ZEND_END_ARG_INFO()

#define arginfo_class_IntlGregorianCalendar_setGregorianChange arginfo_class_IntlCalendar_setTime
Expand Down
153 changes: 81 additions & 72 deletions ext/intl/calendar/gregoriancalendar_methods.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,48 +43,32 @@ static inline GregorianCalendar *fetch_greg(Calendar_object *co) {
return (GregorianCalendar*)co->ucal;
}

static void _php_intlgregcal_constructor_body(
INTERNAL_FUNCTION_PARAMETERS, bool is_constructor)
static void _php_intlgregcal_constructor_body(INTERNAL_FUNCTION_PARAMETERS, bool is_constructor)
{
zval *tz_object = NULL;
zval args_a[6],
*args = &args_a[0];
char *locale = NULL;
size_t locale_len;
zend_long largs[6];
UErrorCode status = U_ZERO_ERROR;
int variant;
intl_error_reset(NULL);

// parameter number validation / variant determination
if (ZEND_NUM_ARGS() > 6 ||
zend_get_parameters_array_ex(ZEND_NUM_ARGS(), args) == FAILURE) {
zend_argument_count_error("Too many arguments");
RETURN_THROWS();
}

for (variant = ZEND_NUM_ARGS();
variant > 0 && Z_TYPE(args[variant - 1]) == IS_NULL;
variant--) {}
if (variant == 4) {
zend_argument_count_error("No variant with 4 arguments (excluding trailing NULLs)");
RETURN_THROWS();
}
UErrorCode status = U_ZERO_ERROR;
zval *timezone_or_year = NULL;
zend_string *locale = NULL;
zend_long month;
bool is_month_null = true;
zend_long day_of_month = 0;
bool is_dom_null = true;
zend_long hour = 0;
zend_long minutes = 0;
zend_long seconds = 0;

ZEND_PARSE_PARAMETERS_START(0, 6)
Z_PARAM_OPTIONAL
Z_PARAM_ZVAL(timezone_or_year)
Z_PARAM_STR_OR_LONG_OR_NULL(locale, month, is_month_null)
Z_PARAM_LONG_OR_NULL(day_of_month, is_dom_null)
Z_PARAM_LONG(hour)
Z_PARAM_LONG(minutes)
Z_PARAM_LONG(seconds)
ZEND_PARSE_PARAMETERS_END();

// argument parsing
if (variant <= 2) {
if (zend_parse_parameters(MIN(ZEND_NUM_ARGS(), 2),
"|z!s!", &tz_object, &locale, &locale_len) == FAILURE) {
RETURN_THROWS();
}
}
if (variant > 2 && zend_parse_parameters(ZEND_NUM_ARGS(),
"lll|lll", &largs[0], &largs[1], &largs[2], &largs[3], &largs[4],
&largs[5]) == FAILURE) {
RETURN_THROWS();
}
intl_error_reset(NULL);

// instantion of ICU object
// instantiation of ICU object
Calendar_object *co = Z_INTL_CALENDAR_P(return_value);
GregorianCalendar *gcal = NULL;

Expand All @@ -93,27 +77,33 @@ static void _php_intlgregcal_constructor_body(
RETURN_THROWS();
}

if (variant <= 2) {
// From timezone and locale (0 to 2 arguments)
TimeZone *tz = timezone_process_timezone_argument(tz_object, NULL,
if (!timezone_or_year || Z_TYPE_P(timezone_or_year) != IS_LONG) {
if (!locale && !is_month_null) {
zend_argument_value_error(2, "must be ?string if argument 1 is a timezone");
RETURN_THROWS();
}

TimeZone *tz = timezone_process_timezone_argument(timezone_or_year, NULL,
"intlgregcal_create_instance");
if (tz == NULL) {
if (!EG(exception)) {
zend_throw_exception(IntlException_ce_ptr, "Constructor failed", 0);
}
if (!is_constructor) {
zval_ptr_dtor(return_value);
RETVAL_NULL();
RETURN_NULL();
}
return;
RETURN_THROWS();
}
if (!locale) {
locale = const_cast<char*>(intl_locale_get_default());
char *intl_locale;
if (locale) {
intl_locale = ZSTR_VAL(locale);
} else {
intl_locale = const_cast<char*>(intl_locale_get_default());
}

gcal = new GregorianCalendar(tz, Locale::createFromName(locale),
status);
// Should this throw?
gcal = new GregorianCalendar(tz, Locale::createFromName(intl_locale), status);
// Should this always throw?
if (U_FAILURE(status)) {
intl_error_set(NULL, status, "intlgregcal_create_instance: error "
"creating ICU GregorianCalendar from time zone and locale", 0);
Expand All @@ -123,31 +113,50 @@ static void _php_intlgregcal_constructor_body(
delete tz;
if (!is_constructor) {
zval_ptr_dtor(return_value);
RETVAL_NULL();
RETURN_NULL();
}
return;
RETURN_THROWS();
}
} else {
// From date/time (3, 5 or 6 arguments)
for (int i = 0; i < variant; i++) {
if (largs[i] < INT32_MIN || largs[i] > INT32_MAX) {
zend_argument_value_error(getThis() ? (i-1) : i,
"must be between %d and %d", INT32_MIN, INT32_MAX);
RETURN_THROWS();
}
ZEND_ASSERT(Z_TYPE_P(timezone_or_year) == IS_LONG);
if (locale || is_month_null) {
zend_argument_value_error(2, "must be int if argument 1 is an int");
RETURN_THROWS();
}
if (is_dom_null) {
zend_argument_value_error(3, "must be provided");
RETURN_THROWS();
}

if (variant == 3) {
gcal = new GregorianCalendar((int32_t)largs[0], (int32_t)largs[1],
(int32_t)largs[2], status);
} else if (variant == 5) {
gcal = new GregorianCalendar((int32_t)largs[0], (int32_t)largs[1],
(int32_t)largs[2], (int32_t)largs[3], (int32_t)largs[4], status);
} else if (variant == 6) {
gcal = new GregorianCalendar((int32_t)largs[0], (int32_t)largs[1],
(int32_t)largs[2], (int32_t)largs[3], (int32_t)largs[4], (int32_t)largs[5],
status);
zend_long year = Z_LVAL_P(timezone_or_year);
if (year < INT32_MIN || year > INT32_MAX) {
zend_argument_value_error(3, "must be between %d and %d", INT32_MIN, INT32_MAX);
RETURN_THROWS();
}
if (month < INT32_MIN || month > INT32_MAX) {
zend_argument_value_error(3, "must be between %d and %d", INT32_MIN, INT32_MAX);
RETURN_THROWS();
}
if (day_of_month < INT32_MIN || day_of_month > INT32_MAX) {
zend_argument_value_error(3, "must be between %d and %d", INT32_MIN, INT32_MAX);
RETURN_THROWS();
}
if (hour < INT32_MIN || hour > INT32_MAX) {
zend_argument_value_error(4, "must be between %d and %d", INT32_MIN, INT32_MAX);
RETURN_THROWS();
}
if (minutes < INT32_MIN || minutes > INT32_MAX) {
zend_argument_value_error(5, "must be between %d and %d", INT32_MIN, INT32_MAX);
RETURN_THROWS();
}
if (seconds < INT32_MIN || seconds > INT32_MAX) {
zend_argument_value_error(6, "must be between %d and %d", INT32_MIN, INT32_MAX);
RETURN_THROWS();
}

gcal = new GregorianCalendar((int32_t)year, (int32_t)month, (int32_t)day_of_month,
(int32_t)hour, (int32_t)minutes, (int32_t)seconds, status);

if (U_FAILURE(status)) {
intl_error_set(NULL, status, "intlgregcal_create_instance: error "
"creating ICU GregorianCalendar from date", 0);
Expand All @@ -156,9 +165,9 @@ static void _php_intlgregcal_constructor_body(
}
if (!is_constructor) {
zval_ptr_dtor(return_value);
RETVAL_NULL();
RETURN_NULL();
}
return;
RETURN_THROWS();
}

timelib_tzinfo *tzinfo = get_timezone_info();
Expand All @@ -171,9 +180,9 @@ static void _php_intlgregcal_constructor_body(
delete gcal;
if (!is_constructor) {
zval_ptr_dtor(return_value);
RETVAL_NULL();
RETURN_NULL();
}
return;
RETURN_THROWS();
}

TimeZone *tz = TimeZone::createTimeZone(tzstr);
Expand Down
2 changes: 1 addition & 1 deletion ext/intl/tests/gregoriancalendar___construct_basic.phpt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ $intlcal = new IntlGregorianCalendar('Europe/Lisbon', 'pt_PT');
var_dump($intlcal->getTimeZone()->getId());
var_dump($intlcal->getLocale(1));

$intlcal = new IntlGregorianCalendar('Europe/Paris', 'fr_CA', NULL, NULL, NULL, NULL);
$intlcal = new IntlGregorianCalendar('Europe/Paris', 'fr_CA');
var_dump($intlcal->getTimeZone()->getId());
var_dump($intlcal->getLocale(1));

Expand Down
18 changes: 9 additions & 9 deletions ext/intl/tests/gregoriancalendar___construct_error.phpt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,18 +12,18 @@ try {
echo $e->getMessage(), "\n";
}
try {
var_dump(intlgregcal_create_instance(1,2,3,4,5,6,7,8));
} catch (ArgumentCountError $e) {
var_dump(new IntlGregorianCalendar(null, 1));
} catch (ValueError $e) {
echo $e->getMessage(), "\n";
}
try {
var_dump(intlgregcal_create_instance(1,2,3,4));
} catch (ArgumentCountError $e) {
var_dump(new IntlGregorianCalendar(1, null));
} catch (ValueError $e) {
echo $e->getMessage(), "\n";
}
try {
var_dump(new IntlGregorianCalendar(1,2,NULL,4));
} catch (ArgumentCountError $e) {
} catch (ValueError $e) {
echo $e->getMessage(), "\n";
}
try {
Expand All @@ -40,9 +40,9 @@ try {
}
?>
--EXPECT--
Too many arguments
Too many arguments
No variant with 4 arguments (excluding trailing NULLs)
No variant with 4 arguments (excluding trailing NULLs)
intlgregcal_create_instance() expects at most 6 arguments, 7 given
IntlGregorianCalendar::__construct(): Argument #2 ($localeOrMonth) must be ?string if argument 1 is a timezone
IntlGregorianCalendar::__construct(): Argument #2 ($localeOrMonth) must be int if argument 1 is an int
IntlGregorianCalendar::__construct(): Argument #3 ($day) must be provided
IntlGregorianCalendar::__construct(): Argument #6 ($second) must be of type int, array given
IntlGregorianCalendar object is already constructed