Skip to content

Commit a22dc1f

Browse files
sparkprimesparkprime
committed
Fix join, and also some sign/unsigned mismatches
1 parent e9dfa54 commit a22dc1f

File tree

2 files changed

+67
-61
lines changed

2 files changed

+67
-61
lines changed

‎core/vm.cpp

Lines changed: 65 additions & 61 deletions
Original file line numberDiff line numberDiff line change
@@ -133,6 +133,7 @@ struct Frame {
133133

134134
/** Used for accumulating a joined string. */
135135
UString str;
136+
bool first;
136137

137138
/** The context is used in error messages to attempt to find a reasonable name for the
138139
* object, function, or thunk value being executed. If it is a thunk, it is filled
@@ -1330,12 +1331,12 @@ class Interpreter {
13301331
const auto *str = static_cast<const HeapString *>(args[0].v.h);
13311332
const auto *c = static_cast<const HeapString *>(args[1].v.h);
13321333
long maxsplits = long(args[2].v.d);
1333-
int start = 0;
1334-
int test = 0;
1334+
unsigned start = 0;
1335+
unsigned test = 0;
13351336
scratch = makeArray({});
13361337
auto &elements = static_cast<HeapArray *>(scratch.v.h)->elements;
13371338
while (test < str->value.size() && (maxsplits == -1 ||
1338-
maxsplits > elements.size())) {
1339+
size_t(maxsplits) > elements.size())) {
13391340
if (c->value[0] == str->value[test]) {
13401341
auto *th = makeHeap<HeapThunk>(idArrayElement, nullptr, 0, nullptr);
13411342
elements.push_back(th);
@@ -1359,14 +1360,20 @@ class Interpreter {
13591360
const auto *str = static_cast<const HeapString *>(args[0].v.h);
13601361
long from = long(args[1].v.d);
13611362
long len = long(args[2].v.d);
1362-
if (len + from > str->value.size()) {
1363-
len = str->value.size() - from;
1363+
if (from < 0) {
1364+
std::stringstream ss;
1365+
ss << "substr second parameter should be greater than zero, got " << from;
1366+
throw makeError(loc, ss.str());
13641367
}
1365-
if (len <= 0) {
1366-
scratch = makeString(U"");
1367-
} else {
1368-
scratch = makeString(str->value.substr(from, len));
1368+
if (len < 0) {
1369+
std::stringstream ss;
1370+
ss << "substr third parameter should be greater than zero, got " << len;
1371+
throw makeError(loc, ss.str());
1372+
}
1373+
if (size_t(len + from) > str->value.size()) {
1374+
len = str->value.size() - from;
13691375
}
1376+
scratch = makeString(str->value.substr(from, len));
13701377
return nullptr;
13711378
}
13721379

@@ -1416,7 +1423,7 @@ class Interpreter {
14161423
validateBuiltinArgs(loc, "asciiLower", args, {Value::STRING});
14171424
const auto *str = static_cast<const HeapString *>(args[0].v.h);
14181425
UString new_str(str->value);
1419-
for (int i = 0; i < new_str.size(); ++i) {
1426+
for (size_t i = 0; i < new_str.size(); ++i) {
14201427
if (new_str[i] >= 'A' && new_str[i] <= 'Z') {
14211428
new_str[i] = new_str[i] - 'A' + 'a';
14221429
}
@@ -1430,7 +1437,7 @@ class Interpreter {
14301437
validateBuiltinArgs(loc, "asciiUpper", args, {Value::STRING});
14311438
const auto *str = static_cast<const HeapString *>(args[0].v.h);
14321439
UString new_str(str->value);
1433-
for (int i = 0; i < new_str.size(); ++i) {
1440+
for (size_t i = 0; i < new_str.size(); ++i) {
14341441
if (new_str[i] >= 'a' && new_str[i] <= 'z') {
14351442
new_str[i] = new_str[i] - 'a' + 'A';
14361443
}
@@ -1508,7 +1515,7 @@ class Interpreter {
15081515
}
15091516
}
15101517

1511-
void joinString(UString &running, const Value &sep, unsigned idx, const Value &elt)
1518+
void joinString(bool &first, UString &running, const Value &sep, unsigned idx, const Value &elt)
15121519
{
15131520
if (elt.t == Value::NULL_TYPE) {
15141521
return;
@@ -1518,39 +1525,33 @@ class Interpreter {
15181525
ss << "expected string but arr[" << idx << "] was " << type_str(elt);
15191526
throw makeError(stack.top().location, ss.str());
15201527
}
1521-
if (!running.empty()) {
1528+
if (!first) {
15221529
running.append(static_cast<HeapString *>(sep.v.h)->value);
15231530
}
1531+
first = false;
15241532
running.append(static_cast<HeapString *>(elt.v.h)->value);
15251533
}
15261534

1527-
const AST *joinStrings(const Value &sep, const Value &arr, unsigned idx, UString running)
1535+
const AST *joinStrings(void)
15281536
{
1529-
const auto& elements = static_cast<HeapArray*>(arr.v.h)->elements;
1530-
if (idx >= elements.size()) {
1531-
scratch = makeString(running);
1532-
} else {
1533-
for (int i = idx; i < elements.size(); ++i) {
1534-
auto *th = elements[i];
1535-
if (th->filled) {
1536-
joinString(running, sep, i, th->content);
1537-
} else {
1538-
Frame &f = stack.top();
1539-
f.kind = FRAME_BUILTIN_JOIN_STRINGS;
1540-
f.val = sep;
1541-
f.val2 = arr;
1542-
f.str = running;
1543-
f.elementId = i;
1544-
stack.newCall(f.location, th, th->self, th->offset, th->upValues);
1545-
return th->body;
1546-
}
1537+
Frame &f = stack.top();
1538+
const auto& elements = static_cast<HeapArray*>(f.val2.v.h)->elements;
1539+
while (f.elementId < elements.size()) {
1540+
auto *th = elements[f.elementId];
1541+
if (th->filled) {
1542+
joinString(f.first, f.str, f.val, f.elementId, th->content);
1543+
f.elementId++;
1544+
} else {
1545+
stack.newCall(f.location, th, th->self, th->offset, th->upValues);
1546+
return th->body;
15471547
}
1548-
scratch = makeString(running);
15491548
}
1549+
scratch = makeString(f.str);
15501550
return nullptr;
15511551
}
15521552

1553-
void joinArray(std::vector<HeapThunk*> &running, const Value &sep, unsigned idx, const Value &elt)
1553+
void joinArray(bool &first, std::vector<HeapThunk*> &running, const Value &sep, unsigned idx,
1554+
const Value &elt)
15541555
{
15551556
if (elt.t == Value::NULL_TYPE) {
15561557
return;
@@ -1560,37 +1561,30 @@ class Interpreter {
15601561
ss << "expected array but arr[" << idx << "] was " << type_str(elt);
15611562
throw makeError(stack.top().location, ss.str());
15621563
}
1563-
if (!running.empty()) {
1564+
if (!first) {
15641565
auto& elts = static_cast<HeapArray *>(sep.v.h)->elements;
15651566
running.insert(running.end(), elts.begin(), elts.end());
15661567
}
1568+
first = false;
15671569
auto& elts = static_cast<HeapArray *>(elt.v.h)->elements;
15681570
running.insert(running.end(), elts.begin(), elts.end());
15691571
}
15701572

1571-
const AST *joinArrays(const Value &sep, const Value &arr, unsigned idx, std::vector<HeapThunk*> &running)
1573+
const AST *joinArrays(void)
15721574
{
1573-
const auto& elements = static_cast<HeapArray*>(arr.v.h)->elements;
1574-
if (idx >= elements.size()) {
1575-
scratch = makeArray(running);
1576-
} else {
1577-
for (int i = idx; i < elements.size(); ++i) {
1578-
auto *th = elements[i];
1579-
if (th->filled) {
1580-
joinArray(running, sep, i, th->content);
1581-
} else {
1582-
Frame &f = stack.top();
1583-
f.kind = FRAME_BUILTIN_JOIN_ARRAYS;
1584-
f.val = sep;
1585-
f.val2 = arr;
1586-
f.thunks = running;
1587-
f.elementId = i;
1588-
stack.newCall(f.location, th, th->self, th->offset, th->upValues);
1589-
return th->body;
1590-
}
1575+
Frame &f = stack.top();
1576+
const auto& elements = static_cast<HeapArray*>(f.val2.v.h)->elements;
1577+
while (f.elementId < elements.size()) {
1578+
auto *th = elements[f.elementId];
1579+
if (th->filled) {
1580+
joinArray(f.first, f.thunks, f.val, f.elementId, th->content);
1581+
f.elementId++;
1582+
} else {
1583+
stack.newCall(f.location, th, th->self, th->offset, th->upValues);
1584+
return th->body;
15911585
}
1592-
scratch = makeArray(running);
15931586
}
1587+
scratch = makeArray(f.thunks);
15941588
return nullptr;
15951589
}
15961590

@@ -1608,11 +1602,21 @@ class Interpreter {
16081602
}
16091603
Frame &f = stack.top();
16101604
if (args[0].t == Value::STRING) {
1605+
f.kind = FRAME_BUILTIN_JOIN_STRINGS;
1606+
f.val = args[0]; // sep
1607+
f.val2 = args[1]; // arr
16111608
f.str.clear();
1612-
return joinStrings(args[0], args[1], 0, f.str);
1609+
f.first = true;
1610+
f.elementId = 0;
1611+
return joinStrings();
16131612
} else {
1613+
f.kind = FRAME_BUILTIN_JOIN_ARRAYS;
1614+
f.val = args[0]; // sep
1615+
f.val2 = args[1]; // arr
16141616
f.thunks.clear();
1615-
return joinArrays(args[0], args[1], 0, f.thunks);
1617+
f.first = true;
1618+
f.elementId = 0;
1619+
return joinArrays();
16161620
}
16171621
}
16181622

@@ -2832,19 +2836,19 @@ class Interpreter {
28322836
} break;
28332837

28342838
case FRAME_BUILTIN_JOIN_STRINGS: {
2835-
joinString(f.str, f.val, f.elementId, scratch);
2839+
joinString(f.first, f.str, f.val, f.elementId, scratch);
28362840
f.elementId++;
2837-
auto *ast = joinStrings(f.val, f.val2, f.elementId, f.str);
2841+
auto *ast = joinStrings();
28382842
if (ast != nullptr) {
28392843
ast_ = ast;
28402844
goto recurse;
28412845
}
28422846
} break;
28432847

28442848
case FRAME_BUILTIN_JOIN_ARRAYS: {
2845-
joinArray(f.thunks, f.val, f.elementId, scratch);
2849+
joinArray(f.first, f.thunks, f.val, f.elementId, scratch);
28462850
f.elementId++;
2847-
auto *ast = joinArrays(f.val, f.val2, f.elementId, f.thunks);
2851+
auto *ast = joinArrays();
28482852
if (ast != nullptr) {
28492853
ast_ = ast;
28502854
goto recurse;

‎test_suite/stdlib.jsonnet

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -208,10 +208,12 @@ std.assertEqual(std.join([], [[1, 2], [3, 4, 5], [6]]), [1, 2, 3, 4, 5, 6]) &&
208208
std.assertEqual(std.join(['a', 'b'], [[]]), []) &&
209209
std.assertEqual(std.join(['a', 'b'], []), []) &&
210210
std.assertEqual(std.join(['a', 'b'], [null, [1, 2], null, [3, 4, 5], [6], null]), [1, 2, 'a', 'b', 3, 4, 5, 'a', 'b', 6]) &&
211+
std.assertEqual(std.join(['a', 'b'], [[], [1, 2]]), ['a', 'b', 1, 2]) &&
211212
std.assertEqual(std.join('', [null, '12', null, '345', '6', null]), '123456') &&
212213
std.assertEqual(std.join('ab', ['']), '') &&
213214
std.assertEqual(std.join('ab', []), '') &&
214215
std.assertEqual(std.join('ab', [null, '12', null, '345', '6', null]), '12ab345ab6') &&
216+
std.assertEqual(std.join('ab', ['', '12']), 'ab12') &&
215217
std.assertEqual(std.lines(['a', null, 'b']), 'a\nb\n') &&
216218

217219
std.assertEqual(std.flattenArrays([[1, 2, 3], [4, 5, 6], []]), [1, 2, 3, 4, 5, 6]) &&

0 commit comments

Comments
 (0)