From 5b5f2c1f40212c76c7f796cfb26dbb3eb35f1119 Mon Sep 17 00:00:00 2001
From: Ed Page <eopage@gmail.com>
Date: Thu, 11 Aug 2022 15:33:23 -0500
Subject: [PATCH] fix!: Track original Ids, rather than a hash

This is a step towards #1041
- `ArgGroup` no longer takes a lifetime
- One less field type needs a lifetime

For now, we are using a more brute force type (`String`) so we can
establish performance base lines.  I was torn on whether to use `&str`
everywhere or make an `IdRef`.  The latter would add a lot of noise that
I'm concerned about, so i left it simple for now.  `IdRef` would help to
communicate the types involved though.

Speaking of communicating types, I'm also torn on whether we should use
`Id` for all strings or if we should have `Id`, `Name`, etc types to
avoid people mixing and matching.

This added 18.7 KB.

Compared to `HEAD~` on `06_rustup`:
- build: 6.23us -> 7.41us
- parse: 8.17us -> 9.36us
- parse_sc: 7.65us -> 9.29us
---
 clap_mangen/src/render.rs         |   4 +-
 src/builder/arg.rs                | 118 +++++++++++-----------------
 src/builder/arg_group.rs          |  49 +++++-------
 src/builder/command.rs            |  38 +++++----
 src/builder/debug_asserts.rs      |  44 ++++++-----
 src/mkeymap.rs                    |   5 +-
 src/output/help.rs                |   2 +-
 src/parser/arg_matcher.rs         |   6 +-
 src/parser/error.rs               |   6 +-
 src/parser/matches/arg_matches.rs |  82 +++++++++-----------
 src/parser/parser.rs              |   6 +-
 src/util/fnv.rs                   |  46 -----------
 src/util/id.rs                    | 123 ++++++++++++++++--------------
 src/util/mod.rs                   |   3 -
 tests/builder/groups.rs           |   6 +-
 tests/builder/help.rs             |   2 +-
 tests/builder/tests.rs            |   2 +-
 17 files changed, 222 insertions(+), 320 deletions(-)
 delete mode 100644 src/util/fnv.rs

