Skip to content

Commit

Permalink
Fixes for-in, num-to-str conversion for large n
Browse files Browse the repository at this point in the history
- ToString only uses exponential form for numbers >1e21. Had already
  fixed this for printing, but missed it for converting. It was
  preventing large array indexes (e.g. 1e9) from being assigned to.
  • Loading branch information
ndreynolds committed Nov 14, 2012
1 parent 0ffd90c commit 5e7527a
Show file tree
Hide file tree
Showing 9 changed files with 181 additions and 109 deletions.
9 changes: 6 additions & 3 deletions Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@
# OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE.

CC = gcc
CFLAGS = -Wall
CFLAGS = -Wall -O3
YACC = bison -y -d -t -v
LEX = flex

Expand Down Expand Up @@ -90,8 +90,11 @@ test:
test-node:
node test/runner.js --exec node

test-v8:
node test/runner.js --exec v8 --args "test/harness.js [test]"

test-sm:
node test/runner.js --exec js --args "-f test/rhino-harness.js -f [test]"
node test/runner.js --exec js --args "-f test/harness.js -f [test]"

test-rhino:
node test/runner.js --exec rhino --timeout 10000 --args "-f test/rhino-harness.js -f [test]"
node test/runner.js --exec rhino --timeout 10000 --args "-f test/harness.js -f [test]"
3 changes: 1 addition & 2 deletions src/debug.c
Original file line number Diff line number Diff line change
Expand Up @@ -156,9 +156,8 @@ fh_debug_num(FILE *stream, js_val *num)
char *fmt = "%f";
if (fmod(num->number.val, 1) == 0)
fmt = "%.0f";
if (fabs(num->number.val) > 1e21) {
if (fabs(num->number.val) > 1e21)
fmt = "%g";
}
cfprintf(stream, ANSI_ORANGE, fmt, num->number.val);
}
}
Expand Down
58 changes: 35 additions & 23 deletions src/eval.c
Original file line number Diff line number Diff line change
Expand Up @@ -121,7 +121,8 @@ fh_var_dec_scan(js_val *ctx, ast_node *node)
node->sval = "=";
}
else {
node->type = NODE_EMPT_STMT;
// Signals that it has been hoisted
node->val = 1;
}
}

Expand Down Expand Up @@ -297,13 +298,15 @@ fh_assign(js_val *ctx, ast_node *node)
fh_member(ctx, member->e2) :
fh_get_rec(ctx, fh_str_from_node(ctx, member->e2)->string.ptr);

key = fh_str_from_node(old_ctx, member->e1)->string.ptr;

// Set the array length.
if (IS_ARR(ctx) && member->e1->type == NODE_NUM) {
int val = member->e1->val;
if (val >= ctx->object.length)
fh_set_len(ctx, val + 1);
if (IS_ARR(ctx)) {
char *err;
unsigned long val = strtod(key, &err);
if (*err == 0 && val >= ctx->object.length)
fh_set_len(ctx, val + 1);
}
key = fh_str_from_node(old_ctx, member->e1)->string.ptr;
}

if (IS_OBJ(ctx))
Expand All @@ -327,10 +330,14 @@ fh_do_assign(js_val *obj, char *name, js_val *val, char *op)
js_val *
fh_var_dec(js_val *ctx, ast_node *node, bool ignore_rval)
{
if (node->e2 == NULL || ignore_rval)
fh_set_prop(ctx, node->e1->sval, JSUNDEF(), P_WRITE | P_ENUM);
else
fh_set_prop(ctx, node->e1->sval, fh_eval(ctx, node->e2), P_WRITE | P_ENUM);
// If node->val is set, the variable declaration has already been hoisted and
// should not be touched.
if (!node->val) {
if (node->e2 == NULL || ignore_rval)
fh_set_prop(ctx, node->e1->sval, JSUNDEF(), P_WRITE | P_ENUM);
else
fh_set_prop(ctx, node->e1->sval, fh_eval(ctx, node->e2), P_WRITE | P_ENUM);
}
return JSUNDEF();
}

