Re: [PATCH 15/16] gdb: make cmd_list_element var an optional union

[Lancelot SIX <lsix@lancelotsix.com>]

On Wed, Jul 14, 2021 at 03:22:41PM -0400, Simon Marchi wrote:
> 
> I am certainly for this kind of consistency checks, making sure we
> access the right data the right way.  This is what I did with struct
> dynamic_prop, for example.
> 
> I was thinking of adding accessors too, eventually.  I am not very good
> with advanced C++ templates though, so what I had in mind was more
> something like the obvious:
> 
>   struct cmd_list_element
>   {
>     int *var_as_int () // or int &
>     {
>       gdb_assert (m_var_type == a
> 		  || m_var_type == b);
>       return m_var.int_var;
>     }
> 
>     unsigned int *var_as_unsigned_int () // or unsigned int &
>     {
>       gdb_assert (m_var_type == c
> 		  || m_var_type == c);
>       return m_var.unsigned_int_var;
>     }
> 
>     // And so on.
>   };
> 
> Your last proposition is a bit better in the sense that you ask "give me
> the variable location for a var_zuinteger".  With mine, you still have
> to know that a var_zuinteger uses the unsigned int member.  But at
> least, there's a gdb_assert to catch mistakes at runtime.
> 
> Whether it's worth the extra complexity... I can't tell, I would have to
> see the complete patch.  I would also have to see how that fits with the
> upcoming fix I have for bug 28085.  In the mean time, does it make your
> life more difficult if we merge my patch?  I would guess not, since
> you'll need to modify all the call sites (places that access setting
> variables) anyway.  You can then decide to keep the union or not in your
> patch.
> 
> Simon

I have not yet finished updating all callsites, but here is a
(hopefully) working patch if you want to see what it looks like.

I have not had a look at 28085 yet, so I do not yet if my approach gets
in the way of fixing it.  For the moment, I do not have something
similar to the gdb_optional you introduce, but it should not be too
difficult to have something similar.

Anyway, to not hold back on my account.  I have not read your series
carefully yet, but it seems a real improvement from the current
situation.  If it helps fixing a bug, it is even better.  I can rework
my patch to work on top of this one (at least if what it brings is
considered worthwhile).

Lancelot.

---
From 5b578bb0ed51bf9a7c03cc9b390ec636a49d68d5 Mon Sep 17 00:00:00 2001
From: Lancelot SIX <lsix@lancelotsix.com>
Date: Wed, 14 Jul 2021 22:30:14 +0000
Subject: [PATCH] [WIP] Add typesafe getter/setter for cmd_list_element.var

cmd_list_element can contain a pointer to data that can be set and / or
shown.  This is achieved with a void* that points to the data that can
be accessed, while the var_type (of type enum var_types) tells how to
interpret the data pointed to.

With this pattern, the user needs to know what the storage type
associated with a given VAR_TYPES tag, and needs to do the proper
casting.

This looks something like:

if (c->var_type == var_uinteger)
  unsigned_int v = *(unsigned int*)v->var_type;

With this approach, it is easy to make an error.

This patch is a proof of concept for an alternative approach.

We add templated get_var / set_var member functions to the
cmd_list_element.  The template parameter is the VAR_TYPES we want to
the access the data for.  The template is written so the return type of
the get_var / parameter type of the set_var matches the data type used
to store the actual data.

For example, instantiating the member functions with var_boolean will
be something similar to:

	bool get_var<var_boolean> () const
	{
	  gdb_assert (this->var_type == var_boolean);
	  return *static_cast<bool *> (this->var);
	}
	void set_var<var_boolean> (bool var)
	{
	  gdb_assert (this->var_type == var_boolean);
	  *static_cast<bool *> (this->var) = var;
	}

With those new methods, the caller does not need to know the underlying
storage nor he does need to perform the cast manually.

Given that the processing of some options is common between VAR_TYPES,
the templates can be instantiated with more than one VAR_TYPES.  In such
situation, all the template parameters need to share the same underlying
type.

