Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
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 .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -8,5 +8,5 @@
*.exe

# Generated by Cargo
/target/
target/
Cargo.lock
15 changes: 15 additions & 0 deletions decimal-macros/Cargo.toml
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
[package]
name = "decimal-macros"
version = "0.1.0"
authors = ["Callum Tolley <cgtrolley@gmail.com>"]

[lib]
name = "decimal_macros"
plugin = true

[dependencies]
decimal = { path = "../" }
libc = "~0.2"

[build-dependencies]
gcc = "~0.3"
19 changes: 19 additions & 0 deletions decimal-macros/examples/test.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
#![feature(plugin)]
#![plugin(decimal_macros)]
extern crate decimal;

fn main() {
let a = d128!(0.1);
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

How about better tests?

d128!(0.1) should be equal to 1 as dec128 / 10 as dec128, etc

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

True, I should - was just kinda rushing to get it out the door and working :p I had so much trouble with it

Copy link
Copy Markdown
Contributor Author

@trolleyman trolleyman May 10, 2016

Choose a reason for hiding this comment

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

I chose this code because it fails in f32 land: http://0.30000000000000004.com/

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Does this fail?

d128!(0.1) == (1 as dec128) / (10 as dec128)

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Added a test with this

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

This test is different. Let me explain. What you are testing here is that the result of d128!(0.1) is equal to the result of d128!(1) / d128!(10). This might pass even if d128!(10) does not equal a dec128 of decimal value 10. This is why I suggest that the left hand side uses the macro and the right hand side uses conversion from int to dec128 which are trivial.

Also is this test organization going to work with cargo test? If not can you put these tests inside decimal-macros/src/lib.rs inside a test module and annotate appropriately so that it works?

let b = d128!(0.2);
let c = d128!(0.3);
let res = a + b;
let eq = res == c;
if eq {
println!("{} + {} = {}", a, b, res);
} else {
println!("{} + {} = {} (expected {})", a, b, res, c);
}
assert!(eq);

assert_eq!(d128!(0.1), d128!(1) / d128!(10));
}
91 changes: 91 additions & 0 deletions decimal-macros/src/lib.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,91 @@
#![feature(plugin_registrar, rustc_private)]

extern crate libc;
extern crate decimal;

extern crate rustc_plugin;
extern crate syntax;

use rustc_plugin::Registry;
use syntax::ext::base::{DummyResult, MacEager, ExtCtxt, MacResult};
use syntax::ext::build::AstBuilder;
use syntax::ext::source_util;
use syntax::codemap::Span;
use syntax::ast::{ExprKind, TokenTree, LitKind, StrStyle};

use decimal::d128;

fn d128_lit<'cx>(cx: &'cx mut ExtCtxt, sp: Span, tts: &[TokenTree]) -> Box<MacResult + 'cx> {
// Turn input into a string literal
// e.g. d128!(0.1) -> "0.1", d128!(NaN) -> "NaN"
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Thanks for this comment. It makes it easier for the syntex noob (me) out there.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Aha so was I before this - this and this really helped. Also the docs.

let mac_res = source_util::expand_stringify(cx, sp, tts);

let ex = match mac_res.make_expr() {
Some(ex) => ex,
None => {
// I don't know when this occurs
cx.span_err(sp, "one argument needed");
return DummyResult::any(sp);
}
};

match &ex.node {
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Why do you need this ampersand?

&ExprKind::Lit(ref lit) => match &lit.node {
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Also this one: &lit.node

&LitKind::Str(ref s, StrStyle::Cooked) => {
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Does s need to be a ref? InternedString should be cheap to copy.

Copy link
Copy Markdown
Contributor Author

@trolleyman trolleyman May 10, 2016

Choose a reason for hiding this comment

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

I don't think it has to be a ref. I could remove it if you want, along with all the others?

Copy link
Copy Markdown
Contributor Author

@trolleyman trolleyman May 10, 2016

Choose a reason for hiding this comment

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

Just tried to, turns out I can't because of the borrow checker.

Same goes for the others (ex.node and lit.node).

// Check for empty argument
if (*s).len() == 0 {
cx.span_err(sp, "one argument needed");
return DummyResult::any(sp);
}
let num = match from_str(&*s) {
Ok(num) => num,
Err(s) => {
cx.span_err(lit.span, s);
return DummyResult::any(sp);
}
};
let num = unsafe { ::std::mem::transmute::<d128, [u8; 16]>(num) };

// Create array literal
let mut vec = Vec::new();
for i in 0..16 {
vec.push(cx.expr_u8(lit.span, num[i]));
}
let vec = cx.expr_vec(lit.span, vec);
let ids = vec![cx.ident_of("decimal"), cx.ident_of("d128"), cx.ident_of("from_bytes")];
let ex = cx.expr_call_global(lit.span, ids, vec![vec]);

return MacEager::expr(ex);
},
_ => {}
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

What about LitKind::Int. LitKind::Float? Can we handle those properly as well?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

The source_util::expand_stringify at the beginning ensures that the item is a string literal. I chose this because it's the easiest way to parse the number: Just defer it to the function in d128.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

If it isn't a string (somehow), the code at the end is run that outputs an error.

},
_ => {}
}
// This should never happen.
cx.span_err(sp, "not a valid d128 number");
DummyResult::any(sp)
}

#[plugin_registrar]
pub fn plugin_registrar(reg: &mut Registry) {
reg.register_macro("d128", d128_lit)
}

fn from_str(s: &str) -> Result<d128, &'static str> {
use std::str::FromStr;
d128::set_status(decimal::Status::empty());
let res = d128::from_str(s);

let status = d128::get_status();
if status.contains(decimal::CONVERSION_SYNTAX) {
Err("not a valid d128 number")
} else if status.contains(decimal::OVERFLOW) {
Err("too large for a d128 number")
} else if status.contains(decimal::UNDERFLOW) {
Err("too small for a d128 number")
} else if !status.is_empty() {
Err("not a valid d128 number")
} else {
Ok(res.unwrap())
}
}
9 changes: 9 additions & 0 deletions src/dec128.rs
Original file line number Diff line number Diff line change
Expand Up @@ -436,6 +436,15 @@ impl d128 {
}
}
}

/// Hidden function for initializing a d128 from an array of bytes. When using this function
/// you should be careful about endianness issues.
#[doc(hidden)]
pub fn from_bytes(bytes: [u8; 16]) -> d128 {
Copy link
Copy Markdown
Owner

@alkis alkis May 9, 2016

Choose a reason for hiding this comment

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

Is it possible to make this private? If not can we use dec128::from_hex()? Maybe we can use expr_call_ident?

Copy link
Copy Markdown
Contributor Author

@trolleyman trolleyman May 10, 2016

Choose a reason for hiding this comment

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

Unfortunately this is the thing that I struggled with. If we use dec128::from_hex() won't that just negate the performance benefits, and being able to use d128 in constant expressions?

We could possibly use ::std::mem::transmute::<[u8; 16], ::decimal::d128>(). I need to research the generics though.

The major problem is that it can't store a resolved identifier, because the compiler hasn't done that stage yet (I think). There was an issue somewhere on it, I should've noted it down.

Currently decimal_macros needs decimal in the crate root.
This is because this:

d128!(5.5);

expands to this:

::decimal::d128::from_bytes([5, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 192, 7, 34])

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Let's try with transmute.

d128 {
bytes: unsafe { ::std::mem::transmute(bytes) }
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Will this work in Big Endian machines?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

It will, as it's just calling the same code that from_str calls.

}
}

// Utilities and conversions, extractors, etc.

Expand Down