Expand Down Expand Up @@ -369,26 +376,29 @@ fh_for(js_val *ctx, ast_node *exp_grp, ast_node *stmt)
void
fh_forin(js_val *ctx, ast_node *node)
{
js_val *result, *obj, *env, *name;

obj = fh_eval(ctx, node->e2);
env = ctx;

if (node->e1->type == NODE_MEMBER) {
env = fh_member_parent(ctx, node->e1);
name = fh_member_child(ctx, node->e1);
}
else
name = fh_str_from_node(ctx, node->e1);

js_prop *p;
js_val *result,
*proto = fh_eval(ctx, node->e2),
*name = fh_str_from_node(ctx, node->e1);

// FIXME:
// - Doesn't get name out of hoisted variable declaration.
// - Doesn't properly handle member expressions and other permitted lhs hijinks

// Note that during the first iteration, the prototype is the object.
while (proto != NULL) {
OBJ_ITER(proto, p) {
while (obj != NULL) {
OBJ_ITER(obj, p) {
if (p->enumerable) {
// Assign to name, possibly undeclared assignment.
fh_set_rec(ctx, name->string.ptr, JSSTR(p->name));
fh_set_rec(env, name->string.ptr, JSSTR(p->name));
result = fh_eval(ctx, node->e3);
if (result->signal == S_BREAK) break;
}
}
proto = proto->proto;
obj = obj->proto;
}
}

Expand Down Expand Up @@ -959,6 +969,8 @@ fh_str_from_node(js_val *ctx, ast_node *node)

if (node->type == NODE_IDENT)
return JSSTR(node->sval);
if (node->type == NODE_VAR_DEC)
return fh_str_from_node(ctx, node->e1);
return TO_STR(fh_eval(ctx, node));
}

Expand Down
9 changes: 7 additions & 2 deletions src/flathead.c
Original file line number Diff line number Diff line change
Expand Up @@ -323,9 +323,14 @@ fh_to_string(js_val *val)
// TODO: check spec
if (val->number.is_nan) return JSSTR("NaN");
if (val->number.is_inf) return JSSTR("Infinity");
int size = snprintf(NULL, 0, "%g", val->number.val) + 1;
char *fmt = "%f";
if (fmod(val->number.val, 1) == 0)
fmt = "%.0f";
if (fabs(val->number.val) > 1e21)
fmt = "%g";
int size = snprintf(NULL, 0, fmt, val->number.val) + 1;
char *num = malloc(size);
snprintf(num, size, "%g", val->number.val);
snprintf(num, size, fmt, val->number.val);
return JSSTR(num);
}
if (IS_OBJ(val))
Expand Down
11 changes: 4 additions & 7 deletions src/gc.c
Original file line number Diff line number Diff line change
Expand Up @@ -148,16 +148,13 @@ fh_gc_mark(js_val *val)

val->marked = true;

if (val->proto) fh_gc_mark(val->proto);

if (IS_FUNC(val)) {
fh_gc_mark(val->object.scope);
fh_gc_mark(val->object.bound_this);
fh_gc_mark(val->object.instance);
}
fh_gc_mark(val->proto);

if (IS_OBJ(val)) {
fh_gc_mark(val->object.primitive);
fh_gc_mark(val->object.bound_this);
fh_gc_mark(val->object.scope);
fh_gc_mark(val->object.instance);
fh_gc_mark(val->object.parent);
}

