Skip to content

ext/intl: GregorianCalendar addressing TODO. #19225

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.

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

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
18 changes: 8 additions & 10 deletions ext/intl/calendar/calendar_class.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,7 @@ U_CFUNC Calendar *calendar_fetch_native_calendar(zend_object *object)
{
Calendar_object *co = php_intl_calendar_fetch_object(object);

return co->ucal;
return co->ucal.get();
}

U_CFUNC void calendar_object_construct(zval *object,
Expand All @@ -71,7 +71,7 @@ U_CFUNC void calendar_object_construct(zval *object,

CALENDAR_METHOD_FETCH_OBJECT_NO_CHECK; //populate to from object
assert(co->ucal == NULL);
co->ucal = calendar;
co->ucal.reset(calendar);
}

/* {{{ clone handler for Calendar */
Expand All @@ -90,7 +90,7 @@ static zend_object *Calendar_clone_obj(zend_object *object)
if (UNEXPECTED(!newCalendar)) {
zend_throw_error(NULL, "Failed to clone IntlCalendar");
} else {
co_new->ucal = newCalendar;
co_new->ucal.reset(newCalendar);
}
} else {
zend_throw_error(NULL, "Cannot clone uninitialized IntlCalendar");
Expand Down Expand Up @@ -143,7 +143,7 @@ static HashTable *Calendar_get_debug_info(zend_object *object, int *is_temp)
debug_info = zend_new_array(8);

co = php_intl_calendar_fetch_object(object);
cal = co->ucal;
cal = co->ucal.get();

if (cal == NULL) {
ZVAL_FALSE(&zv);
Expand Down Expand Up @@ -221,10 +221,7 @@ static void Calendar_objects_free(zend_object *object)
{
Calendar_object* co = php_intl_calendar_fetch_object(object);

if (co->ucal) {
delete co->ucal;
co->ucal = NULL;
}
co->ucal.reset();
intl_error_reset(CALENDAR_ERROR_P(co));

zend_object_std_dtor(&co->zo);
Expand All @@ -237,7 +234,7 @@ static zend_object *Calendar_object_create(zend_class_entry *ce)
Calendar_object* intern = (Calendar_object*)zend_object_alloc(sizeof(Calendar_object), ce);

zend_object_std_init(&intern->zo, ce);
object_properties_init(&intern->zo, ce);
object_properties_init(&intern->zo, ce);
calendar_object_init(intern);

return &intern->zo;
Expand All @@ -250,13 +247,14 @@ static zend_object *Calendar_object_create(zend_class_entry *ce)
void calendar_register_IntlCalendar_class(void)
{
/* Create and register 'IntlCalendar' class. */
Calendar_object empty;
Calendar_ce_ptr = register_class_IntlCalendar();
Calendar_ce_ptr->default_object_handlers = &Calendar_handlers;
Calendar_ce_ptr->create_object = Calendar_object_create;

memcpy( &Calendar_handlers, &std_object_handlers,
sizeof Calendar_handlers);
Calendar_handlers.offset = XtOffsetOf(Calendar_object, zo);
Calendar_handlers.offset = reinterpret_cast<char *>(&empty.zo) - reinterpret_cast<char *>(&empty);
Calendar_handlers.clone_obj = Calendar_clone_obj;
Calendar_handlers.get_debug_info = Calendar_get_debug_info;
Calendar_handlers.free_obj = Calendar_objects_free;
Expand Down
10 changes: 10 additions & 0 deletions ext/intl/calendar/calendar_class.h
Original file line number Diff line number Diff line change
Expand Up @@ -33,13 +33,23 @@ typedef struct {
intl_error err;

// ICU calendar
#ifndef __cplusplus
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not so sure this trick is a good idea. Is there any guarantee in the C++ standard on the layout of a unique_ptr?
Perhaps we should convert the entire intl extension to C++ first before doing a change in a common file.

Calendar* ucal;
#else
std::unique_ptr<Calendar> ucal;
#endif

zend_object zo;
} Calendar_object;

static inline Calendar_object *php_intl_calendar_fetch_object(zend_object *obj) {
#ifndef __cplusplus
return (Calendar_object *)((char*)(obj) - XtOffsetOf(Calendar_object, zo));
#else
Calendar_object empty;
size_t offset = reinterpret_cast<char *>(&empty.zo) - reinterpret_cast<char *>(&empty);
return reinterpret_cast<Calendar_object *>(reinterpret_cast<char *>(obj) - offset);
#endif
}
#define Z_INTL_CALENDAR_P(zv) php_intl_calendar_fetch_object(Z_OBJ_P(zv))

Expand Down
6 changes: 3 additions & 3 deletions ext/intl/calendar/calendar_methods.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -227,7 +227,7 @@ static void _php_intlcal_field_uec_ret_in32t_method(

CALENDAR_METHOD_FETCH_OBJECT;

int32_t result = (co->ucal->*func)(
int32_t result = (co->ucal.get()->*func)(
(UCalendarDateFields)field, CALENDAR_ERROR_CODE(co));
INTL_METHOD_CHECK_STATUS(co, "Call to ICU method has failed");

Expand Down Expand Up @@ -349,7 +349,7 @@ static void _php_intlcal_before_after(
RETURN_THROWS();
}

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

RETURN_BOOL((int)res);
Expand Down Expand Up @@ -610,7 +610,7 @@ static void _php_intlcal_field_ret_in32t_method(

CALENDAR_METHOD_FETCH_OBJECT;

int32_t result = (co->ucal->*func)((UCalendarDateFields)field);
int32_t result = (co->ucal.get()->*func)((UCalendarDateFields)field);
INTL_METHOD_CHECK_STATUS(co, "Call to ICU method has failed");

RETURN_LONG((zend_long)result);
Expand Down
9 changes: 4 additions & 5 deletions ext/intl/calendar/gregoriancalendar_methods.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ using icu::StringPiece;
}

static inline GregorianCalendar *fetch_greg(Calendar_object *co) {
return static_cast<GregorianCalendar *>(co->ucal);
return static_cast<GregorianCalendar *>(co->ucal.get());
}

static bool set_gregorian_calendar_time_zone(GregorianCalendar *gcal, UErrorCode status)
Expand Down Expand Up @@ -204,7 +204,7 @@ static void _php_intlgregcal_constructor_body(
}
}

co->ucal = gcal.release();
co->ucal = std::move(gcal);
}

U_CFUNC PHP_FUNCTION(intlgregcal_create_instance)
Expand Down Expand Up @@ -256,7 +256,7 @@ U_CFUNC PHP_METHOD(IntlGregorianCalendar, createFromDate)

object_init_ex(return_value, GregorianCalendar_ce_ptr);
co = Z_INTL_CALENDAR_P(return_value);
co->ucal = gcal.release();
co->ucal = std::move(gcal);

cleanup:
zend_restore_error_handling(&error_handling);
Expand Down Expand Up @@ -304,8 +304,7 @@ U_CFUNC PHP_METHOD(IntlGregorianCalendar, createFromDateTime)

object_init_ex(return_value, GregorianCalendar_ce_ptr);
co = Z_INTL_CALENDAR_P(return_value);
// TODO: trying to get passed the ownership change step
co->ucal = gcal.release();
co->ucal = std::move(gcal);

cleanup:
zend_restore_error_handling(&error_handling);
Expand Down