Skip to content

Commit 963c1f7

Browse files
committed
Merge branch 'bugfix/division'
2 parents 68c69e1 + 3f1eec3 commit 963c1f7

File tree

7 files changed

+207
-23
lines changed

7 files changed

+207
-23
lines changed

src/clojure/core.clj

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -47,7 +47,7 @@
4747
(defmacro time [expr]
4848
(list (quote let) [(quote start) (quote (System/nanoTime)) (quote ret) expr]
4949
(quote (do
50-
(println (str "Elapsed time: " (_slash_ (- (System/nanoTime) start) 1000000.0) " msecs"))
50+
(println (str "Elapsed time: " (/ (- (System/nanoTime) start) 1000000.0) " msecs"))
5151
ret))))
5252

5353
(defn slurp [f & opts]

src/environment.rs

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -278,6 +278,7 @@ impl Environment {
278278
let subtract_fn = rust_core::SubtractFn {};
279279
let multiply_fn = rust_core::MultiplyFn {};
280280
let divide_fn = rust_core::DivideFn {};
281+
let rem_fn = rust_core::RemFn {};
281282
let rand_fn = rust_core::RandFn {};
282283
let rand_int_fn = rust_core::RandIntFn {};
283284
let str_fn = rust_core::StrFn {};
@@ -352,7 +353,8 @@ impl Environment {
352353
environment.insert(Symbol::intern("+"), add_fn.to_rc_value());
353354
environment.insert(Symbol::intern("-"), subtract_fn.to_rc_value());
354355
environment.insert(Symbol::intern("*"), multiply_fn.to_rc_value());
355-
environment.insert(Symbol::intern("_slash_"), divide_fn.to_rc_value());
356+
environment.insert(Symbol::intern("/"), divide_fn.to_rc_value());
357+
environment.insert(Symbol::intern("rem"), rem_fn.to_rc_value());
356358
environment.insert(Symbol::intern("rand"), rand_fn.to_rc_value());
357359
environment.insert(Symbol::intern("rand-int"), rand_int_fn.to_rc_value());
358360
environment.insert(Symbol::intern("let"), let_macro.to_rc_value());

src/reader.rs

Lines changed: 106 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -107,14 +107,16 @@ fn cons_str(head: char, tail: &str) -> String {
107107
/// - `$`,
108108
/// - `*`,
109109
/// - `!`,
110-
fn is_identifier_char(chr: char) -> bool {
111-
chr.is_alphanumeric() || "|?<>+-_=^%&$*!.".contains(chr)
110+
fn is_identifier_char(ch: char) -> bool {
111+
ch.is_alphanumeric() || "|?<>+-_=^%&$*!.".contains(ch)
112112
}
113113

114-
/// Returns whether if a character can be in the head of an identifier.
114+
/// Returns true if a character is an acceptable (non numeric) identifier char
115115
///
116-
/// An identifier is composed of a head (its first char) and a tail (the other
117-
/// chars).
116+
/// An identifier is either a non numeric identifier char, followed by any number
117+
/// of identifier chars, or is a '/' and nothing else.
118+
///
119+
/// A separate function will be used to detect if an identifier is possibly just '/'
118120
///
119121
/// A character is an identifier char if it is alphabetic or if it is one of:
120122
/// - `|`,
@@ -131,20 +133,55 @@ fn is_identifier_char(chr: char) -> bool {
131133
/// - `$`,
132134
/// - `*`,
133135
/// - `!`,
134-
fn is_non_numeric_identifier_char(chr: char) -> bool {
135-
chr.is_alphabetic() || "|?<>+-_=^%&$*!.".contains(chr)
136+
fn is_non_numeric_identifier_char(ch: char) -> bool {
137+
ch.is_alphabetic() || "|?<>+-_=^%&$*!.".contains(ch)
138+
}
139+
140+
/// Returns true if a character is an acceptable (non numeric) identifier char, or '/'
141+
///
142+
/// An identifier is either a non numeric identifier char, followed by any number
143+
/// of identifier chars, or is a '/' and nothing else.
144+
///
145+
/// The reason we check if this is *either* a non numeric identifier char, or a '/',
146+
/// is because we will want to use it to parse either
147+
/// 1.a normal identifier
148+
/// 2.'/',
149+
/// 3. something like '/blah'
150+
/// And then, if we have '/blah', we will proactively make the read fail
151+
///
152+
/// We need to explicitly look for this '/blah' case is otherwise, if we just check for 1 and 2,
153+
/// then in the case where someone types in '/blah' it will count as two valid separate reads --
154+
/// first the symbol '/' and then the symbol 'blah'.
155+
///
156+
/// This function passes if the char is alphabetic, a '/', or one of:
157+
/// - `|`,
158+
/// - `?`,
159+
/// - `<`,
160+
/// - `>`,
161+
/// - `+`,
162+
/// - `-`,
163+
/// - `_`,
164+
/// - `=`,
165+
/// - `^`,
166+
/// - `%`,
167+
/// - `&`,
168+
/// - `$`,
169+
/// - `*`,
170+
/// - `!`,
171+
fn is_non_numeric_identifier_char_or_slash(ch: char) -> bool {
172+
ch == '/' || is_non_numeric_identifier_char(ch)
136173
}
137174

138175
/// Returns true if given character is a minus character
139176
/// - `-`,
140-
fn is_minus_char(chr: char) -> bool {
141-
chr == '-'
177+
fn is_minus_char(ch: char) -> bool {
178+
ch == '-'
142179
}
143180

144181
/// Returns true if given character is a period character
145182
/// - `-`,
146-
fn is_period_char(chr: char) -> bool {
147-
chr == '.'
183+
fn is_period_char(ch: char) -> bool {
184+
ch == '.'
148185
}
149186

150187
/// Returns whether if a given character is a whitespace.
@@ -210,27 +247,40 @@ fn identifier_tail(input: &str) -> IResult<&str, &str> {
210247
}
211248

212249
/// Parses valid Clojure identifiers
213-
/// Example Successes: ab, cat, -12+3, |blah|, <well>
214-
/// Example Failures: 'a, 12b, ,cat
250+
/// Example Successes: ab, cat, -12+3, |blah|, <well>, / (edge case)
251+
/// Example Failures: 'a, 12b, ,cat , /ab
215252
pub fn identifier_parser(input: &str) -> IResult<&str, String> {
216-
named!(identifier_head<&str, char>,
253+
// We will try to parse either a valid identifier, *or* the invalid identifier
254+
// '/slashwithmorecharacters'
255+
// Because if we do get the '/blah', we want to know and actively fail, otherwise '/blah'
256+
// will just count as two valid reads; one for '/' and one for 'blah'
257+
// So, we call these parsers 'maybe_valid_identifier_..', as they are also trying to catch
258+
// this one invalid case
259+
named!(maybe_invalid_identifier_head_parser<&str, char>,
217260
map!(
218-
take_while_m_n!(1, 1, is_non_numeric_identifier_char),
261+
take_while_m_n!(1, 1, is_non_numeric_identifier_char_or_slash),
219262
first_char
220263
)
221264
);
222265

223-
// identifier_tail<&str,&str> defined above to have magic 'complete' powers
266+
// identifier_tail<&str,&str> defined above so it can be a 'completion' parser instead of a
267+
// 'streaming' parser -- look into nom's documentation for more info
224268

225-
named!(identifier <&str, String>,
269+
named!(maybe_invalid_identifier_parser <&str, String>,
226270
do_parse!(
227-
head: identifier_head >>
271+
head: maybe_invalid_identifier_head_parser >>
228272
rest_input: identifier_tail >>
229273
(cons_str(head, &rest_input))
230274
)
231275
);
232276

233-
identifier(input)
277+
named!(valid_identifier_parser <&str,String>,
278+
verify!(maybe_invalid_identifier_parser,|identifier| {
279+
first_char(&identifier) != '/' ||
280+
identifier == "/"
281+
}));
282+
283+
valid_identifier_parser(input)
234284
}
235285

236286
/// Parses valid Clojure symbol
@@ -1012,6 +1062,43 @@ mod tests {
10121062
_ => panic!("try_read_meta \"^:cat a\" should return a symbol")
10131063
}
10141064
}
1065+
#[test]
1066+
fn try_read_forward_slash_test() {
1067+
assert_eq!(
1068+
Value::Symbol(Symbol::intern(&"/")),
1069+
try_read("/ ").ok().unwrap().1
1070+
);
1071+
}
1072+
#[test]
1073+
fn try_read_forward_slash_with_letters_and_fails_test() {
1074+
assert!(try_read("/ab ").ok().is_none());
1075+
}
1076+
1077+
#[test]
1078+
fn try_read_forward_slash_keyword_test() {
1079+
assert_eq!(
1080+
Value::Keyword(Keyword::intern(&"/")),
1081+
try_read(":/ ").ok().unwrap().1
1082+
);
1083+
}
1084+
1085+
#[test]
1086+
fn try_read_forward_slash_keyword_with_letters_and_fails_test() {
1087+
assert!(try_read(":/ab ").ok().is_none());
1088+
}
1089+
1090+
#[test]
1091+
fn try_read_forward_slash_keyword_with_ns_test() {
1092+
assert_eq!(
1093+
Value::Keyword(Keyword::intern_with_ns("core", "/")),
1094+
try_read(":core// ").ok().unwrap().1
1095+
);
1096+
}
1097+
1098+
#[test]
1099+
fn try_read_forward_slash_keyword_with_ns_with_letters_and_fails_test() {
1100+
assert!(try_read(":core//ab ").ok().is_none());
1101+
}
10151102
}
10161103

10171104
mod regex_tests {

src/rust_core.rs

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -37,6 +37,9 @@ pub use self::_divide_::*;
3737
pub(crate) mod _multiply_;
3838
pub use self::_multiply_::*;
3939

40+
pub(crate) mod rem;
41+
pub use self::rem::*;
42+
4043
pub(crate) mod rand;
4144
pub use self::rand::*;
4245

src/rust_core/_divide_.rs

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,8 @@ impl IFn for DivideFn {
2020
1 => {
2121
let val = args.get(0).unwrap().to_value();
2222
match val {
23+
Value::I32(0) => Value::Condition("Divide by zero".to_string()),
24+
Value::F64(0.0) => Value::Condition("Divide by zero".to_string()),
2325
Value::I32(a_) => Value::F64(1.0 / a_ as f64),
2426
Value::F64(f_) => Value::F64(1.0 / f_),
2527
_ => Value::Condition(format!(
@@ -34,6 +36,8 @@ impl IFn for DivideFn {
3436
let first_arg = args_iterator.next().unwrap();
3537
args_iterator.fold(first_arg.to_value(), |a, b| match a {
3638
Value::I32(a_) => match *b {
39+
Value::I32(0) => Value::Condition("Divide by zero".to_string()),
40+
Value::F64(0.0) => Value::Condition("Divide by zero".to_string()),
3741
Value::I32(b_) => Value::I32(a_ / b_),
3842
Value::F64(b_) => Value::F64(a_ as f64 / b_),
3943
_ => Value::Condition(format!(
@@ -43,6 +47,8 @@ impl IFn for DivideFn {
4347
)),
4448
},
4549
Value::F64(a_) => match *b {
50+
Value::I32(0) => Value::Condition("Divide by zero".to_string()),
51+
Value::F64(0.0) => Value::Condition("Divide by zero".to_string()),
4652
Value::I32(b_) => Value::F64(a_ / b_ as f64),
4753
Value::F64(b_) => Value::F64(a_ / b_),
4854
_ => Value::Condition(format!(

src/rust_core/rem.rs

Lines changed: 82 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,82 @@
1+
use crate::ifn::IFn;
2+
use crate::value::{ToValue, Value};
3+
use std::rc::Rc;
4+
5+
use crate::error_message;
6+
7+
/// (rem x y)
8+
/// Calculate remainder
9+
#[derive(Debug, Clone)]
10+
pub struct RemFn {}
11+
impl ToValue for RemFn {
12+
fn to_value(&self) -> Value {
13+
Value::IFn(Rc::new(self.clone()))
14+
}
15+
}
16+
impl IFn for RemFn {
17+
fn invoke(&self, args: Vec<Rc<Value>>) -> Value {
18+
match args.len() {
19+
2 => {
20+
match args.get(0).unwrap().to_value() {
21+
Value::I32(a_) => match args.get(1).unwrap().to_value() {
22+
Value::I32(0) => Value::Condition("Divide by zero".to_string()),
23+
Value::F64(0.0) => Value::Condition("Divide by zero".to_string()),
24+
Value::I32(b_) => Value::I32(a_ % b_),
25+
Value::F64(b_) => Value::F64(a_ as f64 % b_),
26+
_b => Value::Condition(format!(
27+
// TODO: what error message should be returned regarding using typetags?
28+
"Type mismatch; Expecting: (i32 | i64 | f32 | f64), Found: {}",
29+
_b.type_tag()
30+
)),
31+
},
32+
Value::F64(a_) => match args.get(1).unwrap().to_value() {
33+
Value::I32(0) => Value::Condition("Divide by zero".to_string()),
34+
Value::F64(0.0) => Value::Condition("Divide by zero".to_string()),
35+
Value::I32(b_) => Value::F64(a_ % b_ as f64),
36+
Value::F64(b_) => Value::F64(a_ % b_),
37+
_b => Value::Condition(format!(
38+
// TODO: what error message should be returned regarding using typetags?
39+
"Type mismatch; Expecting: (i32 | i64 | f32 | f64), Found: {}",
40+
_b.type_tag()
41+
)),
42+
},
43+
_a => Value::Condition(format!(
44+
// TODO: what error message should be returned regarding using typetags?
45+
"Type mismatch: Expecting: (i32 | i64 | f32 | f64), Found: {}",
46+
_a.type_tag()
47+
)),
48+
}
49+
}
50+
_ => error_message::wrong_arg_count(2, args.len()),
51+
}
52+
}
53+
}
54+
55+
#[cfg(test)]
56+
mod tests {
57+
mod rem_tests {
58+
use crate::ifn::IFn;
59+
use crate::rust_core::rem::RemFn;
60+
use crate::value::Value;
61+
use std::rc::Rc;
62+
63+
#[test]
64+
fn rem_without_arguments_returns_error() {
65+
let rem = RemFn {};
66+
let args = vec![];
67+
assert_eq!(
68+
Value::Condition(String::from(
69+
"Wrong number of arguments given to function (Given: 0, Expected: 2)"
70+
)),
71+
rem.invoke(args)
72+
);
73+
}
74+
75+
#[test]
76+
fn rem_with_two_integer_argument_returns_remainder() {
77+
let rem = RemFn {};
78+
let args = vec![Rc::new(Value::I32(10)), Rc::new(Value::I32(3))];
79+
assert_eq!(Value::I32(1), rem.invoke(args));
80+
}
81+
}
82+
}

src/symbol.rs

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -41,8 +41,12 @@ impl Symbol {
4141
// we need to make sure each 'character' in this case
4242
// is 1 byte, and not some other grapheme abstraction
4343
// else,these are two different indexes
44-
ns = &name[..ind];
45-
name = &name[ind + 1..];
44+
45+
// support interning of the symbol '/' for division
46+
if ind > 0 || name.len() > 1 {
47+
ns = &name[..ind];
48+
name = &name[ind + 1..];
49+
}
4650
}
4751
Symbol::intern_with_ns(ns, name)
4852
}

0 commit comments

Comments
 (0)