[PATCH 0/3] Some language cleanups

[PATCH 0/3] Some language cleanups

[Simon Marchi]

These are some cleanups I found possible while writing a bigger patch
series.  I a sending them on their own, because I think they make sense
on their own.

Simon Marchi (3):
  gdb: remove return value of set_language
  gdb: switch "set language" to getter/setter
  gdb: remove language_auto

 gdb/ada-lang.c    |   2 +-
 gdb/defs.h        |   1 -
 gdb/language.c    | 175 +++++++++++++++++-----------------------------
 gdb/language.h    |   5 +-
 gdb/mi/mi-parse.c |   3 +-
 gdb/minsyms.c     |   2 +-
 gdb/symmisc.c     |   3 +-
 gdb/symtab.c      |   7 +-
 8 files changed, 72 insertions(+), 126 deletions(-)

-- 
2.40.0

[PATCH 1/3] gdb: remove return value of set_language

[Simon Marchi]

set_language returns the previous language, but nothing uses it.  Remove
the return value.  This lets us remove the assignment to
current_language, in _initialize_language.

Change-Id: Ifccf9b488434c1addf4626130a74e159a37d8c17
---
 gdb/language.c | 14 ++------------
 gdb/language.h |  5 +++--
 2 files changed, 5 insertions(+), 14 deletions(-)

diff --git a/gdb/language.c b/gdb/language.c
index fb065ef6a759..1ab356597d74 100644
--- a/gdb/language.c
+++ b/gdb/language.c
@@ -353,18 +353,13 @@ set_range_case (void)
     case_sensitivity = current_language->case_sensitivity ();
 }
 
-/* Set current language to (enum language) LANG.  Returns previous
-   language.  */
+/* See language.h.  */
 
-enum language
+void
 set_language (enum language lang)
 {
-  enum language prev_language;
-
-  prev_language = current_language->la_language;
   current_language = language_def (lang);
   set_range_case ();
-  return prev_language;
 }
 \f
 
@@ -1125,11 +1120,6 @@ For Fortran the default is off; for other languages the default is on."),
 			show_case_command,
 			&setlist, &showlist);
 
-  /* In order to call SET_LANGUAGE (below) we need to make sure that
-     CURRENT_LANGUAGE is not NULL.  So first set the language to unknown,
-     then we can change the language to 'auto'.  */
-  current_language = language_def (language_unknown);
-
   add_set_language_command ();
 
   /* Have the above take effect.  */
diff --git a/gdb/language.h b/gdb/language.h
index 57df8acd0a3b..4c91776d94dc 100644
--- a/gdb/language.h
+++ b/gdb/language.h
@@ -761,8 +761,9 @@ struct symbol *
 
 extern void language_info ();
 
-extern enum language set_language (enum language);
-\f
+/* Set the current language to LANG.  */
+
+extern void set_language (enum language lang);
 
 /* Test a character to decide whether it can be printed in literal form
    or needs to be printed in another representation.  For example,
-- 
2.40.0

[PATCH 2/3] gdb: switch "set language" to getter/setter

[Simon Marchi]

The `language` global variable is mostly a scratch variable used for the
setting.  The source of truth is really current_language and
language_mode (auto vs manual), which are set by the
set_language_command callback.

Switch the setting to use the add_setshow_enum_cmd overload that takes a
value getter and setter.

Change-Id: Ief5b2f93fd7337eed7ec96023639ae3dfe62250b
---
 gdb/language.c | 28 +++++++++++++++++++---------
 1 file changed, 19 insertions(+), 9 deletions(-)

diff --git a/gdb/language.c b/gdb/language.c
index 1ab356597d74..42bce92c647c 100644
--- a/gdb/language.c
+++ b/gdb/language.c
@@ -93,7 +93,6 @@ const struct language_defn *language_defn::languages[nr_languages];
 
 /* The current values of the "set language/range/case-sensitive" enum
    commands.  */
