Repository: clap-rs/clap
Pull request #4096 (2022-08-19 16:55:23)

fix: Switch to owned OsStr type
Another step towards #1041

This allowed removing a dependency as well which helps towards #2037


Patch:
diff --git a/Cargo.toml b/Cargo.toml
index 214c230e..eed7a7d9 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", "dep:once_cell"]
+derive = ["clap_derive"]
 cargo = ["dep:once_cell"] # Disable if you're not using Cargo, enables Cargo-env-var-dependent macros
 wrap_help = ["dep:terminal_size", "textwrap/terminal_size"]
 env = [] # Use environment variables during arg parsing
diff --git a/clap_derive/src/attrs.rs b/clap_derive/src/attrs.rs
index 52ce9b46..79c109d3 100644
--- a/clap_derive/src/attrs.rs
+++ b/clap_derive/src/attrs.rs
@@ -504,11 +504,10 @@ impl Attrs {
                         })
                     } else {
                         quote_spanned!(ident.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)
-                            });
-                            &*DEFAULT_VALUE
+                            }
                         })
                     };
 
@@ -558,10 +557,7 @@ impl Attrs {
 
                                 }
 
-                                static DEFAULT_VALUES: clap::__macro_refs::once_cell::sync::Lazy<Vec<&str>> = clap::__macro_refs::once_cell::sync::Lazy::new(|| {
-                                    iter_to_vals(#expr)
-                                });
-                                DEFAULT_VALUES.iter().copied()
+                                iter_to_vals(#expr)
                             }
                         })
                     } else {
@@ -575,14 +571,7 @@ impl Attrs {
 
                                 }
 
-                                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)
-                                });
-
-                                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()
+                                iter_to_vals(#expr)
                             }
                         })
                     };
@@ -619,11 +608,10 @@ impl Attrs {
                         })
                     } else {
                         quote_spanned!(ident.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)
-                            });
-                            &*DEFAULT_VALUE
+                            }
                         })
                     };
 
@@ -674,10 +662,7 @@ impl Attrs {
 
                                 }
 
-                                static DEFAULT_VALUES: clap::__macro_refs::once_cell::sync::Lazy<Vec<&::std::ffi::OsStr>> = clap::__macro_refs::once_cell::sync::Lazy::new(|| {
-                                    iter_to_vals(#expr)
-                                });
-                                DEFAULT_VALUES.iter().copied()
+                                iter_to_vals(#expr)
                             }
                         })
                     } else {
@@ -691,14 +676,7 @@ impl Attrs {
 
                                 }
 
-                                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)
-                                });
-
-                                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()
+                                iter_to_vals(#expr)
                             }
                         })
                     };
diff --git a/src/builder/arg.rs b/src/builder/arg.rs
index ffebc3d1..648f495e 100644
--- a/src/builder/arg.rs
+++ b/src/builder/arg.rs
@@ -1,11 +1,11 @@
 // Std
 #[cfg(feature = "env")]
 use std::env;
+#[cfg(feature = "env")]
+use std::ffi::OsString;
 use std::{
     borrow::Cow,
     cmp::{Ord, Ordering},
-    ffi::OsStr,
-    ffi::OsString,
     fmt::{self, Display, Formatter},
     str,
 };