diff --git a/clap_mangen/src/render.rs b/clap_mangen/src/render.rs
index 1655af38a2a..b437125bd50 100644
--- a/clap_mangen/src/render.rs
+++ b/clap_mangen/src/render.rs
@@ -63,7 +63,7 @@ pub(crate) fn synopsis(roff: &mut Roff, cmd: &clap::Command) {
         if let Some(value) = arg.get_value_names() {
             line.push(italic(value.join(" ")));
         } else {
-            line.push(italic(arg.get_id()));
+            line.push(italic(arg.get_id().as_str()));
         }
         line.push(roman(rhs));
         line.push(roman(" "));
@@ -129,7 +129,7 @@ pub(crate) fn options(roff: &mut Roff, cmd: &clap::Command) {
         if let Some(value) = pos.get_value_names() {
             header.push(italic(value.join(" ")));
         } else {
-            header.push(italic(pos.get_id()));
+            header.push(italic(pos.get_id().as_str()));
         };
         header.push(roman(rhs));
 
diff --git a/src/builder/arg.rs b/src/builder/arg.rs
index 7f023cd9e65..dd14a95e0bd 100644
--- a/src/builder/arg.rs
+++ b/src/builder/arg.rs
@@ -50,7 +50,6 @@ use crate::INTERNAL_ERROR_MSG;
 #[derive(Default, Clone)]
 pub struct Arg<'help> {
     pub(crate) id: Id,
-    name: &'help str,
     pub(crate) help: Option<&'help str>,
     pub(crate) long_help: Option<&'help str>,
     pub(crate) action: Option<ArgAction>,
@@ -102,7 +101,7 @@ impl<'help> Arg<'help> {
     /// # ;
     /// ```
     /// [`Arg::action(ArgAction::Set)`]: Arg::action()
-    pub fn new(id: impl Into<&'help str>) -> Self {
+    pub fn new(id: impl Into<Id>) -> Self {
         Arg::default().id(id)
     }
 
@@ -110,10 +109,8 @@ impl<'help> Arg<'help> {
     ///
     /// See [`Arg::new`] for more details.
     #[must_use]
-    pub fn id(mut self, id: impl Into<&'help str>) -> Self {
-        let id = id.into();
-        self.id = Id::from(id);
-        self.name = id;
+    pub fn id(mut self, id: impl Into<Id>) -> Self {
+        self.id = id.into();
         self
     }
 
@@ -660,9 +657,8 @@ impl<'help> Arg<'help> {
     /// [Conflicting]: Arg::conflicts_with()
     /// [override]: Arg::overrides_with()
     #[must_use]
-    pub fn requires(mut self, arg_id: impl Into<&'help str>) -> Self {
-        self.requires
-            .push((ArgPredicate::IsPresent, arg_id.into().into()));
+    pub fn requires(mut self, arg_id: impl Into<Id>) -> Self {
+        self.requires.push((ArgPredicate::IsPresent, arg_id.into()));
         self
     }
 
@@ -2443,8 +2439,8 @@ impl<'help> Arg<'help> {
     ///
     /// [`ArgGroup`]: crate::ArgGroup
     #[must_use]
-    pub fn group(mut self, group_id: impl Into<&'help str>) -> Self {
-        self.groups.push(group_id.into().into());
+    pub fn group(mut self, group_id: impl Into<Id>) -> Self {
+        self.groups.push(group_id.into());
         self
     }
 
@@ -2484,9 +2480,8 @@ impl<'help> Arg<'help> {
     ///
     /// [`ArgGroup`]: crate::ArgGroup
     #[must_use]
-    pub fn groups(mut self, group_ids: impl IntoIterator<Item = impl Into<&'help str>>) -> Self {
-        self.groups
-            .extend(group_ids.into_iter().map(|n| n.into().into()));
+    pub fn groups(mut self, group_ids: impl IntoIterator<Item = impl Into<Id>>) -> Self {
+        self.groups.extend(group_ids.into_iter().map(Into::into));
         self
     }
 
@@ -2605,7 +2600,7 @@ impl<'help> Arg<'help> {
     #[must_use]
     pub fn default_value_if(
         self,
-        arg_id: impl Into<&'help str>,
+        arg_id: impl Into<Id>,
         val: Option<&'help str>,
         default: Option<&'help str>,
     ) -> Self {
@@ -2620,12 +2615,12 @@ impl<'help> Arg<'help> {
     #[must_use]
     pub fn default_value_if_os(
         mut self,
-        arg_id: impl Into<&'help str>,
+        arg_id: impl Into<Id>,
         val: Option<&'help OsStr>,
         default: Option<&'help OsStr>,
     ) -> Self {
         self.default_vals_ifs
-            .push((arg_id.into().into(), val.into(), default));
+            .push((arg_id.into(), val.into(), default));
         self
     }
 
@@ -2712,13 +2707,7 @@ impl<'help> Arg<'help> {
     #[must_use]
     pub fn default_value_ifs(
         mut self,
-        ifs: impl IntoIterator<
-            Item = (
-                impl Into<&'help str>,
-                Option<&'help str>,
-                Option<&'help str>,
-            ),
-        >,
+        ifs: impl IntoIterator<Item = (impl Into<Id>, Option<&'help str>, Option<&'help str>)>,
     ) -> Self {
         for (arg, val, default) in ifs {
             self = self.default_value_if_os(arg, val.map(OsStr::new), default.map(OsStr::new));
@@ -2734,13 +2723,7 @@ impl<'help> Arg<'help> {
     #[must_use]
     pub fn default_value_ifs_os(
         mut self,
-        ifs: impl IntoIterator<
-            Item = (
-                impl Into<&'help str>,
-                Option<&'help OsStr>,
-                Option<&'help OsStr>,
-            ),
-        >,
+        ifs: impl IntoIterator<Item = (impl Into<Id>, Option<&'help OsStr>, Option<&'help OsStr>)>,
     ) -> Self {
         for (arg, val, default) in ifs {
             self = self.default_value_if_os(arg.into(), val, default);
@@ -2802,8 +2785,8 @@ impl<'help> Arg<'help> {
     /// ```
     /// [required]: Arg::required()
     #[must_use]
-    pub fn required_unless_present(mut self, arg_id: impl Into<&'help str>) -> Self {
-        self.r_unless.push(arg_id.into().into());
+    pub fn required_unless_present(mut self, arg_id: impl Into<Id>) -> Self {
+        self.r_unless.push(arg_id.into());
         self
     }
 
@@ -2877,10 +2860,9 @@ impl<'help> Arg<'help> {
     #[must_use]
     pub fn required_unless_present_all(
         mut self,
-        names: impl IntoIterator<Item = impl Into<&'help str>>,
+        names: impl IntoIterator<Item = impl Into<Id>>,
     ) -> Self {
-        self.r_unless_all
-            .extend(names.into_iter().map(|n| n.into().into()));
+        self.r_unless_all.extend(names.into_iter().map(Into::into));
         self
     }
 
@@ -2956,10 +2938,9 @@ impl<'help> Arg<'help> {
     #[must_use]
     pub fn required_unless_present_any(
         mut self,
-        names: impl IntoIterator<Item = impl Into<&'help str>>,
+        names: impl IntoIterator<Item = impl Into<Id>>,
     ) -> Self {
-        self.r_unless
-            .extend(names.into_iter().map(|n| n.into().into()));
+        self.r_unless.extend(names.into_iter().map(Into::into));
         self
     }
 
@@ -3043,8 +3024,8 @@ impl<'help> Arg<'help> {
     /// [Conflicting]: Arg::conflicts_with()
     /// [required]: Arg::required()
     #[must_use]
-    pub fn required_if_eq(mut self, arg_id: impl Into<&'help str>, val: &'help str) -> Self {
-        self.r_ifs.push((arg_id.into().into(), val));
+    pub fn required_if_eq(mut self, arg_id: impl Into<Id>, val: &'help str) -> Self {
+        self.r_ifs.push((arg_id.into(), val));
         self
     }
 
@@ -3124,10 +3105,10 @@ impl<'help> Arg<'help> {
     #[must_use]
     pub fn required_if_eq_any(
         mut self,
-        ifs: impl IntoIterator<Item = (impl Into<&'help str>, &'help str)>,
+        ifs: impl IntoIterator<Item = (impl Into<Id>, &'help str)>,
     ) -> Self {
         self.r_ifs
-            .extend(ifs.into_iter().map(|(id, val)| (id.into().into(), val)));
+            .extend(ifs.into_iter().map(|(id, val)| (id.into(), val)));
         self
     }
 
@@ -3205,10 +3186,10 @@ impl<'help> Arg<'help> {
     #[must_use]
     pub fn required_if_eq_all(
         mut self,
-        ifs: impl IntoIterator<Item = (impl Into<&'help str>, &'help str)>,
+        ifs: impl IntoIterator<Item = (impl Into<Id>, &'help str)>,
     ) -> Self {
         self.r_ifs_all
-            .extend(ifs.into_iter().map(|(id, val)| (id.into().into(), val)));
+            .extend(ifs.into_iter().map(|(id, val)| (id.into(), val)));
         self
     }
 
@@ -3268,9 +3249,9 @@ impl<'help> Arg<'help> {
     /// [Conflicting]: Arg::conflicts_with()
     /// [override]: Arg::overrides_with()
     #[must_use]
-    pub fn requires_if(mut self, val: &'help str, arg_id: impl Into<&'help str>) -> Self {
+    pub fn requires_if(mut self, val: &'help str, arg_id: impl Into<Id>) -> Self {
         self.requires
-            .push((ArgPredicate::Equals(OsStr::new(val)), arg_id.into().into()));
+            .push((ArgPredicate::Equals(OsStr::new(val)), arg_id.into()));
         self
     }
 
@@ -3321,11 +3302,11 @@ impl<'help> Arg<'help> {
     #[must_use]
     pub fn requires_ifs(
         mut self,
-        ifs: impl IntoIterator<Item = (&'help str, impl Into<&'help str>)>,
+        ifs: impl IntoIterator<Item = (&'help str, impl Into<Id>)>,
     ) -> Self {
         self.requires.extend(
             ifs.into_iter()
-                .map(|(val, arg)| (ArgPredicate::Equals(OsStr::new(val)), arg.into().into())),
+                .map(|(val, arg)| (ArgPredicate::Equals(OsStr::new(val)), arg.into())),
         );
         self
     }
@@ -3389,11 +3370,11 @@ impl<'help> Arg<'help> {
     /// [Conflicting]: Arg::conflicts_with()
     /// [override]: Arg::overrides_with()
     #[must_use]
-    pub fn requires_all(mut self, names: impl IntoIterator<Item = impl Into<&'help str>>) -> Self {
+    pub fn requires_all(mut self, names: impl IntoIterator<Item = impl Into<Id>>) -> Self {
         self.requires.extend(
             names
                 .into_iter()
-                .map(|s| (ArgPredicate::IsPresent, s.into().into())),
+                .map(|s| (ArgPredicate::IsPresent, s.into())),
         );
         self
     }
@@ -3443,8 +3424,8 @@ impl<'help> Arg<'help> {
     /// [`Arg::conflicts_with_all(names)`]: Arg::conflicts_with_all()
     /// [`Arg::exclusive(true)`]: Arg::exclusive()
     #[must_use]
-    pub fn conflicts_with(mut self, arg_id: impl Into<&'help str>) -> Self {
-        self.blacklist.push(arg_id.into().into());
+    pub fn conflicts_with(mut self, arg_id: impl Into<Id>) -> Self {
+        self.blacklist.push(arg_id.into());
         self
     }
 
@@ -3493,12 +3474,8 @@ impl<'help> Arg<'help> {
     /// [`Arg::conflicts_with`]: Arg::conflicts_with()
     /// [`Arg::exclusive(true)`]: Arg::exclusive()
     #[must_use]
-    pub fn conflicts_with_all(
-        mut self,
-        names: impl IntoIterator<Item = impl Into<&'help str>>,
-    ) -> Self {
-        self.blacklist
-            .extend(names.into_iter().map(|n| n.into().into()));
+    pub fn conflicts_with_all(mut self, names: impl IntoIterator<Item = impl Into<Id>>) -> Self {
+        self.blacklist.extend(names.into_iter().map(Into::into));
         self
     }
 
@@ -3535,8 +3512,8 @@ impl<'help> Arg<'help> {
     /// assert!(!*m.get_one::<bool>("flag").unwrap());
     /// ```
     #[must_use]
-    pub fn overrides_with(mut self, arg_id: impl Into<&'help str>) -> Self {
-        self.overrides.push(arg_id.into().into());
+    pub fn overrides_with(mut self, arg_id: impl Into<Id>) -> Self {
+        self.overrides.push(arg_id.into());
         self
     }
 
@@ -3571,12 +3548,8 @@ impl<'help> Arg<'help> {
     /// assert!(!*m.get_one::<bool>("flag").unwrap());
     /// ```
     #[must_use]
-    pub fn overrides_with_all(
-        mut self,
-        names: impl IntoIterator<Item = impl Into<&'help str>>,
-    ) -> Self {
-        self.overrides
-            .extend(names.into_iter().map(|n| n.into().into()));
+    pub fn overrides_with_all(mut self, names: impl IntoIterator<Item = impl Into<Id>>) -> Self {
+        self.overrides.extend(names.into_iter().map(Into::into));
         self
     }
 }
@@ -3585,8 +3558,8 @@ impl<'help> Arg<'help> {
 impl<'help> Arg<'help> {
     /// Get the name of the argument
     #[inline]
-    pub fn get_id(&self) -> &'help str {
-        self.name
+    pub fn get_id(&self) -> &Id {
+        &self.id
     }
 
     /// Get the help specified for this argument, if any
@@ -4017,7 +3990,7 @@ impl<'help> Arg<'help> {
             }
         } else {
             debug!("Arg::name_no_brackets: just name");
-            Cow::Borrowed(self.get_id())
+            Cow::Borrowed(self.get_id().as_str())
         }
     }
 
@@ -4051,7 +4024,7 @@ impl<'help> PartialOrd for Arg<'help> {
 
 impl<'help> Ord for Arg<'help> {
     fn cmp(&self, other: &Arg) -> Ordering {
-        self.get_id().cmp(other.get_id())
+        self.id.cmp(&other.id)
     }
 }
 
@@ -4110,7 +4083,6 @@ impl<'help> fmt::Debug for Arg<'help> {
         #[allow(unused_mut)]
         let mut ds = ds
             .field("id", &self.id)
-            .field("name", &self.name)
             .field("help", &self.help)
             .field("long_help", &self.long_help)
             .field("action", &self.action)
@@ -4155,7 +4127,7 @@ pub(crate) fn render_arg_val(arg: &Arg) -> String {
 
     let arg_name_storage;
     let val_names = if arg.val_names.is_empty() {
-        arg_name_storage = [arg.get_id()];
+        arg_name_storage = [arg.get_id().as_str()];
         &arg_name_storage
     } else {
         arg.val_names.as_slice()
diff --git a/src/builder/arg_group.rs b/src/builder/arg_group.rs
index a376e131c4b..ffc88c2c1ca 100644
--- a/src/builder/arg_group.rs
+++ b/src/builder/arg_group.rs
@@ -75,9 +75,8 @@ use crate::util::Id;
 /// [conflict]: crate::Arg::conflicts_with()
 /// [requirement]: crate::Arg::requires()
 #[derive(Default, Clone, Debug, PartialEq, Eq)]
-pub struct ArgGroup<'help> {
+pub struct ArgGroup {
     pub(crate) id: Id,
-    name: &'help str,
     pub(crate) args: Vec<Id>,
     pub(crate) required: bool,
     pub(crate) requires: Vec<Id>,
@@ -86,14 +85,7 @@ pub struct ArgGroup<'help> {
 }
 
 /// # Builder
-impl<'help> ArgGroup<'help> {
-    pub(crate) fn with_id(id: Id) -> Self {
-        ArgGroup {
-            id,
-            ..ArgGroup::default()
-        }
-    }
-
+impl ArgGroup {
     /// Create a `ArgGroup` using a unique name.
     ///
     /// The name will be used to get values from the group or refer to the group inside of conflict
@@ -106,7 +98,7 @@ impl<'help> ArgGroup<'help> {
     /// ArgGroup::new("config")
     /// # ;
     /// ```
-    pub fn new(id: impl Into<&'help str>) -> Self {
+    pub fn new(id: impl Into<Id>) -> Self {
         ArgGroup::default().id(id)
     }
 
@@ -120,10 +112,8 @@ impl<'help> ArgGroup<'help> {
     /// # ;
     /// ```
     #[must_use]
-    pub fn id(mut self, id: impl Into<&'help str>) -> Self {
-        let id = id.into();
+    pub fn id(mut self, id: impl Into<Id>) -> Self {
         self.id = id.into();
-        self.name = id;
         self
     }
 
@@ -151,8 +141,8 @@ impl<'help> ArgGroup<'help> {
     /// ```
     /// [argument]: crate::Arg
     #[must_use]
-    pub fn arg(mut self, arg_id: impl Into<&'help str>) -> Self {
-        self.args.push(arg_id.into().into());
+    pub fn arg(mut self, arg_id: impl Into<Id>) -> Self {
+        self.args.push(arg_id.into());
         self
     }
 
@@ -179,7 +169,7 @@ impl<'help> ArgGroup<'help> {
     /// ```
     /// [arguments]: crate::Arg
     #[must_use]
-    pub fn args(mut self, ns: impl IntoIterator<Item = impl Into<&'help str>>) -> Self {
+    pub fn args(mut self, ns: impl IntoIterator<Item = impl Into<Id>>) -> Self {
         for n in ns {
             self = self.arg(n);
         }
@@ -317,8 +307,8 @@ impl<'help> ArgGroup<'help> {
     /// [required group]: ArgGroup::required()
     /// [argument requirement rules]: crate::Arg::requires()
     #[must_use]
-    pub fn requires(mut self, id: impl Into<&'help str>) -> Self {
-        self.requires.push(id.into().into());
+    pub fn requires(mut self, id: impl Into<Id>) -> Self {
+        self.requires.push(id.into());
         self
     }
 
@@ -360,7 +350,7 @@ impl<'help> ArgGroup<'help> {
     /// [required group]: ArgGroup::required()
     /// [argument requirement rules]: crate::Arg::requires_all()
     #[must_use]
-    pub fn requires_all(mut self, ns: impl IntoIterator<Item = impl Into<&'help str>>) -> Self {
+    pub fn requires_all(mut self, ns: impl IntoIterator<Item = impl Into<Id>>) -> Self {
         for n in ns {
             self = self.requires(n);
         }
@@ -400,8 +390,8 @@ impl<'help> ArgGroup<'help> {
     /// ```
     /// [argument exclusion rules]: crate::Arg::conflicts_with()
     #[must_use]
-    pub fn conflicts_with(mut self, id: impl Into<&'help str>) -> Self {
-        self.conflicts.push(id.into().into());
+    pub fn conflicts_with(mut self, id: impl Into<Id>) -> Self {
+        self.conflicts.push(id.into());
         self
     }
 
@@ -442,10 +432,7 @@ impl<'help> ArgGroup<'help> {
     ///
     /// [argument exclusion rules]: crate::Arg::conflicts_with_all()
     #[must_use]
-    pub fn conflicts_with_all(
-        mut self,
-        ns: impl IntoIterator<Item = impl Into<&'help str>>,
-    ) -> Self {
+    pub fn conflicts_with_all(mut self, ns: impl IntoIterator<Item = impl Into<Id>>) -> Self {
         for n in ns {
             self = self.conflicts_with(n);
         }
@@ -454,16 +441,16 @@ impl<'help> ArgGroup<'help> {
 }
 
 /// # Reflection
-impl<'help> ArgGroup<'help> {
+impl ArgGroup {
     /// Get the name of the group
     #[inline]
-    pub fn get_id(&self) -> &'help str {
-        self.name
+    pub fn get_id(&self) -> &Id {
+        &self.id
     }
 }
 
-impl<'help> From<&'_ ArgGroup<'help>> for ArgGroup<'help> {
-    fn from(g: &Self) -> Self {
+impl From<&'_ ArgGroup> for ArgGroup {
+    fn from(g: &ArgGroup) -> Self {
         g.clone()
     }
 }
diff --git a/src/builder/command.rs b/src/builder/command.rs
index 32f75762d01..393ce3dd6e3 100644
--- a/src/builder/command.rs
+++ b/src/builder/command.rs
@@ -93,7 +93,7 @@ pub struct Command<'help> {
     args: MKeyMap<'help>,
     subcommands: Vec<Command<'help>>,
     replacers: FlatMap<&'help str, &'help [&'help str]>,
-    groups: Vec<ArgGroup<'help>>,
+    groups: Vec<ArgGroup>,
     current_help_heading: Option<&'help str>,
     current_disp_ord: Option<usize>,
     subcommand_value_name: Option<&'help str>,
@@ -230,17 +230,15 @@ impl<'help> Command<'help> {
     /// assert!(res.is_ok());
     /// ```
     #[must_use]
-    pub fn mut_arg<F>(mut self, arg_id: impl Into<&'help str>, f: F) -> Self
+    pub fn mut_arg<F>(mut self, arg_id: impl Into<Id>, f: F) -> Self
     where
         F: FnOnce(Arg<'help>) -> Arg<'help>,
     {
-        let arg_id: &str = arg_id.into();
-        let id = Id::from(arg_id);
-
+        let id = arg_id.into();
         let a = self
             .args
-            .remove_by_name(&id)
-            .unwrap_or_else(|| Arg::new(arg_id));
+            .remove_by_name(id.as_str())
+            .unwrap_or_else(|| Arg::new(id));
 
         self.args.push(f(a));
         self
@@ -320,7 +318,7 @@ impl<'help> Command<'help> {
     /// ```
     #[inline]
     #[must_use]
-    pub fn group<G: Into<ArgGroup<'help>>>(mut self, group: G) -> Self {
+    pub fn group<G: Into<ArgGroup>>(mut self, group: G) -> Self {
         self.groups.push(group.into());
         self
     }
@@ -351,7 +349,7 @@ impl<'help> Command<'help> {
     pub fn groups<I, T>(mut self, groups: I) -> Self
     where
         I: IntoIterator<Item = T>,
-        T: Into<ArgGroup<'help>>,
+        T: Into<ArgGroup>,
     {
         for g in groups.into_iter() {
             self = self.group(g.into());
@@ -3383,7 +3381,7 @@ impl<'help> Command<'help> {
 
     /// Iterate through the set of groups.
     #[inline]
-    pub fn get_groups(&self) -> impl Iterator<Item = &ArgGroup<'help>> {
+    pub fn get_groups(&self) -> impl Iterator<Item = &ArgGroup> {
         self.groups.iter()
     }
 
@@ -3763,10 +3761,10 @@ impl<'help> Command<'help> {
                 // Fill in the groups
                 for g in &a.groups {
                     if let Some(ag) = self.groups.iter_mut().find(|grp| grp.id == *g) {
-                        ag.args.push(a.id.clone());
+                        ag.args.push(a.get_id().clone());
                     } else {
-                        let mut ag = ArgGroup::with_id(g.clone());
-                        ag.args.push(a.id.clone());
+                        let mut ag = ArgGroup::new(g);
+                        ag.args.push(a.get_id().clone());
                         self.groups.push(ag);
                     }
                 }
@@ -3992,11 +3990,11 @@ impl<'help> Command<'help> {
 
     pub(crate) fn _panic_on_missing_help(&self, help_required_globally: bool) {
         if self.is_set(AppSettings::HelpExpected) || help_required_globally {
-            let args_missing_help: Vec<String> = self
+            let args_missing_help: Vec<Id> = self
                 .args
                 .args()
                 .filter(|arg| arg.help.is_none() && arg.long_help.is_none())
-                .map(|arg| arg.get_id().to_owned())
+                .map(|arg| arg.get_id().clone())
                 .collect();
 
             assert!(args_missing_help.is_empty(),
@@ -4021,9 +4019,9 @@ impl<'help> Command<'help> {
 
     // just in case
     #[allow(unused)]
-    fn two_groups_of<F>(&self, condition: F) -> Option<(&ArgGroup<'help>, &ArgGroup<'help>)>
+    fn two_groups_of<F>(&self, condition: F) -> Option<(&ArgGroup, &ArgGroup)>
     where
-        F: Fn(&ArgGroup<'help>) -> bool,
+        F: Fn(&ArgGroup) -> bool,
     {
         two_elements_of(self.groups.iter().filter(|a| condition(a)))
     }
@@ -4088,7 +4086,7 @@ impl<'help> Command<'help> {
 
         if !self.is_disable_help_flag_set() {
             debug!("Command::_check_help_and_version: Building default --help");
-            let arg = Arg::new("help")
+            let arg = Arg::new(Id::HELP)
                 .short('h')
                 .long("help")
                 .action(ArgAction::Help)
@@ -4099,7 +4097,7 @@ impl<'help> Command<'help> {
         }
         if !self.is_disable_version_flag_set() {
             debug!("Command::_check_help_and_version: Building default --version");
-            let arg = Arg::new("version")
+            let arg = Arg::new(Id::VERSION)
                 .short('V')
                 .long("version")
                 .action(ArgAction::Version)
@@ -4273,7 +4271,7 @@ impl<'help> Command<'help> {
             .map(|grp| grp.id.clone())
     }
 
-    pub(crate) fn find_group(&self, group_id: &Id) -> Option<&ArgGroup<'help>> {
+    pub(crate) fn find_group(&self, group_id: &Id) -> Option<&ArgGroup> {
         self.groups.iter().find(|g| g.id == *group_id)
     }
 
diff --git a/src/builder/debug_asserts.rs b/src/builder/debug_asserts.rs
index 5c181db5c49..77b0dbb5993 100644
--- a/src/builder/debug_asserts.rs
+++ b/src/builder/debug_asserts.rs
@@ -67,20 +67,26 @@ pub(crate) fn assert_app(cmd: &Command) {
         );
 
         if let Some(s) = arg.short.as_ref() {
-            short_flags.push(Flag::Arg(format!("-{}", s), arg.get_id()));
+            short_flags.push(Flag::Arg(format!("-{}", s), arg.get_id().as_str()));
         }
 
         for (short_alias, _) in &arg.short_aliases {
-            short_flags.push(Flag::Arg(format!("-{}", short_alias), arg.get_id()));
+            short_flags.push(Flag::Arg(
+                format!("-{}", short_alias),
+                arg.get_id().as_str(),
+            ));
         }
 
         if let Some(l) = arg.long.as_ref() {
             assert!(!l.starts_with('-'), "Argument {}: long {:?} must not start with a `-`, that will be handled by the parser", arg.get_id(), l);
-            long_flags.push(Flag::Arg(format!("--{}", l), arg.get_id()));
+            long_flags.push(Flag::Arg(format!("--{}", l), arg.get_id().as_str()));
         }
 
         for (long_alias, _) in &arg.aliases {
-            long_flags.push(Flag::Arg(format!("--{}", long_alias), arg.get_id()));
+            long_flags.push(Flag::Arg(
+                format!("--{}", long_alias),
+                arg.get_id().as_str(),
+            ));
         }
 
         // Name conflicts
@@ -144,7 +150,7 @@ pub(crate) fn assert_app(cmd: &Command) {
         for req in &arg.requires {
             assert!(
                 cmd.id_exists(&req.1),
-                "Command {}: Argument or group '{:?}' specified in 'requires*' for '{}' does not exist",
+                "Command {}: Argument or group '{}' specified in 'requires*' for '{}' does not exist",
                 cmd.get_name(),
                 req.1,
                 arg.get_id(),
@@ -159,7 +165,7 @@ pub(crate) fn assert_app(cmd: &Command) {
             );
             assert!(
                 cmd.id_exists(&req.0),
-                "Command {}: Argument or group '{:?}' specified in 'required_if_eq*' for '{}' does not exist",
+                "Command {}: Argument or group '{}' specified in 'required_if_eq*' for '{}' does not exist",
                     cmd.get_name(),
                 req.0,
                 arg.get_id()
@@ -174,7 +180,7 @@ pub(crate) fn assert_app(cmd: &Command) {
             );
             assert!(
                 cmd.id_exists(&req.0),
-                "Command {}: Argument or group '{:?}' specified in 'required_if_eq_all' for '{}' does not exist",
+                "Command {}: Argument or group '{}' specified in 'required_if_eq_all' for '{}' does not exist",
                     cmd.get_name(),
                 req.0,
                 arg.get_id()
@@ -189,7 +195,7 @@ pub(crate) fn assert_app(cmd: &Command) {
             );
             assert!(
                 cmd.id_exists(req),
-                "Command {}: Argument or group '{:?}' specified in 'required_unless*' for '{}' does not exist",
+                "Command {}: Argument or group '{}' specified in 'required_unless*' for '{}' does not exist",
                     cmd.get_name(),
                 req,
                 arg.get_id(),
@@ -204,7 +210,7 @@ pub(crate) fn assert_app(cmd: &Command) {
             );
             assert!(
                 cmd.id_exists(req),
-                "Command {}: Argument or group '{:?}' specified in 'required_unless*' for '{}' does not exist",
+                "Command {}: Argument or group '{}' specified in 'required_unless*' for '{}' does not exist",
                     cmd.get_name(),
                 req,
                 arg.get_id(),
@@ -215,7 +221,7 @@ pub(crate) fn assert_app(cmd: &Command) {
         for req in &arg.blacklist {
             assert!(
                 cmd.id_exists(req),
-                "Command {}: Argument or group '{:?}' specified in 'conflicts_with*' for '{}' does not exist",
+                "Command {}: Argument or group '{}' specified in 'conflicts_with*' for '{}' does not exist",
                     cmd.get_name(),
                 req,
                 arg.get_id(),
@@ -226,7 +232,7 @@ pub(crate) fn assert_app(cmd: &Command) {
         for req in &arg.overrides {
             assert!(
                 cmd.id_exists(req),
-                "Command {}: Argument or group '{:?}' specified in 'overrides_with*' for '{}' does not exist",
+                "Command {}: Argument or group '{}' specified in 'overrides_with*' for '{}' does not exist",
                     cmd.get_name(),
                 req,
                 arg.get_id(),
@@ -283,7 +289,7 @@ pub(crate) fn assert_app(cmd: &Command) {
 
         // Groups should not have naming conflicts with Args
         assert!(
-            !cmd.get_arguments().any(|x| x.id == group.id),
+            !cmd.get_arguments().any(|x| x.get_id() == group.get_id()),
             "Command {}: Argument group name '{}' must not conflict with argument name",
             cmd.get_name(),
             group.get_id(),
@@ -292,8 +298,8 @@ pub(crate) fn assert_app(cmd: &Command) {
         for arg in &group.args {
             // Args listed inside groups should exist
             assert!(
-                cmd.get_arguments().any(|x| x.id == *arg),
-                "Command {}: Argument group '{}' contains non-existent argument '{:?}'",
+                cmd.get_arguments().any(|x| x.get_id() == arg),
+                "Command {}: Argument group '{}' contains non-existent argument '{}'",
                 cmd.get_name(),
                 group.get_id(),
                 arg
@@ -350,12 +356,10 @@ pub(crate) fn assert_app(cmd: &Command) {
 }
 
 fn duplicate_tip(cmd: &Command<'_>, first: &Arg<'_>, second: &Arg<'_>) -> &'static str {
-    if !cmd.is_disable_help_flag_set()
-        && (first.id == Id::help_hash() || second.id == Id::help_hash())
-    {
+    if !cmd.is_disable_help_flag_set() && (first.id == Id::HELP || second.id == Id::HELP) {
         " (call `cmd.disable_help_flag(true)` to remove the auto-generated `--help`)"
     } else if !cmd.is_disable_version_flag_set()
-        && (first.id == Id::version_hash() || second.id == Id::version_hash())
+        && (first.id == Id::VERSION || second.id == Id::VERSION)
     {
         " (call `cmd.disable_version_flag(true)` to remove the auto-generated `--version`)"
     } else {
@@ -703,9 +707,9 @@ fn assert_arg(arg: &Arg) {
             arg.is_takes_value_set(),
             "Argument '{}` is positional, it must take a value{}",
             arg.get_id(),
-            if arg.id == Id::help_hash() {
+            if arg.id == Id::HELP {
                 " (`mut_arg` no longer works with implicit `--help`)"
-            } else if arg.id == Id::version_hash() {
+            } else if arg.id == Id::VERSION {
                 " (`mut_arg` no longer works with implicit `--version`)"
             } else {
                 ""
diff --git a/src/mkeymap.rs b/src/mkeymap.rs
index 56f22b6977f..28eb268bc61 100644
--- a/src/mkeymap.rs
+++ b/src/mkeymap.rs
@@ -2,7 +2,6 @@ use std::iter::Iterator;
 use std::ops::Index;
 use std::{ffi::OsStr, ffi::OsString};
 
-use crate::util::Id;
 use crate::Arg;
 use crate::INTERNAL_ERROR_MSG;
 
@@ -144,10 +143,10 @@ impl<'help> MKeyMap<'help> {
 
     /// Remove an arg in the graph by Id, usually used by `mut_arg`. Return
     /// `Some(arg)` if removed.
-    pub(crate) fn remove_by_name(&mut self, name: &Id) -> Option<Arg<'help>> {
+    pub(crate) fn remove_by_name(&mut self, name: &str) -> Option<Arg<'help>> {
         self.args
             .iter()
-            .position(|arg| &arg.id == name)
+            .position(|arg| arg.id == name)
             // since it's a cold function, using this wouldn't hurt much
             .map(|i| self.args.remove(i))
     }
diff --git a/src/output/help.rs b/src/output/help.rs
index b6a867bfce9..26620a05e86 100644
--- a/src/output/help.rs
+++ b/src/output/help.rs
@@ -217,7 +217,7 @@ impl<'help, 'cmd, 'writer> Help<'help, 'cmd, 'writer> {
                 x.to_string()
             } else {
                 let mut s = '{'.to_string();
-                s.push_str(arg.get_id());
+                s.push_str(arg.id.as_str());
                 s
             };
             ord_v.push((arg.get_display_order(), key, arg));
diff --git a/src/parser/arg_matcher.rs b/src/parser/arg_matcher.rs
index c872f45fb96..ed03e35d76f 100644
--- a/src/parser/arg_matcher.rs
+++ b/src/parser/arg_matcher.rs
@@ -170,9 +170,11 @@ impl ArgMatcher {
     }
 
     pub(crate) fn start_occurrence_of_external(&mut self, cmd: &crate::Command) {
-        let id = Id::empty_hash();
+        let id = Id::EXTERNAL;
         debug!("ArgMatcher::start_occurrence_of_external: id={:?}", id,);
-        let ma = self.entry(id).or_insert(MatchedArg::new_external(cmd));
+        let ma = self
+            .entry(id.into())
+            .or_insert(MatchedArg::new_external(cmd));
         debug_assert_eq!(
             ma.type_id(),
             Some(
diff --git a/src/parser/error.rs b/src/parser/error.rs
index caeba4b8fee..fb143f88df6 100644
--- a/src/parser/error.rs
+++ b/src/parser/error.rs
@@ -1,5 +1,3 @@
-use crate::util::Id;
-
 /// Violation of [`ArgMatches`][crate::ArgMatches] assumptions
 #[derive(Clone, Debug)]
 #[allow(missing_copy_implementations)] // We might add non-Copy types in the future
@@ -22,7 +20,7 @@ pub enum MatchesError {
 
 impl MatchesError {
     #[track_caller]
-    pub(crate) fn unwrap<T>(id: &Id, r: Result<T, MatchesError>) -> T {
+    pub(crate) fn unwrap<T>(id: &str, r: Result<T, MatchesError>) -> T {
         let err = match r {
             Ok(t) => {
                 return t;
@@ -30,7 +28,7 @@ impl MatchesError {
             Err(err) => err,
         };
         panic!(
-            "Mismatch between definition and access of `{:?}`. {}",
+            "Mismatch between definition and access of `{}`. {}",
             id, err
         )
     }
diff --git a/src/parser/matches/arg_matches.rs b/src/parser/matches/arg_matches.rs
index 5655c26eb0f..7b77a54500e 100644
--- a/src/parser/matches/arg_matches.rs
+++ b/src/parser/matches/arg_matches.rs
@@ -12,7 +12,7 @@ use crate::parser::MatchedArg;
 use crate::parser::MatchesError;
 use crate::parser::ValueSource;
 use crate::util::FlatMap;
-use crate::util::{Id, Key};
+use crate::util::Id;
 use crate::INTERNAL_ERROR_MSG;
 
 /// Container for parse results.
@@ -108,8 +108,7 @@ impl ArgMatches {
     /// [`default_value`]: crate::Arg::default_value()
     #[track_caller]
     pub fn get_one<T: Any + Clone + Send + Sync + 'static>(&self, id: &str) -> Option<&T> {
-        let internal_id = Id::from(id);
-        MatchesError::unwrap(&internal_id, self.try_get_one(id))
+        MatchesError::unwrap(id, self.try_get_one(id))
     }
 
     /// Iterate over values of a specific option or positional argument.
@@ -149,8 +148,7 @@ impl ArgMatches {
         &self,
         id: &str,
     ) -> Option<ValuesRef<T>> {
-        let internal_id = Id::from(id);
-        MatchesError::unwrap(&internal_id, self.try_get_many(id))
+        MatchesError::unwrap(id, self.try_get_many(id))
     }
 
     /// Iterate over the original argument values.
@@ -196,8 +194,7 @@ impl ArgMatches {
     /// [`String`]: std::string::String
     #[track_caller]
     pub fn get_raw(&self, id: &str) -> Option<RawValues<'_>> {
-        let internal_id = Id::from(id);
-        MatchesError::unwrap(&internal_id, self.try_get_raw(id))
+        MatchesError::unwrap(id, self.try_get_raw(id))
     }
 
     /// Returns the value of a specific option or positional argument.
@@ -235,8 +232,7 @@ impl ArgMatches {
     /// [`default_value`]: crate::Arg::default_value()
     #[track_caller]
     pub fn remove_one<T: Any + Clone + Send + Sync + 'static>(&mut self, id: &str) -> Option<T> {
-        let internal_id = Id::from(id);
-        MatchesError::unwrap(&internal_id, self.try_remove_one(id))
+        MatchesError::unwrap(id, self.try_remove_one(id))
     }
 
     /// Return values of a specific option or positional argument.
@@ -274,8 +270,7 @@ impl ArgMatches {
         &mut self,
         id: &str,
     ) -> Option<Values2<T>> {
-        let internal_id = Id::from(id);
-        MatchesError::unwrap(&internal_id, self.try_remove_many(id))
+        MatchesError::unwrap(id, self.try_remove_many(id))
     }
 
     /// Check if values are present for the argument or group id
@@ -305,8 +300,7 @@ impl ArgMatches {
     ///
     /// [`default_value`]: crate::Arg::default_value()
     pub fn contains_id(&self, id: &str) -> bool {
-        let internal_id = Id::from(id);
-        MatchesError::unwrap(&internal_id, self.try_contains_id(id))
+        MatchesError::unwrap(id, self.try_contains_id(id))
     }
 
     /// Check if any args were present on the command line
@@ -367,9 +361,8 @@ impl ArgMatches {
     /// [`Iterator`]: std::iter::Iterator
     #[cfg(feature = "unstable-grouped")]
     #[cfg_attr(debug_assertions, track_caller)]
-    pub fn grouped_values_of<T: Key>(&self, id: T) -> Option<GroupedValues> {
-        let id = Id::from(id);
-        let arg = self.get_arg(&id)?;
+    pub fn grouped_values_of(&self, id: &str) -> Option<GroupedValues> {
+        let arg = self.get_arg(id)?;
         let v = GroupedValues {
             iter: arg.vals().map(|g| g.iter().map(unwrap_string).collect()),
             len: arg.vals().len(),
@@ -401,10 +394,8 @@ impl ArgMatches {
     ///
     /// [`default_value`]: crate::Arg::default_value()
     #[cfg_attr(debug_assertions, track_caller)]
-    pub fn value_source<T: Key>(&self, id: T) -> Option<ValueSource> {
-        let id = Id::from(id);
-
-        let value = self.get_arg(&id);
+    pub fn value_source(&self, id: &str) -> Option<ValueSource> {
+        let value = self.get_arg(id);
 
         value.and_then(MatchedArg::source)
     }
@@ -552,8 +543,8 @@ impl ArgMatches {
     /// ```
     /// [delimiter]: crate::Arg::value_delimiter()
     #[cfg_attr(debug_assertions, track_caller)]
-    pub fn index_of<T: Key>(&self, id: T) -> Option<usize> {
-        let arg = self.get_arg(&Id::from(id))?;
+    pub fn index_of(&self, id: &str) -> Option<usize> {
+        let arg = self.get_arg(id)?;
         let i = arg.get_index(0)?;
         Some(i)
     }
@@ -634,8 +625,8 @@ impl ArgMatches {
     /// [`ArgMatches::index_of`]: ArgMatches::index_of()
     /// [delimiter]: Arg::value_delimiter()
     #[cfg_attr(debug_assertions, track_caller)]
-    pub fn indices_of<T: Key>(&self, id: T) -> Option<Indices<'_>> {
-        let arg = self.get_arg(&Id::from(id))?;
+    pub fn indices_of(&self, id: &str) -> Option<Indices<'_>> {
+        let arg = self.get_arg(id)?;
         let i = Indices {
             iter: arg.indices(),
             len: arg.num_vals(),
@@ -843,7 +834,7 @@ impl ArgMatches {
         #[cfg(debug_assertions)]
         {
             let _name = _name.to_owned();
-            _name.is_empty() || self.valid_subcommands.contains(&_name)
+            _name == String::default() || self.valid_subcommands.contains(&_name)
         }
         #[cfg(not(debug_assertions))]
         {
@@ -859,8 +850,7 @@ impl ArgMatches {
         &self,
         id: &str,
     ) -> Result<Option<&T>, MatchesError> {
-        let id = Id::from(id);
-        let arg = self.try_get_arg_t::<T>(&id)?;
+        let arg = self.try_get_arg_t::<T>(id)?;
         let value = match arg.and_then(|a| a.first()) {
             Some(value) => value,
             None => {
@@ -878,8 +868,7 @@ impl ArgMatches {
         &self,
         id: &str,
     ) -> Result<Option<ValuesRef<T>>, MatchesError> {
-        let id = Id::from(id);
-        let arg = match self.try_get_arg_t::<T>(&id)? {
+        let arg = match self.try_get_arg_t::<T>(id)? {
             Some(arg) => arg,
             None => return Ok(None),
         };
@@ -895,8 +884,7 @@ impl ArgMatches {
 
     /// Non-panicking version of [`ArgMatches::get_raw`]
     pub fn try_get_raw(&self, id: &str) -> Result<Option<RawValues<'_>>, MatchesError> {
-        let id = Id::from(id);
-        let arg = match self.try_get_arg(&id)? {
+        let arg = match self.try_get_arg(id)? {
             Some(arg) => arg,
             None => return Ok(None),
         };
@@ -914,8 +902,7 @@ impl ArgMatches {
         &mut self,
         id: &str,
     ) -> Result<Option<T>, MatchesError> {
-        let id = Id::from(id);
-        match self.try_remove_arg_t::<T>(&id)? {
+        match self.try_remove_arg_t::<T>(id)? {
             Some(values) => Ok(values
                 .into_vals_flatten()
                 // enforced by `try_get_arg_t`
@@ -930,8 +917,7 @@ impl ArgMatches {
         &mut self,
         id: &str,
     ) -> Result<Option<Values2<T>>, MatchesError> {
-        let id = Id::from(id);
-        let arg = match self.try_remove_arg_t::<T>(&id)? {
+        let arg = match self.try_remove_arg_t::<T>(id)? {
             Some(arg) => arg,
             None => return Ok(None),
         };
@@ -947,11 +933,9 @@ impl ArgMatches {
 
     /// Non-panicking version of [`ArgMatches::contains_id`]
     pub fn try_contains_id(&self, id: &str) -> Result<bool, MatchesError> {
-        let id = Id::from(id);
-
-        self.verify_arg(&id)?;
+        self.verify_arg(id)?;
 
-        let presence = self.args.contains_key(&id);
+        let presence = self.args.contains_key(id);
         Ok(presence)
     }
 }
@@ -959,7 +943,7 @@ impl ArgMatches {
 // Private methods
 impl ArgMatches {
     #[inline]
-    fn try_get_arg(&self, arg: &Id) -> Result<Option<&MatchedArg>, MatchesError> {
+    fn try_get_arg(&self, arg: &str) -> Result<Option<&MatchedArg>, MatchesError> {
         self.verify_arg(arg)?;
         Ok(self.args.get(arg))
     }
@@ -967,7 +951,7 @@ impl ArgMatches {
     #[inline]
     fn try_get_arg_t<T: Any + Send + Sync + 'static>(
         &self,
-        arg: &Id,
+        arg: &str,
     ) -> Result<Option<&MatchedArg>, MatchesError> {
         let arg = match self.try_get_arg(arg)? {
             Some(arg) => arg,
@@ -982,7 +966,7 @@ impl ArgMatches {
     #[inline]
     fn try_remove_arg_t<T: Any + Send + Sync + 'static>(
         &mut self,
-        arg: &Id,
+        arg: &str,
     ) -> Result<Option<MatchedArg>, MatchesError> {
         self.verify_arg(arg)?;
         let matched = match self.args.remove(arg) {
@@ -997,7 +981,7 @@ impl ArgMatches {
         if actual == expected {
             Ok(Some(matched))
         } else {
-            self.args.insert(arg.clone(), matched);
+            self.args.insert(Id::from(arg.to_owned()), matched);
             Err(MatchesError::Downcast { actual, expected })
         }
     }
@@ -1016,10 +1000,11 @@ impl ArgMatches {
     }
 
     #[inline]
-    fn verify_arg(&self, _arg: &Id) -> Result<(), MatchesError> {
+    fn verify_arg(&self, _arg: &str) -> Result<(), MatchesError> {
         #[cfg(debug_assertions)]
         {
-            if *_arg == Id::empty_hash() || self.valid_args.contains(_arg) {
+            let _arg = Id::from(_arg.to_owned());
+            if _arg == Id::EXTERNAL || self.valid_args.contains(&_arg) {
             } else {
                 debug!(
                     "`{:?}` is not an id of an argument or a group.\n\
@@ -1035,10 +1020,11 @@ impl ArgMatches {
 
     #[inline]
     #[cfg_attr(debug_assertions, track_caller)]
-    fn get_arg(&self, arg: &Id) -> Option<&MatchedArg> {
+    fn get_arg<'s>(&'s self, arg: &str) -> Option<&'s MatchedArg> {
         #[cfg(debug_assertions)]
         {
-            if *arg == Id::empty_hash() || self.valid_args.contains(arg) {
+            let arg = Id::from(arg.to_owned());
+            if arg == Id::EXTERNAL || self.valid_args.contains(&arg) {
             } else {
                 panic!(
                     "`{:?}` is not an id of an argument or a group.\n\
@@ -1058,7 +1044,7 @@ impl ArgMatches {
         #[cfg(debug_assertions)]
         {
             let name = name.to_owned();
-            if name.is_empty() || self.valid_subcommands.contains(&name) {
+            if name == String::default() || self.valid_subcommands.contains(&name) {
             } else {
                 panic!("`{}` is not a name of a subcommand.", name);
             }
diff --git a/src/parser/parser.rs b/src/parser/parser.rs
index 9ed587ee983..27c20fdcbc4 100644
--- a/src/parser/parser.rs
+++ b/src/parser/parser.rs
@@ -433,8 +433,8 @@ impl<'help, 'cmd> Parser<'help, 'cmd> {
 
                 for raw_val in raw_args.remaining(&mut args_cursor) {
                     let val = external_parser.parse_ref(self.cmd, None, raw_val)?;
-                    let external_id = &Id::empty_hash();
-                    sc_m.add_val_to(external_id, val, raw_val.to_os_string());
+                    let external_id = Id::from(Id::EXTERNAL);
+                    sc_m.add_val_to(&external_id, val, raw_val.to_os_string());
                 }
 
                 matcher.subcommand(SubCommand {
@@ -1208,7 +1208,7 @@ impl<'help, 'cmd> Parser<'help, 'cmd> {
             ArgAction::Count => {
                 let raw_vals = if raw_vals.is_empty() {
                     let existing_value = *matcher
-                        .get_one::<crate::builder::CountType>(arg.get_id())
+                        .get_one::<crate::builder::CountType>(arg.get_id().as_str())
                         .unwrap_or(&0);
                     let next_value = existing_value.saturating_add(1);
                     vec![OsString::from(next_value.to_string())]
diff --git a/src/util/fnv.rs b/src/util/fnv.rs
deleted file mode 100644
index 4602300a498..00000000000
--- a/src/util/fnv.rs
+++ /dev/null
@@ -1,46 +0,0 @@
-use std::{
-    fmt::Display,
-    hash::{Hash, Hasher},
-};
-
-const MAGIC_INIT: u64 = 0x811C_9DC5;
-
-// TODO: Docs
-pub trait Key: Hash + Display {
-    fn key(&self) -> u64;
-}
-
-impl<T> Key for T
-where
-    T: Hash + Display,
-{
-    fn key(&self) -> u64 {
-        let mut hasher = FnvHasher::new();
-        self.hash(&mut hasher);
-        hasher.finish()
-    }
-}
-
-pub(crate) struct FnvHasher(u64);
-
-impl FnvHasher {
-    pub(crate) fn new() -> Self {
-        FnvHasher(MAGIC_INIT)
-    }
-}
-
-impl Hasher for FnvHasher {
-    fn finish(&self) -> u64 {
-        self.0
-    }
-    fn write(&mut self, bytes: &[u8]) {
-        let FnvHasher(mut hash) = *self;
-
-        for byte in bytes.iter() {
-            hash ^= u64::from(*byte);
-            hash = hash.wrapping_mul(0x0100_0000_01b3);
-        }
-
-        *self = FnvHasher(hash);
-    }
-}
diff --git a/src/util/id.rs b/src/util/id.rs
index f481dd12a17..193bce0325f 100644
--- a/src/util/id.rs
+++ b/src/util/id.rs
@@ -1,83 +1,88 @@
-use crate::util::fnv::Key;
+#[derive(Default, Clone, Eq, PartialEq, PartialOrd, Ord, Hash)]
+#[repr(transparent)]
+pub struct Id {
+    name: String,
+}
 
-use std::{
-    fmt::{Debug, Formatter, Result},
-    hash::{Hash, Hasher},
-    ops::Deref,
-};
+impl Id {
+    pub(crate) const HELP: &'static str = "help";
+    pub(crate) const VERSION: &'static str = "version";
+    pub(crate) const EXTERNAL: &'static str = "";
 
-#[derive(Clone, Eq, Default)]
-#[cfg_attr(not(debug_assertions), repr(transparent))]
-pub(crate) struct Id {
-    #[cfg(debug_assertions)]
-    name: String,
-    id: u64,
+    pub fn as_str(&self) -> &str {
+        self.name.as_str()
+    }
+}
+
+impl<'s> From<&'s Id> for Id {
+    fn from(id: &'s Id) -> Self {
+        id.clone()
+    }
+}
+
+impl From<String> for Id {
+    fn from(name: String) -> Self {
+        Self { name }
+    }
 }
 
-macro_rules! precomputed_hashes {
-    ($($fn_name:ident, $const:expr, $name:expr;)*) => {
-        impl Id {
-            $(
-                #[allow(dead_code)]
-                pub(crate) fn $fn_name() -> Self {
-                    Id {
-                        #[cfg(debug_assertions)]
-                        name: $name.into(),
-                        id: $const,
-                    }
-                }
-            )*
-        }
-    };
+impl<'s> From<&'s String> for Id {
+    fn from(name: &'s String) -> Self {
+        name.to_owned().into()
+    }
 }
 
-// precompute some common values
-precomputed_hashes! {
-    empty_hash,   0x1C9D_3ADB_639F_298E, "";
-    help_hash,    0x5963_6393_CFFB_FE5F, "help";
-    version_hash, 0x30FF_0B7C_4D07_9478, "version";
+impl From<&'static str> for Id {
+    fn from(name: &'static str) -> Self {
+        name.to_owned().into()
+    }
 }
 
-impl Debug for Id {
-    fn fmt(&self, f: &mut Formatter) -> Result {
-        #[cfg(debug_assertions)]
-        write!(f, "{}", self.name)?;
-        #[cfg(not(debug_assertions))]
-        write!(f, "[hash: {:X}]", self.id)?;
+impl std::fmt::Display for Id {
+    #[inline]
+    fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
+        std::fmt::Display::fmt(self.as_str(), f)
+    }
+}
 
-        Ok(())
+impl std::fmt::Debug for Id {
+    #[inline]
+    fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
+        std::fmt::Debug::fmt(self.as_str(), f)
     }
 }
 
-impl Deref for Id {
-    type Target = u64;
+impl AsRef<str> for Id {
+    #[inline]
+    fn as_ref(&self) -> &str {
+        self.as_str()
+    }
+}
 
-    fn deref(&self) -> &Self::Target {
-        &self.id
+impl std::borrow::Borrow<str> for Id {
+    #[inline]
+    fn borrow(&self) -> &str {
+        self.as_str()
     }
 }
 
-impl<T: Key> From<T> for Id {
-    fn from(val: T) -> Self {
-        Id {
-            #[cfg(debug_assertions)]
-            name: val.to_string(),
-            id: val.key(),
-        }
+impl PartialEq<str> for Id {
+    #[inline]
+    fn eq(&self, other: &str) -> bool {
+        PartialEq::eq(self.as_str(), other)
     }
 }
 
-impl Hash for Id {
-    fn hash<H>(&self, state: &mut H)
-    where
-        H: Hasher,
-    {
-        self.id.hash(state)
+impl<'s> PartialEq<&'s str> for Id {
+    #[inline]
+    fn eq(&self, other: &&str) -> bool {
+        PartialEq::eq(self.as_str(), *other)
     }
 }
 
-impl PartialEq for Id {
-    fn eq(&self, other: &Id) -> bool {
-        self.id == other.id
+impl PartialEq<String> for Id {
+    #[inline]
+    fn eq(&self, other: &String) -> bool {
+        PartialEq::eq(self.as_str(), other.as_str())
     }
 }
diff --git a/src/util/mod.rs b/src/util/mod.rs
index 73b7ad81a97..d80df9656d5 100644
--- a/src/util/mod.rs
+++ b/src/util/mod.rs
@@ -2,13 +2,10 @@
 
 mod flat_map;
 mod flat_set;
-mod fnv;
 mod graph;
 mod id;
 mod str_to_bool;
 
-pub use self::fnv::Key;
-
 pub(crate) use self::flat_map::Entry;
 pub(crate) use self::flat_map::FlatMap;
 pub(crate) use self::flat_set::FlatSet;
diff --git a/tests/builder/groups.rs b/tests/builder/groups.rs
index edd43063064..4ff054b8e44 100644
--- a/tests/builder/groups.rs
+++ b/tests/builder/groups.rs
@@ -43,7 +43,7 @@ fn required_group_missing_arg() {
 
 #[cfg(debug_assertions)]
 #[test]
-#[should_panic = "Argument group 'req' contains non-existent argument"]
+#[should_panic = "Command group: Argument group 'req' contains non-existent argument"]
 fn non_existing_arg() {
     let _ = Command::new("group")
         .arg(arg!(-f --flag "some flag"))
@@ -54,7 +54,7 @@ fn non_existing_arg() {
 
 #[cfg(debug_assertions)]
 #[test]
-#[should_panic = "Argument group name must be unique\n\n\t'req' is already in use"]
+#[should_panic = "Command group: Argument group name must be unique\n\n\t'req' is already in use"]
 fn unique_group_name() {
     let _ = Command::new("group")
         .arg(arg!(-f --flag "some flag"))
@@ -66,7 +66,7 @@ fn unique_group_name() {
 
 #[cfg(debug_assertions)]
 #[test]
-#[should_panic = "Command group: Argument group name '' must not conflict with argument name"]
+#[should_panic = "Command group: Argument group name 'a' must not conflict with argument name"]
 fn groups_new_of_arg_name() {
     let _ = Command::new("group")
         .arg(Arg::new("a").long("a").group("a"))
diff --git a/tests/builder/help.rs b/tests/builder/help.rs
index ba07139563b..f84a47839c8 100644
--- a/tests/builder/help.rs
+++ b/tests/builder/help.rs
@@ -2586,7 +2586,7 @@ fn disable_help_flag_affects_help_subcommand() {
         .find_subcommand("help")
         .unwrap()
         .get_arguments()
-        .map(|a| a.get_id())
+        .map(|a| a.get_id().as_str())
         .collect::<Vec<_>>();
     assert!(
         !args.contains(&"help"),
diff --git a/tests/builder/tests.rs b/tests/builder/tests.rs
index b93a4d601cb..ea976c3ee8f 100644
--- a/tests/builder/tests.rs
+++ b/tests/builder/tests.rs
@@ -418,7 +418,7 @@ fn mut_arg_all() {
     let arg_names = cmd
         .get_arguments()
         .map(|a| a.get_id().clone())
-        .filter(|a| *a != "version" && *a != "help")
+        .filter(|a| a != "version" && a != "help")
         .collect::<Vec<_>>();
 
     for arg_name in arg_names {