Skip to content
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

Use decnum for addition #1997

Open
wants to merge 5 commits into
base: master
Choose a base branch
from
Open

Use decnum for addition #1997

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
05e5984
Use decnum to add numbers
wtlangford Oct 22, 2019
2e51cf5
Clean up context creation
wtlangford Oct 22, 2019
11de034
fixup! Use decnum to add numbers
wtlangford Oct 22, 2019
08d6dec
fixup! Use decnum to add numbers
wtlangford Oct 23, 2019
d812abb
fixup! Use decnum to add numbers
wtlangford Oct 23, 2019
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
6 changes: 1 addition & 5 deletions src/builtin.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -90,11 +90,7 @@ static jv f_plus(jq_state *jq, jv input, jv a, jv b) {
jv_free(b);
return a;
} else if (jv_get_kind(a) == JV_KIND_NUMBER && jv_get_kind(b) == JV_KIND_NUMBER) {
jv r = jv_number(jv_number_value(a) +
jv_number_value(b));
jv_free(a);
jv_free(b);
return r;
return jv_number_add(a, b);
} else if (jv_get_kind(a) == JV_KIND_STRING && jv_get_kind(b) == JV_KIND_STRING) {
return jv_string_concat(a, b);
} else if (jv_get_kind(a) == JV_KIND_ARRAY && jv_get_kind(b) == JV_KIND_ARRAY) {
Expand Down
68 changes: 48 additions & 20 deletions src/jv.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -230,28 +230,24 @@ static decContext* tsd_dec_ctx_get(pthread_key_t *key) {
return ctx;
}

decContext _ctx = {
0,
DEC_MAX_EMAX,
DEC_MIN_EMAX,
DEC_ROUND_HALF_UP,
0, /*no errors*/
0, /*status*/
0, /*no clamping*/
};
ctx = malloc(sizeof(decContext));
if (!ctx) {
return ctx;
}

// Initialize the context, clear any traps
decContextDefault(ctx, DEC_INIT_BASE);
ctx->traps = 0;

if (key == &dec_ctx_key) {
_ctx.digits = DEC_MAX_DIGITS;
ctx->digits = DEC_MAX_DIGITS;
} else if (key == &dec_ctx_dbl_key) {
_ctx.digits = BIN64_DEC_PRECISION;
ctx->digits = BIN64_DEC_PRECISION;
}

ctx = malloc(sizeof(decContext));
if (ctx) {
*ctx = _ctx;
if (pthread_setspecific(*key, ctx) != 0) {
fprintf(stderr, "error: cannot store thread specific data");
abort();
}
if (pthread_setspecific(*key, ctx) != 0) {
fprintf(stderr, "error: cannot store thread specific data");
abort();
}
return ctx;
}
Expand Down Expand Up @@ -291,8 +287,9 @@ static decNumber* jvp_dec_number_ptr(jv j) {

static jvp_literal_number* jvp_literal_number_alloc(unsigned literal_length) {

/* The number of units needed is ceil(DECNUMDIGITS/DECDPUN) */
int units = ((literal_length+DECDPUN-1)/DECDPUN);
// The number of units needed is ceil(DECNUMDIGITS/DECDPUN)
// We already have one in the decNumber, so we subtract one from that
int units = ((literal_length+DECDPUN-1)/DECDPUN)-1;

jvp_literal_number* n = jv_mem_alloc(
sizeof(jvp_literal_number)
Expand All @@ -314,6 +311,7 @@ static jv jvp_literal_number_new(const char * literal) {

if (ctx->status & DEC_Conversion_syntax) {
jv_mem_free(n);
decContextClearStatus(ctx, DEC_Conversion_syntax);
return JV_INVALID;
}

Expand Down Expand Up @@ -484,6 +482,36 @@ int jvp_number_cmp(jv a, jv b) {
}
}

jv jv_number_add(jv a, jv b) {
if (jvp_number_is_literal(a) && jvp_number_is_literal(b)) {
decNumber *da = jvp_dec_number_ptr(a);
decNumber *db = jvp_dec_number_ptr(b);

unsigned digits = MAX(da->digits+da->exponent, db->digits+db->exponent) -
MIN(da->exponent, db->exponent) + 1;
jvp_literal_number *nlit = jvp_literal_number_alloc(digits);
nlit->refcnt = JV_REFCNT_INIT;
nlit->literal_data = NULL;
nlit->num_double = NAN;

decContext *ctx = DEC_CONTEXT();
decNumberAdd(&nlit->num_decimal, da, db, ctx);
decNumberTrim(&nlit->num_decimal);
// TODO: Check context for error?

jv_free(a);
jv_free(b);

jv r = {JVP_FLAGS_NUMBER_LITERAL, 0, 0, JV_NUMBER_SIZE_INIT, {&nlit->refcnt}};
return r;
}

jv r = jv_number(jv_number_value(a) + jv_number_value(b));
jv_free(a);
jv_free(b);
return r;
}

/*
* Arrays (internal helpers)
*/
Expand Down
1 change: 1 addition & 0 deletions src/jv.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,7 @@ jv jv_number(double);
jv jv_number_with_literal(const char*);
double jv_number_value(jv);
int jv_is_integer(jv);
jv jv_number_add(jv a, jv b);

int jv_number_has_literal(jv n);
const char* jv_number_get_literal(jv);
Expand Down
9 changes: 7 additions & 2 deletions src/parser.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -369,10 +369,13 @@ static block constant_fold(block a, block b, int op) {
double na = jv_number_value(jv_a);
double nb = jv_number_value(jv_b);

int cmp = jv_cmp(jv_a, jv_b);
// Once we implement the rest of the arithmetic operations, all of these
// copy/frees can be optimized away, if we move the call to jv_cmp inside
// the switch cases. We'd just need to move the final frees into the default
int cmp = jv_cmp(jv_copy(jv_a), jv_copy(jv_b));

switch (op) {
case '+': res = jv_number(na + nb); break;
case '+': res = jv_number_add(jv_copy(jv_a), jv_copy(jv_b)); break;
case '-': res = jv_number(na - nb); break;
case '*': res = jv_number(na * nb); break;
case '/': res = jv_number(na / nb); break;
Expand All @@ -384,6 +387,8 @@ static block constant_fold(block a, block b, int op) {
case GREATEREQ: res = (cmp >= 0 ? jv_true() : jv_false()); break;
default: break;
}
jv_free(jv_a);
jv_free(jv_b);
} else if (op == '+' && block_const_kind(a) == JV_KIND_STRING) {
res = jv_string_concat(block_const(a), block_const(b));
} else {
Expand Down
9 changes: 7 additions & 2 deletions src/parser.y
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -222,10 +222,13 @@ static block constant_fold(block a, block b, int op) {
double na = jv_number_value(jv_a);
double nb = jv_number_value(jv_b);

int cmp = jv_cmp(jv_a, jv_b);
// Once we implement the rest of the arithmetic operations, all of these
// copy/frees can be optimized away, if we move the call to jv_cmp inside
// the switch cases. We'd just need to move the final frees into the default
int cmp = jv_cmp(jv_copy(jv_a), jv_copy(jv_b));

switch (op) {
case '+': res = jv_number(na + nb); break;
case '+': res = jv_number_add(jv_copy(jv_a), jv_copy(jv_b)); break;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Isn't it too late to copy the arguments here? jv_cmp above has already consumed those, hasn't it?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You're right, it has. I'll fix that.

case '-': res = jv_number(na - nb); break;
case '*': res = jv_number(na * nb); break;
case '/': res = jv_number(na / nb); break;
Expand All @@ -237,6 +240,8 @@ static block constant_fold(block a, block b, int op) {
case GREATEREQ: res = (cmp >= 0 ? jv_true() : jv_false()); break;
default: break;
}
jv_free(jv_a);
jv_free(jv_b);
} else if (op == '+' && block_const_kind(a) == JV_KIND_STRING) {
res = jv_string_concat(block_const(a), block_const(b));
} else {
Expand Down
7 changes: 6 additions & 1 deletion tests/jq.test
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1698,8 +1698,13 @@ map(. == 1)
null
false

# Applying addition to the value will not truncate the result to double

# Applying arithmetic to the value will truncate the result to double
. + 1
418502930602131457
418502930602131458

# Applying other arithmetic to the value will truncate the result to double

. - 10
13911860366432393
Expand Down