@@ -13,9 +13,11 @@ use std::{
 // Internal
 use super::{ArgFlags, ArgSettings};
 use crate::builder::ArgPredicate;
+use crate::builder::IntoResettable;
 use crate::builder::PossibleValue;
 use crate::builder::ValueRange;
 use crate::util::Id;
+use crate::util::OsStr;
 use crate::ArgAction;
 use crate::ValueHint;
 use crate::INTERNAL_ERROR_MSG;
@@ -60,8 +62,8 @@ pub struct Arg<'help> {
     pub(crate) overrides: Vec<Id>,
     pub(crate) groups: Vec<Id>,
     pub(crate) requires: Vec<(ArgPredicate, Id)>,
-    pub(crate) r_ifs: Vec<(Id, OsString)>,
-    pub(crate) r_ifs_all: Vec<(Id, OsString)>,
+    pub(crate) r_ifs: Vec<(Id, OsStr)>,
+    pub(crate) r_ifs_all: Vec<(Id, OsStr)>,
     pub(crate) r_unless: Vec<Id>,
     pub(crate) r_unless_all: Vec<Id>,
     pub(crate) short: Option<char>,
@@ -72,11 +74,11 @@ pub struct Arg<'help> {
     pub(crate) val_names: Vec<&'help str>,
     pub(crate) num_vals: Option<ValueRange>,
     pub(crate) val_delim: Option<char>,
-    pub(crate) default_vals: Vec<&'help OsStr>,
-    pub(crate) default_vals_ifs: Vec<(Id, ArgPredicate, Option<&'help OsStr>)>,
-    pub(crate) default_missing_vals: Vec<&'help OsStr>,
+    pub(crate) default_vals: Vec<OsStr>,
+    pub(crate) default_vals_ifs: Vec<(Id, ArgPredicate, Option<OsStr>)>,
+    pub(crate) default_missing_vals: Vec<OsStr>,
     #[cfg(feature = "env")]
-    pub(crate) env: Option<(&'help OsStr, Option<OsString>)>,
+    pub(crate) env: Option<(OsStr, Option<OsString>)>,
     pub(crate) terminator: Option<&'help str>,
     pub(crate) index: Option<usize>,
     pub(crate) help_heading: Option<Option<&'help str>>,
@@ -1514,8 +1516,8 @@ impl<'help> Arg<'help> {
     /// [`Arg::default_value_if`]: Arg::default_value_if()
     #[inline]
     #[must_use]
-    pub fn default_value(self, val: &'help str) -> Self {
-        self.default_values_os([OsStr::new(val)])
+    pub fn default_value(self, val: impl Into<OsStr>) -> Self {
+        self.default_values_os([val])
     }
 
     /// Value for the argument when not present.
@@ -1526,7 +1528,7 @@ impl<'help> Arg<'help> {
     /// [`OsStr`]: std::ffi::OsStr
     #[inline]
     #[must_use]
-    pub fn default_value_os(self, val: &'help OsStr) -> Self {
+    pub fn default_value_os(self, val: impl Into<OsStr>) -> Self {
         self.default_values_os([val])
     }
 
@@ -1537,9 +1539,8 @@ impl<'help> Arg<'help> {
     /// [`Arg::default_value`]: Arg::default_value()
     #[inline]
     #[must_use]
-    pub fn default_values(self, vals: impl IntoIterator<Item = impl Into<&'help str>>) -> Self {
-        let vals_vec = vals.into_iter().map(|val| OsStr::new(val.into()));
-        self.default_values_os(vals_vec)
+    pub fn default_values(self, vals: impl IntoIterator<Item = impl Into<OsStr>>) -> Self {
+        self.default_values_os(vals)
     }
 
     /// Value for the argument when not present.
@@ -1550,10 +1551,7 @@ impl<'help> Arg<'help> {
     /// [`OsStr`]: std::ffi::OsStr
     #[inline]
     #[must_use]
-    pub fn default_values_os(
-        mut self,
-        vals: impl IntoIterator<Item = impl Into<&'help OsStr>>,
-    ) -> Self {
+    pub fn default_values_os(mut self, vals: impl IntoIterator<Item = impl Into<OsStr>>) -> Self {
         self.default_vals = vals.into_iter().map(|s| s.into()).collect();
         self
     }
@@ -1649,8 +1647,8 @@ impl<'help> Arg<'help> {
     /// [`Arg::default_value`]: Arg::default_value()
     #[inline]
     #[must_use]
-    pub fn default_missing_value(self, val: &'help str) -> Self {
-        self.default_missing_values_os([OsStr::new(val)])
+    pub fn default_missing_value(self, val: impl Into<OsStr>) -> Self {
+        self.default_missing_values_os([val])
     }
 
     /// Value for the argument when the flag is present but no value is specified.
@@ -1661,7 +1659,7 @@ impl<'help> Arg<'help> {
     /// [`OsStr`]: std::ffi::OsStr
     #[inline]
     #[must_use]
-    pub fn default_missing_value_os(self, val: &'help OsStr) -> Self {
+    pub fn default_missing_value_os(self, val: impl Into<OsStr>) -> Self {
         self.default_missing_values_os([val])
     }
 
@@ -1672,12 +1670,8 @@ impl<'help> Arg<'help> {
     /// [`Arg::default_missing_value`]: Arg::default_missing_value()
     #[inline]
     #[must_use]
-    pub fn default_missing_values(
-        self,
-        vals: impl IntoIterator<Item = impl Into<&'help str>>,
-    ) -> Self {
-        let vals_vec = vals.into_iter().map(|val| OsStr::new(val.into()));
-        self.default_missing_values_os(vals_vec)
+    pub fn default_missing_values(self, vals: impl IntoIterator<Item = impl Into<OsStr>>) -> Self {
+        self.default_missing_values_os(vals)
     }
 
     /// Value for the argument when the flag is present but no value is specified.
@@ -1690,7 +1684,7 @@ impl<'help> Arg<'help> {
     #[must_use]
     pub fn default_missing_values_os(
         mut self,
-        vals: impl IntoIterator<Item = impl Into<&'help OsStr>>,
+        vals: impl IntoIterator<Item = impl Into<OsStr>>,
     ) -> Self {
         self.default_missing_vals = vals.into_iter().map(|s| s.into()).collect();
         self
@@ -1844,8 +1838,8 @@ impl<'help> Arg<'help> {
     #[cfg(feature = "env")]
     #[inline]
     #[must_use]
-    pub fn env(self, name: &'help str) -> Self {
-        self.env_os(OsStr::new(name))
+    pub fn env(self, name: impl Into<OsStr>) -> Self {
+        self.env_os(name)
     }
 
     /// Read from `name` environment variable when argument is not present.
@@ -1854,8 +1848,10 @@ impl<'help> Arg<'help> {
     #[cfg(feature = "env")]
     #[inline]
     #[must_use]
-    pub fn env_os(mut self, name: &'help OsStr) -> Self {
-        self.env = Some((name, env::var_os(name)));
+    pub fn env_os(mut self, name: impl Into<OsStr>) -> Self {
+        let name = name.into();
+        let value = env::var_os(&name);
+        self.env = Some((name, value));
         self
     }
 }
@@ -2615,9 +2611,9 @@ impl<'help> Arg<'help> {
         self,
         arg_id: impl Into<Id>,
         val: impl Into<ArgPredicate>,
-        default: Option<&'help str>,
+        default: impl IntoResettable<OsStr>,
     ) -> Self {
-        self.default_value_if_os(arg_id, val.into(), default.map(OsStr::new))
+        self.default_value_if_os(arg_id, val, default)
     }
 
     /// Provides a conditional default value in the exact same manner as [`Arg::default_value_if`]
@@ -2630,10 +2626,13 @@ impl<'help> Arg<'help> {
         mut self,
         arg_id: impl Into<Id>,
         val: impl Into<ArgPredicate>,
-        default: Option<&'help OsStr>,
+        default: impl IntoResettable<OsStr>,
     ) -> Self {
-        self.default_vals_ifs
-            .push((arg_id.into(), val.into(), default));
+        self.default_vals_ifs.push((
+            arg_id.into(),
+            val.into(),
+            default.into_resettable().into_option(),
+        ));
         self
     }
 
@@ -2721,10 +2720,16 @@ impl<'help> Arg<'help> {
     #[must_use]
     pub fn default_value_ifs(
         mut self,
-        ifs: impl IntoIterator<Item = (impl Into<Id>, impl Into<ArgPredicate>, Option<&'help str>)>,
+        ifs: impl IntoIterator<
+            Item = (
+                impl Into<Id>,
+                impl Into<ArgPredicate>,
+                impl IntoResettable<OsStr>,
+            ),
+        >,
     ) -> Self {
         for (arg, val, default) in ifs {
-            self = self.default_value_if_os(arg, val, default.map(OsStr::new));
+            self = self.default_value_if_os(arg, val, default);
         }
         self
     }
@@ -2737,10 +2742,16 @@ impl<'help> Arg<'help> {
     #[must_use]
     pub fn default_value_ifs_os(
         mut self,
-        ifs: impl IntoIterator<Item = (impl Into<Id>, impl Into<ArgPredicate>, Option<&'help OsStr>)>,
+        ifs: impl IntoIterator<
+            Item = (
+                impl Into<Id>,
+                impl Into<ArgPredicate>,
+                impl IntoResettable<OsStr>,
+            ),
+        >,
     ) -> Self {
         for (arg, val, default) in ifs {
-            self = self.default_value_if_os(arg.into(), val, default);
+            self = self.default_value_if_os(arg, val, default);
         }
         self
     }
@@ -3038,7 +3049,7 @@ impl<'help> Arg<'help> {
     /// [Conflicting]: Arg::conflicts_with()
     /// [required]: Arg::required()
     #[must_use]
-    pub fn required_if_eq(mut self, arg_id: impl Into<Id>, val: impl Into<OsString>) -> Self {
+    pub fn required_if_eq(mut self, arg_id: impl Into<Id>, val: impl Into<OsStr>) -> Self {
         self.r_ifs.push((arg_id.into(), val.into()));
         self
     }
@@ -3119,7 +3130,7 @@ impl<'help> Arg<'help> {
     #[must_use]
     pub fn required_if_eq_any(
         mut self,
-        ifs: impl IntoIterator<Item = (impl Into<Id>, impl Into<OsString>)>,
+        ifs: impl IntoIterator<Item = (impl Into<Id>, impl Into<OsStr>)>,
     ) -> Self {
         self.r_ifs
             .extend(ifs.into_iter().map(|(id, val)| (id.into(), val.into())));
@@ -3200,7 +3211,7 @@ impl<'help> Arg<'help> {
     #[must_use]
     pub fn required_if_eq_all(
         mut self,
-        ifs: impl IntoIterator<Item = (impl Into<Id>, impl Into<OsString>)>,
+        ifs: impl IntoIterator<Item = (impl Into<Id>, impl Into<OsStr>)>,
     ) -> Self {
         self.r_ifs_all
             .extend(ifs.into_iter().map(|(id, val)| (id.into(), val.into())));
@@ -3713,14 +3724,14 @@ impl<'help> Arg<'help> {
     /// # Examples
     ///
     /// ```rust
-    /// # use std::ffi::OsStr;
+    /// # use clap::OsStr;
     /// # use clap::Arg;
     /// let arg = Arg::new("foo").env("ENVIRONMENT");
-    /// assert_eq!(Some(OsStr::new("ENVIRONMENT")), arg.get_env());
+    /// assert_eq!(arg.get_env(), Some(&OsStr::from("ENVIRONMENT")));
     /// ```
     #[cfg(feature = "env")]
     pub fn get_env(&self) -> Option<&OsStr> {
-        self.env.as_ref().map(|x| x.0)
+        self.env.as_ref().map(|x| &x.0)
     }
 
     /// Get the default values specified for this argument, if any
@@ -3730,9 +3741,9 @@ impl<'help> Arg<'help> {
     /// ```rust
     /// # use clap::Arg;
     /// let arg = Arg::new("foo").default_value("default value");
-    /// assert_eq!(&["default value"], arg.get_default_values());
+    /// assert_eq!(arg.get_default_values(), &["default value"]);
     /// ```
-    pub fn get_default_values(&self) -> &[&OsStr] {
+    pub fn get_default_values(&self) -> &[OsStr] {
         &self.default_vals
     }
 
@@ -3743,10 +3754,10 @@ impl<'help> Arg<'help> {
     /// ```
     /// # use clap::Arg;
     /// let arg = Arg::new("foo");
-    /// assert_eq!(true, arg.is_positional());
+    /// assert_eq!(arg.is_positional(), true);
     ///
     /// let arg = Arg::new("foo").long("foo");
-    /// assert_eq!(false, arg.is_positional());
+    /// assert_eq!(arg.is_positional(), false);
     /// ```
     pub fn is_positional(&self) -> bool {
         self.long.is_none() && self.short.is_none()
@@ -3892,12 +3903,12 @@ impl<'help> Arg<'help> {
         if let Some(action) = self.action.as_ref() {
             if let Some(default_value) = action.default_value() {
                 if self.default_vals.is_empty() {
-                    self.default_vals = vec![default_value];
+                    self.default_vals = vec![default_value.into()];
                 }
             }
             if let Some(default_value) = action.default_missing_value() {
                 if self.default_missing_vals.is_empty() {
-                    self.default_missing_vals = vec![default_value];
+                    self.default_missing_vals = vec![default_value.into()];
                 }
             }
         }
diff --git a/src/builder/debug_asserts.rs b/src/builder/debug_asserts.rs
index 77b0dbb5..f52a6383 100644
--- a/src/builder/debug_asserts.rs
+++ b/src/builder/debug_asserts.rs
@@ -7,6 +7,7 @@ use crate::mkeymap::KeyType;
 use crate::util::FlatSet;
 use crate::util::Id;
 use crate::ArgAction;
+use crate::OsStr;
 use crate::INTERNAL_ERROR_MSG;
 use crate::{Arg, Command, ValueHint};
 
@@ -766,18 +767,18 @@ fn assert_arg(arg: &Arg) {
 
     assert_arg_flags(arg);
 
-    assert_defaults(arg, "default_value", arg.default_vals.iter().copied());
+    assert_defaults(arg, "default_value", arg.default_vals.iter());
     assert_defaults(
         arg,
         "default_missing_value",
-        arg.default_missing_vals.iter().copied(),
+        arg.default_missing_vals.iter(),
     );
     assert_defaults(
         arg,
         "default_value_if",
         arg.default_vals_ifs
             .iter()
-            .filter_map(|(_, _, default)| *default),
+            .filter_map(|(_, _, default)| default.as_ref()),
     );
 }
 
@@ -813,7 +814,7 @@ fn assert_arg_flags(arg: &Arg) {
 fn assert_defaults<'d>(
     arg: &Arg,
     field: &'static str,
-    defaults: impl IntoIterator<Item = &'d std::ffi::OsStr>,
+    defaults: impl IntoIterator<Item = &'d OsStr>,
 ) {
     for default_os in defaults {
         let value_parser = arg.get_value_parser();
diff --git a/src/builder/mod.rs b/src/builder/mod.rs
index 16eaeffd..23ad15a9 100644
--- a/src/builder/mod.rs
+++ b/src/builder/mod.rs
@@ -9,6 +9,7 @@ mod arg_settings;
 mod command;
 mod possible_value;
 mod range;
+mod resettable;
 mod value_hint;
 mod value_parser;
 
@@ -25,6 +26,8 @@ pub use arg_predicate::ArgPredicate;
 pub use command::Command;
 pub use possible_value::PossibleValue;
 pub use range::ValueRange;
+pub use resettable::IntoResettable;
+pub use resettable::Resettable;
 pub use value_hint::ValueHint;
 pub use value_parser::_AutoValueParser;
 pub use value_parser::via_prelude;
diff --git a/src/builder/resettable.rs b/src/builder/resettable.rs
new file mode 100644
index 00000000..0f269d7e
--- /dev/null
+++ b/src/builder/resettable.rs
@@ -0,0 +1,53 @@
+// Unlike `impl Into<Option<T>>` or `Option<impl Into<T>>`, this isn't ambiguous for the `None`
+// case.
+
+/// Clearable builder value
+#[derive(Copy, Clone, Debug, PartialEq, Eq, PartialOrd, Ord, Hash)]
+pub enum Resettable<T> {
+    /// Overwrite builder value
+    Value(T),
+    /// Reset builder value
+    Reset,
+}
+
+impl<T> Resettable<T> {
+    pub(crate) fn into_option(self) -> Option<T> {
+        match self {
+            Self::Value(t) => Some(t),
+            Self::Reset => None,
+        }
+    }
+}
+
+/// Convert to the intended resettable type
+pub trait IntoResettable<T> {
+    /// Convert to the intended resettable type
+    fn into_resettable(self) -> Resettable<T>;
+}
+
+impl IntoResettable<crate::OsStr> for Option<&'static str> {
+    fn into_resettable(self) -> Resettable<crate::OsStr> {
+        match self {
+            Some(s) => Resettable::Value(s.into()),
+            None => Resettable::Reset,
+        }
+    }
+}
+
+impl<I: Into<crate::OsStr>> IntoResettable<crate::OsStr> for I {
+    fn into_resettable(self) -> Resettable<crate::OsStr> {
+        Resettable::Value(self.into())
+    }
+}
+
+impl<I: Into<crate::Str>> IntoResettable<crate::Str> for I {
+    fn into_resettable(self) -> Resettable<crate::Str> {
+        Resettable::Value(self.into())
+    }
+}
+
+impl<I: Into<crate::Id>> IntoResettable<crate::Id> for I {
+    fn into_resettable(self) -> Resettable<crate::Id> {
+        Resettable::Value(self.into())
+    }
+}
diff --git a/src/lib.rs b/src/lib.rs
index cf87ddd0..37aa246f 100644
--- a/src/lib.rs
+++ b/src/lib.rs
@@ -109,7 +109,7 @@ pub use crate::util::color::ColorChoice;
 #[allow(unused_imports)]
 pub(crate) use crate::util::color::ColorChoice;
 pub use crate::util::Id;
-pub(crate) use crate::util::OsStr;
+pub use crate::util::OsStr;
 pub(crate) use crate::util::Str;
 
 pub use crate::derive::{Args, CommandFactory, FromArgMatches, Parser, Subcommand, ValueEnum};
@@ -131,7 +131,7 @@ pub mod _tutorial;
 
 #[doc(hidden)]
 pub mod __macro_refs {
-    #[cfg(any(feature = "derive", feature = "cargo"))]
+    #[cfg(feature = "cargo")]
     #[doc(hidden)]
     pub use once_cell;
 }
diff --git a/src/output/help.rs b/src/output/help.rs
index 26620a05..932e5b09 100644
--- a/src/output/help.rs
+++ b/src/output/help.rs
@@ -597,7 +597,7 @@ impl<'help, 'cmd, 'writer> Help<'help, 'cmd, 'writer> {
             let pvs = a
                 .default_vals
                 .iter()
-                .map(|&pvs| pvs.to_string_lossy())
+                .map(|pvs| pvs.to_string_lossy())
                 .map(|pvs| {
                     if pvs.contains(char::is_whitespace) {
                         Cow::from(format!("{:?}", pvs))
diff --git a/src/parser/parser.rs b/src/parser/parser.rs
index 1eea0d17..444b822f 100644
--- a/src/parser/parser.rs
+++ b/src/parser/parser.rs
@@ -1123,8 +1123,7 @@ impl<'help, 'cmd> Parser<'help, 'cmd> {
                 raw_vals.extend(
                     arg.default_missing_vals
                         .iter()
-                        .copied()
-                        .map(ToOwned::to_owned),
+                        .map(|s| s.as_os_str().to_owned()),
                 );
             }
         }
@@ -1388,8 +1387,8 @@ impl<'help, 'cmd> Parser<'help, 'cmd> {
                     };
 
                     if add {
-                        if let Some(default) = *default {
-                            let arg_values = vec![default.to_owned()];
+                        if let Some(default) = default {
+                            let arg_values = vec![default.to_os_string()];
                             let trailing_idx = None;
                             let _ = self.react(
                                 None,
@@ -1424,8 +1423,7 @@ impl<'help, 'cmd> Parser<'help, 'cmd> {
                 let arg_values: Vec<_> = arg
                     .default_vals
                     .iter()
-                    .copied()
-                    .map(ToOwned::to_owned)
+                    .map(crate::OsStr::to_os_string)
                     .collect();
                 let trailing_idx = None;
                 let _ = self.react(
diff --git a/src/util/os_str.rs b/src/util/os_str.rs
index e9d7767a..592e2b69 100644
--- a/src/util/os_str.rs
+++ b/src/util/os_str.rs
@@ -23,10 +23,15 @@ impl OsStr {
         }
     }
 
-    /// Get the raw string of the `OsStr`
+    /// Get the raw string as an `std::ffi::OsStr`
     pub fn as_os_str(&self) -> &std::ffi::OsStr {
         self.name.as_os_str()
     }
+
+    /// Get the raw string as an `OsString`
+    pub fn to_os_string(&self) -> std::ffi::OsString {
+        self.as_os_str().to_owned()
+    }
 }
 
 impl From<&'_ OsStr> for OsStr {


Test patch:
diff --git a/tests/builder/default_vals.rs b/tests/builder/default_vals.rs
index fc01816a..d2f2fc08 100644
--- a/tests/builder/default_vals.rs
+++ b/tests/builder/default_vals.rs
@@ -624,7 +624,7 @@ fn default_value_ifs_os() {
             Arg::new("other")
                 .long("other")
                 .value_parser(value_parser!(OsString))
-                .default_value_ifs_os([("flag", "标记2", Some(OsStr::new("flag=标记2")))]),
+                .default_value_ifs_os([("flag", "标记2", OsStr::new("flag=标记2"))]),
         );
     let result = cmd.try_get_matches_from(["my_cargo", "--flag", "标记2"]);
     assert!(result.is_ok(), "{}", result.unwrap_err());
diff --git a/tests/derive/default_value.rs b/tests/derive/default_value.rs
index 70141836..8f0fdf14 100644
--- a/tests/derive/default_value.rs
+++ b/tests/derive/default_value.rs
@@ -166,11 +166,9 @@ fn default_values_os_t() {
 
 #[test]
 fn detect_os_variant() {
-    #![allow(unused_parens)] // needed for `as_ref` call
-
     #[derive(clap::Parser)]
     pub struct Options {
-        #[clap(default_value_os = ("123".as_ref()))]
+        #[clap(default_value_os = "123")]
         x: String,
     }
     Options::command().debug_assert();