* [PATCH 0/3] Some language cleanups
@ 2023-04-18 20:27 Simon Marchi
2023-04-18 20:27 ` [PATCH 1/3] gdb: remove return value of set_language Simon Marchi
` (3 more replies)
0 siblings, 4 replies; 6+ messages in thread
From: Simon Marchi @ 2023-04-18 20:27 UTC (permalink / raw)
To: gdb-patches; +Cc: 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
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH 1/3] gdb: remove return value of set_language
2023-04-18 20:27 [PATCH 0/3] Some language cleanups Simon Marchi
@ 2023-04-18 20:27 ` Simon Marchi
2023-04-18 20:27 ` [PATCH 2/3] gdb: switch "set language" to getter/setter Simon Marchi
` (2 subsequent siblings)
3 siblings, 0 replies; 6+ messages in thread
From: Simon Marchi @ 2023-04-18 20:27 UTC (permalink / raw)
To: gdb-patches; +Cc: 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
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH 2/3] gdb: switch "set language" to getter/setter
2023-04-18 20:27 [PATCH 0/3] Some language cleanups Simon Marchi
2023-04-18 20:27 ` [PATCH 1/3] gdb: remove return value of set_language Simon Marchi
@ 2023-04-18 20:27 ` Simon Marchi
2023-04-18 20:27 ` [PATCH 3/3] gdb: remove language_auto Simon Marchi
2023-04-21 14:26 ` [PATCH 0/3] Some language cleanups Tom Tromey
3 siblings, 0 replies; 6+ messages in thread
From: Simon Marchi @ 2023-04-18 20:27 UTC (permalink / raw)
To: gdb-patches; +Cc: 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
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH 3/3] gdb: remove language_auto
2023-04-18 20:27 [PATCH 0/3] Some language cleanups Simon Marchi
2023-04-18 20:27 ` [PATCH 1/3] gdb: remove return value of set_language Simon Marchi
2023-04-18 20:27 ` [PATCH 2/3] gdb: switch "set language" to getter/setter Simon Marchi
@ 2023-04-18 20:27 ` Simon Marchi
2023-04-21 14:26 ` [PATCH 0/3] Some language cleanups Tom Tromey
3 siblings, 0 replies; 6+ messages in thread
From: Simon Marchi @ 2023-04-18 20:27 UTC (permalink / raw)
To: gdb-patches; +Cc: 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
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 0/3] Some language cleanups
2023-04-18 20:27 [PATCH 0/3] Some language cleanups Simon Marchi
` (2 preceding siblings ...)
2023-04-18 20:27 ` [PATCH 3/3] gdb: remove language_auto Simon Marchi
@ 2023-04-21 14:26 ` Tom Tromey
2023-04-21 18:10 ` Simon Marchi
3 siblings, 1 reply; 6+ messages in thread
From: Tom Tromey @ 2023-04-21 14:26 UTC (permalink / raw)
To: Simon Marchi via Gdb-patches; +Cc: Simon Marchi
>>>>> "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
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 0/3] Some language cleanups
2023-04-21 14:26 ` [PATCH 0/3] Some language cleanups Tom Tromey
@ 2023-04-21 18:10 ` Simon Marchi
0 siblings, 0 replies; 6+ messages in thread
From: Simon Marchi @ 2023-04-21 18:10 UTC (permalink / raw)
To: Tom Tromey, Simon Marchi via Gdb-patches; +Cc: 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
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2023-04-21 18:10 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-04-18 20:27 [PATCH 0/3] Some language cleanups Simon Marchi
2023-04-18 20:27 ` [PATCH 1/3] gdb: remove return value of set_language Simon Marchi
2023-04-18 20:27 ` [PATCH 2/3] gdb: switch "set language" to getter/setter Simon Marchi
2023-04-18 20:27 ` [PATCH 3/3] gdb: remove language_auto Simon Marchi
2023-04-21 14:26 ` [PATCH 0/3] Some language cleanups Tom Tromey
2023-04-21 18:10 ` Simon Marchi
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).