Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

perf: Switch to &'static str by default #4223

Merged
merged 6 commits into from Sep 16, 2022
Merged

perf: Switch to &'static str by default #4223

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
1e13109
refactor(parser): Track subcommands with Box<str>
epage Sep 16, 2022
7ccaebb
refactor(parser): Switch from `Box<str>` to `String`
epage Sep 16, 2022
10854cd
Revert "refactor: Remove once_cell dependency"
epage Sep 16, 2022
bbbaca2
perf: Hint to the compiler when once_cell isn't needed
epage Sep 16, 2022
fe43f0c
Revert "fix: Remove once_cell dependency from derive"
epage Sep 16, 2022
c165b60
perf: Switch to `&'static str` by default
epage Sep 16, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
1 change: 1 addition & 0 deletions Cargo.lock
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

9 changes: 5 additions & 4 deletions Cargo.toml
View file
Open in desktop
Expand Up @@ -60,7 +60,7 @@ default = [
"suggestions",
]
debug = ["clap_derive/debug", "dep:backtrace"] # Enables debug messages
unstable-doc = ["derive", "cargo", "wrap_help", "env", "unicode", "perf", "unstable-replace", "unstable-grouped"] # for docs.rs
unstable-doc = ["derive", "cargo", "wrap_help", "env", "unicode", "string", "unstable-replace", "unstable-grouped"] # for docs.rs

# Used in default
std = [] # support for no_std in a backwards-compatible way
Expand All @@ -69,12 +69,12 @@ suggestions = ["dep:strsim"]

# Optional
deprecated = ["clap_derive?/deprecated"] # Guided experience to prepare for next breaking release (at different stages of development, this may become default)
derive = ["clap_derive"]
cargo = [] # Disable if you're not using Cargo, enables Cargo-env-var-dependent macros
derive = ["clap_derive", "dep:once_cell"]
cargo = ["dep:once_cell"] # Disable if you're not using Cargo, enables Cargo-env-var-dependent macros
wrap_help = ["dep:terminal_size"]
env = [] # Use environment variables during arg parsing
unicode = ["dep:unicode-width", "dep:unicase"] # Support for unicode characters in arguments and help messages
perf = [] # Optimize for runtime performance
string = [] # Allow runtime generated strings

# In-work features
unstable-replace = []
Expand All @@ -95,6 +95,7 @@ termcolor = { version = "1.1.1", optional = true }
terminal_size = { version = "0.2.1", optional = true }
backtrace = { version = "0.3", optional = true }
unicode-width = { version = "0.1.9", optional = true }
once_cell = { version = "1.12.0", optional = true }

