Skip to content

ICU-23447 MessageFormat ICU4C: Make Function* pointers const #4072

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

Open
catamorphism wants to merge 4 commits into
base: main
Choose a base branch
from
+60 −46
Open

ICU-23447 MessageFormat ICU4C: Make Function* pointers const #4072

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
6ae3801
ICU-23447 MessageFormat ICU4C: Make Function* pointers const
catamorphism Jul 9, 2026
e5d614a
ICU-23447 Fix LocalPointer template usage for Counter
srl295 Aug 20, 2026
d91f1b5
ICU-23447 merge from main
srl295 Aug 20, 2026
f65e49b
ICU-23447 update mf2 test for constness
srl295 Aug 20, 2026
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
2 changes: 1 addition & 1 deletion icu4c/source/i18n/messageformat2.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -254,7 +254,7 @@ static UnicodeString reserialize(const UnicodeString& s) {
UnicodeString fallbackStr = functionFallback(rand, functionName);

// Look up the function name
Function* function = lookupFunction(functionName, status);
const Function* function = lookupFunction(functionName, status);

if (U_FAILURE(status)) { // Handle unknown function
// Set error and use the fallback value
Expand Down
4 changes: 2 additions & 2 deletions icu4c/source/i18n/messageformat2_formatter.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -278,14 +278,14 @@ namespace message2 {
return standardMFFunctionRegistry.hasFunction(functionName);
}

Function*
const Function*
MessageFormatter::lookupFunction(const FunctionName& functionName,
UErrorCode& status) const {
NULL_ON_ERROR(status);

if (hasCustomMFFunctionRegistry()) {
const MFFunctionRegistry& customMFFunctionRegistry = getCustomMFFunctionRegistry();
Function* function = customMFFunctionRegistry.getFunction(functionName);
const Function* function = customMFFunctionRegistry.getFunction(functionName);
if (function != nullptr) {
return function;
}
Expand Down
13 changes: 6 additions & 7 deletions icu4c/source/i18n/messageformat2_function_registry.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -107,10 +107,9 @@ MFFunctionRegistry::Builder::~Builder() {
}

// Returns non-owned pointer. Returns pointer rather than reference because it can fail.
// Returns non-const because Function is mutable.
Function* MFFunctionRegistry::getFunction(const FunctionName& functionName) const {
const Function* MFFunctionRegistry::getFunction(const FunctionName& functionName) const {
U_ASSERT(functions != nullptr);
return static_cast<Function*>(functions->get(functionName));
return static_cast<const Function*>(functions->get(functionName));
}

UBool MFFunctionRegistry::getDefaultFormatterNameByType(const UnicodeString& type, FunctionName& name) const {
Expand Down Expand Up @@ -550,7 +549,7 @@ StandardFunctions::Number::create(StandardFunctions::NumberType numberType, UErr
LocalPointer<FunctionValue> StandardFunctions::Number::call(const FunctionContext& context,
const FunctionValue& operand,
const FunctionOptions& options,
UErrorCode& errorCode) {
UErrorCode& errorCode) const {
if (U_FAILURE(errorCode)) {
return LocalPointer<FunctionValue>();
}
Expand Down Expand Up @@ -1304,7 +1303,7 @@ LocalPointer<FunctionValue>
StandardFunctions::DateTime::call(const FunctionContext& context,
const FunctionValue& val,
const FunctionOptions& opts,
UErrorCode& errorCode) {
UErrorCode& errorCode) const {
if (U_FAILURE(errorCode)) {
return LocalPointer<FunctionValue>();
}
Expand Down Expand Up @@ -1807,7 +1806,7 @@ LocalPointer<FunctionValue>
StandardFunctions::String::call(const FunctionContext& context,
const FunctionValue& val,
const FunctionOptions& opts,
UErrorCode& errorCode) {
UErrorCode& errorCode) const {
if (U_FAILURE(errorCode)) {
return LocalPointer<FunctionValue>();
}
Expand Down Expand Up @@ -1892,7 +1891,7 @@ StandardFunctions::TestFunctionValue::~TestFunctionValue() {}
LocalPointer<FunctionValue> StandardFunctions::TestFunction::call(const FunctionContext& context,
const FunctionValue& operand,
const FunctionOptions& options,
UErrorCode& errorCode) {
UErrorCode& errorCode) const {
if (U_FAILURE(errorCode)) {
return LocalPointer<FunctionValue>();
}
Expand Down
8 changes: 4 additions & 4 deletions icu4c/source/i18n/messageformat2_function_registry_internal.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -159,7 +159,7 @@ static constexpr std::u16string_view YEAR = u"year";
LocalPointer<FunctionValue> call(const FunctionContext& context,
const FunctionValue& operand,
const FunctionOptions& options,
UErrorCode& errorCode) override;
UErrorCode& errorCode) const override;
virtual ~DateTime();

private:
Expand Down Expand Up @@ -194,7 +194,7 @@ static constexpr std::u16string_view YEAR = u"year";
LocalPointer<FunctionValue> call(const FunctionContext& context,
const FunctionValue& operand,
const FunctionOptions& options,
UErrorCode& errorCode) override;
UErrorCode& errorCode) const override;
virtual ~Number();

private:
Expand Down Expand Up @@ -279,7 +279,7 @@ static constexpr std::u16string_view YEAR = u"year";
LocalPointer<FunctionValue> call(const FunctionContext& context,
const FunctionValue& val,
const FunctionOptions& opts,
UErrorCode& errorCode) override;
UErrorCode& errorCode) const override;
static String* string(UErrorCode& status);
virtual ~String();

Expand All @@ -301,7 +301,7 @@ static constexpr std::u16string_view YEAR = u"year";
LocalPointer<FunctionValue> call(const FunctionContext& context,
const FunctionValue& operand,
const FunctionOptions& options,
UErrorCode& errorCode) override;
UErrorCode& errorCode) const override;
virtual ~TestFunction();
private:
friend class TestFunctionValue;
Expand Down
3 changes: 1 addition & 2 deletions icu4c/source/i18n/unicode/messageformat2.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -487,8 +487,7 @@ namespace message2 {
bool isBuiltInFunction(const FunctionName&) const;
bool isFunction(const FunctionName& fn) const { return isBuiltInFunction(fn) || isCustomFunction(fn); }
void setNotSelectableError(MessageContext&, const InternalValue&, UErrorCode&) const;
// Result is not adopted
Function* lookupFunction(const FunctionName&, UErrorCode&) const;
const Function* lookupFunction(const FunctionName&, UErrorCode&) const;
bool getDefaultFormatterNameByType(const UnicodeString&, FunctionName&) const;

// Checking for resolution errors
Expand Down
9 changes: 4 additions & 5 deletions icu4c/source/i18n/unicode/messageformat2_function_registry.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -50,9 +50,8 @@ namespace message2 {

public:
/**
* Looks up a function by the name of the function. The result is non-const,
* since functions may have local state. Returns the result by pointer
* rather than by reference since it can fail.
* Looks up a function by the name of the function.
* Returns the result by pointer rather than by reference since it can fail.
*
* @param functionName Name of the desired function.
* @return A pointer to the function registered under `functionName`, or null
Expand All @@ -62,7 +61,7 @@ namespace message2 {
* @internal ICU 79 technology preview
* @deprecated This API is for technology preview only.
*/
Function* getFunction(const FunctionName& functionName) const;
const Function* getFunction(const FunctionName& functionName) const;
/**
* Looks up a function by a type tag. This method gets the name of the default formatter registered
* for that type. If no formatter was explicitly registered for this type, it returns false.
Expand Down Expand Up @@ -306,7 +305,7 @@ namespace message2 {
virtual LocalPointer<FunctionValue> call(const FunctionContext& context,
const FunctionValue& operand,
const FunctionOptions& options,
UErrorCode& status) = 0;
UErrorCode& status) const = 0;
/**
* Destructor.
*
Expand Down
4 changes: 2 additions & 2 deletions icu4c/source/test/intltest/messageformat2test.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -515,7 +515,7 @@ class FixedDateFunction : public Function {
public:
FixedDateFunction(UErrorCode& status);
LocalPointer<FunctionValue> call(const FunctionContext &, const FunctionValue &,
const FunctionOptions &, UErrorCode &) override;
const FunctionOptions &, UErrorCode &) const override;
virtual ~FixedDateFunction();
};

Expand All @@ -526,7 +526,7 @@ FixedDateFunction::FixedDateFunction(UErrorCode& status) {
LocalPointer<FunctionValue> FixedDateFunction::call(const FunctionContext &context,
const FunctionValue &arg,
const FunctionOptions &opts,
UErrorCode &errorCode) {
UErrorCode &errorCode) const {
(void)context;

if (U_FAILURE(errorCode)) {
Expand Down
31 changes: 20 additions & 11 deletions icu4c/source/test/intltest/messageformat2test.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -124,7 +124,7 @@ class PersonNameFunction : public Function {
LocalPointer<FunctionValue> call(const FunctionContext&,
const FunctionValue&,
const FunctionOptions&,
UErrorCode&) override;
UErrorCode&) const override;
virtual ~PersonNameFunction();
PersonNameFunction() {}
};
Expand Down Expand Up @@ -155,7 +155,7 @@ class FormattableProperties : public FormattableObject {

class GrammarCasesFunction : public Function {
public:
LocalPointer<FunctionValue> call(const FunctionContext&, const FunctionValue&, const FunctionOptions&, UErrorCode&) override;
LocalPointer<FunctionValue> call(const FunctionContext&, const FunctionValue&, const FunctionOptions&, UErrorCode&) const override;
static MFFunctionRegistry customRegistry(UErrorCode&);
};

Expand All @@ -174,7 +174,7 @@ class GrammarCasesValue : public FunctionValue {

class ListFunction : public Function {
public:
LocalPointer<FunctionValue> call(const FunctionContext&, const FunctionValue&, const FunctionOptions&, UErrorCode&) override;
LocalPointer<FunctionValue> call(const FunctionContext&, const FunctionValue&, const FunctionOptions&, UErrorCode&) const override;
static MFFunctionRegistry customRegistry(UErrorCode&);
ListFunction() {}
virtual ~ListFunction();
Expand Down Expand Up @@ -225,7 +225,7 @@ class AdjectiveValue : public FunctionValue {

class ResourceManager : public Function {
public:
LocalPointer<FunctionValue> call(const FunctionContext&, const FunctionValue&, const FunctionOptions&, UErrorCode&) override;
LocalPointer<FunctionValue> call(const FunctionContext&, const FunctionValue&, const FunctionOptions&, UErrorCode&) const override;
static MFFunctionRegistry customRegistry(UErrorCode&);
static Hashtable* properties(UErrorCode&);
static UnicodeString propertiesAsString(const Hashtable&);
Expand All @@ -250,25 +250,34 @@ class ResourceManagerValue : public FunctionValue {

class NounFunction : public Function {
public:
LocalPointer<FunctionValue> call(const FunctionContext&, const FunctionValue&, const FunctionOptions&, UErrorCode&) override;
LocalPointer<FunctionValue> call(const FunctionContext&, const FunctionValue&, const FunctionOptions&, UErrorCode&) const override;
NounFunction() { }
virtual ~NounFunction();
};

class AdjectiveFunction : public Function {
public:
LocalPointer<FunctionValue> call(const FunctionContext&, const FunctionValue&, const FunctionOptions&, UErrorCode&) override;
LocalPointer<FunctionValue> call(const FunctionContext&, const FunctionValue&, const FunctionOptions&, UErrorCode&) const override;
AdjectiveFunction() { }
virtual ~AdjectiveFunction();
};

class Counter {
public:
void increment() { value++; }
int32_t get() const { return value; }
Counter() {}
private:
int32_t value = 0;
};

class CounterFunction : public Function {
public:
LocalPointer<FunctionValue> call(const FunctionContext&, const FunctionValue&, const FunctionOptions&, UErrorCode&) override;
CounterFunction() { }
LocalPointer<FunctionValue> call(const FunctionContext&, const FunctionValue&, const FunctionOptions&, UErrorCode&) const override;
CounterFunction(UErrorCode& errorCode);
virtual ~CounterFunction();
private:
int32_t count = 0; // Number of times the function was called
LocalPointer<Counter> count; // Number of times the function was called
Comment thread
srl295 marked this conversation as resolved.
};

class CounterFunctionValue : public FunctionValue {
Expand All @@ -278,9 +287,9 @@ class CounterFunctionValue : public FunctionValue {
virtual ~CounterFunctionValue();
private:
friend class CounterFunction;
int32_t& count;
Counter* count;

CounterFunctionValue(int32_t&,
CounterFunctionValue(Counter*,
const FunctionValue&,
const FunctionOptions&,
UErrorCode&);
Expand Down
32 changes: 20 additions & 12 deletions icu4c/source/test/intltest/messageformat2test_custom.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -285,7 +285,7 @@ static bool hasStringOption(const FunctionOptionsMap& opt,
LocalPointer<FunctionValue> PersonNameFunction::call(const FunctionContext& context,
const FunctionValue& arg,
const FunctionOptions& opts,
UErrorCode& errorCode) {
UErrorCode& errorCode) const {
(void) context;

if (U_FAILURE(errorCode)) {
Expand Down Expand Up @@ -404,7 +404,7 @@ LocalPointer<FunctionValue>
GrammarCasesFunction::call(const FunctionContext& context,
const FunctionValue& arg,
const FunctionOptions& opts,
UErrorCode& errorCode) {
UErrorCode& errorCode) const {
(void) context;

if (U_FAILURE(errorCode)) {
Expand Down Expand Up @@ -535,7 +535,7 @@ LocalPointer<FunctionValue>
ListFunction::call(const FunctionContext& context,
const FunctionValue& arg,
const FunctionOptions& opts,
UErrorCode& errorCode) {
UErrorCode& errorCode) const {
if (U_FAILURE(errorCode)) {
return LocalPointer<FunctionValue>();
}
Expand Down Expand Up @@ -711,7 +711,7 @@ LocalPointer<FunctionValue>
ResourceManager::call(const FunctionContext&,
const FunctionValue& arg,
const FunctionOptions& options,
UErrorCode& errorCode) {
UErrorCode& errorCode) const {
if (U_FAILURE(errorCode)) {
return LocalPointer<FunctionValue>();
}
Expand Down Expand Up @@ -878,7 +878,7 @@ LocalPointer<FunctionValue>
NounFunction::call(const FunctionContext&,
const FunctionValue& arg,
const FunctionOptions& opts,
UErrorCode& errorCode) {
UErrorCode& errorCode) const {
if (U_FAILURE(errorCode)) {
return LocalPointer<FunctionValue>();
}
Expand Down Expand Up @@ -938,7 +938,7 @@ LocalPointer<FunctionValue>
AdjectiveFunction::call(const FunctionContext&,
const FunctionValue& arg,
const FunctionOptions& opts,
UErrorCode& errorCode) {
UErrorCode& errorCode) const {
if (U_FAILURE(errorCode)) {
return LocalPointer<FunctionValue>();
}
Expand Down Expand Up @@ -1014,7 +1014,7 @@ void TestMessageFormat2::testSingleEvaluation(IcuTestErrorCode& errorCode) {

MFFunctionRegistry customRegistry(MFFunctionRegistry::Builder(errorCode)
.adoptFunction(FunctionName("counter"),
new CounterFunction(),
new CounterFunction(errorCode),
errorCode)
.build());
UnicodeString name = "name";
Expand All @@ -1032,25 +1032,33 @@ void TestMessageFormat2::testSingleEvaluation(IcuTestErrorCode& errorCode) {
TestUtils::runTestCase(*this, test, errorCode);
}

CounterFunction::CounterFunction(UErrorCode& errorCode) {
if (U_FAILURE(errorCode)) {
return;
}
Counter* c = new Counter();
count = LocalPointer<Counter>(c, errorCode);
}

LocalPointer<FunctionValue>
CounterFunction::call(const FunctionContext&,
const FunctionValue& arg,
const FunctionOptions& opts,
UErrorCode& errorCode) {
UErrorCode& errorCode) const {
if (U_FAILURE(errorCode)) {
return LocalPointer<FunctionValue>();
}

LocalPointer<FunctionValue>
v(new CounterFunctionValue(count, arg, std::move(opts), errorCode));
v(new CounterFunctionValue(count.getAlias(), arg, std::move(opts), errorCode));
if (!v.isValid()) {
errorCode = U_MEMORY_ALLOCATION_ERROR;
}
count++;
count->increment();
return v;
}

CounterFunctionValue::CounterFunctionValue(int32_t& c,
CounterFunctionValue::CounterFunctionValue(Counter* c,
const FunctionValue&,
const FunctionOptions&,
UErrorCode&) : count(c) {
Expand All @@ -1062,7 +1070,7 @@ UnicodeString CounterFunctionValue::formatToString(UErrorCode& status) const {
return {};
}
number::UnlocalizedNumberFormatter nf = number::NumberFormatter::with();
number::FormattedNumber formattedNumber = nf.locale("en-US").formatInt(count, status);
number::FormattedNumber formattedNumber = nf.locale("en-US").formatInt(count->get(), status);
return formattedNumber.toString(status);
}

Expand Down