8000 Command line parsing improvements (#197) · kddnewton/ruby@7e95ae9 · GitHub
[go: up one dir, main page]
Skip to content

Commit 7e95ae9

Browse files
XrXrXrXr
authored
Command line parsing improvements (ruby#197)
* Accept when user passes exactly "--yjit" * Refactor command line parsing - Use an `Option(())` so we could use the `?` operator instead of having to type out `return false;` - Use an exhaustive match instead of repeating similar if-elses - Return parse failure instead of panicking when input is not utf8 - Remove unnecessary `str.to_owned()` which allocates and copies the string. * Explicit matchinig instead of Result.map
1 parent 130be0b commit 7e95ae9

File tree

2 files changed

+22
-14
lines changed

2 files changed

+22
-14
lines changed

‎yjit/src/options.rs

Lines changed: 21 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -47,39 +47,45 @@ macro_rules! get_option {
4747
}
4848
pub(crate) use get_option;
4949

50-
pub fn parse_option(str_ptr: *const std::os::raw::c_char) -> bool
50+
/// Expected to receive what comes after the third dash in "--yjit-*".
51+
/// Empty string means user passed only "--yjit". C code rejects when
52+
/// they pass exact "--yjit-".
53+
pub fn parse_option(str_ptr: *const std::os::raw::c_char) -> Option<()>
5154
{
5255
let c_str: &CStr = unsafe { CStr::from_ptr(str_ptr) };
53-
let str_slice: &str = c_str.to_str().unwrap();
54-
let opt_str: String = str_slice.to_owned();
56+
let opt_str: &str = c_str.to_str().ok()?;
5557
//println!("{}", opt_str);
5658

5759
// Split the option name and value strings
5860
// Note that some options do not contain an assignment
5961
let parts = opt_str.split_once("=");
60-
let opt_name = if parts.is_some() { parts.unwrap().0 } else { &opt_str };
61-
let opt_val = if parts.is_some() { parts.unwrap().1 } else { "" };
62+
let (opt_name, opt_val) = match parts {
63+
Some((before_eq, after_eq)) => (before_eq, after_eq),
64+
None => (opt_str, "")
65+
};
6266

6367
// Match on the option name and value strings
6468
match (opt_name, opt_val) {
69+
("", "") => (), // Simply --yjit
70+
6571
("exec-mem-size", _) => {
6672
match opt_val.parse::<usize>() {
67-
Ok(n) => { unsafe { OPTIONS.exec_mem_size = n }}
68-
Err(e) => { return false; }
73+
Ok(n) => unsafe { OPTIONS.exec_mem_size = n },
74+
Err(_) => { return None; }
6975
}
7076
},
7177

7278
("call-threshold", _) => {
7379
match opt_val.parse::<usize>() {
74-
Ok(n) => { unsafe { OPTIONS.call_threshold = n }}
75-
Err(e) => { return false; }
80+
Ok(n) => unsafe { OPTIONS.call_threshold = n },
81+
Err(_) => { return None; }
7682
}
7783
},
7884

7985
("max-versions", _) => {
8086
match opt_val.parse::<usize>() {
81-
Ok(n) => { unsafe { OPTIONS.max_versions = n }}
82-
Err(e) => { return false; }
87+
Ok(n) => unsafe { OPTIONS.max_versions = n },
88+
Err(_) => { return None; }
8389
}
8490
},
8591

@@ -89,10 +95,12 @@ pub fn parse_option(str_ptr: *const std::os::raw::c_char) -> bool
8995

9096
// Option name not recognized
9197
_ => {
92-
return false;
98+
return None;
9399
}
94100
}
95101

102+
// dbg!(unsafe {OPTIONS});
103+
96104
// Option successfully parsed
97-
return true;
105+
return Some(());
98106
}

‎yjit/src/yjit.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,7 @@ static YJIT_ENABLED: AtomicBool = AtomicBool::new(false);
1616
#[no_mangle]
1717
pub extern "C" fn rb_yjit_parse_option(str_ptr: *const raw::c_char) -> bool
1818
{
19-
return parse_option(str_ptr);
19+
return parse_option(str_ptr).is_some();
2020
}
2121

2222
/// Is YJIT on? The interpreter uses this function to decide whether to increment

0 commit comments

Comments
 (0)
0