Here is another example of what an instantiation looks like with 2
parameters:

	int get_var<var_integer, var_zinteger> ()
	{
	  gdb_assert (this->var_type == var_integer
	              || this->var_type == var_zinteger);
	  return *static_cast<int *> (this->var);
	}
	void set_var<var_integer, var_zinteger> (int v)
	{
	  gdb_assert (this->var_type == var_integer
	              || this->var_type == var_zinteger);
	  *static_cast<int *> (this->var) = v;
	}

For convenience reasons, the getter is written in such a way that it
accepts template parameters that have different underlying types, as long
as they are compatible (i.e. can all be implicitly converted to).

	char *get_var<var_string> ()
	{
	  gdb_assert (this->var_type == var_string);
	  return *static_cast<char **> (this->var);
	}
	const char *get_var<var_string, var_filename, var_enum> ()
	{
	  gdb_assert (this->var_type == var_string
	              || this->var_type == var_filename
	              || this->var_type == var_enum);
	  return *static_cast<const char **> (this->var);
	}

Eventually the 'var' and 'var_type' members of cmd_list_element will
have to be made private members to make sure all access is done through
the getter / setter, but since this is a POC, only a few callsites have
been updated.
---
 gdb/cli/cli-decode.c  | 114 ++++++++++++++++----------------
 gdb/cli/cli-decode.h  |  49 ++++++++++++++
 gdb/cli/cli-setshow.c |  91 ++++++++++++++------------
 gdb/command.h         | 147 ++++++++++++++++++++++++++++++++++++++++++
 4 files changed, 305 insertions(+), 96 deletions(-)

diff --git a/gdb/cli/cli-decode.c b/gdb/cli/cli-decode.c
index 633a3ad9309..35ab7c4d015 100644
--- a/gdb/cli/cli-decode.c
+++ b/gdb/cli/cli-decode.c
@@ -481,12 +481,12 @@ empty_sfunc (const char *args, int from_tty, struct cmd_list_element *c)
    VAR is address of the variable being controlled by this command.
    DOC is the documentation string.  */
 
