Skip to content

Commit a592f87

Browse files
authored
Remove env_var and command_line from options (#1240)
Removed `from_env_var` and `from_command_line` from `MMTkOption` because now all options can be set in both ways. Replaced `Options::set_from_command_line` and `Options::set_from_env_var` with a single `Options::set_from_string`. Moved several methods of `Options` out of the `options!` macro if they don't need to be generated via macro expansion. Now only two methods are generated by the macro. - `set_from_string_inner`: Requires matching field names against user-provided strings. - `new`: Initializing fields using default values. In order to replicate the behavior that `Options::read_env_var_settings` silently ignores unrecognized option keys in `MMTK_*` environment variables, we introduced an error type `SetOptionByStringError` which `Options::set_from_string_inner` returns so that `Options::read_env_var_settings` and `Options::set_bulk_from_string` can behave differently when encountering invalid keys.
1 parent 2520fd6 commit a592f87

File tree

3 files changed

+189
-131
lines changed

3 files changed

+189
-131
lines changed

docs/userguide/src/migration/prefix.md

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,29 @@ Notes for the mmtk-core developers:
3030

3131
<!-- Insert new versions here -->
3232

33+
## 0.32.0
34+
35+
### `Options` no longer differentiates between environment variables and command line arguments.
36+
37+
```admonish tldr
38+
We replaced both `Options::set_from_command_line` and `Options::set_from_env_var` with
39+
`Options::set_from_string` because all options can now be set via either environment variable or
40+
command line arguments.
41+
```
42+
43+
API changes:
44+
45+
- module `util::options`
46+
+ `Options::set_from_command_line`: Removed.
47+
* Use `Options::set_from_string` instead.
48+
+ `Options::set_from_env_var`: Removed.
49+
* Use `Options::set_from_string` instead.
50+
+ `Options::set_bulk_from_command_line`: Removed.
51+
* Use `Options::set_bulk_from_string` instead.
52+
+ All `<T>` in `MMTKOption<T>` now must implement `FromStr`.
53+
* This means you can parse a string into `T` when setting an `MMTKOption<T>`. For
54+
example, `options.plan.set(user_input.parse()?);`.
55+
3356
## 0.30.0
3457

3558
### `live_bytes_in_last_gc` becomes a runtime option, and returns a map for live bytes in each space

src/mmtk.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -81,13 +81,13 @@ impl MMTKBuilder {
8181

8282
/// Set an option.
8383
pub fn set_option(&mut self, name: &str, val: &str) -> bool {
84-
self.options.set_from_command_line(name, val)
84+
self.options.set_from_string(name, val)
8585
}
8686

8787
/// Set multiple options by a string. The string should be key-value pairs separated by white spaces,
8888
/// such as `threads=1 stress_factor=4096`.
8989
pub fn set_options_bulk_by_str(&mut self, options: &str) -> bool {
90-
self.options.set_bulk_from_command_line(options)
90+
self.options.set_bulk_from_string(options)
9191
}
9292

9393
/// Custom VM layout constants. VM bindings may use this function for compressed or 39-bit heap support.

0 commit comments

Comments
 (0)