-static const char *language;
 static const char *range;
 static const char *case_sensitive;
 
@@ -135,10 +134,10 @@ show_language_command (struct ui_file *file, int from_tty,
     }
 }
 
-/* Set command.  Change the current working language.  */
+/* Set callback for the "set/show language" setting.  */
+
 static void
-set_language_command (const char *ignore,
-		      int from_tty, struct cmd_list_element *c)
+set_language (const char *language)
 {
   enum language flang = language_unknown;
 
@@ -192,6 +191,17 @@ set_language_command (const char *ignore,
 		  language);
 }
 
+/* Get callback for the "set/show language" setting.  */
+
+static const char *
+get_language ()
+{
+  if (language_mode == language_mode_auto)
+    return "auto";
+
+  return current_language->name ();
+}
+
 /* Show command.  Display a warning if the range setting does
    not match the current language.  */
 static void
@@ -372,7 +382,7 @@ language_info ()
     return;
 
   expected_language = current_language;
-  gdb_printf (_("Current language:  %s\n"), language);
+  gdb_printf (_("Current language:  %s\n"), get_language ());
   show_language_command (gdb_stdout, 1, NULL, NULL);
 }
 \f
@@ -465,8 +475,7 @@ add_set_language_command ()
   /* Display "auto", "local" and "unknown" first, and then the rest,
      alpha sorted.  */
   const char **language_names_p = language_names;
-  language = language_def (language_auto)->name ();
-  *language_names_p++ = language;
+  *language_names_p++ = language_def (language_auto)->name ();;
   *language_names_p++ = "local";
   *language_names_p++ = language_def (language_unknown)->name ();
   const char **sort_begin = language_names_p;
@@ -509,10 +518,11 @@ add_set_language_command ()
 
   add_setshow_enum_cmd ("language", class_support,
 			language_names,
-			&language,
 			doc.c_str (),
 			_("Show the current source language."),
-			NULL, set_language_command,
+			NULL,
+			set_language,
+			get_language,
 			show_language_command,
 			&setlist, &showlist);
 }
-- 
2.40.0

[PATCH 3/3] gdb: remove language_auto

[Simon Marchi]

I think that the language_auto enumerator and the auto_language class
can be removed.  There isn't really an "auto" language, it's only a
construct of the "set language" command to say "pick the appropriate
language automatically".  But "auto" is never the current language.  The
`current_language` points to the current effective language, and the
fact that we're in "auto language" mode is noted by the language_mode
global.

 - Change set_language to handle the "auto" (and "local", which is a
   synonym) early, instead of in the for loop.  I think it makes the two
   cases (auto vs explicit language) more clearly separated anyway.

 - Adjust add_set_language_command to hard-code the "auto" string,
   instead of using the "auto" language definition.

 - Remove auto_language, rename auto_or_unknown_language to
   unknown_language and move the bits of the existing unknown_language
   in there.

 - Remove the set_language at the end of _initialize_language.  I think
   it's not needed, because we call set_language in gdb_init, after all
   _initialize functions are called.  There is some chance that an
   _initialize function that runs after _initialize_language implicitly
   depends on current_language being set, but my testsuite runs haven't
   found anything like that.

 - Use language_unknown instead of language_auto when creating a minimal
   symbol (minimal_symbol_reader::record_full).  I think that this value
   is used to indicate that we don't know the symbol of the minimal
   symbol (yet), so language_unknown makes sense to me.  Update a
   condition accordingly in ada-lang.c.  symbol_find_demangled_name also
   appears to "normalize" this value from "unknown" to "auto", remove
   that part and update the condition to just check for
   language_unknown.

Change-Id: I47bcd6c15f607d9818f2e6e413053c2dc8ec5034
---
 gdb/ada-lang.c    |   2 +-
 gdb/defs.h        |   1 -
 gdb/language.c    | 135 +++++++++++++++-------------------------------
 gdb/mi/mi-parse.c |   3 +-
 gdb/minsyms.c     |   2 +-
 gdb/symmisc.c     |   3 +-
 gdb/symtab.c      |   7 +--
 7 files changed, 49 insertions(+), 104 deletions(-)

