Skip to content

Commit

Permalink
ICU-22520 Use operator* instead of calling std::optional::value().
Browse files Browse the repository at this point in the history
There's a subtle difference between these two ways of accessing the
value of an optional and that is that the value() method can throw an
exception if there isn't any value, but operator* won't do that (it's
just undefined behavior if there isn't any value).

ICU4C code never tries to access any optional value without first
checking that it exists, but the ability of the value() method to throw
an exception in case there wasn't any such check first is the reason why
std::exception symbols previously could show up in debug builds.

This reverts the changes that were made to dependencies.txt by
commit dc70b5a.
  • Loading branch information
roubert committed Mar 4, 2024
1 parent 73744ea commit 232362b
Show file tree
Hide file tree
Showing 3 changed files with 15 additions and 21 deletions.
10 changes: 5 additions & 5 deletions icu4c/source/common/localematcher.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -610,7 +610,7 @@ const Locale *LocaleMatcher::getBestMatch(const Locale &desiredLocale, UErrorCod
std::optional<int32_t> suppIndex = getBestSuppIndex(
getMaximalLsrOrUnd(likelySubtags, desiredLocale, errorCode),
nullptr, errorCode);
return U_SUCCESS(errorCode) && suppIndex.has_value() ? supportedLocales[suppIndex.value()]
return U_SUCCESS(errorCode) && suppIndex.has_value() ? supportedLocales[*suppIndex]
: defaultLocale;
}

Expand All @@ -622,7 +622,7 @@ const Locale *LocaleMatcher::getBestMatch(Locale::Iterator &desiredLocales,
}
LocaleLsrIterator lsrIter(likelySubtags, desiredLocales, ULOCMATCH_TEMPORARY_LOCALES);
std::optional<int32_t> suppIndex = getBestSuppIndex(lsrIter.next(errorCode), &lsrIter, errorCode);
return U_SUCCESS(errorCode) && suppIndex.has_value() ? supportedLocales[suppIndex.value()]
return U_SUCCESS(errorCode) && suppIndex.has_value() ? supportedLocales[*suppIndex]
: defaultLocale;
}

Expand All @@ -645,7 +645,7 @@ LocaleMatcher::Result LocaleMatcher::getBestMatchResult(
if (U_FAILURE(errorCode) || !suppIndex.has_value()) {
return Result(nullptr, defaultLocale, -1, -1, false);
} else {
return Result(&desiredLocale, supportedLocales[suppIndex.value()], 0, suppIndex.value(), false);
return Result(&desiredLocale, supportedLocales[*suppIndex], 0, *suppIndex, false);
}
}

Expand All @@ -659,8 +659,8 @@ LocaleMatcher::Result LocaleMatcher::getBestMatchResult(
if (U_FAILURE(errorCode) || !suppIndex.has_value()) {
return Result(nullptr, defaultLocale, -1, -1, false);
} else {
return Result(lsrIter.orphanRemembered(), supportedLocales[suppIndex.value()],
lsrIter.getBestDesiredIndex(), suppIndex.value(), true);
return Result(lsrIter.orphanRemembered(), supportedLocales[*suppIndex],
lsrIter.getBestDesiredIndex(), *suppIndex, true);
}
}

Expand Down
20 changes: 10 additions & 10 deletions icu4c/source/common/uloc.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1154,12 +1154,12 @@ std::optional<int16_t> _findIndex(const char* const* list, const char* key)
U_CFUNC const char*
uloc_getCurrentCountryID(const char* oldID){
std::optional<int16_t> offset = _findIndex(DEPRECATED_COUNTRIES, oldID);
return offset.has_value() ? REPLACEMENT_COUNTRIES[offset.value()] : oldID;
return offset.has_value() ? REPLACEMENT_COUNTRIES[*offset] : oldID;
}
U_CFUNC const char*
uloc_getCurrentLanguageID(const char* oldID){
std::optional<int16_t> offset = _findIndex(DEPRECATED_LANGUAGES, oldID);
return offset.has_value() ? REPLACEMENT_LANGUAGES[offset.value()] : oldID;
return offset.has_value() ? REPLACEMENT_LANGUAGES[*offset] : oldID;
}

namespace {
Expand Down Expand Up @@ -1225,7 +1225,7 @@ _getLanguage(const char* localeID,
buffer[3] = '\0';
std::optional<int16_t> offset = _findIndex(LANGUAGES_3, buffer);
if (offset.has_value()) {
const char* const alias = LANGUAGES[offset.value()];
const char* const alias = LANGUAGES[*offset];
sink->Append(alias, (int32_t)uprv_strlen(alias));
return;
}
Expand Down Expand Up @@ -1306,7 +1306,7 @@ _getRegion(const char* localeID,
buffer[3] = '\0';
std::optional<int16_t> offset = _findIndex(COUNTRIES_3, buffer);
if (offset.has_value()) {
const char* const alias = COUNTRIES[offset.value()];
const char* const alias = COUNTRIES[*offset];
sink->Append(alias, (int32_t)uprv_strlen(alias));
return;
}
Expand Down Expand Up @@ -1465,10 +1465,10 @@ ulocimp_getSubtags(

ulocimp_getSubtags(
localeID,
languageSink.has_value() ? &languageSink.value() : nullptr,
scriptSink.has_value() ? &scriptSink.value() : nullptr,
regionSink.has_value() ? &regionSink.value() : nullptr,
variantSink.has_value() ? &variantSink.value() : nullptr,
languageSink.has_value() ? &*languageSink : nullptr,
scriptSink.has_value() ? &*scriptSink : nullptr,
regionSink.has_value() ? &*regionSink : nullptr,
variantSink.has_value() ? &*variantSink : nullptr,
pEnd,
status);
}
Expand Down Expand Up @@ -2183,7 +2183,7 @@ uloc_getISO3Language(const char* localeID)
if (U_FAILURE(err))
return "";
std::optional<int16_t> offset = _findIndex(LANGUAGES, lang.data());
return offset.has_value() ? LANGUAGES_3[offset.value()] : "";
return offset.has_value() ? LANGUAGES_3[*offset] : "";
}

U_CAPI const char* U_EXPORT2
Expand All @@ -2199,7 +2199,7 @@ uloc_getISO3Country(const char* localeID)
if (U_FAILURE(err))
return "";
std::optional<int16_t> offset = _findIndex(COUNTRIES, cntry.data());
return offset.has_value() ? COUNTRIES_3[offset.value()] : "";
return offset.has_value() ? COUNTRIES_3[*offset] : "";
}

U_CAPI uint32_t U_EXPORT2
Expand Down
6 changes: 0 additions & 6 deletions icu4c/source/test/depstest/dependencies.txt
Original file line number Diff line number Diff line change
Expand Up @@ -139,12 +139,6 @@ group: cplusplus
# "Calls the current terminate handler."
std::terminate()

# ICU4C doesn't actually use C++ exceptions, but the standard library does,
# so these symbols can end up in debug builds.
"std::exception::~exception()"
"typeinfo for std::exception"
"vtable for std::exception"

group: iostream
"std::basic_ios<char, std::char_traits<char> >::clear(std::_Ios_Iostate)"
"std::basic_ios<char, std::char_traits<char> >::eof() const"
Expand Down

0 comments on commit 232362b

Please sign in to comment.