Skip to content

Commit 6419a0d

Browse files
epageepage
committed
perf!(lex): Build faster by removing os_str_bytes
We are doing direct transmutes between `OsStr` and `[u8]`. rust-lang/rust#95290 would make this natively supported but I got tired of waitin for it. This only saves about 1/4s off of `cargo build`. This took 2.9 KiB off of `cargo bloat --release --example git`
1 parent 9712987 commit 6419a0d

File tree

10 files changed

+432
-164
lines changed

10 files changed

+432
-164
lines changed

‎Cargo.lock‎

Lines changed: 0 additions & 4 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎clap_builder/src/builder/debug_asserts.rs‎

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
use std::cmp::Ordering;
22

3-
use clap_lex::RawOsStr;
3+
use clap_lex::OsStrExt as _;
44

55
use crate::builder::OsStr;
66
use crate::builder::ValueRange;
@@ -841,16 +841,16 @@ fn assert_defaults<'d>(
841841
for default_os in defaults {
842842
let value_parser = arg.get_value_parser();
843843
let assert_cmd = Command::new("assert");
844-
if let Some(delim) = arg.get_value_delimiter() {
845-
let default_os = RawOsStr::new(default_os);
846-
for part in default_os.split(delim) {
847-
if let Err(err) = value_parser.parse_ref(&assert_cmd, Some(arg), &part.to_os_str())
848-
{
844+
if let Some(val_delim) = arg.get_value_delimiter() {
845+
let mut val_delim_buffer = [0; 4];
846+
let val_delim = val_delim.encode_utf8(&mut val_delim_buffer);
847+
for part in default_os.split(val_delim) {
848+
if let Err(err) = value_parser.parse_ref(&assert_cmd, Some(arg), part) {
849849
panic!(
850850
"Argument `{}`'s {}={:?} failed validation: {}",
851851
arg.get_id(),
852852
field,
853-
part.to_str_lossy(),
853+
part.to_string_lossy(),
854854
err
855855
);
856856
}

‎clap_builder/src/parser/parser.rs‎

Lines changed: 20 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -4,9 +4,7 @@ use std::{
44
ffi::{OsStr, OsString},
55
};
66

7-
// Third Party
8-
use clap_lex::RawOsStr;
9-
use clap_lex::RawOsString;
7+
use clap_lex::OsStrExt as _;
108

119
// Internal
1210
use crate::builder::{Arg, Command};
@@ -93,9 +91,8 @@ impl<'cmd> Parser<'cmd> {
9391
}
9492

9593
debug!(
96-
"Parser::get_matches_with: Begin parsing '{:?}' ({:?})",
94+
"Parser::get_matches_with: Begin parsing '{:?}'",
9795
arg_os.to_value_os(),
98-
arg_os.to_value_os().as_raw_bytes()
9996
);
10097

10198
// Has the user already passed '--'? Meaning only positional args follow
@@ -291,7 +288,7 @@ impl<'cmd> Parser<'cmd> {
291288
} else {
292289
let trailing_values = false;
293290
let arg_values = matcher.pending_values_mut(id, None, trailing_values);
294-
arg_values.push(arg_os.to_value_os().to_os_str().into_owned());
291+
arg_values.push(arg_os.to_value_os().to_owned());
295292
if matcher.needs_more_vals(arg) {
296293
ParseResult::Opt(arg.get_id().clone())
297294
} else {
@@ -411,7 +408,7 @@ impl<'cmd> Parser<'cmd> {
411408
Some(Identifier::Index),
412409
trailing_values,
413410
);
414-
arg_values.push(arg_os.to_value_os().to_os_str().into_owned());
411+
arg_values.push(arg_os.to_value_os().to_owned());
415412
}
416413

417414
// Only increment the positional counter if it doesn't allow multiples
@@ -548,7 +545,7 @@ impl<'cmd> Parser<'cmd> {
548545
// Checks if the arg matches a subcommand name, or any of its aliases (if defined)
549546
fn possible_subcommand(
550547
&self,
551-
arg: Result<&str, &RawOsStr>,
548+
arg: Result<&str, &OsStr>,
552549
valid_arg_found: bool,
553550
) -> Option<&str> {
554551
debug!("Parser::possible_subcommand: arg={:?}", arg);
@@ -723,8 +720,8 @@ impl<'cmd> Parser<'cmd> {
723720
fn parse_long_arg(
724721
&mut self,
725722
matcher: &mut ArgMatcher,
726-
long_arg: Result<&str, &RawOsStr>,
727-
long_value: Option<&RawOsStr>,
723+
long_arg: &str,
724+
long_value: Option<&OsStr>,
728725
parse_state: &ParseState,
729726
pos_counter: usize,
730727
valid_arg_found: &mut bool,
@@ -741,14 +738,6 @@ impl<'cmd> Parser<'cmd> {
741738
}
742739

743740
debug!("Parser::parse_long_arg: Does it contain '='...");
744-
let long_arg = match long_arg {
745-
Ok(long_arg) => long_arg,
746-
Err(long_arg) => {
747-
return Ok(ParseResult::NoMatchingArg {
748-
arg: long_arg.to_str_lossy().into_owned(),
749-
});
750-
}
751-
};
752741
if long_arg.is_empty() {
753742
debug_assert!(
754743
long_value.is_some(),
@@ -805,7 +794,7 @@ impl<'cmd> Parser<'cmd> {
805794
used.push(arg.get_id().clone());
806795

807796
Ok(ParseResult::UnneededAttachedValue {
808-
rest: rest.to_str_lossy().into_owned(),
797+
rest: rest.to_string_lossy().into_owned(),
809798
used,
810799
arg: arg.to_string(),
811800
})
@@ -902,7 +891,7 @@ impl<'cmd> Parser<'cmd> {
902891
Ok(c) => c,
903892
Err(rest) => {
904893
return Ok(ParseResult::NoMatchingArg {
905-
arg: format!("-{}", rest.to_str_lossy()),
894+
arg: format!("-{}", rest.to_string_lossy()),
906895
});
907896
}
908897
};
@@ -938,8 +927,8 @@ impl<'cmd> Parser<'cmd> {
938927
// Cloning the iterator, so we rollback if it isn't there.
939928
let val = short_arg.clone().next_value_os().unwrap_or_default();
940929
debug!(
941-
"Parser::parse_short_arg:iter:{}: val={:?} (bytes), val={:?} (ascii), short_arg={:?}",
942-
c, val, val.as_raw_bytes(), short_arg
930+
"Parser::parse_short_arg:iter:{}: val={:?}, short_arg={:?}",
931+
c, val, short_arg
943932
);
944933
let val = Some(val).filter(|v| !v.is_empty());
945934

@@ -950,7 +939,7 @@ impl<'cmd> Parser<'cmd> {
950939
//
951940
// e.g. `-xvf`, when require_equals && x.min_vals == 0, we don't
952941
// consume the `vf`, even if it's provided as value.
953-
let (val, has_eq) = if let Some(val) = val.and_then(|v| v.strip_prefix('=')) {
942+
let (val, has_eq) = if let Some(val) = val.and_then(|v| v.strip_prefix("=")) {
954943
(Some(val), true)
955944
} else {
956945
(val, false)
@@ -991,7 +980,7 @@ impl<'cmd> Parser<'cmd> {
991980
fn parse_opt_value(
992981
&self,
993982
ident: Identifier,
994-
attached_value: Option<&RawOsStr>,
983+
attached_value: Option<&OsStr>,
995984
arg: &Arg,
996985
matcher: &mut ArgMatcher,
997986
has_eq: bool,
@@ -1032,7 +1021,7 @@ impl<'cmd> Parser<'cmd> {
10321021
})
10331022
}
10341023
} else if let Some(v) = attached_value {
1035-
let arg_values = vec![v.to_os_str().into_owned()];
1024+
let arg_values = vec![v.to_owned()];
10361025
let trailing_idx = None;
10371026
let react_result = ok!(self.react(
10381027
Some(ident),
@@ -1054,13 +1043,8 @@ impl<'cmd> Parser<'cmd> {
10541043
}
10551044
}
10561045

1057-
fn check_terminator(&self, arg: &Arg, val: &RawOsStr) -> Option<ParseResult> {
1058-
if Some(val)
1059-
== arg
1060-
.terminator
1061-
.as_ref()
1062-
.map(|s| RawOsStr::from_str(s.as_str()))
1063-
{
1046+
fn check_terminator(&self, arg: &Arg, val: &OsStr) -> Option<ParseResult> {
1047+
if Some(val) == arg.terminator.as_ref().map(|s| OsStr::new(s.as_str())) {
10641048
debug!("Parser::check_terminator: terminator={:?}", arg.terminator);
10651049
Some(ParseResult::ValuesDone)
10661050
} else {
@@ -1156,17 +1140,17 @@ impl<'cmd> Parser<'cmd> {
11561140
if self.cmd.is_dont_delimit_trailing_values_set() && trailing_idx == Some(0) {
11571141
// Nothing to do
11581142
} else {
1143+
let mut val_delim_buffer = [0; 4];
1144+
let val_delim = val_delim.encode_utf8(&mut val_delim_buffer);
11591145
let mut split_raw_vals = Vec::with_capacity(raw_vals.len());
11601146
for (i, raw_val) in raw_vals.into_iter().enumerate() {
1161-
let raw_val = RawOsString::new(raw_val);
11621147
if !raw_val.contains(val_delim)
11631148
|| (self.cmd.is_dont_delimit_trailing_values_set()
11641149
&& trailing_idx == Some(i))
11651150
{
1166-
split_raw_vals.push(raw_val.into_os_string());
1151+
split_raw_vals.push(raw_val);
11671152
} else {
1168-
split_raw_vals
1169-
.extend(raw_val.split(val_delim).map(|x| x.to_os_str().into_owned()));
1153+
split_raw_vals.extend(raw_val.split(val_delim).map(|x| x.to_owned()));
11701154
}
11711155
}
11721156
raw_vals = split_raw_vals

‎clap_complete/Cargo.toml‎

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -35,7 +35,6 @@ bench = false
3535
clap = { path = "../", version = "4.1.0", default-features = false, features = ["std"] }
3636
clap_lex = { path = "../clap_lex", version = "0.3.0", optional = true }
3737
is_executable = { version = "1.0.1", optional = true }
38-
os_str_bytes = { version = "6.0.0", default-features = false, features = ["raw_os_str"], optional = true }
3938
pathdiff = { version = "0.2.1", optional = true }
4039
shlex = { version = "1.1.0", optional = true }
4140
unicode-xid = { version = "0.2.2", optional = true }
@@ -52,5 +51,5 @@ required-features = ["unstable-dynamic"]
5251

5352
[features]
5453
default = []
55-
unstable-dynamic = ["dep:clap_lex", "dep:shlex", "dep:unicode-xid", "dep:os_str_bytes", "clap/derive", "dep:is_executable", "dep:pathdiff"]
54+
unstable-dynamic = ["dep:clap_lex", "dep:shlex", "dep:unicode-xid", "clap/derive", "dep:is_executable", "dep:pathdiff"]
5655
debug = ["clap/debug"]

‎clap_complete/src/dynamic.rs‎

Lines changed: 26 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -2,9 +2,11 @@
22
33
/// Complete commands within bash
44
pub mod bash {
5+
use std::ffi::OsStr;
56
use std::ffi::OsString;
67
use std::io::Write;
78

9+
use clap_lex::OsStrExt as _;
810
use unicode_xid::UnicodeXID;
911

1012
#[derive(clap::Subcommand)]
@@ -320,11 +322,7 @@ complete OPTIONS -F _clap_complete_NAME EXECUTABLES
320322
return complete_arg(&arg, current_cmd, current_dir, pos_index, is_escaped);
321323
}
322324

323-
debug!(
324-
"complete::next: Begin parsing '{:?}' ({:?})",
325-
arg.to_value_os(),
326-
arg.to_value_os().as_raw_bytes()
327-
);
325+
debug!("complete::next: Begin parsing '{:?}'", arg.to_value_os(),);
328326

329327
if let Ok(value) = arg.to_value() {
330328
if let Some(next_cmd) = current_cmd.find_subcommand(value) {
@@ -370,28 +368,23 @@ complete OPTIONS -F _clap_complete_NAME EXECUTABLES
370368

371369
if !is_escaped {
372370
if let Some((flag, value)) = arg.to_long() {
373-
if let Ok(flag) = flag {
374-
if let Some(value) = value {
375-
if let Some(arg) = cmd.get_arguments().find(|a| a.get_long() == Some(flag))
376-
{
377-
completions.extend(
378-
complete_arg_value(value.to_str().ok_or(value), arg, current_dir)
379-
.into_iter()
380-
.map(|os| {
381-
// HACK: Need better `OsStr` manipulation
382-
format!("--{}={}", flag, os.to_string_lossy()).into()
383-
}),
384-
)
385-
}
386-
} else {
371+
if let Some(value) = value {
372+
if let Some(arg) = cmd.get_arguments().find(|a| a.get_long() == Some(flag)) {
387373
completions.extend(
388-
crate::generator::utils::longs_and_visible_aliases(cmd)
374+
complete_arg_value(value.to_str().ok_or(value), arg, current_dir)
389375
.into_iter()
390-
.filter_map(|f| {
391-
f.starts_with(flag).then(|| format!("--{}", f).into())
376+
.map(|os| {
377+
// HACK: Need better `OsStr` manipulation
378+
format!("--{}={}", flag, os.to_string_lossy()).into()
392379
}),
393-
);
380+
)
394381
}
382+
} else {
383+
completions.extend(
384+
crate::generator::utils::longs_and_visible_aliases(cmd)
385+
.into_iter()
386+
.filter_map(|f| f.starts_with(flag).then(|| format!("--{}", f).into())),
387+
);
395388
}
396389
} else if arg.is_escape() || arg.is_stdio() || arg.is_empty() {
397390
// HACK: Assuming knowledge of is_escape / is_stdio
@@ -408,7 +401,7 @@ complete OPTIONS -F _clap_complete_NAME EXECUTABLES
408401
crate::generator::utils::shorts_and_visible_aliases(cmd)
409402
.into_iter()
410403
// HACK: Need better `OsStr` manipulation
411-
.map(|f| format!("{}{}", arg.to_value_os().to_str_lossy(), f).into()),
404+
.map(|f| format!("{}{}", arg.to_value_os().to_string_lossy(), f).into()),
412405
);
413406
}
414407
}
@@ -428,7 +421,7 @@ complete OPTIONS -F _clap_complete_NAME EXECUTABLES
428421
}
429422

430423
fn complete_arg_value(
431-
value: Result<&str, &clap_lex::RawOsStr>,
424+
value: Result<&str, &OsStr>,
432425
arg: &clap::Arg,
433426
current_dir: Option<&std::path::Path>,
434427
) -> Vec<OsString> {
@@ -444,7 +437,7 @@ complete OPTIONS -F _clap_complete_NAME EXECUTABLES
444437
}
445438
} else {
446439
let value_os = match value {
447-
Ok(value) => clap_lex::RawOsStr::from_str(value),
440+
Ok(value) => OsStr::new(value),
448441
Err(value_os) => value_os,
449442
};
450443
match arg.get_value_hint() {
@@ -485,7 +478,7 @@ complete OPTIONS -F _clap_complete_NAME EXECUTABLES
485478
}
486479

487480
fn complete_path(
488-
value_os: &clap_lex::RawOsStr,
481+
value_os: &OsStr,
489482
current_dir: Option<&std::path::Path>,
490483
is_wanted: impl Fn(&std::path::Path) -> bool,
491484
) -> Vec<OsString> {
@@ -499,19 +492,20 @@ complete OPTIONS -F _clap_complete_NAME EXECUTABLES
499492
}
500493
};
501494
let (existing, prefix) = value_os
502-
.split_once('\\')
503-
.unwrap_or((clap_lex::RawOsStr::from_str(""), value_os));
504-
let root = current_dir.join(existing.to_os_str());
495+
.split_once("\\")
496+
.unwrap_or((OsStr::new(""), value_os));
497+
let root = current_dir.join(existing);
505498
debug!("complete_path: root={:?}, prefix={:?}", root, prefix);
499+
let prefix = prefix.to_string_lossy();
506500

507501
for entry in std::fs::read_dir(&root)
508502
.ok()
509503
.into_iter()
510504
.flatten()
511505
.filter_map(Result::ok)
512506
{
513-
let raw_file_name = clap_lex::RawOsString::new(entry.file_name());
514-
if !raw_file_name.starts_with_os(prefix) {
507+
let raw_file_name = OsString::from(entry.file_name());
508+
if !raw_file_name.starts_with(&prefix) {
515509
continue;
516510
}
517511

‎clap_lex/Cargo.toml‎

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,3 @@ pre-release-replacements = [
2828

2929
[lib]
3030
bench = false
31-
32-
[dependencies]
33-
os_str_bytes = { version = "6.0.0", default-features = false, features = ["raw_os_str"] }

0 commit comments

Comments
 (0)