From 1e13109a2679e306be0e895e1fb034635a3831fa Mon Sep 17 00:00:00 2001 From: Ed Page Date: Fri, 16 Sep 2022 14:01:50 -0500 Subject: [PATCH 1/6] refactor(parser): Track subcommands with Box The overall hope is to allow a `&'static str`-only version of clap, so we need to move off of `Str` where dynamic strings are required. We need it here for subcommands due to external subcommands. This had minimal impact on code size (567.2 to 567.5 KiB) --- clap_derive/src/derives/subcommand.rs | 4 ++-- src/builder/command.rs | 1 + src/parser/matches/arg_matches.rs | 7 ++++--- src/parser/parser.rs | 5 ++--- 4 files changed, 9 insertions(+), 8 deletions(-) diff --git a/clap_derive/src/derives/subcommand.rs b/clap_derive/src/derives/subcommand.rs index 1989e82fded..83a055f5668 100644 --- a/clap_derive/src/derives/subcommand.rs +++ b/clap_derive/src/derives/subcommand.rs @@ -491,7 +491,7 @@ fn gen_from_arg_matches(variants: &[(&Variant, Item)]) -> TokenStream { }; quote! { - if #subcommand_name_var == #sub_name && !#sub_arg_matches_var.contains_id("") { + if #subcommand_name_var.as_ref() == #sub_name && !#sub_arg_matches_var.contains_id("") { return ::std::result::Result::Ok(Self :: #variant_name #constructor_block) } } @@ -522,7 +522,7 @@ fn gen_from_arg_matches(variants: &[(&Variant, Item)]) -> TokenStream { let wildcard = match ext_subcmd { Some((span, var_name, str_ty)) => quote_spanned! { span=> ::std::result::Result::Ok(Self::#var_name( - ::std::iter::once(#str_ty::from(#subcommand_name_var)) + ::std::iter::once(#str_ty::from(#subcommand_name_var.as_ref())) .chain( #sub_arg_matches_var .remove_many::<#str_ty>("") diff --git a/src/builder/command.rs b/src/builder/command.rs index c659e22c4af..78acfd0837c 100644 --- a/src/builder/command.rs +++ b/src/builder/command.rs @@ -3167,6 +3167,7 @@ impl Command { } #[inline] + #[cfg(debug_assertions)] pub(crate) fn get_name_str(&self) -> &Str { &self.name } diff --git a/src/parser/matches/arg_matches.rs b/src/parser/matches/arg_matches.rs index cfbae26ef1f..67dcd85e5ee 100644 --- a/src/parser/matches/arg_matches.rs +++ b/src/parser/matches/arg_matches.rs @@ -6,6 +6,7 @@ use std::iter::{Cloned, Flatten, Map}; use std::slice::Iter; // Internal +#[cfg(debug_assertions)] use crate::builder::Str; use crate::parser::AnyValue; use crate::parser::AnyValueId; @@ -839,7 +840,7 @@ impl ArgMatches { /// } /// ``` /// [subcommand]: crate::Command::subcommand - pub fn remove_subcommand(&mut self) -> Option<(Str, ArgMatches)> { + pub fn remove_subcommand(&mut self) -> Option<(Box, ArgMatches)> { self.subcommand.take().map(|sc| (sc.name, sc.matches)) } @@ -1138,7 +1139,7 @@ impl ArgMatches { } if let Some(ref sc) = self.subcommand { - if sc.name == name { + if sc.name.as_ref() == name { return Some(sc); } } @@ -1149,7 +1150,7 @@ impl ArgMatches { #[derive(Debug, Clone, PartialEq, Eq)] pub(crate) struct SubCommand { - pub(crate) name: Str, + pub(crate) name: Box, pub(crate) matches: ArgMatches, } diff --git a/src/parser/parser.rs b/src/parser/parser.rs index d736d0cfdf0..2579d181411 100644 --- a/src/parser/parser.rs +++ b/src/parser/parser.rs @@ -9,7 +9,6 @@ use clap_lex::RawOsStr; use clap_lex::RawOsString; // Internal -use crate::builder::Str; use crate::builder::{Arg, Command}; use crate::error::Error as ClapError; use crate::error::Result as ClapResult; @@ -418,7 +417,7 @@ impl<'cmd> Parser<'cmd> { { // Get external subcommand name let sc_name = match arg_os.to_value() { - Ok(s) => Str::from(s.to_owned()), + Ok(s) => Box::from(s), Err(_) => { let _ = self.resolve_pending(matcher); return Err(ClapError::invalid_utf8( @@ -702,7 +701,7 @@ impl<'cmd> Parser<'cmd> { } } matcher.subcommand(SubCommand { - name: sc.get_name_str().clone(), + name: Box::from(sc.get_name()), matches: sc_matcher.into_inner(), }); } From 7ccaebb65d4838eb61e7a8e84988bd4a5c8dbbff Mon Sep 17 00:00:00 2001 From: Ed Page Date: Fri, 16 Sep 2022 14:05:53 -0500 Subject: [PATCH 2/6] refactor(parser): Switch from `Box` to `String` This is a more familiar type for people and it didn't cost us anything (in fact it undid the code size change in the last commit). --- clap_derive/src/derives/subcommand.rs | 4 ++-- src/parser/matches/arg_matches.rs | 6 +++--- src/parser/parser.rs | 4 ++-- 3 files changed, 7 insertions(+), 7 deletions(-) diff --git a/clap_derive/src/derives/subcommand.rs b/clap_derive/src/derives/subcommand.rs index 83a055f5668..1989e82fded 100644 --- a/clap_derive/src/derives/subcommand.rs +++ b/clap_derive/src/derives/subcommand.rs @@ -491,7 +491,7 @@ fn gen_from_arg_matches(variants: &[(&Variant, Item)]) -> TokenStream { }; quote! { - if #subcommand_name_var.as_ref() == #sub_name && !#sub_arg_matches_var.contains_id("") { + if #subcommand_name_var == #sub_name && !#sub_arg_matches_var.contains_id("") { return ::std::result::Result::Ok(Self :: #variant_name #constructor_block) } } @@ -522,7 +522,7 @@ fn gen_from_arg_matches(variants: &[(&Variant, Item)]) -> TokenStream { let wildcard = match ext_subcmd { Some((span, var_name, str_ty)) => quote_spanned! { span=> ::std::result::Result::Ok(Self::#var_name( - ::std::iter::once(#str_ty::from(#subcommand_name_var.as_ref())) + ::std::iter::once(#str_ty::from(#subcommand_name_var)) .chain( #sub_arg_matches_var .remove_many::<#str_ty>("") diff --git a/src/parser/matches/arg_matches.rs b/src/parser/matches/arg_matches.rs index 67dcd85e5ee..e743ca876cd 100644 --- a/src/parser/matches/arg_matches.rs +++ b/src/parser/matches/arg_matches.rs @@ -840,7 +840,7 @@ impl ArgMatches { /// } /// ``` /// [subcommand]: crate::Command::subcommand - pub fn remove_subcommand(&mut self) -> Option<(Box, ArgMatches)> { + pub fn remove_subcommand(&mut self) -> Option<(String, ArgMatches)> { self.subcommand.take().map(|sc| (sc.name, sc.matches)) } @@ -1139,7 +1139,7 @@ impl ArgMatches { } if let Some(ref sc) = self.subcommand { - if sc.name.as_ref() == name { + if sc.name == name { return Some(sc); } } @@ -1150,7 +1150,7 @@ impl ArgMatches { #[derive(Debug, Clone, PartialEq, Eq)] pub(crate) struct SubCommand { - pub(crate) name: Box, + pub(crate) name: String, pub(crate) matches: ArgMatches, } diff --git a/src/parser/parser.rs b/src/parser/parser.rs index 2579d181411..1db8495eff6 100644 --- a/src/parser/parser.rs +++ b/src/parser/parser.rs @@ -417,7 +417,7 @@ impl<'cmd> Parser<'cmd> { { // Get external subcommand name let sc_name = match arg_os.to_value() { - Ok(s) => Box::from(s), + Ok(s) => s.to_owned(), Err(_) => { let _ = self.resolve_pending(matcher); return Err(ClapError::invalid_utf8( @@ -701,7 +701,7 @@ impl<'cmd> Parser<'cmd> { } } matcher.subcommand(SubCommand { - name: Box::from(sc.get_name()), + name: sc.get_name().to_owned(), matches: sc_matcher.into_inner(), }); } From 10854cd262a590fcce053c8069ec6edfcbf40324 Mon Sep 17 00:00:00 2001 From: Ed Page Date: Fri, 16 Sep 2022 15:35:15 -0500 Subject: [PATCH 3/6] Revert "refactor: Remove once_cell dependency" This reverts commit c9d883a8c61357de487c0e08b7ab20d40a01ecef. --- Cargo.lock | 1 + Cargo.toml | 3 ++- src/lib.rs | 7 +++++++ src/macros.rs | 13 +++++++------ 4 files changed, 17 insertions(+), 7 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 52fe3fb89de..1015d0136be 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -118,6 +118,7 @@ dependencies = [ "clap_derive", "clap_lex", "humantime", + "once_cell", "rustversion", "shlex", "snapbox", diff --git a/Cargo.toml b/Cargo.toml index d38c7c64b18..1dd22c220db 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -70,7 +70,7 @@ 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 +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 @@ -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" diff --git a/src/lib.rs b/src/lib.rs index 0f9d25a8450..eabcb8260cc 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -127,6 +127,13 @@ pub mod _features; #[cfg(feature = "unstable-doc")] pub mod _tutorial; +#[doc(hidden)] +pub mod __macro_refs { + #[cfg(feature = "cargo")] + #[doc(hidden)] + pub use once_cell; +} + #[macro_use] #[allow(missing_docs)] mod macros; diff --git a/src/macros.rs b/src/macros.rs index 3bc04647e3c..7a9c3f9d524 100644 --- a/src/macros.rs +++ b/src/macros.rs @@ -46,12 +46,13 @@ macro_rules! crate_version { #[macro_export] macro_rules! crate_authors { ($sep:expr) => {{ - let authors = env!("CARGO_PKG_AUTHORS"); - if authors.contains(':') { - $crate::builder::Str::from(authors.replace(':', $sep)) - } else { - $crate::builder::Str::from(authors) - } + static CACHED: clap::__macro_refs::once_cell::sync::Lazy = + clap::__macro_refs::once_cell::sync::Lazy::new(|| { + env!("CARGO_PKG_AUTHORS").replace(':', $sep) + }); + + let s: &'static str = &*CACHED; + s }}; () => { env!("CARGO_PKG_AUTHORS") From bbbaca2ffebeb5b004a18f26500bfe6a08cb360e Mon Sep 17 00:00:00 2001 From: Ed Page Date: Fri, 16 Sep 2022 15:40:21 -0500 Subject: [PATCH 4/6] perf: Hint to the compiler when once_cell isn't needed --- src/macros.rs | 16 +++++++++------- tests/builder/cargo.rs | 14 ++++++++++++++ 2 files changed, 23 insertions(+), 7 deletions(-) diff --git a/src/macros.rs b/src/macros.rs index 7a9c3f9d524..c1754558137 100644 --- a/src/macros.rs +++ b/src/macros.rs @@ -46,13 +46,15 @@ macro_rules! crate_version { #[macro_export] macro_rules! crate_authors { ($sep:expr) => {{ - static CACHED: clap::__macro_refs::once_cell::sync::Lazy = - clap::__macro_refs::once_cell::sync::Lazy::new(|| { - env!("CARGO_PKG_AUTHORS").replace(':', $sep) - }); - - let s: &'static str = &*CACHED; - s + static authors: &str = env!("CARGO_PKG_AUTHORS"); + if authors.contains(':') { + static CACHED: clap::__macro_refs::once_cell::sync::Lazy = + clap::__macro_refs::once_cell::sync::Lazy::new(|| authors.replace(':', $sep)); + let s: &'static str = &*CACHED; + s + } else { + authors + } }}; () => { env!("CARGO_PKG_AUTHORS") diff --git a/tests/builder/cargo.rs b/tests/builder/cargo.rs index 376bda7451c..21336cb071c 100644 --- a/tests/builder/cargo.rs +++ b/tests/builder/cargo.rs @@ -70,6 +70,20 @@ fn crate_authors() { assert_eq!(err.to_string(), AUTHORS_ONLY); } +#[test] +fn crate_authors_with_sep() { + let res = Command::new("prog") + .version("1") + .author(crate_authors!(", ")) + .help_template(utils::FULL_TEMPLATE) + .try_get_matches_from(vec!["prog", "--help"]); + + assert!(res.is_err()); + let err = res.unwrap_err(); + assert_eq!(err.kind(), ErrorKind::DisplayHelp); + assert_eq!(err.to_string(), AUTHORS_ONLY); +} + #[test] fn crate_name() { let res = Command::new(crate_name!()) From fe43f0c9450487b850acae3c149a98bff3c6bb52 Mon Sep 17 00:00:00 2001 From: Ed Page Date: Fri, 16 Sep 2022 15:49:38 -0500 Subject: [PATCH 5/6] Revert "fix: Remove once_cell dependency from derive" This reverts commit 429143af4299327a816b9d50e13a4cfc3ba247a7. --- Cargo.toml | 2 +- clap_derive/src/item.rs | 74 ++++++++++++++++++++++++++++++----------- src/lib.rs | 2 +- 3 files changed, 56 insertions(+), 22 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index 1dd22c220db..528e194da68 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -69,7 +69,7 @@ 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"] +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 diff --git a/clap_derive/src/item.rs b/clap_derive/src/item.rs index 3808364b6fe..627d11b52e1 100644 --- a/clap_derive/src/item.rs +++ b/clap_derive/src/item.rs @@ -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 = 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 = 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 }) }; @@ -599,21 +603,34 @@ impl Item { }) } - iter_to_vals(#expr) + static DEFAULT_STRINGS: clap::__macro_refs::once_cell::sync::Lazy> = clap::__macro_refs::once_cell::sync::Lazy::new(|| { + iter_to_vals(#expr).collect() + }); + + static DEFAULT_VALUES: clap::__macro_refs::once_cell::sync::Lazy> = 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(iterable: impl IntoIterator) -> Vec + fn iter_to_vals(iterable: impl IntoIterator) -> impl Iterator 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> = clap::__macro_refs::once_cell::sync::Lazy::new(|| { + iter_to_vals(#expr).collect() + }); + + static DEFAULT_VALUES: clap::__macro_refs::once_cell::sync::Lazy> = clap::__macro_refs::once_cell::sync::Lazy::new(|| { + DEFAULT_STRINGS.iter().map(::std::string::String::as_str).collect() + }); + DEFAULT_VALUES.iter().copied() } }) }; @@ -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 }) }; @@ -703,32 +724,45 @@ impl Item { { quote_spanned!(attr.name.clone().span()=> { { - fn iter_to_vals(iterable: impl IntoIterator) -> impl Iterator + fn iter_to_vals(iterable: impl IntoIterator) -> impl Iterator 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> = clap::__macro_refs::once_cell::sync::Lazy::new(|| { + iter_to_vals(#expr).collect() + }); + + static DEFAULT_VALUES: clap::__macro_refs::once_cell::sync::Lazy> = 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(iterable: impl IntoIterator) -> Vec<::std::ffi::OsString> + fn iter_to_vals(iterable: impl IntoIterator) -> impl Iterator 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> = clap::__macro_refs::once_cell::sync::Lazy::new(|| { + iter_to_vals(#expr).collect() + }); + + static DEFAULT_VALUES: clap::__macro_refs::once_cell::sync::Lazy> = clap::__macro_refs::once_cell::sync::Lazy::new(|| { + DEFAULT_OS_STRINGS.iter().map(::std::ffi::OsString::as_os_str).collect() + }); + DEFAULT_VALUES.iter().copied() } }) }; diff --git a/src/lib.rs b/src/lib.rs index eabcb8260cc..0059ca39338 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -129,7 +129,7 @@ pub mod _tutorial; #[doc(hidden)] pub mod __macro_refs { - #[cfg(feature = "cargo")] + #[cfg(any(feature = "derive", feature = "cargo"))] #[doc(hidden)] pub use once_cell; } From c165b601acb4232b2748f8f455d3df4cf458f2c9 Mon Sep 17 00:00:00 2001 From: Ed Page Date: Fri, 16 Sep 2022 11:52:55 -0500 Subject: [PATCH 6/6] perf: Switch to `&'static str` by default MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Originally, clap carried a lifetime parameter. When moving away from that, we took the approach that dynamically generated strings are always supported and `&'static str` was just an optimization. The problem is the code size increase from this is dramatic. So we're taking the opposite approach and making dynamic formatting opt-in under the `string` feature flag. When deciding on an implementation, I favored the faster one rather than the one with smaller code size since small code size can be gotten through other means. Before: 567.2 KiB, 15.975 µs After: 541.1 KiB, 9.7855 µs With `string`: 576.6 KiB, 13.016 µs --- Cargo.toml | 4 ++-- Makefile | 4 ++-- clap_complete/src/generator/mod.rs | 6 +++--- src/_features.rs | 2 +- src/builder/command.rs | 20 +++++++++--------- src/builder/os_str.rs | 33 ++++++++++++++---------------- src/builder/resettable.rs | 6 ++++++ src/builder/str.rs | 23 ++++++++------------- src/util/id.rs | 2 ++ tests/examples.rs | 2 ++ tests/ui.rs | 2 ++ 11 files changed, 54 insertions(+), 50 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index 528e194da68..b18b4ca1f61 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -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 @@ -74,7 +74,7 @@ cargo = ["dep:once_cell"] # Disable if you're not using Cargo, enables Cargo-env 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 = [] diff --git a/Makefile b/Makefile index 5a0ce9d74fb..48f91ce88af 100644 --- a/Makefile +++ b/Makefile @@ -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 diff --git a/clap_complete/src/generator/mod.rs b/clap_complete/src/generator/mod.rs index dd9c0584f22..6f34d345dfb 100644 --- a/clap_complete/src/generator/mod.rs +++ b/clap_complete/src/generator/mod.rs @@ -170,7 +170,7 @@ pub fn generate_to( ) -> Result where G: Generator, - S: Into, + S: Into, T: Into, { cmd.set_bin_name(bin_name); @@ -223,7 +223,7 @@ where pub fn generate(gen: G, cmd: &mut clap::Command, bin_name: S, buf: &mut dyn Write) where G: Generator, - S: Into, + S: Into, { cmd.set_bin_name(bin_name); _generate::(gen, cmd, buf) @@ -232,7 +232,7 @@ where fn _generate(gen: G, cmd: &mut clap::Command, buf: &mut dyn Write) where G: Generator, - S: Into, + S: Into, { cmd.build(); diff --git a/src/_features.rs b/src/_features.rs index 40009ee6caf..7c12ef1510c 100644 --- a/src/_features.rs +++ b/src/_features.rs @@ -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 //! diff --git a/src/builder/command.rs b/src/builder/command.rs index 78acfd0837c..a400f13ed0d 100644 --- a/src/builder/command.rs +++ b/src/builder/command.rs @@ -70,8 +70,8 @@ pub struct Command { name: Str, long_flag: Option, short_flag: Option, - display_name: Option, - bin_name: Option, + display_name: Option, + bin_name: Option, author: Option, version: Option, long_version: Option, @@ -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()); } } } @@ -1376,7 +1376,7 @@ impl Command { /// # ; /// ``` #[must_use] - pub fn bin_name(mut self, name: impl IntoResettable) -> Self { + pub fn bin_name(mut self, name: impl IntoResettable) -> Self { self.bin_name = name.into_resettable().into_option(); self } @@ -1392,7 +1392,7 @@ impl Command { /// # ; /// ``` #[must_use] - pub fn display_name(mut self, name: impl IntoResettable) -> Self { + pub fn display_name(mut self, name: impl IntoResettable) -> Self { self.display_name = name.into_resettable().into_option(); self } @@ -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) { + pub fn set_bin_name(&mut self, name: impl Into) { self.bin_name = Some(name.into()); } @@ -3889,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 { @@ -3911,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 @@ -3990,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 {} ({:?})", @@ -4018,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 {} ({:?})", diff --git a/src/builder/os_str.rs b/src/builder/os_str.rs index 9e815d4d030..f379b415337 100644 --- a/src/builder/os_str.rs +++ b/src/builder/os_str.rs @@ -7,12 +7,14 @@ pub struct OsStr { } impl OsStr { + #[cfg(feature = "string")] pub(crate) fn from_string(name: std::ffi::OsString) -> Self { Self { name: Inner::from_string(name), } } + #[cfg(feature = "string")] pub(crate) fn from_ref(name: &std::ffi::OsStr) -> Self { Self { name: Inner::from_ref(name), @@ -42,7 +44,7 @@ impl From<&'_ OsStr> for OsStr { } } -#[cfg(feature = "perf")] +#[cfg(feature = "string")] impl From for OsStr { fn from(id: Str) -> Self { match id.into_inner() { @@ -52,10 +54,10 @@ impl From for OsStr { } } -#[cfg(not(feature = "perf"))] +#[cfg(not(feature = "string"))] impl From for OsStr { fn from(id: Str) -> Self { - Self::from_ref(std::ffi::OsStr::new(id.as_str())) + Self::from_static_ref(std::ffi::OsStr::new(id.into_inner().0)) } } @@ -69,31 +71,34 @@ impl From<&'_ Str> for OsStr { } } -#[cfg(not(feature = "perf"))] impl From<&'_ Str> for OsStr { fn from(id: &'_ Str) -> Self { - Self::from_ref(std::ffi::OsStr::new(id.as_str())) + id.clone().into() } } +#[cfg(feature = "string")] impl From for OsStr { fn from(name: std::ffi::OsString) -> Self { Self::from_string(name) } } +#[cfg(feature = "string")] impl From<&'_ std::ffi::OsString> for OsStr { fn from(name: &'_ std::ffi::OsString) -> Self { Self::from_ref(name.as_os_str()) } } +#[cfg(feature = "string")] impl From for OsStr { fn from(name: std::string::String) -> Self { Self::from_string(name.into()) } } +#[cfg(feature = "string")] impl From<&'_ std::string::String> for OsStr { fn from(name: &'_ std::string::String) -> Self { Self::from_ref(name.as_str().as_ref()) @@ -238,7 +243,7 @@ impl PartialEq for std::ffi::OsString { } } -#[cfg(feature = "perf")] +#[cfg(feature = "string")] pub(crate) mod inner { #[derive(Clone)] pub(crate) enum Inner { @@ -272,26 +277,18 @@ pub(crate) mod inner { } } -#[cfg(not(feature = "perf"))] +#[cfg(not(feature = "string"))] pub(crate) mod inner { #[derive(Clone)] - pub(crate) struct Inner(Box); + pub(crate) struct Inner(&'static std::ffi::OsStr); impl Inner { - pub(crate) fn from_string(name: std::ffi::OsString) -> Self { - Self(name.into_boxed_os_str()) - } - - pub(crate) fn from_ref(name: &std::ffi::OsStr) -> Self { - Self(Box::from(name)) - } - pub(crate) fn from_static_ref(name: &'static std::ffi::OsStr) -> Self { - Self::from_ref(name) + Self(name) } pub(crate) fn as_os_str(&self) -> &std::ffi::OsStr { - &self.0 + self.0 } pub(crate) fn into_os_string(self) -> std::ffi::OsString { diff --git a/src/builder/resettable.rs b/src/builder/resettable.rs index 9b20b602ef1..0176d398454 100644 --- a/src/builder/resettable.rs +++ b/src/builder/resettable.rs @@ -162,6 +162,12 @@ impl> IntoResettable for I { } } +impl> IntoResettable for I { + fn into_resettable(self) -> Resettable { + Resettable::Value(self.into()) + } +} + impl> IntoResettable for I { fn into_resettable(self) -> Resettable { Resettable::Value(self.into()) diff --git a/src/builder/str.rs b/src/builder/str.rs index 03c3ee3d779..8dd9a7f454f 100644 --- a/src/builder/str.rs +++ b/src/builder/str.rs @@ -5,12 +5,14 @@ pub struct Str { } impl Str { + #[cfg(feature = "string")] pub(crate) fn from_string(name: std::string::String) -> Self { Self { name: Inner::from_string(name), } } + #[cfg(feature = "string")] pub(crate) fn from_ref(name: &str) -> Self { Self { name: Inner::from_ref(name), @@ -23,7 +25,6 @@ impl Str { } } - #[cfg(feature = "perf")] pub(crate) fn into_inner(self) -> Inner { self.name } @@ -40,12 +41,14 @@ impl From<&'_ Str> for Str { } } +#[cfg(feature = "string")] impl From for Str { fn from(name: std::string::String) -> Self { Self::from_string(name) } } +#[cfg(feature = "string")] impl From<&'_ std::string::String> for Str { fn from(name: &'_ std::string::String) -> Self { Self::from_ref(name.as_str()) @@ -211,7 +214,7 @@ impl PartialEq for std::string::String { } } -#[cfg(feature = "perf")] +#[cfg(feature = "string")] pub(crate) mod inner { #[derive(Clone)] pub(crate) enum Inner { @@ -245,26 +248,18 @@ pub(crate) mod inner { } } -#[cfg(not(feature = "perf"))] +#[cfg(not(feature = "string"))] pub(crate) mod inner { #[derive(Clone)] - pub(crate) struct Inner(Box); + pub(crate) struct Inner(pub(crate) &'static str); impl Inner { - pub(crate) fn from_string(name: std::string::String) -> Self { - Self(name.into_boxed_str()) - } - - pub(crate) fn from_ref(name: &str) -> Self { - Self(Box::from(name)) - } - pub(crate) fn from_static_ref(name: &'static str) -> Self { - Self::from_ref(name) + Self(name) } pub(crate) fn as_str(&self) -> &str { - &self.0 + self.0 } pub(crate) fn into_string(self) -> String { diff --git a/src/util/id.rs b/src/util/id.rs index a6ee26ef9dd..710d2ea7d1c 100644 --- a/src/util/id.rs +++ b/src/util/id.rs @@ -45,12 +45,14 @@ impl From<&'_ Str> for Id { } } +#[cfg(feature = "string")] impl From for Id { fn from(name: std::string::String) -> Self { Self(name.into()) } } +#[cfg(feature = "string")] impl From<&'_ std::string::String> for Id { fn from(name: &'_ std::string::String) -> Self { Self(name.into()) diff --git a/tests/examples.rs b/tests/examples.rs index b3fb1d15bb1..2dddf81157d 100644 --- a/tests/examples.rs +++ b/tests/examples.rs @@ -18,6 +18,8 @@ fn example_tests() { "suggestions", #[cfg(feature = "unicode")] "unicode", + #[cfg(feature = "string")] + "string", #[cfg(feature = "wrap_help")] "wrap_help", #[cfg(feature = "unstable-replace")] diff --git a/tests/ui.rs b/tests/ui.rs index 77bcac71c5c..733e821e10c 100644 --- a/tests/ui.rs +++ b/tests/ui.rs @@ -18,6 +18,8 @@ fn ui_tests() { "suggestions", #[cfg(feature = "unicode")] "unicode", + #[cfg(feature = "string")] + "string", #[cfg(feature = "wrap_help")] "wrap_help", #[cfg(feature = "unstable-replace")]