Skip to content

Commit

Permalink
Rollup merge of rust-lang#79809 - Eric-Arellano:split-once, r=matklad
Browse files Browse the repository at this point in the history
Dogfood `str_split_once()`

Part of rust-lang#74773.

Beyond increased clarity, this fixes some instances of a common confusion with how `splitn(2)` behaves: the first element will always be `Some()`, regardless of the delimiter, and even if the value is empty.

Given this code:

```rust
fn main() {
    let val = "...";
    let mut iter = val.splitn(2, '=');
    println!("Input: {:?}, first: {:?}, second: {:?}", val, iter.next(), iter.next());
}
```

We get:

```
Input: "no_delimiter", first: Some("no_delimiter"), second: None
Input: "k=v", first: Some("k"), second: Some("v")
Input: "=", first: Some(""), second: Some("")
```

Using `str_split_once()` makes more clear what happens when the delimiter is not found.
  • Loading branch information
Dylan-DPC committed Dec 10, 2020
2 parents 6e26af9 + 989edf4 commit 63bc8ce
Show file tree
Hide file tree
Showing 22 changed files with 196 additions and 190 deletions.
1 change: 1 addition & 0 deletions compiler/rustc_mir/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ Rust MIR: a lowered representation of Rust.
#![feature(or_patterns)]
#![feature(once_cell)]
#![feature(control_flow_enum)]
#![feature(str_split_once)]
#![recursion_limit = "256"]

