Skip to content

Commit 207a010

Browse files
adazem009adazem009
committed
Use output parameter in value to string conversion functions
1 parent 4387cea commit 207a010

File tree

9 files changed

+352
-198
lines changed

9 files changed

+352
-198
lines changed

‎include/scratchcpp/list.h‎

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -163,7 +163,9 @@ class LIBSCRATCHCPP_EXPORT List : public Entity
163163

164164
for (i = 0; i < m_size; i++) {
165165
const ValueData *item = &m_dataPtr->operator[](i);
166-
strings.push_back(value_toStringPtr(item));
166+
StringPtr *str = string_pool_new();
167+
value_toStringPtr(item, str);
168+
strings.push_back(str);
167169
size += strings.back()->size;
168170

169171
if (value_isValidNumber(item) && !value_isBool(item) && strings.back()->size > 0) {
@@ -198,7 +200,8 @@ class LIBSCRATCHCPP_EXPORT List : public Entity
198200
}
199201

200202
for (; i < m_size; i++) {
201-
StringPtr *item = value_toStringPtr(&m_dataPtr->operator[](i));
203+
StringPtr *item = string_pool_new();
204+
value_toStringPtr(&m_dataPtr->operator[](i), item);
202205
size += item->size + 1;
203206
string_alloc(ret, size);
204207
memcpy(ret->data + ret->size, item->data, item->size * sizeof(char16_t));
@@ -219,7 +222,8 @@ class LIBSCRATCHCPP_EXPORT List : public Entity
219222
}
220223

221224
for (; i < m_size; i++) {
222-
StringPtr *item = value_toStringPtr(&m_dataPtr->operator[](i));
225+
StringPtr *item = string_pool_new();
226+
value_toStringPtr(&m_dataPtr->operator[](i), item);
223227
size += item->size + 1;
224228
string_alloc(ret, size);
225229
memcpy(ret->data + ret->size, item->data, item->size * sizeof(char16_t));

‎include/scratchcpp/value_functions.h‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -75,14 +75,14 @@ extern "C"
7575
LIBSCRATCHCPP_EXPORT double value_toDouble(const ValueData *v);
7676
LIBSCRATCHCPP_EXPORT bool value_toBool(const ValueData *v);
7777
LIBSCRATCHCPP_EXPORT void value_toString(const ValueData *v, std::string *dst);
78-
LIBSCRATCHCPP_EXPORT StringPtr *value_toStringPtr(const ValueData *v);
78+
LIBSCRATCHCPP_EXPORT void value_toStringPtr(const ValueData *v, StringPtr *dst);
7979
LIBSCRATCHCPP_EXPORT void value_toUtf16(const ValueData *v, std::u16string *dst);
8080
LIBSCRATCHCPP_EXPORT Rgb value_toRgba(const ValueData *v);
8181
LIBSCRATCHCPP_EXPORT const void *value_toPointer(const ValueData *v);
8282

8383
LIBSCRATCHCPP_EXPORT bool value_doubleIsInt(double v);
8484

85-
LIBSCRATCHCPP_EXPORT StringPtr *value_doubleToStringPtr(double v);
85+
LIBSCRATCHCPP_EXPORT void value_doubleToStringPtr(double v, StringPtr *dst);
8686
LIBSCRATCHCPP_EXPORT const StringPtr *value_boolToStringPtr(bool v);
8787
LIBSCRATCHCPP_EXPORT double value_stringToDouble(const StringPtr *s);
8888
LIBSCRATCHCPP_EXPORT double value_stringToDoubleWithCheck(const StringPtr *s, bool *ok);

‎src/engine/internal/llvm/llvmbuildutils.cpp‎

Lines changed: 11 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -629,7 +629,8 @@ llvm::Value *LLVMBuildUtils::castValue(LLVMRegister *reg, Compiler::StaticType t
629629
llvm::Value *intCast = m_builder.CreateSIToFP(reg->intValue, m_builder.getDoubleTy());
630630
llvm::Value *value = m_builder.CreateSelect(reg->isInt, intCast, doubleValue);
631631

632-
llvm::Value *numberResult = m_builder.CreateCall(m_functions.resolve_value_doubleToStringPtr(), value);
632+
llvm::Value *numberResult = m_builder.CreateCall(m_functions.resolve_string_pool_new(), { m_builder.getInt1(true) });
633+
m_builder.CreateCall(m_functions.resolve_value_doubleToStringPtr(), { value, numberResult });
633634
m_builder.CreateBr(mergeBlock);
634635
results.push_back({ numberBlock, numberResult });
635636
}
@@ -1307,8 +1308,11 @@ llvm::Value *LLVMBuildUtils::createStringComparison(LLVMRegister *arg1, LLVMRegi
13071308

13081309
if (arg1->isConst() && arg2->isConst()) {
13091310
// If both operands are constant, perform the comparison at compile time
1310-
StringPtr *str1 = value_toStringPtr(&arg1->constValue().data());
1311-
StringPtr *str2 = value_toStringPtr(&arg2->constValue().data());
1311+
StringPtr *str1 = string_pool_new();
1312+
StringPtr *str2 = string_pool_new();
1313+
value_toStringPtr(&arg1->constValue().data(), str1);
1314+
value_toStringPtr(&arg2->constValue().data(), str2);
1315+
13121316
bool result;
13131317

13141318
if (caseSensitive)
@@ -1496,7 +1500,8 @@ llvm::Value *LLVMBuildUtils::castRawValue(LLVMRegister *reg, Compiler::StaticTyp
14961500
// Convert double/int to string
14971501
llvm::Value *intCast = m_builder.CreateSIToFP(reg->intValue, m_builder.getDoubleTy());
14981502
llvm::Value *doubleValue = m_builder.CreateSelect(reg->isInt, intCast, reg->value);
1499-
llvm::Value *ptr = m_builder.CreateCall(m_functions.resolve_value_doubleToStringPtr(), doubleValue);
1503+
llvm::Value *ptr = m_builder.CreateCall(m_functions.resolve_string_pool_new(), { m_builder.getInt1(true) });
1504+
m_builder.CreateCall(m_functions.resolve_value_doubleToStringPtr(), { doubleValue, ptr });
15001505
freeStringLater(ptr);
15011506
return ptr;
15021507
}
@@ -1791,7 +1796,8 @@ llvm::Value *LLVMBuildUtils::createNumberAndStringComparison(LLVMRegister *arg1,
17911796

17921797
// String comparison
17931798
m_builder.SetInsertPoint(stringBlock);
1794-
llvm::Value *stringValue = m_builder.CreateCall(m_functions.resolve_value_doubleToStringPtr(), { value1 });
1799+
llvm::Value *stringValue = m_builder.CreateCall(m_functions.resolve_string_pool_new(), { m_builder.getInt1(true) });
1800+
m_builder.CreateCall(m_functions.resolve_value_doubleToStringPtr(), { value1, stringValue });
17951801
llvm::Value *cmp = m_builder.CreateCall(m_functions.resolve_string_compare_case_insensitive(), { stringValue, value2 });
17961802
m_builder.CreateCall(m_functions.resolve_string_pool_free(), { stringValue }); // free the string immediately
17971803

‎src/engine/internal/llvm/llvmfunctions.cpp‎

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -104,14 +104,18 @@ llvm::FunctionCallee LLVMFunctions::resolve_value_toBool()
104104

105105
llvm::FunctionCallee LLVMFunctions::resolve_value_toStringPtr()
106106
{
107-
// NOTE: This function can't be marked read-only because it allocates on the heap
108-
return resolveFunction("value_toStringPtr", llvm::FunctionType::get(m_stringPtrType->getPointerTo(), m_valueDataType->getPointerTo(), false));
107+
llvm::FunctionCallee callee = resolveFunction("value_toStringPtr", llvm::FunctionType::get(m_builder->getVoidTy(), { m_valueDataType->getPointerTo(), m_stringPtrType->getPointerTo() }, false));
108+
llvm::Function *func = llvm::cast<llvm::Function>(callee.getCallee());
109+
func->addFnAttr(llvm::Attribute::ReadOnly);
110+
return callee;
109111
}
110112

111113
llvm::FunctionCallee LLVMFunctions::resolve_value_doubleToStringPtr()
112114
{
113-
// NOTE: This function can't be marked read-only because it allocates on the heap
114-
return resolveFunction("value_doubleToStringPtr", llvm::FunctionType::get(m_stringPtrType->getPointerTo(), m_builder->getDoubleTy(), false));
115+
llvm::FunctionCallee callee = resolveFunction("value_doubleToStringPtr", llvm::FunctionType::get(m_builder->getVoidTy(), { m_builder->getDoubleTy(), m_stringPtrType->getPointerTo() }, false));
116+
llvm::Function *func = llvm::cast<llvm::Function>(callee.getCallee());
117+
func->addFnAttr(llvm::Attribute::ReadOnly);
118+
return callee;
115119
}
116120

117121
llvm::FunctionCallee LLVMFunctions::resolve_value_boolToStringPtr()

‎src/scratch/value_functions.cpp‎

Lines changed: 75 additions & 102 deletions
Original file line numberDiff line numberDiff line change
@@ -281,44 +281,31 @@ extern "C"
281281
/*! Writes the string representation of the given value to dst. */
282282
void value_toString(const ValueData *v, std::string *dst)
283283
{
284-
StringPtr *str = value_toStringPtr(v);
284+
StringPtr *str = string_pool_new();
285+
value_toStringPtr(v, str);
285286
dst->assign(utf8::utf16to8(std::u16string(str->data)));
286287
string_pool_free(str);
287288
}
288289

289-
/*!
290-
* Returns the string representation of the given value.
291-
* \note It is the caller's responsibility to free allocated memory.
292-
*/
293-
StringPtr *value_toStringPtr(const ValueData *v)
290+
/*! Writes the string representation of the given value to dst. */
291+
void value_toStringPtr(const ValueData *v, StringPtr *dst)
294292
{
295-
if (v->type == ValueType::String) {
296-
StringPtr *ret = string_pool_new();
297-
string_assign(ret, v->stringValue);
298-
return ret;
299-
} else if (v->type == ValueType::Number)
300-
return value_doubleToStringPtr(v->numberValue);
293+
if (v->type == ValueType::String)
294+
string_assign(dst, v->stringValue);
295+
else if (v->type == ValueType::Number)
296+
value_doubleToStringPtr(v->numberValue, dst);
301297
else if (v->type == ValueType::Bool) {
302-
if (v->boolValue) {
303-
StringPtr *ret = string_pool_new();
304-
string_assign(ret, &TRUE_STR);
305-
return ret;
306-
} else {
307-
StringPtr *ret = string_pool_new();
308-
string_assign(ret, &FALSE_STR);
309-
return ret;
310-
}
311-
} else {
312-
StringPtr *ret = string_pool_new();
313-
string_assign(ret, &ZERO_STR);
314-
return ret;
315-
}
298+
const StringPtr *boolStr = value_boolToStringPtr(v->boolValue);
299+
string_assign(dst, boolStr);
300+
} else
301+
string_assign(dst, &ZERO_STR);
316302
}
317303

318304
/*! Writes the UTF-16 representation of the given value to dst. */
319305
void value_toUtf16(const libscratchcpp::ValueData *v, std::u16string *dst)
320306
{
321-
StringPtr *str = value_toStringPtr(v);
307+
StringPtr *str = string_pool_new();
308+
value_toStringPtr(v, str);
322309
dst->assign(str->data);
323310
string_pool_free(str);
324311
}
@@ -395,90 +382,76 @@ extern "C"
395382
return v == intpart;
396383
}
397384

398-
/*!
399-
* Converts the given number to string.
400-
* \note It is the caller's responsibility to free the string.
401-
*/
402-
StringPtr *value_doubleToStringPtr(double v)
385+
/*! Converts the given number to string and stores the result in dst. */
386+
void value_doubleToStringPtr(double v, StringPtr *dst)
403387
{
404-
if (v == 0) {
405-
StringPtr *ret = string_pool_new();
406-
string_assign_cstring(ret, "0");
407-
return ret;
408-
} else if (std::isinf(v)) {
409-
if (v > 0) {
410-
StringPtr *ret = string_pool_new();
411-
string_assign(ret, &INFINITY_STR);
412-
return ret;
388+
if (v == 0)
389+
string_assign_cstring(dst, "0");
390+
else if (std::isinf(v)) {
391+
if (v > 0)
392+
string_assign(dst, &INFINITY_STR);
393+
else
394+
string_assign(dst, &NEGATIVE_INFINITY_STR);
395+
} else if (std::isnan(v))
396+
string_assign(dst, &NAN_STR);
397+
else {
398+
// snprintf() is locale-dependent
399+
std::string oldLocale = std::setlocale(LC_NUMERIC, nullptr);
400+
std::setlocale(LC_NUMERIC, "C");
401+
402+
const int maxlen = 26; // should be enough for any number
403+
char *buffer = (char *)malloc((maxlen + 1) * sizeof(char));
404+
405+
// Constants for significant digits and thresholds
406+
const int significantDigits = 17 - value_intDigitCount(std::floor(std::fabs(v)));
407+
constexpr double scientificThreshold = 1e21;
408+
constexpr double minScientificThreshold = 1e-6;
409+
410+
// Use scientific notation if the number is very large or very small
411+
if (std::fabs(v) >= scientificThreshold || std::fabs(v) < minScientificThreshold) {
412+
int ret = snprintf(buffer, maxlen, "%.*g", significantDigits - 1, v);
413+
assert(ret >= 0);
413414
} else {
414-
StringPtr *ret = string_pool_new();
415-
string_assign(ret, &NEGATIVE_INFINITY_STR);
416-
return ret;
417-
}
418-
} else if (std::isnan(v)) {
419-
StringPtr *ret = string_pool_new();
420-
string_assign(ret, &NAN_STR);
421-
return ret;
422-
}
423-
424-
// snprintf() is locale-dependent
425-
std::string oldLocale = std::setlocale(LC_NUMERIC, nullptr);
426-
std::setlocale(LC_NUMERIC, "C");
427-
428-
const int maxlen = 26; // should be enough for any number
429-
char *buffer = (char *)malloc((maxlen + 1) * sizeof(char));
430-
431-
// Constants for significant digits and thresholds
432-
const int significantDigits = 17 - value_intDigitCount(std::floor(std::fabs(v)));
433-
constexpr double scientificThreshold = 1e21;
434-
constexpr double minScientificThreshold = 1e-6;
435-
436-
// Use scientific notation if the number is very large or very small
437-
if (std::fabs(v) >= scientificThreshold || std::fabs(v) < minScientificThreshold) {
438-
int ret = snprintf(buffer, maxlen, "%.*g", significantDigits - 1, v);
439-
assert(ret >= 0);
440-
} else {
441-
snprintf(buffer, maxlen, "%.*f", significantDigits - 1, v);
442-
443-
// Remove trailing zeros from the fractional part
444-
char *dot = std::strchr(buffer, '.');
445-
446-
if (dot) {
447-
char *end = buffer + std::strlen(buffer) - 1;
448-
while (end > dot && *end == '0') {
449-
*end-- = '\0';
450-
}
451-
if (*end == '.') {
452-
*end = '\0'; // Remove trailing dot
415+
snprintf(buffer, maxlen, "%.*f", significantDigits - 1, v);
416+
417+
// Remove trailing zeros from the fractional part
418+
char *dot = std::strchr(buffer, '.');
419+
420+
if (dot) {
421+
char *end = buffer + std::strlen(buffer) - 1;
422+
while (end > dot && *end == '0') {
423+
*end-- = '\0';
424+
}
425+
if (*end == '.') {
426+
*end = '\0'; // Remove trailing dot
427+
}
453428
}
454429
}
455-
}
456430

457-
// Remove leading zeros after e+/e-
458-
for (int i = 0; i < 2; i++) {
459-
const char *target = (i == 0) ? "e+" : "e-";
460-
char *index = strstr(buffer, target);
461-
462-
if (index != nullptr) {
463-
char *ptr = index + 2;
464-
while (*(ptr + 1) != '\0' && *ptr == '0') {
465-
// Shift the characters left to erase '0'
466-
char *shiftPtr = ptr;
467-
do {
468-
*shiftPtr = *(shiftPtr + 1);
469-
shiftPtr++;
470-
} while (*shiftPtr != '\0');
431+
// Remove leading zeros after e+/e-
432+
for (int i = 0; i < 2; i++) {
433+
const char *target = (i == 0) ? "e+" : "e-";
434+
char *index = strstr(buffer, target);
435+
436+
if (index != nullptr) {
437+
char *ptr = index + 2;
438+
while (*(ptr + 1) != '\0' && *ptr == '0') {
439+
// Shift the characters left to erase '0'
440+
char *shiftPtr = ptr;
441+
do {
442+
*shiftPtr = *(shiftPtr + 1);
443+
shiftPtr++;
444+
} while (*shiftPtr != '\0');
445+
}
471446
}
472447
}
473-
}
474448

475-
// Restore old locale
476-
std::setlocale(LC_NUMERIC, oldLocale.c_str());
449+
// Restore old locale
450+
std::setlocale(LC_NUMERIC, oldLocale.c_str());
477451

478-
StringPtr *ret = string_pool_new();
479-
string_assign_cstring(ret, buffer);
480-
free(buffer);
481-
return ret;
452+
string_assign_cstring(dst, buffer);
453+
free(buffer);
454+
}
482455
}
483456

484457
/*!

‎src/scratch/value_functions_p.h‎

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -486,8 +486,11 @@ extern "C"
486486
if (!ok) {
487487
// At least one argument can't be converted to a number
488488
// Scratch compares strings as case insensitive
489-
StringPtr *s1 = value_toStringPtr(v1);
490-
StringPtr *s2 = value_toStringPtr(v2);
489+
StringPtr *s1 = string_pool_new();
490+
StringPtr *s2 = string_pool_new();
491+
value_toStringPtr(v1, s1);
492+
value_toStringPtr(v2, s2);
493+
491494
int ret = string_compare_case_insensitive(s1, s2);
492495
string_pool_free(s1);
493496
string_pool_free(s2);

‎test/llvm/llvmtestutils.cpp‎

Lines changed: 24 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -197,11 +197,31 @@ Value LLVMTestUtils::getExpectedOpResult(OpType type, const Value &v1, const Val
197197
case OpType::CmpLT:
198198
return v1 < v2;
199199

200-
case OpType::StrCmpEQCS:
201-
return string_compare_case_sensitive(value_toStringPtr(&v1.data()), value_toStringPtr(&v2.data())) == 0;
200+
case OpType::StrCmpEQCS: {
201+
StringPtr *s1 = string_pool_new();
202+
StringPtr *s2 = string_pool_new();
203+
value_toStringPtr(&v1.data(), s1);
204+
value_toStringPtr(&v2.data(), s2);
202205

203-
case OpType::StrCmpEQCI:
204-
return string_compare_case_insensitive(value_toStringPtr(&v1.data()), value_toStringPtr(&v2.data())) == 0;
206+
bool ret = string_compare_case_sensitive(s1, s2) == 0;
207+
208+
string_pool_free(s1);
209+
string_pool_free(s2);
210+
return ret;
211+
}
212+
213+
case OpType::StrCmpEQCI: {
214+
StringPtr *s1 = string_pool_new();
215+
StringPtr *s2 = string_pool_new();
216+
value_toStringPtr(&v1.data(), s1);
217+
value_toStringPtr(&v2.data(), s2);
218+
219+
bool ret = string_compare_case_insensitive(s1, s2) == 0;
220+
221+
string_pool_free(s1);
222+
string_pool_free(s2);
223+
return ret;
224+
}
205225

206226
case OpType::And:
207227
return v1.toBool() && v2.toBool();

0 commit comments

Comments
 (0)