[dev-dependencies]
trybuild = "1.0.18"
Expand Down
4 changes: 2 additions & 2 deletions Makefile
View file
Open in desktop
Expand Up @@ -15,8 +15,8 @@ MSRV?=1.60.0
_FEATURES = minimal default wasm full debug release
_FEATURES_minimal = --no-default-features --features "std"
_FEATURES_default =
_FEATURES_wasm = --features "deprecated derive cargo env unicode perf unstable-replace unstable-grouped"
_FEATURES_full = --features "deprecated derive cargo env unicode perf unstable-replace unstable-grouped wrap_help"
_FEATURES_wasm = --features "deprecated derive cargo env unicode string unstable-replace unstable-grouped"
_FEATURES_full = --features "deprecated derive cargo env unicode string unstable-replace unstable-grouped wrap_help"
_FEATURES_next = ${_FEATURES_full} --features unstable-v5
_FEATURES_debug = ${_FEATURES_full} --features debug --features clap_complete/debug
_FEATURES_release = ${_FEATURES_full} --release
Expand Down
6 changes: 3 additions & 3 deletions clap_complete/src/generator/mod.rs
View file
Open in desktop
Expand Up @@ -170,7 +170,7 @@ pub fn generate_to<G, S, T>(
) -> Result<PathBuf, Error>
where
G: Generator,
S: Into<clap::builder::Str>,
S: Into<String>,
T: Into<OsString>,
{
cmd.set_bin_name(bin_name);
Expand Down Expand Up @@ -223,7 +223,7 @@ where
pub fn generate<G, S>(gen: G, cmd: &mut clap::Command, bin_name: S, buf: &mut dyn Write)
where
G: Generator,
S: Into<clap::builder::Str>,
S: Into<String>,
{
cmd.set_bin_name(bin_name);
_generate::<G, S>(gen, cmd, buf)
Expand All @@ -232,7 +232,7 @@ where
fn _generate<G, S>(gen: G, cmd: &mut clap::Command, buf: &mut dyn Write)
where
G: Generator,
S: Into<clap::builder::Str>,
S: Into<String>,
{
cmd.build();

Expand Down
74 changes: 54 additions & 20 deletions clap_derive/src/item.rs
View file
Open in desktop
Expand Up @@ -535,17 +535,21 @@ impl Item {
.any(|a| a.magic == Some(MagicAttrName::ValueEnum))
{
quote_spanned!(attr.name.clone().span()=> {
{
static DEFAULT_VALUE: clap::__macro_refs::once_cell::sync::Lazy<String> = clap::__macro_refs::once_cell::sync::Lazy::new(|| {
let val: #ty = #val;
clap::ValueEnum::to_possible_value(&val).unwrap().get_name().to_owned()
}
});
let s: &'static str = &*DEFAULT_VALUE;
s
})
} else {
quote_spanned!(attr.name.clone().span()=> {
{
static DEFAULT_VALUE: clap::__macro_refs::once_cell::sync::Lazy<String> = clap::__macro_refs::once_cell::sync::Lazy::new(|| {
let val: #ty = #val;
::std::string::ToString::to_string(&val)
}
});
let s: &'static str = &*DEFAULT_VALUE;
s
})
};

Expand Down Expand Up @@ -599,21 +603,34 @@ impl Item {
})
}

iter_to_vals(#expr)
static DEFAULT_STRINGS: clap::__macro_refs::once_cell::sync::Lazy<Vec<::std::string::String>> = clap::__macro_refs::once_cell::sync::Lazy::new(|| {
iter_to_vals(#expr).collect()
});

static DEFAULT_VALUES: clap::__macro_refs::once_cell::sync::Lazy<Vec<&str>> = clap::__macro_refs::once_cell::sync::Lazy::new(|| {
DEFAULT_STRINGS.iter().map(::std::string::String::as_str).collect()
});
DEFAULT_VALUES.iter().copied()
}
})
} else {
quote_spanned!(attr.name.clone().span()=> {
{
fn iter_to_vals<T>(iterable: impl IntoIterator<Item = T>) -> Vec<String>
fn iter_to_vals<T>(iterable: impl IntoIterator<Item = T>) -> impl Iterator<Item=String>
where
T: ::std::borrow::Borrow<#inner_type>
{
iterable.into_iter().map(|val| val.borrow().to_string()).collect()

iterable.into_iter().map(|val| val.borrow().to_string())
}

iter_to_vals(#expr)
static DEFAULT_STRINGS: clap::__macro_refs::once_cell::sync::Lazy<Vec<::std::string::String>> = clap::__macro_refs::once_cell::sync::Lazy::new(|| {
iter_to_vals(#expr).collect()
});

static DEFAULT_VALUES: clap::__macro_refs::once_cell::sync::Lazy<Vec<&str>> = clap::__macro_refs::once_cell::sync::Lazy::new(|| {
DEFAULT_STRINGS.iter().map(::std::string::String::as_str).collect()
});
DEFAULT_VALUES.iter().copied()
}
})
};
Expand Down Expand Up @@ -650,17 +667,21 @@ impl Item {
.any(|a| a.magic == Some(MagicAttrName::ValueEnum))
{
quote_spanned!(attr.name.clone().span()=> {
{
static DEFAULT_VALUE: clap::__macro_refs::once_cell::sync::Lazy<::std::ffi::OsString> = clap::__macro_refs::once_cell::sync::Lazy::new(|| {
let val: #ty = #val;
clap::ValueEnum::to_possible_value(&val).unwrap().get_name().to_owned()
}
});
let s: &'static ::std::ffi::OsStr = &*DEFAULT_VALUE;
s
})
} else {
quote_spanned!(attr.name.clone().span()=> {
{
static DEFAULT_VALUE: clap::__macro_refs::once_cell::sync::Lazy<::std::ffi::OsString> = clap::__macro_refs::once_cell::sync::Lazy::new(|| {
let val: #ty = #val;
::std::ffi::OsString::from(val)
}
});
let s: &'static ::std::ffi::OsStr = &*DEFAULT_VALUE;
s
})
};

Expand Down Expand Up @@ -703,32 +724,45 @@ impl Item {
{
quote_spanned!(attr.name.clone().span()=> {
{
fn iter_to_vals<T>(iterable: impl IntoIterator<Item = T>) -> impl Iterator<Item=String>
fn iter_to_vals<T>(iterable: impl IntoIterator<Item = T>) -> impl Iterator<Item=::std::ffi::OsString>
where
T: ::std::borrow::Borrow<#inner_type>
{
iterable
.into_iter()
.map(|val| {
clap::ValueEnum::to_possible_value(val.borrow()).unwrap().get_name().to_owned()
clap::ValueEnum::to_possible_value(val.borrow()).unwrap().get_name().to_owned().into()
})
}

iter_to_vals(#expr)
static DEFAULT_OS_STRINGS: clap::__macro_refs::once_cell::sync::Lazy<Vec<::std::ffi::OsString>> = clap::__macro_refs::once_cell::sync::Lazy::new(|| {
iter_to_vals(#expr).collect()
});

static DEFAULT_VALUES: clap::__macro_refs::once_cell::sync::Lazy<Vec<&::std::ffi::OsStr>> = clap::__macro_refs::once_cell::sync::Lazy::new(|| {
DEFAULT_OS_STRINGS.iter().map(::std::ffi::OsString::as_os_str).collect()
});
DEFAULT_VALUES.iter().copied()
}
})
} else {
quote_spanned!(attr.name.clone().span()=> {
{
fn iter_to_vals<T>(iterable: impl IntoIterator<Item = T>) -> Vec<::std::ffi::OsString>
fn iter_to_vals<T>(iterable: impl IntoIterator<Item = T>) -> impl Iterator<Item=::std::ffi::OsString>
where
T: ::std::borrow::Borrow<#inner_type>
{
iterable.into_iter().map(|val| val.borrow().into()).collect()

iterable.into_iter().map(|val| val.borrow().into())
}

iter_to_vals(#expr)
static DEFAULT_OS_STRINGS: clap::__macro_refs::once_cell::sync::Lazy<Vec<::std::ffi::OsString>> = clap::__macro_refs::once_cell::sync::Lazy::new(|| {
iter_to_vals(#expr).collect()
});

static DEFAULT_VALUES: clap::__macro_refs::once_cell::sync::Lazy<Vec<&::std::ffi::OsStr>> = clap::__macro_refs::once_cell::sync::Lazy::new(|| {
DEFAULT_OS_STRINGS.iter().map(::std::ffi::OsString::as_os_str).collect()
});
DEFAULT_VALUES.iter().copied()
}
})
};
Expand Down
2 changes: 1 addition & 1 deletion src/_features.rs
View file
Open in desktop
Expand Up @@ -16,7 +16,7 @@
//! * **env**: Turns on the usage of environment variables during parsing.
//! * **unicode**: Turns on support for unicode characters (including emoji) in arguments and help messages.
//! * **wrap_help**: Turns on the help text wrapping feature, based on the terminal size.
//! * **perf**: Optimized for runtime performance.
//! * **string**: Allow runtime generated strings
//!
//! #### Experimental features
//!
Expand Down
21 changes: 11 additions & 10 deletions src/builder/command.rs
View file
Open in desktop
Expand Up @@ -70,8 +70,8 @@ pub struct Command {
name: Str,
long_flag: Option<Str>,
short_flag: Option<char>,
display_name: Option<Str>,
bin_name: Option<Str>,
display_name: Option<String>,
bin_name: Option<String>,
author: Option<Str>,
version: Option<Str>,
long_version: Option<Str>,
Expand Down Expand Up @@ -695,7 +695,7 @@ impl Command {
if let Some(f) = p.file_name() {
if let Some(s) = f.to_str() {
if self.bin_name.is_none() {
self.bin_name = Some(Str::from(s.to_owned()));
self.bin_name = Some(s.to_owned());
}
}
}
Expand Down Expand Up @@ -1376,7 +1376,7 @@ impl Command {
/// # ;
/// ```
#[must_use]
pub fn bin_name(mut self, name: impl IntoResettable<Str>) -> Self {
pub fn bin_name(mut self, name: impl IntoResettable<String>) -> Self {
self.bin_name = name.into_resettable().into_option();
self
}
Expand All @@ -1392,7 +1392,7 @@ impl Command {
/// # ;
/// ```
#[must_use]
pub fn display_name(mut self, name: impl IntoResettable<Str>) -> Self {
pub fn display_name(mut self, name: impl IntoResettable<String>) -> Self {
self.display_name = name.into_resettable().into_option();
self
}
Expand Down Expand Up @@ -3156,7 +3156,7 @@ impl Command {
}

/// Set binary name. Uses `&mut self` instead of `self`.
pub fn set_bin_name(&mut self, name: impl Into<Str>) {
pub fn set_bin_name(&mut self, name: impl Into<String>) {
self.bin_name = Some(name.into());
}

Expand All @@ -3167,6 +3167,7 @@ impl Command {
}

#[inline]
#[cfg(debug_assertions)]
pub(crate) fn get_name_str(&self) -> &Str {
&self.name
}
Expand Down Expand Up @@ -3888,7 +3889,7 @@ impl Command {
"Command::_build_subcommand Setting bin_name of {} to {:?}",
sc.name, bin_name
);
sc.bin_name = Some(Str::from(bin_name));
sc.bin_name = Some(bin_name);

if sc.display_name.is_none() {
let self_display_name = if is_multicall_set {
Expand All @@ -3910,7 +3911,7 @@ impl Command {
"Command::_build_subcommand Setting display_name of {} to {:?}",
sc.name, display_name
);
sc.display_name = Some(Str::from(display_name));
sc.display_name = Some(display_name);
}

// Ensure all args are built and ready to parse
Expand Down Expand Up @@ -3989,7 +3990,7 @@ impl Command {
"Command::_build_bin_names:iter: Setting bin_name of {} to {:?}",
sc.name, bin_name
);
sc.bin_name = Some(Str::from(bin_name));
sc.bin_name = Some(bin_name);
} else {
debug!(
"Command::_build_bin_names::iter: Using existing bin_name of {} ({:?})",
Expand Down Expand Up @@ -4017,7 +4018,7 @@ impl Command {
"Command::_build_bin_names:iter: Setting display_name of {} to {:?}",
sc.name, display_name
);
sc.display_name = Some(Str::from(display_name));
sc.display_name = Some(display_name);
} else {
debug!(
"Command::_build_bin_names::iter: Using existing display_name of {} ({:?})",
Expand Down