Skip to content

Commit

Permalink
WIP: fix: Ensure 'ArgMatches' is unwind safe
Browse files Browse the repository at this point in the history
  • Loading branch information
epage committed Jun 28, 2022
1 parent 9962393 commit 436e1f9
Show file tree
Hide file tree
Showing 4 changed files with 94 additions and 18 deletions.
8 changes: 7 additions & 1 deletion src/builder/command.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5110,5 +5110,11 @@ where

#[test]
fn check_auto_traits() {
static_assertions::assert_impl_all!(Command: Send, Sync, Unpin);
static_assertions::assert_impl_all!(
Command: Send,
Sync,
std::panic::RefUnwindSafe,
std::panic::UnwindSafe,
Unpin
);
}
21 changes: 17 additions & 4 deletions src/builder/value_parser.rs
Original file line number Diff line number Diff line change
Expand Up @@ -108,7 +108,7 @@ impl ValueParser {
pub fn new<P>(other: P) -> Self
where
P: TypedValueParser,
P::Value: Send + Sync + Clone,
P::Value: Send + Sync + Clone + std::panic::UnwindSafe + std::panic::RefUnwindSafe,
{
Self(ValueParserInner::Other(Box::new(other)))
}
Expand Down Expand Up @@ -284,7 +284,7 @@ impl ValueParser {
impl<P> From<P> for ValueParser
where
P: TypedValueParser + Send + Sync + 'static,
P::Value: Send + Sync + Clone,
P::Value: Send + Sync + Clone + std::panic::UnwindSafe + std::panic::RefUnwindSafe,
{
fn from(p: P) -> Self {
Self::new(p)
Expand Down Expand Up @@ -563,7 +563,13 @@ trait AnyValueParser: Send + Sync + 'static {

impl<T, P> AnyValueParser for P
where
T: std::any::Any + Clone + Send + Sync + 'static,
T: std::any::Any
+ Clone
+ Send
+ Sync
+ std::panic::UnwindSafe
+ std::panic::RefUnwindSafe
+ 'static,
P: TypedValueParser<Value = T>,
{
fn parse_ref(
Expand Down Expand Up @@ -1963,7 +1969,14 @@ pub mod via_prelude {
}
impl<FromStr> _ValueParserViaFromStr for _AutoValueParser<FromStr>
where
FromStr: std::str::FromStr + std::any::Any + Clone + Send + Sync + 'static,
FromStr: std::str::FromStr
+ std::any::Any
+ Clone
+ Send
+ Sync
+ std::panic::UnwindSafe
+ std::panic::RefUnwindSafe
+ 'static,
<FromStr as std::str::FromStr>::Err:
Into<Box<dyn std::error::Error + Send + Sync + 'static>>,
{
Expand Down
37 changes: 33 additions & 4 deletions src/parser/matches/any_value.rs
Original file line number Diff line number Diff line change
@@ -1,25 +1,54 @@
#[derive(Clone)]
pub(crate) struct AnyValue {
inner: std::sync::Arc<dyn std::any::Any + Send + Sync + 'static>,
inner: std::sync::Arc<
dyn std::any::Any
+ Send
+ Sync
+ std::panic::UnwindSafe
+ std::panic::RefUnwindSafe
+ 'static,
>,
// While we can extract `TypeId` from `inner`, the debug repr is of a number, so let's track
// the type_name in debug builds.
id: AnyValueId,
}

impl AnyValue {
pub(crate) fn new<V: std::any::Any + Clone + Send + Sync + 'static>(inner: V) -> Self {
pub(crate) fn new<
V: std::any::Any
+ Clone
+ Send
+ Sync
+ std::panic::UnwindSafe
+ std::panic::RefUnwindSafe
+ 'static,
>(
inner: V,
) -> Self {
let id = AnyValueId::of::<V>();
let inner = std::sync::Arc::new(inner);
Self { inner, id }
}

pub(crate) fn downcast_ref<T: std::any::Any + Clone + Send + Sync + 'static>(
pub(crate) fn downcast_ref<
T: std::any::Any
+ Clone
+ Send
+ Sync
+ std::panic::UnwindSafe
+ std::panic::RefUnwindSafe
+ 'static,
>(
&self,
) -> Option<&T> {
self.inner.downcast_ref::<T>()
}

pub(crate) fn downcast_into<T: std::any::Any + Clone + Send + Sync>(self) -> Result<T, Self> {
pub(crate) fn downcast_into<
T: std::any::Any + Clone + Send + Sync + std::panic::UnwindSafe + std::panic::RefUnwindSafe,
>(
self,
) -> Result<T, Self> {
let id = self.id;
let value =
std::sync::Arc::downcast::<T>(self.inner).map_err(|inner| Self { inner, id })?;
Expand Down
46 changes: 37 additions & 9 deletions src/parser/matches/arg_matches.rs
Original file line number Diff line number Diff line change
Expand Up @@ -114,7 +114,12 @@ impl ArgMatches {
/// [positional]: crate::Arg::index()
/// [`default_value`]: crate::Arg::default_value()
#[track_caller]
pub fn get_one<T: Any + Clone + Send + Sync + 'static>(&self, id: &str) -> Option<&T> {
pub fn get_one<
T: Any + Clone + Send + Sync + std::panic::UnwindSafe + std::panic::RefUnwindSafe + 'static,
>(
&self,
id: &str,
) -> Option<&T> {
let internal_id = Id::from(id);
MatchesError::unwrap(&internal_id, self.try_get_one(id))
}
Expand Down Expand Up @@ -153,7 +158,9 @@ impl ArgMatches {
/// assert_eq!(vals, [22, 80, 2020]);
/// ```
#[track_caller]
pub fn get_many<T: Any + Clone + Send + Sync + 'static>(
pub fn get_many<
T: Any + Clone + Send + Sync + std::panic::UnwindSafe + std::panic::RefUnwindSafe + 'static,
>(
&self,
id: &str,
) -> Option<ValuesRef<T>> {
Expand Down Expand Up @@ -243,7 +250,12 @@ impl ArgMatches {
/// [positional]: crate::Arg::index()
/// [`default_value`]: crate::Arg::default_value()
#[track_caller]
pub fn remove_one<T: Any + Clone + Send + Sync + 'static>(&mut self, id: &str) -> Option<T> {
pub fn remove_one<
T: Any + Clone + Send + Sync + std::panic::UnwindSafe + std::panic::RefUnwindSafe + 'static,
>(
&mut self,
id: &str,
) -> Option<T> {
let internal_id = Id::from(id);
MatchesError::unwrap(&internal_id, self.try_remove_one(id))
}
Expand Down Expand Up @@ -280,7 +292,9 @@ impl ArgMatches {
/// assert_eq!(vals, ["file1.txt", "file2.txt", "file3.txt", "file4.txt"]);
/// ```
#[track_caller]
pub fn remove_many<T: Any + Clone + Send + Sync + 'static>(
pub fn remove_many<
T: Any + Clone + Send + Sync + std::panic::UnwindSafe + std::panic::RefUnwindSafe + 'static,
>(
&mut self,
id: &str,
) -> Option<Values2<T>> {
Expand Down Expand Up @@ -1074,7 +1088,9 @@ impl ArgMatches {
/// # Advanced
impl ArgMatches {
/// Non-panicking version of [`ArgMatches::get_one`]
pub fn try_get_one<T: Any + Clone + Send + Sync + 'static>(
pub fn try_get_one<
T: Any + Clone + Send + Sync + std::panic::UnwindSafe + std::panic::RefUnwindSafe + 'static,
>(
&self,
id: &str,
) -> Result<Option<&T>, MatchesError> {
Expand All @@ -1093,7 +1109,9 @@ impl ArgMatches {
}

/// Non-panicking version of [`ArgMatches::get_many`]
pub fn try_get_many<T: Any + Clone + Send + Sync + 'static>(
pub fn try_get_many<
T: Any + Clone + Send + Sync + std::panic::UnwindSafe + std::panic::RefUnwindSafe + 'static,
>(
&self,
id: &str,
) -> Result<Option<ValuesRef<T>>, MatchesError> {
Expand Down Expand Up @@ -1129,7 +1147,9 @@ impl ArgMatches {
}

/// Non-panicking version of [`ArgMatches::remove_one`]
pub fn try_remove_one<T: Any + Clone + Send + Sync + 'static>(
pub fn try_remove_one<
T: Any + Clone + Send + Sync + std::panic::UnwindSafe + std::panic::RefUnwindSafe + 'static,
>(
&mut self,
id: &str,
) -> Result<Option<T>, MatchesError> {
Expand All @@ -1145,7 +1165,9 @@ impl ArgMatches {
}

/// Non-panicking version of [`ArgMatches::remove_many`]
pub fn try_remove_many<T: Any + Clone + Send + Sync + 'static>(
pub fn try_remove_many<
T: Any + Clone + Send + Sync + std::panic::UnwindSafe + std::panic::RefUnwindSafe + 'static,
>(
&mut self,
id: &str,
) -> Result<Option<Values2<T>>, MatchesError> {
Expand Down Expand Up @@ -1743,7 +1765,13 @@ mod tests {

#[test]
fn check_auto_traits() {
static_assertions::assert_impl_all!(ArgMatches: Send, Sync, Unpin);
static_assertions::assert_impl_all!(
ArgMatches: Send,
Sync,
std::panic::RefUnwindSafe,
std::panic::UnwindSafe,
Unpin
);
}

#[test]
Expand Down

0 comments on commit 436e1f9

Please sign in to comment.