diff --git a/gdb/ada-lang.c b/gdb/ada-lang.c
index 97e03f78d5da..3a9f554ce4ce 100644
--- a/gdb/ada-lang.c
+++ b/gdb/ada-lang.c
@@ -13700,7 +13700,7 @@ class ada_language : public language_defn
 	       C++ symbols (using an old mangling scheme), such as "name__2Xn"
 	       -> "Xn::name(void)" and thus some Ada minimal symbols end up
 	       with the wrong language set.  Paper over that issue here.  */
-	    if (symbol_language == language_auto
+	    if (symbol_language == language_unknown
 		|| symbol_language == language_cplus)
 	      symbol_language = language_ada;
 
diff --git a/gdb/defs.h b/gdb/defs.h
index 8dbb68e440b0..454475b203c3 100644
--- a/gdb/defs.h
+++ b/gdb/defs.h
@@ -210,7 +210,6 @@ extern void quit_serial_event_clear (void);
 enum language
   {
     language_unknown,		/* Language not known */
-    language_auto,		/* Placeholder for automatic setting */
     language_c,			/* C */
     language_objc,		/* Objective-C */
     language_rust,		/* Rust */
diff --git a/gdb/language.c b/gdb/language.c
index 42bce92c647c..a1bf4d14ad49 100644
--- a/gdb/language.c
+++ b/gdb/language.c
@@ -142,49 +142,45 @@ set_language (const char *language)
   enum language flang = language_unknown;
 
   /* "local" is a synonym of "auto".  */
-  if (strcmp (language, "local") == 0)
-    language = "auto";
+  if (strcmp (language, "auto") == 0
+      || strcmp (language, "local") == 0)
+    {
+      /* Enter auto mode.  Set to the current frame's language, if
+	 known, or fallback to the initial language.  */
+      language_mode = language_mode_auto;
+      try
+	{
+	  frame_info_ptr frame;
+
+	  frame = get_selected_frame (NULL);
+	  flang = get_frame_language (frame);
+	}
+      catch (const gdb_exception_error &ex)
+	{
+	  flang = language_unknown;
+	}
+
+      if (flang != language_unknown)
+	set_language (flang);
+      else
+	set_initial_language ();
+
+      expected_language = current_language;
+      return;
+    }
 
   /* Search the list of languages for a match.  */
   for (const auto &lang : language_defn::languages)
     {
-      if (strcmp (lang->name (), language) == 0)
-	{
-	  /* Found it!  Go into manual mode, and use this language.  */
-	  if (lang->la_language == language_auto)
-	    {
-	      /* Enter auto mode.  Set to the current frame's language, if
-		 known, or fallback to the initial language.  */
-	      language_mode = language_mode_auto;
-	      try
-		{
-		  frame_info_ptr frame;
-
-		  frame = get_selected_frame (NULL);
-		  flang = get_frame_language (frame);
-		}
-	      catch (const gdb_exception_error &ex)
-		{
-		  flang = language_unknown;
-		}
-
-	      if (flang != language_unknown)
-		set_language (flang);
-	      else
-		set_initial_language ();
-	      expected_language = current_language;
-	      return;
-	    }
-	  else
-	    {
-	      /* Enter manual mode.  Set the specified language.  */
-	      language_mode = language_mode_manual;
-	      current_language = lang;
-	      set_range_case ();
-	      expected_language = current_language;
-	      return;
-	    }
-	}
+      if (strcmp (lang->name (), language) != 0)
+	continue;
+
+      /* Found it!  Go into manual mode, and use this language.  */
+      language_mode = language_mode_manual;
+      current_language = lang;
+      set_range_case ();
+      expected_language = current_language;
+      return;
     }
 
   internal_error ("Couldn't find language `%s' in known languages list.",
@@ -434,9 +430,6 @@ language_enum (const char *str)
     if (strcmp (lang->name (), str) == 0)
       return lang->la_language;
 
-  if (strcmp (str, "local") == 0)
-    return language_auto;
-
   return language_unknown;
 }
 
@@ -468,22 +461,21 @@ add_set_language_command ()
   static const char **language_names;
 
   /* Build the language names array, to be used as enumeration in the
-     "set language" enum command.  +1 for "local" and +1 for NULL
+     "set language" enum command.  +3 for "auto", "local" and NULL
      termination.  */
-  language_names = new const char *[ARRAY_SIZE (language_defn::languages) + 2];
+  language_names = new const char *[ARRAY_SIZE (language_defn::languages) + 3];
 
   /* Display "auto", "local" and "unknown" first, and then the rest,
      alpha sorted.  */
   const char **language_names_p = language_names;
-  *language_names_p++ = language_def (language_auto)->name ();;
+  *language_names_p++ = "auto";
   *language_names_p++ = "local";
   *language_names_p++ = language_def (language_unknown)->name ();
   const char **sort_begin = language_names_p;
   for (const auto &lang : language_defn::languages)
     {
       /* Already handled above.  */
-      if (lang->la_language == language_auto
-	  || lang->la_language == language_unknown)
+      if (lang->la_language == language_unknown)
 	continue;
       *language_names_p++ = lang->name ();
     }
@@ -505,8 +497,7 @@ add_set_language_command ()
   for (const auto &lang : language_defn::languages)
     {
       /* Already dealt with these above.  */
-      if (lang->la_language == language_unknown
-	  || lang->la_language == language_auto)
+      if (lang->la_language == language_unknown)
 	continue;
 
       /* Note that we add the newline at the front, so we don't wind
@@ -703,15 +694,12 @@ language_defn::varobj_ops () const
   return &c_varobj_ops;
 }
 
-/* Parent class for both the "auto" and "unknown" languages.  These two
-   pseudo-languages are very similar so merging their implementations like
-   this makes sense.  */
+/* Class representing the "unknown" language.  */
 
-class auto_or_unknown_language : public language_defn
+class unknown_language : public language_defn
 {
 public:
-  auto_or_unknown_language (enum language lang)
-    : language_defn (lang)
+  unknown_language () : language_defn (language_unknown)
   { /* Nothing.  */ }
 
   /* See language.h.  */
@@ -823,40 +811,6 @@ class auto_or_unknown_language : public language_defn
 
   const char *name_of_this () const override
   { return "this"; }
-};
-
-/* Class representing the fake "auto" language.  */
-
-class auto_language : public auto_or_unknown_language
-{
-public:
-  auto_language ()
-    : auto_or_unknown_language (language_auto)
-  { /* Nothing.  */ }
-
-  /* See language.h.  */
-
-  const char *name () const override
-  { return "auto"; }
-
-  /* See language.h.  */
-
-  const char *natural_name () const override
-  { return "Auto"; }
-};
-
-/* Single instance of the fake "auto" language.  */
-
-static auto_language auto_language_defn;
-
-/* Class representing the unknown language.  */
-
-class unknown_language : public auto_or_unknown_language
-{
-public:
-  unknown_language ()
-    : auto_or_unknown_language (language_unknown)
-  { /* Nothing.  */ }
 
   /* See language.h.  */
 
@@ -1131,7 +1085,4 @@ For Fortran the default is off; for other languages the default is on."),
 			&setlist, &showlist);
 
   add_set_language_command ();
-
-  /* Have the above take effect.  */
-  set_language (language_auto);
 }
diff --git a/gdb/mi/mi-parse.c b/gdb/mi/mi-parse.c
index e334968cc88b..737ec57eddd0 100644
--- a/gdb/mi/mi-parse.c
+++ b/gdb/mi/mi-parse.c
@@ -351,8 +351,7 @@ mi_parse (const char *cmd, char **token)
 	  std::string lang_name = extract_arg (&chp);
 
 	  parse->language = language_enum (lang_name.c_str ());
-	  if (parse->language == language_unknown
-	      || parse->language == language_auto)
+	  if (parse->language == language_unknown)
 	    error (_("Invalid --language argument: %s"), lang_name.c_str ());
 	}
       else
diff --git a/gdb/minsyms.c b/gdb/minsyms.c
index 3fa07f47b9fc..a2c139db24d4 100644
--- a/gdb/minsyms.c
+++ b/gdb/minsyms.c
@@ -1189,7 +1189,7 @@ minimal_symbol_reader::record_full (gdb::string_view name,
       m_msym_bunch = newobj;
     }
   msymbol = &m_msym_bunch->contents[m_msym_bunch_index];
-  msymbol->set_language (language_auto,
+  msymbol->set_language (language_unknown,
 			 &m_objfile->per_bfd->storage_obstack);
 
   if (copy_name)
diff --git a/gdb/symmisc.c b/gdb/symmisc.c
index 3d7fd5609564..4b68f2b1f6fb 100644
--- a/gdb/symmisc.c
+++ b/gdb/symmisc.c
@@ -365,8 +365,7 @@ dump_symtab (struct symtab *symtab, struct ui_file *outfile)
      because certain routines used during dump_symtab() use the current
      language to print an image of the symbol.  We'll restore it later.
      But use only real languages, not placeholders.  */
-  if (symtab->language () != language_unknown
-      && symtab->language () != language_auto)
+  if (symtab->language () != language_unknown)
     {
       scoped_restore_current_language save_lang;
       set_language (symtab->language ());
diff --git a/gdb/symtab.c b/gdb/symtab.c
index 9e9798676cbe..cbd8671f8499 100644
--- a/gdb/symtab.c
+++ b/gdb/symtab.c
@@ -874,10 +874,7 @@ symbol_find_demangled_name (struct general_symbol_info *gsymbol,
   gdb::unique_xmalloc_ptr<char> demangled;
   int i;
 
-  if (gsymbol->language () == language_unknown)
-    gsymbol->m_language = language_auto;
-
-  if (gsymbol->language () != language_auto)
+  if (gsymbol->language () != language_unknown)
     {
       const struct language_defn *lang = language_def (gsymbol->language ());
 
@@ -1015,7 +1012,7 @@ general_symbol_info::compute_and_set_names (gdb::string_view linkage_name,
       (*slot)->demangled = std::move (demangled_name);
       (*slot)->language = language ();
     }
-  else if (language () == language_unknown || language () == language_auto)
+  else if (language () == language_unknown)
     m_language = (*slot)->language;
 
   m_name = (*slot)->mangled.data ();
-- 
2.40.0

Re: [PATCH 0/3] Some language cleanups

[Tom Tromey]

>>>>> "Simon" == Simon Marchi via Gdb-patches <gdb-patches@sourceware.org> writes:

Simon> These are some cleanups I found possible while writing a bigger patch
Simon> series.  I a sending them on their own, because I think they make sense
Simon> on their own.

Thank you for doing this.  I was particularly happy to see the removal
of language_auto.
Reviewed-By: Tom Tromey <tom@tromey.com>

Tom

Re: [PATCH 0/3] Some language cleanups

[Simon Marchi]

On 4/21/23 10:26, Tom Tromey wrote:
>>>>>> "Simon" == Simon Marchi via Gdb-patches <gdb-patches@sourceware.org> writes:
> 
> Simon> These are some cleanups I found possible while writing a bigger patch
> Simon> series.  I a sending them on their own, because I think they make sense
> Simon> on their own.
> 
> Thank you for doing this.  I was particularly happy to see the removal
> of language_auto.
> Reviewed-By: Tom Tromey <tom@tromey.com>
> 
> Tom

Thanks, will push.

Simon