Expand Down
1 change: 1 addition & 0 deletions src/nodes.c
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,7 @@ new_node(enum ast_node_type type, ast_node *e1, ast_node *e2, ast_node *e3,
node->line = line;
node->column = column;

node->val = 0;
if (type == NODE_NUM || type == NODE_BOOL || type == NODE_MEMBER)
node->val = x;

Expand Down
2 changes: 1 addition & 1 deletion test/rhino-harness.js → test/harness.js
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
// Define a basic console in Rhino (or SpiderMonkey) for use in tests.
// Define a basic console for implementations that do not provide one.

if (typeof console !== 'object') {
console = {
Expand Down
129 changes: 66 additions & 63 deletions test/test_arrays.js
Original file line number Diff line number Diff line change
Expand Up @@ -6,69 +6,72 @@ var assertEquals = function(a, b) {
assert(a === b);
};


// ----------------------------------------------------------------------------
// Literals & Indexing
// ----------------------------------------------------------------------------

// Simple homogenous array
var a1 = [1, 2, 3];
assertEquals(1, a1[0]);
assertEquals(2, a1[1]);
assertEquals(3, a1[2]);

// Mixed types
var a2 = [1, 'Dog', false, null, undefined, [1, 2, 3], {a: 12}];
assertEquals(1, a2[0]);
assertEquals('Dog', a2[1]);
assertEquals(false, a2[2]);
assertEquals(null, a2[3]);
assertEquals(undefined, a2[4]);
assertEquals(1, a2[5][0]);
assertEquals(2, a2[5][1]);
assertEquals(3, a2[5][2]);
assertEquals(12, a2[6].a);

// Nested arrays
var a3 = [[[[[[1, 2]]]]]];
assertEquals(1, a3[0][0][0][0][0][0]);
assertEquals(2, a3[0][0][0][0][0][1]);

// Calculated index
var getIndex = function() {
return 1;
var test = function(name, f) {
f();
};
var a4 = ['a', 'b', 'c', 'd'];
assertEquals('c', a4[4 - 2]);
assertEquals('b', a4[getIndex()]);


// ----------------------------------------------------------------------------
// Bracket Assignment
// ----------------------------------------------------------------------------

// Easy stuff

var a5 = [];

a5[0] = 10;
a5[1] = 20;
a5[2] = 30;
a5[3] = 40;

assert(a5.length === 4);
assert(a5[0] === 10);
assert(a5[1] === 20);
assert(a5[2] === 30);
assert(a5[3] === 40);

// Indices do not need to be assigned in order (or at all)

var a6 = [];

a6[12] = 42;
a6[10002] = 'hello';

assert(a6.length === 10003);
assert(a6[12] === 42);
assert(a6[10002] === 'hello');
test('simple homogenous arrays', function() {
var arr = [1, 2, 3];
assertEquals(1, arr[0]);
assertEquals(2, arr[1]);
assertEquals(3, arr[2]);
});

test('heterogeneous arrays', function() {
var arr = [1, 'Dog', false, null, undefined, [1, 2, 3], {a: 12}];
assertEquals(1, arr[0]);
assertEquals('Dog', arr[1]);
assertEquals(false, arr[2]);
assertEquals(null, arr[3]);
assertEquals(undefined, arr[4]);
assertEquals(1, arr[5][0]);
assertEquals(2, arr[5][1]);
assertEquals(3, arr[5][2]);
assertEquals(12, arr[6].a);
});

test('nested arrays', function() {
var arr = [[[[[[1, 2]]]]]];
assertEquals(1, arr[0][0][0][0][0][0]);
assertEquals(2, arr[0][0][0][0][0][1]);
});

test('expression index', function() {
var getIndex = function() { return 1; };
var a4 = ['a', 'b', 'c', 'd'];
assertEquals('c', a4[4 - 2]);
assertEquals('b', a4[getIndex()]);
});

test('bracket assignment', function() {
var arr = [];

arr[0] = 10;
arr[1] = 20;
arr[2] = 30;
arr[3] = 40;

assertEquals(4, arr.length);
assertEquals(10, arr[0]);
assertEquals(20, arr[1]);
assertEquals(30, arr[2]);
assertEquals(40, arr[3]);
});

test('indices do not need to be assigned in order (or at all)', function() {
var arr = [];

arr[12] = 42;
arr[10002] = 'hello';

assertEquals(10003, arr.length);
assertEquals(42, arr[12]);
assertEquals('hello', arr[10002]);
});

test('big arrays', function() {
var arr = [];
arr[Math.pow(2, 31) - 1] = 42;
assertEquals(Math.pow(2, 31), arr.length);
});
68 changes: 60 additions & 8 deletions test/test_forin.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,11 +3,63 @@

var assert = console.assert;

// Basic for-in
var x = 100; // expect x to be redeclared
var obj = {a:1, b:2, c:3};
console.log(x);
for(var x in obj) {
console.log(x);
assert(obj[x] > 0 && obj[x] < 4);
}
var assertBetween = function(x, min, max) {
assert(x > min && x < max); // exclusive
};

var assertEquals = function(a, b) {
assert(a === b);
};

var test = function(name, f) {
f();
};


test('for-in with variable declaration on object', function() {
var obj = { a: 1, b: 2, c: 3 };

for (var x in obj) {
assert(obj[x] > 0 && obj[x] < 4);
}
assertEquals('c', x);
});

test('for-in with variable declaration on array', function() {
var arr = [1, 2, 3, 4, 5, 6, 7, 8, 9];

for (var i in arr) {
assertBetween(arr[i], 0, 10);
}
assertEquals('8', i);
});

test('for-in with lhs expression on object', function() {
var obj = { a: 1, b: 2, c: 3 };

var k;
for (k in obj) {
assertBetween(obj[k], 0, 4);
}
assertEquals('c', k);
});

test('for-in with lhs member expression on object', function() {
var obj = { a: 1, b: 2, c: 3 };
var x = { key: null };

for (x.key in obj) {
assertBetween(obj[x.key], 0, 4);
}
assertEquals('c', x.key);
});

test('for-in with crazy lhs member expression on object', function() {
var obj = { a: 1, b: 2, c: 3 };
var x = { y: { z: { key: null } } };

for (x.y.z['k' + 'e' + 'y'] in obj) {
assertBetween(obj[x.y.z.key], 0, 4);
}
assertEquals('c', x.y.z.key);
});

0 comments on commit 5e7527a

Please sign in to comment.