Skip to content

ext/intl: using a bit more modern c++ memory management features. #19163

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

Merged
merged 2 commits into from
Jul 18, 2025
Merged

ext/intl: using a bit more modern c++ memory management features. #19163

Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
3f70910
ext/intl: using a bit more modern c++ emmory management features.
devnexen Jul 17, 2025
fa30a33
changes from feedback
devnexen Jul 17, 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
51 changes: 20 additions & 31 deletions ext/intl/dateformat/dateformat_format_object.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -70,10 +70,10 @@ U_CFUNC PHP_FUNCTION(datefmt_format_object)
size_t locale_len;
bool pattern = false;
UDate date;
TimeZone *timeZone = NULL;
std::unique_ptr<TimeZone> timeZone = NULL;
Copy link
Member

Choose a reason for hiding this comment

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

I believe it's not necessary to value-initialize these unique_ptrs.

Copy link
Member Author

Choose a reason for hiding this comment

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

yes true

UErrorCode status = U_ZERO_ERROR;
DateFormat *df = NULL;
Calendar *cal = NULL;
std::unique_ptr<DateFormat> df = NULL;
std::unique_ptr<Calendar> cal = NULL;
DateFormat::EStyle dateStyle = DateFormat::kDefault,
timeStyle = DateFormat::kDefault;

Expand Down Expand Up @@ -158,28 +158,28 @@ U_CFUNC PHP_FUNCTION(datefmt_format_object)
"not initialized properly", 0);
RETURN_FALSE;
}
timeZone = obj_cal->getTimeZone().clone();
timeZone = std::unique_ptr<TimeZone>(obj_cal->getTimeZone().clone());
date = obj_cal->getTime(status);
if (U_FAILURE(status)) {
intl_error_set(NULL, status,
"datefmt_format_object: error obtaining instant from "
"IntlCalendar", 0);
RETVAL_FALSE;
goto cleanup;
RETURN_FALSE;
}
cal = obj_cal->clone();
cal = std::unique_ptr<Calendar>(obj_cal->clone());
} else if (instanceof_function(instance_ce, php_date_get_interface_ce())) {
if (intl_datetime_decompose(object, &date, &timeZone, NULL,
TimeZone *tz;
if (intl_datetime_decompose(object, &date, &tz, NULL,
"datefmt_format_object") == FAILURE) {
RETURN_FALSE;
}
cal = new GregorianCalendar(Locale::createFromName(locale_str), status);
timeZone = std::unique_ptr<TimeZone>(tz);
cal = std::unique_ptr<Calendar>(new GregorianCalendar(Locale::createFromName(locale_str), status));
Copy link
Member

Choose a reason for hiding this comment

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

Can't you use make_unique here instead of unique_ptr+new ?

Copy link
Member Author

@devnexen devnexen Jul 17, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

I wish but isn't it C++14 minimum ?

Copy link
Member

Choose a reason for hiding this comment

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

Ah right, and it's the ICU version that decides between C++11/C++17... Nvm then ;)

if (U_FAILURE(status)) {
intl_error_set(NULL, status,
"datefmt_format_object: could not create GregorianCalendar",
0);
RETVAL_FALSE;
goto cleanup;
RETURN_FALSE;
}
} else {
intl_error_set(NULL, status, "datefmt_format_object: the passed object "
Expand All @@ -190,36 +190,32 @@ U_CFUNC PHP_FUNCTION(datefmt_format_object)

if (pattern) {
StringPiece sp(Z_STRVAL_P(format));
df = new SimpleDateFormat(
df = std::unique_ptr<DateFormat>(new SimpleDateFormat(
UnicodeString::fromUTF8(sp),
Locale::createFromName(locale_str),
status);
status));

if (U_FAILURE(status)) {
intl_error_set(NULL, status,
"datefmt_format_object: could not create SimpleDateFormat",
0);
RETVAL_FALSE;
goto cleanup;
RETURN_FALSE;
}
} else {
df = DateFormat::createDateTimeInstance(dateStyle, timeStyle,
Locale::createFromName(locale_str));
df = std::unique_ptr<DateFormat>(DateFormat::createDateTimeInstance(dateStyle, timeStyle,
Locale::createFromName(locale_str)));

if (df == NULL) { /* according to ICU sources, this should never happen */
intl_error_set(NULL, status,
"datefmt_format_object: could not create DateFormat",
0);
RETVAL_FALSE;
goto cleanup;
RETURN_FALSE;
}
}

//must be in this order (or have the cal adopt the tz)
df->adoptCalendar(cal);
cal = NULL;
df->adoptTimeZone(timeZone);
timeZone = NULL;
df->adoptCalendar(cal.release());
df->adoptTimeZone(timeZone.release());

{
zend_string *u8str;
Expand All @@ -231,15 +227,8 @@ U_CFUNC PHP_FUNCTION(datefmt_format_object)
intl_error_set(NULL, status,
"datefmt_format_object: error converting result to UTF-8",
0);
RETVAL_FALSE;
goto cleanup;
RETURN_FALSE;
}
RETVAL_STR(u8str);
}


cleanup:
delete df;
delete timeZone;
delete cal;
}