#[macro_use]
Expand Down
62 changes: 34 additions & 28 deletions compiler/rustc_mir/src/transform/coverage/debug.rs
Original file line number Diff line number Diff line change
Expand Up @@ -148,40 +148,46 @@ impl DebugOptions {

if let Ok(env_debug_options) = std::env::var(RUSTC_COVERAGE_DEBUG_OPTIONS) {
for setting_str in env_debug_options.replace(" ", "").replace("-", "_").split(',') {
let mut setting = setting_str.splitn(2, '=');
match setting.next() {
Some(option) if option == "allow_unused_expressions" => {
allow_unused_expressions = bool_option_val(option, setting.next());
let (option, value) = match setting_str.split_once('=') {
None => (setting_str, None),
Some((k, v)) => (k, Some(v)),
};
match option {
"allow_unused_expressions" => {
allow_unused_expressions = bool_option_val(option, value);
debug!(
"{} env option `allow_unused_expressions` is set to {}",
RUSTC_COVERAGE_DEBUG_OPTIONS, allow_unused_expressions
);
}
Some(option) if option == "counter_format" => {
if let Some(strval) = setting.next() {
counter_format = counter_format_option_val(strval);
debug!(
"{} env option `counter_format` is set to {:?}",
RUSTC_COVERAGE_DEBUG_OPTIONS, counter_format
);
} else {
bug!(
"`{}` option in environment variable {} requires one or more \
plus-separated choices (a non-empty subset of \
`id+block+operation`)",
option,
RUSTC_COVERAGE_DEBUG_OPTIONS
);
}
"counter_format" => {
match value {
None => {
bug!(
"`{}` option in environment variable {} requires one or more \
plus-separated choices (a non-empty subset of \
`id+block+operation`)",
option,
RUSTC_COVERAGE_DEBUG_OPTIONS
);
}
Some(val) => {
counter_format = counter_format_option_val(val);
debug!(
"{} env option `counter_format` is set to {:?}",
RUSTC_COVERAGE_DEBUG_OPTIONS, counter_format
);
}
};
}
Some("") => {}
Some(invalid) => bug!(
"Unsupported setting `{}` in environment variable {}",
invalid,
RUSTC_COVERAGE_DEBUG_OPTIONS
),
None => {}
}
_ => {
bug!(
"Unsupported setting `{}` in environment variable {}",
option,
RUSTC_COVERAGE_DEBUG_OPTIONS
)
}
};
}
}

Expand Down
109 changes: 50 additions & 59 deletions compiler/rustc_session/src/config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1296,8 +1296,10 @@ fn parse_output_types(
if !debugging_opts.parse_only {
for list in matches.opt_strs("emit") {
for output_type in list.split(',') {
let mut parts = output_type.splitn(2, '=');
let shorthand = parts.next().unwrap();
let (shorthand, path) = match output_type.split_once('=') {
None => (output_type, None),
Some((shorthand, path)) => (shorthand, Some(PathBuf::from(path))),
};
let output_type = OutputType::from_shorthand(shorthand).unwrap_or_else(|| {
early_error(
error_format,
Expand All @@ -1308,7 +1310,6 @@ fn parse_output_types(
),
)
});
let path = parts.next().map(PathBuf::from);
output_types.insert(output_type, path);
}
}
Expand Down Expand Up @@ -1452,11 +1453,10 @@ fn parse_opt_level(
let max_c = matches
.opt_strs_pos("C")
.into_iter()
.flat_map(
|(i, s)| {
if let Some("opt-level") = s.splitn(2, '=').next() { Some(i) } else { None }
},
)
.flat_map(|(i, s)| {
// NB: This can match a string without `=`.
if let Some("opt-level") = s.splitn(2, '=').next() { Some(i) } else { None }
})
.max();
if max_o > max_c {
OptLevel::Default
Expand Down Expand Up @@ -1491,11 +1491,10 @@ fn select_debuginfo(
let max_c = matches
.opt_strs_pos("C")
.into_iter()
.flat_map(
|(i, s)| {
if let Some("debuginfo") = s.splitn(2, '=').next() { Some(i) } else { None }
},
)
.flat_map(|(i, s)| {
// NB: This can match a string without `=`.
if let Some("debuginfo") = s.splitn(2, '=').next() { Some(i) } else { None }
})
.max();
if max_g > max_c {
DebugInfo::Full
Expand Down Expand Up @@ -1528,23 +1527,26 @@ fn parse_libs(
.map(|s| {
// Parse string of the form "[KIND=]lib[:new_name]",
// where KIND is one of "dylib", "framework", "static".
let mut parts = s.splitn(2, '=');
let kind = parts.next().unwrap();
let (name, kind) = match (parts.next(), kind) {
(None, name) => (name, NativeLibKind::Unspecified),
(Some(name), "dylib") => (name, NativeLibKind::Dylib),
(Some(name), "framework") => (name, NativeLibKind::Framework),
(Some(name), "static") => (name, NativeLibKind::StaticBundle),
(Some(name), "static-nobundle") => (name, NativeLibKind::StaticNoBundle),
(_, s) => {
early_error(
error_format,
&format!(
"unknown library kind `{}`, expected \
one of dylib, framework, or static",
s
),
);
let (name, kind) = match s.split_once('=') {
None => (s, NativeLibKind::Unspecified),
Some((kind, name)) => {
let kind = match kind {
"dylib" => NativeLibKind::Dylib,
"framework" => NativeLibKind::Framework,
"static" => NativeLibKind::StaticBundle,
"static-nobundle" => NativeLibKind::StaticNoBundle,
s => {
early_error(
error_format,
&format!(
"unknown library kind `{}`, expected \
one of dylib, framework, or static",
s
),
);
}
};
(name.to_string(), kind)
}
};
if kind == NativeLibKind::StaticNoBundle
Expand All @@ -1556,10 +1558,11 @@ fn parse_libs(
accepted on the nightly compiler",
);
}
let mut name_parts = name.splitn(2, ':');
let name = name_parts.next().unwrap();
let new_name = name_parts.next();
(name.to_owned(), new_name.map(|n| n.to_owned()), kind)
let (name, new_name) = match name.split_once(':') {
None => (name, None),
Some((name, new_name)) => (name.to_string(), Some(new_name.to_owned())),
};
(name, new_name, kind)
})
.collect()
}
Expand All @@ -1580,20 +1583,13 @@ pub fn parse_externs(
let is_unstable_enabled = debugging_opts.unstable_options;
let mut externs: BTreeMap<String, ExternEntry> = BTreeMap::new();
for arg in matches.opt_strs("extern") {
let mut parts = arg.splitn(2, '=');
let name = parts
.next()
.unwrap_or_else(|| early_error(error_format, "--extern value must not be empty"));
let path = parts.next().map(|s| s.to_string());

let mut name_parts = name.splitn(2, ':');
let first_part = name_parts.next();
let second_part = name_parts.next();
let (options, name) = match (first_part, second_part) {
(Some(opts), Some(name)) => (Some(opts), name),
(Some(name), None) => (None, name),
(None, None) => early_error(error_format, "--extern name must not be empty"),
_ => unreachable!(),
let (name, path) = match arg.split_once('=') {
None => (arg, None),
Some((name, path)) => (name.to_string(), Some(path.to_string())),
};
let (options, name) = match name.split_once(':') {
None => (None, name),
Some((opts, name)) => (Some(opts), name.to_string()),
};

let entry = externs.entry(name.to_owned());
Expand Down Expand Up @@ -1682,17 +1678,12 @@ fn parse_remap_path_prefix(
matches
.opt_strs("remap-path-prefix")
.into_iter()
.map(|remap| {
let mut parts = remap.rsplitn(2, '='); // reverse iterator
let to = parts.next();
let from = parts.next();
match (from, to) {
(Some(from), Some(to)) => (PathBuf::from(from), PathBuf::from(to)),
_ => early_error(
error_format,
"--remap-path-prefix must contain '=' between FROM and TO",
),
}
.map(|remap| match remap.rsplit_once('=') {
None => early_error(
error_format,
"--remap-path-prefix must contain '=' between FROM and TO",
),
Some((from, to)) => (PathBuf::from(from), PathBuf::from(to)),
})
.collect()
}
Expand Down
1 change: 1 addition & 0 deletions compiler/rustc_session/src/lib.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
#![feature(crate_visibility_modifier)]
#![feature(once_cell)]
#![feature(or_patterns)]
#![feature(str_split_once)]

#[macro_use]
extern crate bitflags;
Expand Down
7 changes: 4 additions & 3 deletions compiler/rustc_session/src/options.rs
Original file line number Diff line number Diff line change
Expand Up @@ -179,9 +179,10 @@ macro_rules! options {
{
let mut op = $defaultfn();
for option in matches.opt_strs($prefix) {
let mut iter = option.splitn(2, '=');
let key = iter.next().unwrap();
let value = iter.next();
let (key, value) = match option.split_once('=') {
None => (option, None),
Some((k, v)) => (k.to_string(), Some(v)),
};
let option_to_lookup = key.replace("-", "_");
let mut found = false;
for &(candidate, setter, type_desc, _) in $stat {
Expand Down
1 change: 1 addition & 0 deletions compiler/rustc_target/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
#![feature(never_type)]
#![feature(associated_type_bounds)]
#![feature(exhaustive_patterns)]
#![feature(str_split_once)]

#[macro_use]
extern crate rustc_macros;
Expand Down
5 changes: 1 addition & 4 deletions compiler/rustc_target/src/spec/apple_base.rs
Original file line number Diff line number Diff line change
Expand Up @@ -54,10 +54,7 @@ fn macos_deployment_target() -> (u32, u32) {
let deployment_target = env::var("MACOSX_DEPLOYMENT_TARGET").ok();
let version = deployment_target
.as_ref()
.and_then(|s| {
let mut i = s.splitn(2, '.');
i.next().and_then(|a| i.next().map(|b| (a, b)))
})
.and_then(|s| s.split_once('.'))
.and_then(|(a, b)| a.parse::<u32>().and_then(|a| b.parse::<u32>().map(|b| (a, b))).ok());

version.unwrap_or((10, 7))
Expand Down
1 change: 1 addition & 0 deletions library/std/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -312,6 +312,7 @@
#![feature(stdsimd)]
#![feature(stmt_expr_attributes)]
#![feature(str_internals)]
#![feature(str_split_once)]
#![feature(test)]
#![feature(thread_local)]
#![feature(thread_local_internals)]
Expand Down
5 changes: 1 addition & 4 deletions library/std/src/sys_common/net.rs
Original file line number Diff line number Diff line change
Expand Up @@ -177,11 +177,8 @@ impl TryFrom<&str> for LookupHost {
}

// split the string by ':' and convert the second part to u16
let mut parts_iter = s.rsplitn(2, ':');
let port_str = try_opt!(parts_iter.next(), "invalid socket address");
let host = try_opt!(parts_iter.next(), "invalid socket address");
let (host, port_str) = try_opt!(s.rsplit_once(':'), "invalid socket address");
let port: u16 = try_opt!(port_str.parse().ok(), "invalid port value");

(host, port).try_into()
}
}
Expand Down
1 change: 1 addition & 0 deletions library/test/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@
#![feature(termination_trait_lib)]
#![feature(test)]
#![feature(total_cmp)]
#![feature(str_split_once)]

// Public reexports
pub use self::bench::{black_box, Bencher};
Expand Down
24 changes: 9 additions & 15 deletions library/test/src/time.rs
Original file line number Diff line number Diff line change
Expand Up @@ -105,30 +105,24 @@ impl TimeThreshold {
/// value.
pub fn from_env_var(env_var_name: &str) -> Option<Self> {
let durations_str = env::var(env_var_name).ok()?;
let (warn_str, critical_str) = durations_str.split_once(',').unwrap_or_else(|| {
panic!(
"Duration variable {} expected to have 2 numbers separated by comma, but got {}",
env_var_name, durations_str
)
});

// Split string into 2 substrings by comma and try to parse numbers.
let mut durations = durations_str.splitn(2, ',').map(|v| {
let parse_u64 = |v| {
u64::from_str(v).unwrap_or_else(|_| {
panic!(
"Duration value in variable {} is expected to be a number, but got {}",
env_var_name, v
)
})
});

// Callback to be called if the environment variable has unexpected structure.
let panic_on_incorrect_value = || {
panic!(
"Duration variable {} expected to have 2 numbers separated by comma, but got {}",
env_var_name, durations_str
);
};

let (warn, critical) = (
durations.next().unwrap_or_else(panic_on_incorrect_value),
durations.next().unwrap_or_else(panic_on_incorrect_value),
);

let warn = parse_u64(warn_str);
let critical = parse_u64(critical_str);
if warn > critical {
panic!("Test execution warn time should be less or equal to the critical time");
}
Expand Down
15 changes: 5 additions & 10 deletions src/librustdoc/config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -397,12 +397,9 @@ impl Options {
matches
.opt_strs("default-setting")
.iter()
.map(|s| {
let mut kv = s.splitn(2, '=');
// never panics because `splitn` always returns at least one element
let k = kv.next().unwrap().to_string();
let v = kv.next().unwrap_or("true").to_string();
(k, v)
.map(|s| match s.split_once('=') {
None => (s.clone(), "true".to_string()),
Some((k, v)) => (k.to_string(), v.to_string()),
})
.collect(),
];
Expand Down Expand Up @@ -707,11 +704,9 @@ fn parse_extern_html_roots(
) -> Result<BTreeMap<String, String>, &'static str> {
let mut externs = BTreeMap::new();
for arg in &matches.opt_strs("extern-html-root-url") {
let mut parts = arg.splitn(2, '=');
let name = parts.next().ok_or("--extern-html-root-url must not be empty")?;
let url = parts.next().ok_or("--extern-html-root-url must be of the form name=url")?;
let (name, url) =
arg.split_once('=').ok_or("--extern-html-root-url must be of the form name=url")?;
externs.insert(name.to_string(), url.to_string());
}

Ok(externs)
}
Loading

0 comments on commit 63bc8ce

Please sign in to comment.