+template<var_types T>
 static struct cmd_list_element *
 add_set_or_show_cmd (const char *name,
 		     enum cmd_types type,
 		     enum command_class theclass,
-		     var_types var_type,
-		     void *var,
+		     typename var_types_storage<T>::type *var,
 		     const char *doc,
 		     struct cmd_list_element **list)
 {
@@ -494,7 +494,7 @@ add_set_or_show_cmd (const char *name,
 
   gdb_assert (type == set_cmd || type == show_cmd);
   c->type = type;
-  c->var_type = var_type;
+  c->var_type = T;
   c->var = var;
   /* This needs to be something besides NULL so that this isn't
      treated as a help class.  */
@@ -511,10 +511,11 @@ add_set_or_show_cmd (const char *name,
 
    Return the newly created set and show commands.  */
 
+template<var_types T>
 static set_show_commands
 add_setshow_cmd_full (const char *name,
 		      enum command_class theclass,
-		      var_types var_type, void *var,
+		      typename var_types_storage<T>::type *var,
 		      const char *set_doc, const char *show_doc,
 		      const char *help_doc,
 		      cmd_const_sfunc_ftype *set_func,
@@ -537,15 +538,15 @@ add_setshow_cmd_full (const char *name,
       full_set_doc = xstrdup (set_doc);
       full_show_doc = xstrdup (show_doc);
     }
-  set = add_set_or_show_cmd (name, set_cmd, theclass, var_type, var,
-			     full_set_doc, set_list);
+  set = add_set_or_show_cmd<T> (name, set_cmd, theclass, var,
+				full_set_doc, set_list);
   set->doc_allocated = 1;
 
   if (set_func != NULL)
     set_cmd_sfunc (set, set_func);
 
-  show = add_set_or_show_cmd (name, show_cmd, theclass, var_type, var,
-			      full_show_doc, show_list);
+  show = add_set_or_show_cmd<T> (name, show_cmd, theclass, var,
+				 full_show_doc, show_list);
   show->doc_allocated = 1;
   show->show_value_func = show_func;
   /* Disable the default symbol completer.  Doesn't make much sense
@@ -574,10 +575,10 @@ add_setshow_enum_cmd (const char *name,
 		      struct cmd_list_element **show_list)
 {
   set_show_commands commands
-    =  add_setshow_cmd_full (name, theclass, var_enum, var,
-			     set_doc, show_doc, help_doc,
-			     set_func, show_func,
-			     set_list, show_list);
+    =  add_setshow_cmd_full<var_enum> (name, theclass, var,
+				       set_doc, show_doc, help_doc,
+				       set_func, show_func,
+				       set_list, show_list);
   commands.set->enums = enumlist;
   return commands;
 }
@@ -602,10 +603,10 @@ add_setshow_auto_boolean_cmd (const char *name,
 			      struct cmd_list_element **show_list)
 {
   set_show_commands commands
-    = add_setshow_cmd_full (name, theclass, var_auto_boolean, var,
-			    set_doc, show_doc, help_doc,
-			    set_func, show_func,
-			    set_list, show_list);
+    = add_setshow_cmd_full<var_auto_boolean> (name, theclass, var,
+					      set_doc, show_doc, help_doc,
+					      set_func, show_func,
+					      set_list, show_list);
 
   commands.set->enums = auto_boolean_enums;
 
@@ -631,10 +632,10 @@ add_setshow_boolean_cmd (const char *name, enum command_class theclass, bool *va
 			 struct cmd_list_element **show_list)
 {
   set_show_commands commands
-    = add_setshow_cmd_full (name, theclass, var_boolean, var,
-			    set_doc, show_doc, help_doc,
-			    set_func, show_func,
-			    set_list, show_list);
+    = add_setshow_cmd_full<var_boolean> (name, theclass, var,
+					 set_doc, show_doc, help_doc,
+					 set_func, show_func,
+					 set_list, show_list);
 
   commands.set->enums = boolean_enums;
 
@@ -655,10 +656,10 @@ add_setshow_filename_cmd (const char *name, enum command_class theclass,
 			  struct cmd_list_element **show_list)
 {
   set_show_commands commands
-    = add_setshow_cmd_full (name, theclass, var_filename, var,
-			    set_doc, show_doc, help_doc,
-			    set_func, show_func,
-			    set_list, show_list);
+    = add_setshow_cmd_full<var_filename> (name, theclass, var,
+					  set_doc, show_doc, help_doc,
+					  set_func, show_func,
+					  set_list, show_list);
 
   set_cmd_completer (commands.set, filename_completer);
 
@@ -679,10 +680,10 @@ add_setshow_string_cmd (const char *name, enum command_class theclass,
 			struct cmd_list_element **show_list)
 {
   set_show_commands commands
-    = add_setshow_cmd_full (name, theclass, var_string, var,
-			    set_doc, show_doc, help_doc,
-			    set_func, show_func,
-			    set_list, show_list);
+    = add_setshow_cmd_full<var_string> (name, theclass, var,
+					set_doc, show_doc, help_doc,
+					set_func, show_func,
+					set_list, show_list);
 
   /* Disable the default symbol completer.  */
   set_cmd_completer (commands.set, nullptr);
@@ -704,10 +705,10 @@ add_setshow_string_noescape_cmd (const char *name, enum command_class theclass,
 				 struct cmd_list_element **show_list)
 {
   set_show_commands commands
-    = add_setshow_cmd_full (name, theclass, var_string_noescape, var,
-			    set_doc, show_doc, help_doc,
-			    set_func, show_func,
-			    set_list, show_list);
+    = add_setshow_cmd_full<var_string_noescape> (name, theclass, var,
+						 set_doc, show_doc, help_doc,
+						 set_func, show_func,
+						 set_list, show_list);
 
   /* Disable the default symbol completer.  */
   set_cmd_completer (commands.set, nullptr);
@@ -729,10 +730,10 @@ add_setshow_optional_filename_cmd (const char *name, enum command_class theclass
 				   struct cmd_list_element **show_list)
 {
   set_show_commands commands
-    = add_setshow_cmd_full (name, theclass, var_optional_filename, var,
-			    set_doc, show_doc, help_doc,
-			    set_func, show_func,
-			    set_list, show_list);
+    = add_setshow_cmd_full<var_optional_filename> (name, theclass, var,
+						   set_doc, show_doc, help_doc,
+						   set_func, show_func,
+						   set_list, show_list);
 		
   set_cmd_completer (commands.set, filename_completer);
 
@@ -773,10 +774,10 @@ add_setshow_integer_cmd (const char *name, enum command_class theclass,
 			 struct cmd_list_element **show_list)
 {
   set_show_commands commands
-    = add_setshow_cmd_full (name, theclass, var_integer, var,
-			    set_doc, show_doc, help_doc,
-			    set_func, show_func,
-			    set_list, show_list);
+    = add_setshow_cmd_full<var_integer> (name, theclass, var,
+					 set_doc, show_doc, help_doc,
+					 set_func, show_func,
+					 set_list, show_list);
 
   set_cmd_completer (commands.set, integer_unlimited_completer);
 
@@ -799,10 +800,10 @@ add_setshow_uinteger_cmd (const char *name, enum command_class theclass,
 			  struct cmd_list_element **show_list)
 {
   set_show_commands commands
-    = add_setshow_cmd_full (name, theclass, var_uinteger, var,
-			    set_doc, show_doc, help_doc,
-			    set_func, show_func,
-			    set_list, show_list);
+    = add_setshow_cmd_full<var_uinteger> (name, theclass, var,
+					  set_doc, show_doc, help_doc,
+					  set_func, show_func,
+					  set_list, show_list);
 
   set_cmd_completer (commands.set, integer_unlimited_completer);
 
@@ -824,10 +825,10 @@ add_setshow_zinteger_cmd (const char *name, enum command_class theclass,
 			  struct cmd_list_element **set_list,
 			  struct cmd_list_element **show_list)
 {
-  return add_setshow_cmd_full (name, theclass, var_zinteger, var,
-			       set_doc, show_doc, help_doc,
-			       set_func, show_func,
-			       set_list, show_list);
+  return add_setshow_cmd_full<var_zinteger> (name, theclass, var,
+					     set_doc, show_doc, help_doc,
+					     set_func, show_func,
+					     set_list, show_list);
 }
 
 set_show_commands
@@ -843,10 +844,11 @@ add_setshow_zuinteger_unlimited_cmd (const char *name,
 				     struct cmd_list_element **show_list)
 {
   set_show_commands commands
-    = add_setshow_cmd_full (name, theclass, var_zuinteger_unlimited, var,
-			    set_doc, show_doc, help_doc,
-			    set_func, show_func,
-			    set_list, show_list);
+    = add_setshow_cmd_full<var_zuinteger_unlimited> (name, theclass, var,
+						     set_doc, show_doc,
+						     help_doc, set_func,
+						     show_func, set_list,
+						     show_list);
 
   set_cmd_completer (commands.set, integer_unlimited_completer);
 
@@ -868,10 +870,10 @@ add_setshow_zuinteger_cmd (const char *name, enum command_class theclass,
 			   struct cmd_list_element **set_list,
 			   struct cmd_list_element **show_list)
 {
-  return add_setshow_cmd_full (name, theclass, var_zuinteger, var,
-			       set_doc, show_doc, help_doc,
-			       set_func, show_func,
-			       set_list, show_list);
+  return add_setshow_cmd_full<var_zuinteger> (name, theclass, var,
+					      set_doc, show_doc, help_doc,
+					      set_func, show_func,
+					      set_list, show_list);
 }
 
 /* Remove the command named NAME from the command list.  Return the
diff --git a/gdb/cli/cli-decode.h b/gdb/cli/cli-decode.h
index 241535ae5b5..d54dff5a997 100644
--- a/gdb/cli/cli-decode.h
+++ b/gdb/cli/cli-decode.h
@@ -26,6 +26,7 @@
 #include "gdb_regex.h"
 #include "cli-script.h"
 #include "completer.h"
+#include <type_traits>
 
 /* Not a set/show command.  Note that some commands which begin with
    "set" or "show" might be in this category, if their syntax does
@@ -102,6 +103,54 @@ struct cmd_list_element
   void *context () const
   { return m_context; }
 
+  /* Return the value of the current element that can be shown.
+     The expected template parameter is the VAR_TYPES of the current instance
+     is.  This is enforced with a runtime check.
+
+     If multiple template parameters are given, select the approapriate return
+     type that satisfies all possibilities.  If such type does not exist, the
+     instantiation will fail.  A runtime check is made to ensure that the
+     current instance's type is one of those given as template parameter.  */
+  template<var_types... Ts>
+  typename std::common_type<typename var_types_storage<Ts>::type...>::type get_var() const
+  {
+    gdb_assert (var_types_storage_helper<Ts...>::covers_type (this->var_type));
+
+    return *static_cast<
+      typename std::common_type<
+	typename var_types_storage<Ts>::type...>::type *> (this->var);
+  }
+
+  /* Update the enclosed VAR to the value given as a parameter.
+
+     If one template argument is given, it must be the VAR_TYPE of the current
+     instance.  This is enforced at runtime.
+     If multiple template parameters are given, they must all share the same
+     underlying type (this is checked at compile time), and THIS must be of
+     the type of one of the template parameters (this is checked at runtime).
+     */
+  template<var_types... Ts>
+  void set_var(typename var_types_storage_helper<Ts...>::type v)
+    {
+      /* Ensure that all Ts share the same underlying type.  */
+      static_assert (var_types_storage_helper<Ts...>::all_same);
+
+      /* Check that the current instance is of one of the supported types for
+         this instantiation.  */
+      gdb_assert (var_types_storage_helper<Ts...>::covers_type (this->var_type));
+
+      *static_cast<typename var_types_storage_helper<Ts...>::type *> (this->var)
+	= v;
+    }
+
+  /* Update the pointer to the VAR referenced by this instance.  */
+  template<var_types T>
+  void set_var_p (typename var_types_storage<T>::type *v)
+  {
+    this->type = T;
+    this->var = static_cast<void *> (v);
+  }
+
   /* Points to next command in this list.  */
   struct cmd_list_element *next = nullptr;
 
diff --git a/gdb/cli/cli-setshow.c b/gdb/cli/cli-setshow.c
index 5fd5fd15c6a..b5931fe0e2e 100644
--- a/gdb/cli/cli-setshow.c
+++ b/gdb/cli/cli-setshow.c
@@ -353,11 +353,11 @@ do_set_command (const char *arg, int from_tty, struct cmd_list_element *c)
 	*q++ = '\0';
 	newobj = (char *) xrealloc (newobj, q - newobj);
 
-	if (*(char **) c->var == NULL
-	    || strcmp (*(char **) c->var, newobj) != 0)
+	if (c->get_var<var_string> () == nullptr
+	    || strcmp (c->get_var<var_string> (), newobj) != 0)
 	  {
-	    xfree (*(char **) c->var);
-	    *(char **) c->var = newobj;
+	    xfree (c->get_var<var_string> ());
+	    c->set_var<var_string> (newobj);
 
 	    option_changed = 1;
 	  }
@@ -366,10 +366,11 @@ do_set_command (const char *arg, int from_tty, struct cmd_list_element *c)
       }
       break;
     case var_string_noescape:
-      if (*(char **) c->var == NULL || strcmp (*(char **) c->var, arg) != 0)
+      if (c->get_var<var_string_noescape> () == nullptr
+	  || strcmp (c->get_var<var_string_noescape> (), arg) != 0)
 	{
-	  xfree (*(char **) c->var);
-	  *(char **) c->var = xstrdup (arg);
+	  xfree (c->get_var<var_string_noescape> ());
+	  c->set_var<var_string_noescape> (xstrdup (arg));
 
 	  option_changed = 1;
 	}
@@ -398,11 +399,11 @@ do_set_command (const char *arg, int from_tty, struct cmd_list_element *c)
 	else
 	  val = xstrdup ("");
 
-	if (*(char **) c->var == NULL
-	    || strcmp (*(char **) c->var, val) != 0)
+	if (c->get_var<var_optional_filename> () == nullptr
+	    || strcmp (c->get_var<var_optional_filename> (), val) != 0)
 	  {
-	    xfree (*(char **) c->var);
-	    *(char **) c->var = val;
+	    xfree (c->get_var<var_optional_filename> ());
+	    c->set_var<var_optional_filename> (val);
 
 	    option_changed = 1;
 	  }
@@ -416,9 +417,9 @@ do_set_command (const char *arg, int from_tty, struct cmd_list_element *c)
 
 	if (val < 0)
 	  error (_("\"on\" or \"off\" expected."));
-	if (val != *(bool *) c->var)
+	if (val != c->get_var<var_boolean> ())
 	  {
-	    *(bool *) c->var = val;
+	    c->set_var<var_boolean> (val);
 
 	    option_changed = 1;
 	  }
@@ -428,9 +429,9 @@ do_set_command (const char *arg, int from_tty, struct cmd_list_element *c)
       {
 	enum auto_boolean val = parse_auto_binary_operation (arg);
 
-	if (*(enum auto_boolean *) c->var != val)
+	if (c->get_var<var_auto_boolean> () != val)
 	  {
-	    *(enum auto_boolean *) c->var = val;
+	    c->set_var<var_auto_boolean> (val);
 
 	    option_changed = 1;
 	  }
@@ -441,9 +442,9 @@ do_set_command (const char *arg, int from_tty, struct cmd_list_element *c)
       {
 	unsigned int val = parse_cli_var_uinteger (c->var_type, &arg, true);
 
-	if (*(unsigned int *) c->var != val)
+	if (c->get_var<var_uinteger, var_zuinteger> () != val)
 	  {
-	    *(unsigned int *) c->var = val;
+	    c->set_var<var_uinteger, var_zuinteger> (val);
 
 	    option_changed = 1;
 	  }
@@ -477,9 +478,9 @@ do_set_command (const char *arg, int from_tty, struct cmd_list_element *c)
 		 || (c->var_type == var_zinteger && val > INT_MAX))
 	  error (_("integer %s out of range"), plongest (val));
 
-	if (*(int *) c->var != val)
+	if (c->get_var<var_integer, var_zinteger> () != val)
 	  {
-	    *(int *) c->var = val;
+	    c->set_var<var_integer, var_zinteger> (val);
 
 	    option_changed = 1;
 	  }
@@ -495,9 +496,9 @@ do_set_command (const char *arg, int from_tty, struct cmd_list_element *c)
 	if (*after != '\0')
 	  error (_("Junk after item \"%.*s\": %s"), len, arg, after);
 
-	if (*(const char **) c->var != match)
+	if (c->get_var<var_enum> () != match)
 	  {
-	    *(const char **) c->var = match;
+	    c->set_var<var_enum> (match);
 
 	    option_changed = 1;
 	  }
@@ -507,9 +508,9 @@ do_set_command (const char *arg, int from_tty, struct cmd_list_element *c)
       {
 	int val = parse_cli_var_zuinteger_unlimited (&arg, true);
 
-	if (*(int *) c->var != val)
+	if (c->get_var<var_zuinteger_unlimited> () != val)
 	  {
-	    *(int *) c->var = val;
+	    c->set_var<var_zuinteger_unlimited> (val);
 	    option_changed = 1;
 	  }
       }
@@ -584,18 +585,24 @@ do_set_command (const char *arg, int from_tty, struct cmd_list_element *c)
 	case var_filename:
 	case var_optional_filename:
 	case var_enum:
-	  gdb::observers::command_param_changed.notify (name, *(char **) c->var);
+	  gdb::observers::command_param_changed.notify
+	    (name,
+	     c->get_var<var_string,
+			var_string_noescape,
+			var_filename,
+			var_optional_filename,
+			var_enum> ());
 	  break;
 	case var_boolean:
 	  {
-	    const char *opt = *(bool *) c->var ? "on" : "off";
+	    const char *opt = c->get_var<var_boolean> () ? "on" : "off";
 
 	    gdb::observers::command_param_changed.notify (name, opt);
 	  }
 	  break;
 	case var_auto_boolean:
 	  {
-	    const char *s = auto_boolean_enums[*(enum auto_boolean *) c->var];
+	    const char *s = auto_boolean_enums[c->get_var<var_auto_boolean> ()];
 
 	    gdb::observers::command_param_changed.notify (name, s);
 	  }
@@ -605,7 +612,7 @@ do_set_command (const char *arg, int from_tty, struct cmd_list_element *c)
 	  {
 	    char s[64];
 
-	    xsnprintf (s, sizeof s, "%u", *(unsigned int *) c->var);
+	    xsnprintf (s, sizeof s, "%u", c->get_var<var_uinteger, var_zuinteger> ());
 	    gdb::observers::command_param_changed.notify (name, s);
 	  }
 	  break;
@@ -615,7 +622,8 @@ do_set_command (const char *arg, int from_tty, struct cmd_list_element *c)
 	  {
 	    char s[64];
 
-	    xsnprintf (s, sizeof s, "%d", *(int *) c->var);
+	    xsnprintf (s, sizeof s, "%d",
+		       c->get_var<var_integer, var_zinteger, var_zuinteger_unlimited> ());
 	    gdb::observers::command_param_changed.notify (name, s);
 	  }
 	  break;
@@ -634,21 +642,24 @@ get_setshow_command_value_string (const cmd_list_element *c)
   switch (c->var_type)
     {
     case var_string:
-      if (*(char **) c->var)
-	stb.putstr (*(char **) c->var, '"');
+      if (c->get_var<var_string> () != nullptr)
+	stb.putstr (c->get_var<var_string> (), '"');
       break;
     case var_string_noescape:
     case var_optional_filename:
     case var_filename:
     case var_enum:
-      if (*(char **) c->var)
-	stb.puts (*(char **) c->var);
+      if (c->get_var<var_string_noescape,
+		     var_optional_filename,
+		     var_filename,
+		     var_enum> () != nullptr)
+	stb.puts (c->get_var<var_enum> ());
       break;
     case var_boolean:
-      stb.puts (*(bool *) c->var ? "on" : "off");
+      stb.puts (c->get_var<var_boolean> () ? "on" : "off");
       break;
     case var_auto_boolean:
-      switch (*(enum auto_boolean*) c->var)
+      switch (c->get_var<var_auto_boolean> ())
 	{
 	case AUTO_BOOLEAN_TRUE:
 	  stb.puts ("on");
@@ -667,25 +678,25 @@ get_setshow_command_value_string (const cmd_list_element *c)
     case var_uinteger:
     case var_zuinteger:
       if (c->var_type == var_uinteger
-	  && *(unsigned int *) c->var == UINT_MAX)
+	  && c->get_var<var_uinteger> () == UINT_MAX)
 	stb.puts ("unlimited");
       else
-	stb.printf ("%u", *(unsigned int *) c->var);
+	stb.printf ("%u", c->get_var<var_uinteger, var_zuinteger> ());
       break;
     case var_integer:
     case var_zinteger:
       if (c->var_type == var_integer
-	  && *(int *) c->var == INT_MAX)
+	  && c->get_var<var_integer> () == INT_MAX)
 	stb.puts ("unlimited");
       else
-	stb.printf ("%d", *(int *) c->var);
+	stb.printf ("%d", c->get_var<var_integer, var_zinteger> ());
       break;
     case var_zuinteger_unlimited:
       {
-	if (*(int *) c->var == -1)
+	if (c->get_var<var_zuinteger_unlimited> () == -1)
 	  stb.puts ("unlimited");
 	else
-	  stb.printf ("%d", *(int *) c->var);
+	  stb.printf ("%d", c->get_var<var_zuinteger_unlimited> ());
       }
       break;
     default:
diff --git a/gdb/command.h b/gdb/command.h
index 711cbdcf43e..8fc36e8eb5e 100644
--- a/gdb/command.h
+++ b/gdb/command.h
@@ -125,6 +125,153 @@ typedef enum var_types
   }
 var_types;
 
+/* Helper classes used to associate a storage type for each possible
+   var_type. */
+
+template<var_types T>
+struct var_types_storage;
+
+template<>
+struct var_types_storage<var_boolean>
+{
+  using type = bool;
+};
+
+template<>
+struct var_types_storage<var_auto_boolean>
+{
+  using type = enum auto_boolean;
+};
+
+template<>
+struct var_types_storage<var_uinteger>
+{
+  using type = unsigned int;
+};
+
+template<>
+struct var_types_storage<var_integer>
+{
+  using type = int;
+};
+
+template<>
+struct var_types_storage<var_string>
+{
+  using type = char *;
+};
+
+template<>
+struct var_types_storage<var_string_noescape>
+{
+  using type = char *;
+};
+
+template<>
+struct var_types_storage<var_optional_filename>
+{
+  using type = char *;
+};
+
+template<>
+struct var_types_storage<var_filename>
+{
+  using type = char *;
+};
+
+template<>
+struct var_types_storage<var_zinteger>
+{
+  using type = int;
+};
+
+template<>
+struct var_types_storage<var_zuinteger>
+{
+  using type = unsigned int;
+};
+
+template<>
+struct var_types_storage<var_zuinteger_unlimited>
+{
+  using type = int;
+};
+
+template<>
+struct var_types_storage<var_enum>
+{
+  using type = const char *;
+};
+
+/* Helper class used to check if multiple var_types are represented
+   using the same underlying type.  This class is meant to be instantiated
+   using any number of var_types, and will be used to assess common properties
+   of the underlying storage type.
+
+   Each template instantiating will define the following static members:
+	- all_same: True if and only if all the var_types are stored on the same
+	underlying storage type.
+	- covers_type (var_types t): True if and only if the parameter T is one
+	the templates parameter.
+	- type: Type alias of the underlying type if all_same is true, unspecified
+	otherwise.
+  */
+
+template<var_types... Ts>
+struct var_types_storage_helper;
+
+/* Specialization of var_types_storage_helper when instantiated with only 1
+   template parameter.  */
+template<var_types T>
+struct var_types_storage_helper<T>
+{
+  static constexpr bool all_same = true;
+
+  using type = typename var_types_storage<T>::type;
+
+  static constexpr bool covers_type (var_types t)
+    {
+      return t == T;
+    }
+};
+
+/* Specialization of var_types_storage_helper when instantiated with exactly
+   2 template parameters.  */
+template<var_types T, var_types U>
+struct var_types_storage_helper<T, U>
+{
+  static constexpr bool all_same
+    = std::is_same<typename var_types_storage<T>::type,
+		   typename var_types_storage<U>::type>::value;
+
+  using type = typename var_types_storage<T>::type;
+
+  static constexpr bool covers_type (var_types t)
+  {
+    return var_types_storage_helper<T>::covers_type (t)
+      || var_types_storage_helper<U>::covers_type (t);
+  }
+};
+
+/* Specialization of var_types_storage_helper when instantiated with 3 or more
+   template parameters.  */
+template<var_types T, var_types U, var_types... Us>
+struct var_types_storage_helper<T, U, Us...>
+{
+  static constexpr bool all_same
+    = var_types_storage_helper<T, U>::all_same
+    && var_types_storage_helper<T, Us...>::all_same;
+
+  using type = typename var_types_storage<T>::type;
+
+  static constexpr bool covers_type (var_types t)
+    {
+      return var_types_storage_helper<T>::covers_type (t)
+	|| var_types_storage_helper<U, Us...>::covers_type (t);
+    }
+};
+
+
 /* This structure records one command'd definition.  */
 struct cmd_list_element;
 
-- 
2.30.2