public inbox for gdb-patches@sourceware.org
 help / color / mirror / Atom feed
* [RFA/commit 1/3] language.h: Add "symtab.h" #include
@ 2013-11-11  6:38 Joel Brobecker
  2013-11-11  6:39 ` [RFA 2/3] New function cli-utils.c:extract_arg_const Joel Brobecker
                   ` (3 more replies)
  0 siblings, 4 replies; 15+ messages in thread
From: Joel Brobecker @ 2013-11-11  6:38 UTC (permalink / raw)
  To: gdb-patches

Hello,

I noticed this when I tried to #include "language.h" from mi-parse.c,
when I got compilation errors from missing type declarations:

In addition to the fact that language.h depends on a number of struct
types declared in symtab.h, language.h also depends on an enumerated
type (domain_enum). So language.h should #include "symtab.h".

gdb/ChangeLog:

        * language.h: Add "symtab.h" #include.

Tested on x86_64-linux.  I think this patch should go in on its own.

---
 gdb/language.h | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/gdb/language.h b/gdb/language.h
index 5e029ea..d730bf5 100644
--- a/gdb/language.h
+++ b/gdb/language.h
@@ -23,6 +23,8 @@
 #if !defined (LANGUAGE_H)
 #define LANGUAGE_H 1
 
+#include "symtab.h"
+
 /* Forward decls for prototypes.  */
 struct value;
 struct objfile;
-- 
1.8.1.2

^ permalink raw reply	[flat|nested] 15+ messages in thread

* [RFA 2/3] New function cli-utils.c:extract_arg_const
  2013-11-11  6:38 [RFA/commit 1/3] language.h: Add "symtab.h" #include Joel Brobecker
@ 2013-11-11  6:39 ` Joel Brobecker
  2013-11-13 19:35   ` Pedro Alves
  2013-11-11  6:55 ` [RFC 3/3] GDB/MI: Add new "--language LANG" command option Joel Brobecker
                   ` (2 subsequent siblings)
  3 siblings, 1 reply; 15+ messages in thread
From: Joel Brobecker @ 2013-11-11  6:39 UTC (permalink / raw)
  To: gdb-patches

Hello,

This function provides the exact same functionality as extract_arg,
except that it takes a "const char**" instead of a "char **". This
will be useful in the context of my next patch.

gdb/ChangeLog:

        * cli/cli-utils.h (extract_arg_const): Add declaration.
        * cli/cli-utils.c (extract_arg_const): New function.

Tested on x86_64-linux. OK to push?

Thank you,
-- 
Joel

---
 gdb/cli/cli-utils.c | 30 ++++++++++++++++++++++++++++++
 gdb/cli/cli-utils.h |  7 +++++++
 2 files changed, 37 insertions(+)

diff --git a/gdb/cli/cli-utils.c b/gdb/cli/cli-utils.c
index f74e6b1..98afe1f 100644
--- a/gdb/cli/cli-utils.c
+++ b/gdb/cli/cli-utils.c
@@ -289,6 +289,36 @@ extract_arg (char **arg)
 
 /* See documentation in cli-utils.h.  */
 
+char *
+extract_arg_const (const char **arg)
+{
+  const char *result;
+  char *copy;
+
+  if (!*arg)
+    return NULL;
+
+  /* Find the start of the argument.  */
+  *arg = skip_spaces_const (*arg);
+  if (!**arg)
+    return NULL;
+  result = *arg;
+
+  /* Find the end of the argument.  */
+  *arg = skip_to_space_const (*arg + 1);
+
+  if (result == *arg)
+    return NULL;
+
+  copy = xmalloc (*arg - result + 1);
+  memcpy (copy, result, *arg - result);
+  copy[*arg - result] = '\0';
+
+  return copy;
+}
+
+/* See documentation in cli-utils.h.  */
+
 int
 check_for_argument (char **str, char *arg, int arg_len)
 {
diff --git a/gdb/cli/cli-utils.h b/gdb/cli/cli-utils.h
index 152fb89..ebae2d2 100644
--- a/gdb/cli/cli-utils.h
+++ b/gdb/cli/cli-utils.h
@@ -118,6 +118,13 @@ extern char *remove_trailing_whitespace (const char *start, char *s);
 
 extern char *extract_arg (char **arg);
 
+/* A const-correct version of "extract_arg".
+
+   Since the returned value is xmalloc'd, it eventually needs to be
+   xfree'ed, which prevents us from making it const as well.  */
+
+extern char *extract_arg_const (const char **arg);
+
 /* A helper function that looks for an argument at the start of a
    string.  The argument must also either be at the end of the string,
    or be followed by whitespace.  Returns 1 if it finds the argument,
-- 
1.8.1.2

^ permalink raw reply	[flat|nested] 15+ messages in thread

* [RFC 3/3] GDB/MI: Add new "--language LANG" command option.
  2013-11-11  6:38 [RFA/commit 1/3] language.h: Add "symtab.h" #include Joel Brobecker
  2013-11-11  6:39 ` [RFA 2/3] New function cli-utils.c:extract_arg_const Joel Brobecker
@ 2013-11-11  6:55 ` Joel Brobecker
  2013-11-11  7:02   ` Joel Brobecker
                     ` (2 more replies)
  2013-11-13 19:34 ` [PATCH] New "make check-headers" rule. (was: Re: [RFA/commit 1/3] language.h: Add "symtab.h" #include) Pedro Alves
  2013-11-14 10:55 ` pushed: [RFA/commit 1/3] language.h: Add "symtab.h" #include Joel Brobecker
  3 siblings, 3 replies; 15+ messages in thread
From: Joel Brobecker @ 2013-11-11  6:55 UTC (permalink / raw)
  To: gdb-patches

Hello,

Frontend sometimes need to evaluate expressions that are
language-specific. For instance, Eclipse uses the following
expression to determine the size of an address on the target:

    -data-evaluate-expression "sizeof (void*)"

Unfortunately, if the main of the program being debugged is not C,
this may not work. For instance, if the main is in Ada, you get...

    -data-evaluate-expression "sizeof (void*)"
    ^error,msg="No definition of \"sizeof\" in current context."

... and apparently decides to stop the debugging session as a result.
The  recommendation sent was to specifically set the language to C
before trying to evaluate the expression.  Something such as:

    1. save current language
    2. set language c
    3. -data-evaluate-expression "sizeof (void*)"
    4. Restore language

This has the same disadvantages as the ones outlined in the "Context
Management" section of the GDB/MI documentation regarding setting
the current thread or the current frame, thus recommending the use of
general command-line switches such as --frame, or --thread instead.

This patch follows the same steps for the language, adding a similar
new command option: --language LANG. Example of use:

    -data-evaluate-expression --language c "sizeof (void*)"
    ^done,value="4"

gdb/ChangeLog:

        * mi/mi-parse.h (struct mi_parse) [language]: New field.
        * mi/mi-main.c (mi_cmd_execute): Temporarily set language to
        PARSE->LANGUAGE during command execution, if set.
        * mi/mi-parse.c: Add "language.h" #include.
        (mi_parse): Add parsing of "--language" command option.

gdb/testsuite/ChangeLog:

        * gdb.mi/mi-language.exp: New file.

gdb/doc/ChangeLog:

        * gdb.texinfo (Show): Add xref anchor for "show language" command.
        (Context management): Place current subsection text into its own
        subsubsection.  Add new subsubsection describing the "--language"
        command option.

Tested on x86_64-linux.  Thoughts?
Thank you,

---
 gdb/doc/gdb.texinfo                  | 22 +++++++++++++
 gdb/mi/mi-main.c                     |  7 ++++
 gdb/mi/mi-parse.c                    | 25 ++++++++++++++-
 gdb/mi/mi-parse.h                    |  4 +++
 gdb/testsuite/gdb.mi/mi-language.exp | 62 ++++++++++++++++++++++++++++++++++++
 5 files changed, 119 insertions(+), 1 deletion(-)
 create mode 100644 gdb/testsuite/gdb.mi/mi-language.exp

diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo
index 9731bbf..37c313d 100644
--- a/gdb/doc/gdb.texinfo
+++ b/gdb/doc/gdb.texinfo
@@ -13340,6 +13340,7 @@ working language, and also what language source files were written in.
 
 @table @code
 @item show language
+@anchor{show language}
 @kindex show language
 Display the current working language.  This is the
 language you can use with commands such as @code{print} to
@@ -28706,6 +28707,8 @@ the user interface.
 @node Context management
 @subsection Context management
 
+@subsubsection Threads and Frames
+
 In most cases when @value{GDBN} accesses the target, this access is
 done in context of a specific thread and frame (@pxref{Frames}).
 Often, even when accessing global data, the target requires that a thread
@@ -28756,6 +28759,25 @@ all subsequent commands.  No frontend is known to do this exactly
 right, so it is suggested to just always pass the @samp{--thread} and
 @samp{--frame} options.
 
+@subsubsection Language
+
+The execution of several commands depends on which language is selected.
+By default, the current language (@pxref{show language}) is used.
+But for commands known to be language-sensitive, it is recommended
+to use the @samp{--language} option.  This option takes one argument,
+which is the name of the language to use while executing the command.
+For instance:
+
+@smallexample
+-data-evaluate-expression --language c "sizeof (void*)"
+^done,value="4"
+(gdb) 
+@end smallexample
+
+The valid language names are the same names accepted by the
+@samp{set language} command (@pxref{Manually}), excluding @samp{auto},
+@samp{local} or @samp{unknown}.
+
 @node Asynchronous and non-stop modes
 @subsection Asynchronous command execution and non-stop mode
 
diff --git a/gdb/mi/mi-main.c b/gdb/mi/mi-main.c
index 8ff7f27..35573af 100644
--- a/gdb/mi/mi-main.c
+++ b/gdb/mi/mi-main.c
@@ -2123,6 +2123,7 @@ static void
 mi_cmd_execute (struct mi_parse *parse)
 {
   struct cleanup *cleanup;
+  enum language saved_language;
 
   cleanup = prepare_execute_command ();
 
@@ -2184,6 +2185,12 @@ mi_cmd_execute (struct mi_parse *parse)
 	error (_("Invalid frame id: %d"), frame);
     }
 
+  if (parse->language != language_unknown)
+    {
+      make_cleanup_restore_current_language ();
+      set_language (parse->language);
+    }
+
   current_context = parse;
 
   if (parse->cmd->suppress_notification != NULL)
diff --git a/gdb/mi/mi-parse.c b/gdb/mi/mi-parse.c
index a3c1849..9994307 100644
--- a/gdb/mi/mi-parse.c
+++ b/gdb/mi/mi-parse.c
@@ -27,6 +27,7 @@
 #include <ctype.h>
 #include "gdb_string.h"
 #include "cli/cli-utils.h"
+#include "language.h"
 
 static const char mi_no_values[] = "--no-values";
 static const char mi_simple_values[] = "--simple-values";
@@ -244,6 +245,7 @@ mi_parse (const char *cmd, char **token)
   parse->thread_group = -1;
   parse->thread = -1;
   parse->frame = -1;
+  parse->language = language_unknown;
 
   cleanup = make_cleanup (mi_parse_cleanup, parse);
 
@@ -292,7 +294,10 @@ mi_parse (const char *cmd, char **token)
      some important commands, like '-break-*' are implemented by
      forwarding to the CLI layer directly.  We want to parse --thread
      and --frame here, so as not to leave those option in the string
-     that will be passed to CLI.  */
+     that will be passed to CLI.
+
+     Same for the --language option.  */
+
   for (;;)
     {
       const char *option;
@@ -300,6 +305,7 @@ mi_parse (const char *cmd, char **token)
       size_t tgs = sizeof ("--thread-group ") - 1;
       size_t ts = sizeof ("--thread ") - 1;
       size_t fs = sizeof ("--frame ") - 1;
+      size_t ls = sizeof ("--language ") - 1;
 
       if (strncmp (chp, "--all ", as) == 0)
 	{
@@ -348,6 +354,23 @@ mi_parse (const char *cmd, char **token)
 	  parse->frame = strtol (chp, &endp, 10);
 	  chp = endp;
 	}
+      else if (strncmp (chp, "--language ", ls) == 0)
+	{
+	  char *lang_name;
+	  struct cleanup *old_chain;
+
+	  option = "--language";
+	  chp += ls;
+	  lang_name = extract_arg_const (&chp);
+	  old_chain = make_cleanup (xfree, lang_name);
+
+	  parse->language = language_enum (lang_name);
+	  if (parse->language == language_unknown
+	      || parse->language == language_auto)
+	    error (_("Invalid --language argument: %s"), lang_name);
+
+	  do_cleanups (old_chain);
+	}
       else
 	break;
 
diff --git a/gdb/mi/mi-parse.h b/gdb/mi/mi-parse.h
index b20a389..325c1e1 100644
--- a/gdb/mi/mi-parse.h
+++ b/gdb/mi/mi-parse.h
@@ -50,6 +50,10 @@ struct mi_parse
     int thread_group; /* At present, the same as inferior number.  */
     int thread;
     int frame;
+
+    /* The language that should be used to evaluate the MI command.
+       Ignored if set to language_unknown.  */
+    enum language language;
   };
 
 /* Attempts to parse CMD returning a ``struct mi_parse''.  If CMD is
diff --git a/gdb/testsuite/gdb.mi/mi-language.exp b/gdb/testsuite/gdb.mi/mi-language.exp
new file mode 100644
index 0000000..550beec
--- /dev/null
+++ b/gdb/testsuite/gdb.mi/mi-language.exp
@@ -0,0 +1,62 @@
+# Copyright 2013 Free Software Foundation, Inc.
+
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 3 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program.  If not, see <http://www.gnu.org/licenses/>.
+
+load_lib mi-support.exp
+set MIFLAGS "-i=mi"
+
+gdb_exit
+if [mi_gdb_start] {
+    continue
+}
+
+mi_gdb_test "set lang ada" \
+            ".*=cmd-param-changed,param=\"language\",value=\"ada\".*" \
+            "set lang ada"
+
+# Evaluate an expression that the Ada language is unable to parse.
+mi_gdb_test "-data-evaluate-expression \"sizeof (void*)\"" \
+            "\\^error,.*" \
+            "sizeof expression using current language"
+
+# Now, ask for the same expression to be parsed, but using the C
+# language.
+mi_gdb_test "-data-evaluate-expression --language c \"sizeof (void*)\"" \
+            "\\^done,value=\"$decimal\"" \
+            "sizeof expression using C language"
+
+# Double-check that the current language has not changed.
+mi_gdb_test "show lang ada" \
+            ".*The current source language is \\\\\"ada\\\\\".*" \
+            "set lang ada"
+
+# Test what happens when specifying an invalid language name...
+mi_gdb_test "-data-evaluate-expression --language invlang \"sizeof (void*)\"" \
+            "\\^error,msg=\"Invalid --language argument: invlang\"" \
+            "data-evaluate-expression with invalid language name"
+
+# Make sure that "--language auto" is also rejected.
+mi_gdb_test "-data-evaluate-expression --language auto \"sizeof (void*)\"" \
+            "\\^error,msg=\"Invalid --language argument: auto\"" \
+            "data-evaluate-expression with language auto"
+
+# Make sure that "--language local" is also rejected.
+mi_gdb_test "-data-evaluate-expression --language local \"sizeof (void*)\"" \
+            "\\^error,msg=\"Invalid --language argument: local\"" \
+            "data-evaluate-expression with language local"
+
+# Make sure that "--language unknown" is also rejected.
+mi_gdb_test "-data-evaluate-expression --language unknown \"sizeof (void*)\"" \
+            "\\^error,msg=\"Invalid --language argument: unknown\"" \
+            "data-evaluate-expression with language unknown"
-- 
1.8.1.2

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [RFC 3/3] GDB/MI: Add new "--language LANG" command option.
  2013-11-11  6:55 ` [RFC 3/3] GDB/MI: Add new "--language LANG" command option Joel Brobecker
@ 2013-11-11  7:02   ` Joel Brobecker
  2013-11-11 17:20     ` Eli Zaretskii
  2013-11-11 16:29   ` Eli Zaretskii
  2013-11-13 19:49   ` Pedro Alves
  2 siblings, 1 reply; 15+ messages in thread
From: Joel Brobecker @ 2013-11-11  7:02 UTC (permalink / raw)
  To: gdb-patches

[-- Attachment #1: Type: text/plain, Size: 846 bytes --]

I just realized that I forgot to add an entry in the NEWS file.
Argh! New patch attached.

gdb/ChangeLog:

        * mi/mi-parse.h (struct mi_parse) [language]: New field.
        * mi/mi-main.c (mi_cmd_execute): Temporarily set language to
        PARSE->LANGUAGE during command execution, if set.
        * mi/mi-parse.c: Add "language.h" #include.
        (mi_parse): Add parsing of "--language" command option.

        * NEWS: Add entry mentioning the new "--language" command option.

gdb/testsuite/ChangeLog:

        * gdb.mi/mi-language.exp: New file.

gdb/doc/ChangeLog:

        * gdb.texinfo (Show): Add xref anchor for "show language" command.
        (Context management): Place current subsection text into its own
        subsubsection.  Add new subsubsection describing the "--language"
        command option.

Thanks,
-- 
Joel

[-- Attachment #2: 0003-GDB-MI-Add-new-language-LANG-command-option.patch --]
[-- Type: text/x-diff, Size: 10753 bytes --]

From f562a28284d5c45f9b67359dd50cb823be484302 Mon Sep 17 00:00:00 2001
From: Joel Brobecker <brobecker@adacore.com>
Date: Mon, 11 Nov 2013 09:21:44 +0400
Subject: [PATCH] GDB/MI: Add new "--language LANG" command option.

Frontend sometimes need to evaluate expressions that are
language-specific. For instance, Eclipse uses the following
expression to determine the size of an address on the target:

    -data-evaluate-expression "sizeof (void*)"

Unfortunately, if the main of the program being debugged is not C,
this may not work. For instance, if the main is in Ada, you get...

    -data-evaluate-expression "sizeof (void*)"
    ^error,msg="No definition of \"sizeof\" in current context."

... and apparently decides to stop the debugging session as a result.
The  recommendation sent was to specifically set the language to C
before trying to evaluate the expression.  Something such as:

    1. save current language
    2. set language c
    3. -data-evaluate-expression "sizeof (void*)"
    4. Restore language

This has the same disadvantages as the ones outlined in the "Context
Management" section of the GDB/MI documentation regarding setting
the current thread or the current frame, thus recommending the use of
general command-line switches such as --frame, or --thread instead.

This patch follows the same steps for the language, adding a similar
new command option: --language LANG. Example of use:

    -data-evaluate-expression --language c "sizeof (void*)"
    ^done,value="4"

gdb/ChangeLog:

        * mi/mi-parse.h (struct mi_parse) [language]: New field.
        * mi/mi-main.c (mi_cmd_execute): Temporarily set language to
        PARSE->LANGUAGE during command execution, if set.
        * mi/mi-parse.c: Add "language.h" #include.
        (mi_parse): Add parsing of "--language" command option.

        * NEWS: Add entry mentioning the new "--language" command option.

gdb/testsuite/ChangeLog:

        * gdb.mi/mi-language.exp: New file.

gdb/doc/ChangeLog:

        * gdb.texinfo (Show): Add xref anchor for "show language" command.
        (Context management): Place current subsection text into its own
        subsubsection.  Add new subsubsection describing the "--language"
        command option.
---
 gdb/NEWS                             |  2 ++
 gdb/doc/gdb.texinfo                  | 22 +++++++++++++
 gdb/mi/mi-main.c                     |  7 ++++
 gdb/mi/mi-parse.c                    | 25 ++++++++++++++-
 gdb/mi/mi-parse.h                    |  4 +++
 gdb/testsuite/gdb.mi/mi-language.exp | 62 ++++++++++++++++++++++++++++++++++++
 6 files changed, 121 insertions(+), 1 deletion(-)
 create mode 100644 gdb/testsuite/gdb.mi/mi-language.exp

diff --git a/gdb/NEWS b/gdb/NEWS
index 6c3c272..cc7ae1b 100644
--- a/gdb/NEWS
+++ b/gdb/NEWS
@@ -145,6 +145,8 @@ show startup-with-shell
 
 * MI changes
 
+  ** All MI commands now accept an optional "--language" option.
+
   ** The -trace-save MI command can optionally save trace buffer in Common
      Trace Format now.
 
diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo
index 9731bbf..37c313d 100644
--- a/gdb/doc/gdb.texinfo
+++ b/gdb/doc/gdb.texinfo
@@ -13340,6 +13340,7 @@ working language, and also what language source files were written in.
 
 @table @code
 @item show language
+@anchor{show language}
 @kindex show language
 Display the current working language.  This is the
 language you can use with commands such as @code{print} to
@@ -28706,6 +28707,8 @@ the user interface.
 @node Context management
 @subsection Context management
 
+@subsubsection Threads and Frames
+
 In most cases when @value{GDBN} accesses the target, this access is
 done in context of a specific thread and frame (@pxref{Frames}).
 Often, even when accessing global data, the target requires that a thread
@@ -28756,6 +28759,25 @@ all subsequent commands.  No frontend is known to do this exactly
 right, so it is suggested to just always pass the @samp{--thread} and
 @samp{--frame} options.
 
+@subsubsection Language
+
+The execution of several commands depends on which language is selected.
+By default, the current language (@pxref{show language}) is used.
+But for commands known to be language-sensitive, it is recommended
+to use the @samp{--language} option.  This option takes one argument,
+which is the name of the language to use while executing the command.
+For instance:
+
+@smallexample
+-data-evaluate-expression --language c "sizeof (void*)"
+^done,value="4"
+(gdb) 
+@end smallexample
+
+The valid language names are the same names accepted by the
+@samp{set language} command (@pxref{Manually}), excluding @samp{auto},
+@samp{local} or @samp{unknown}.
+
 @node Asynchronous and non-stop modes
 @subsection Asynchronous command execution and non-stop mode
 
diff --git a/gdb/mi/mi-main.c b/gdb/mi/mi-main.c
index 8ff7f27..35573af 100644
--- a/gdb/mi/mi-main.c
+++ b/gdb/mi/mi-main.c
@@ -2123,6 +2123,7 @@ static void
 mi_cmd_execute (struct mi_parse *parse)
 {
   struct cleanup *cleanup;
+  enum language saved_language;
 
   cleanup = prepare_execute_command ();
 
@@ -2184,6 +2185,12 @@ mi_cmd_execute (struct mi_parse *parse)
 	error (_("Invalid frame id: %d"), frame);
     }
 
+  if (parse->language != language_unknown)
+    {
+      make_cleanup_restore_current_language ();
+      set_language (parse->language);
+    }
+
   current_context = parse;
 
   if (parse->cmd->suppress_notification != NULL)
diff --git a/gdb/mi/mi-parse.c b/gdb/mi/mi-parse.c
index a3c1849..9994307 100644
--- a/gdb/mi/mi-parse.c
+++ b/gdb/mi/mi-parse.c
@@ -27,6 +27,7 @@
 #include <ctype.h>
 #include "gdb_string.h"
 #include "cli/cli-utils.h"
+#include "language.h"
 
 static const char mi_no_values[] = "--no-values";
 static const char mi_simple_values[] = "--simple-values";
@@ -244,6 +245,7 @@ mi_parse (const char *cmd, char **token)
   parse->thread_group = -1;
   parse->thread = -1;
   parse->frame = -1;
+  parse->language = language_unknown;
 
   cleanup = make_cleanup (mi_parse_cleanup, parse);
 
@@ -292,7 +294,10 @@ mi_parse (const char *cmd, char **token)
      some important commands, like '-break-*' are implemented by
      forwarding to the CLI layer directly.  We want to parse --thread
      and --frame here, so as not to leave those option in the string
-     that will be passed to CLI.  */
+     that will be passed to CLI.
+
+     Same for the --language option.  */
+
   for (;;)
     {
       const char *option;
@@ -300,6 +305,7 @@ mi_parse (const char *cmd, char **token)
       size_t tgs = sizeof ("--thread-group ") - 1;
       size_t ts = sizeof ("--thread ") - 1;
       size_t fs = sizeof ("--frame ") - 1;
+      size_t ls = sizeof ("--language ") - 1;
 
       if (strncmp (chp, "--all ", as) == 0)
 	{
@@ -348,6 +354,23 @@ mi_parse (const char *cmd, char **token)
 	  parse->frame = strtol (chp, &endp, 10);
 	  chp = endp;
 	}
+      else if (strncmp (chp, "--language ", ls) == 0)
+	{
+	  char *lang_name;
+	  struct cleanup *old_chain;
+
+	  option = "--language";
+	  chp += ls;
+	  lang_name = extract_arg_const (&chp);
+	  old_chain = make_cleanup (xfree, lang_name);
+
+	  parse->language = language_enum (lang_name);
+	  if (parse->language == language_unknown
+	      || parse->language == language_auto)
+	    error (_("Invalid --language argument: %s"), lang_name);
+
+	  do_cleanups (old_chain);
+	}
       else
 	break;
 
diff --git a/gdb/mi/mi-parse.h b/gdb/mi/mi-parse.h
index b20a389..325c1e1 100644
--- a/gdb/mi/mi-parse.h
+++ b/gdb/mi/mi-parse.h
@@ -50,6 +50,10 @@ struct mi_parse
     int thread_group; /* At present, the same as inferior number.  */
     int thread;
     int frame;
+
+    /* The language that should be used to evaluate the MI command.
+       Ignored if set to language_unknown.  */
+    enum language language;
   };
 
 /* Attempts to parse CMD returning a ``struct mi_parse''.  If CMD is
diff --git a/gdb/testsuite/gdb.mi/mi-language.exp b/gdb/testsuite/gdb.mi/mi-language.exp
new file mode 100644
index 0000000..550beec
--- /dev/null
+++ b/gdb/testsuite/gdb.mi/mi-language.exp
@@ -0,0 +1,62 @@
+# Copyright 2013 Free Software Foundation, Inc.
+
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 3 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program.  If not, see <http://www.gnu.org/licenses/>.
+
+load_lib mi-support.exp
+set MIFLAGS "-i=mi"
+
+gdb_exit
+if [mi_gdb_start] {
+    continue
+}
+
+mi_gdb_test "set lang ada" \
+            ".*=cmd-param-changed,param=\"language\",value=\"ada\".*" \
+            "set lang ada"
+
+# Evaluate an expression that the Ada language is unable to parse.
+mi_gdb_test "-data-evaluate-expression \"sizeof (void*)\"" \
+            "\\^error,.*" \
+            "sizeof expression using current language"
+
+# Now, ask for the same expression to be parsed, but using the C
+# language.
+mi_gdb_test "-data-evaluate-expression --language c \"sizeof (void*)\"" \
+            "\\^done,value=\"$decimal\"" \
+            "sizeof expression using C language"
+
+# Double-check that the current language has not changed.
+mi_gdb_test "show lang ada" \
+            ".*The current source language is \\\\\"ada\\\\\".*" \
+            "set lang ada"
+
+# Test what happens when specifying an invalid language name...
+mi_gdb_test "-data-evaluate-expression --language invlang \"sizeof (void*)\"" \
+            "\\^error,msg=\"Invalid --language argument: invlang\"" \
+            "data-evaluate-expression with invalid language name"
+
+# Make sure that "--language auto" is also rejected.
+mi_gdb_test "-data-evaluate-expression --language auto \"sizeof (void*)\"" \
+            "\\^error,msg=\"Invalid --language argument: auto\"" \
+            "data-evaluate-expression with language auto"
+
+# Make sure that "--language local" is also rejected.
+mi_gdb_test "-data-evaluate-expression --language local \"sizeof (void*)\"" \
+            "\\^error,msg=\"Invalid --language argument: local\"" \
+            "data-evaluate-expression with language local"
+
+# Make sure that "--language unknown" is also rejected.
+mi_gdb_test "-data-evaluate-expression --language unknown \"sizeof (void*)\"" \
+            "\\^error,msg=\"Invalid --language argument: unknown\"" \
+            "data-evaluate-expression with language unknown"
-- 
1.8.1.2


^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [RFC 3/3] GDB/MI: Add new "--language LANG" command option.
  2013-11-11  6:55 ` [RFC 3/3] GDB/MI: Add new "--language LANG" command option Joel Brobecker
  2013-11-11  7:02   ` Joel Brobecker
@ 2013-11-11 16:29   ` Eli Zaretskii
  2013-11-13 19:49   ` Pedro Alves
  2 siblings, 0 replies; 15+ messages in thread
From: Eli Zaretskii @ 2013-11-11 16:29 UTC (permalink / raw)
  To: Joel Brobecker; +Cc: gdb-patches

> From: Joel Brobecker <brobecker@adacore.com>
> Date: Mon, 11 Nov 2013 10:37:35 +0400
> 
> gdb/ChangeLog:
> 
>         * mi/mi-parse.h (struct mi_parse) [language]: New field.
>         * mi/mi-main.c (mi_cmd_execute): Temporarily set language to
>         PARSE->LANGUAGE during command execution, if set.
>         * mi/mi-parse.c: Add "language.h" #include.
>         (mi_parse): Add parsing of "--language" command option.
> 
> gdb/testsuite/ChangeLog:
> 
>         * gdb.mi/mi-language.exp: New file.
> 
> gdb/doc/ChangeLog:
> 
>         * gdb.texinfo (Show): Add xref anchor for "show language" command.
>         (Context management): Place current subsection text into its own
>         subsubsection.  Add new subsubsection describing the "--language"
>         command option.
> 
> Tested on x86_64-linux.  Thoughts?

The documentation part is OK.

(Does this need a NEWS entry?)

Thanks.

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [RFC 3/3] GDB/MI: Add new "--language LANG" command option.
  2013-11-11  7:02   ` Joel Brobecker
@ 2013-11-11 17:20     ` Eli Zaretskii
  0 siblings, 0 replies; 15+ messages in thread
From: Eli Zaretskii @ 2013-11-11 17:20 UTC (permalink / raw)
  To: Joel Brobecker; +Cc: gdb-patches

> Date: Mon, 11 Nov 2013 10:55:31 +0400
> From: Joel Brobecker <brobecker@adacore.com>
> 
> I just realized that I forgot to add an entry in the NEWS file.
> Argh! New patch attached.

OK for the NEWS part.

^ permalink raw reply	[flat|nested] 15+ messages in thread

* [PATCH] New "make check-headers" rule. (was: Re: [RFA/commit 1/3] language.h: Add "symtab.h" #include)
  2013-11-11  6:38 [RFA/commit 1/3] language.h: Add "symtab.h" #include Joel Brobecker
  2013-11-11  6:39 ` [RFA 2/3] New function cli-utils.c:extract_arg_const Joel Brobecker
  2013-11-11  6:55 ` [RFC 3/3] GDB/MI: Add new "--language LANG" command option Joel Brobecker
@ 2013-11-13 19:34 ` Pedro Alves
  2013-11-14 10:00   ` Joel Brobecker
  2013-11-14 21:32   ` [PATCH] New "make check-headers" rule Tom Tromey
  2013-11-14 10:55 ` pushed: [RFA/commit 1/3] language.h: Add "symtab.h" #include Joel Brobecker
  3 siblings, 2 replies; 15+ messages in thread
From: Pedro Alves @ 2013-11-13 19:34 UTC (permalink / raw)
  To: Joel Brobecker; +Cc: gdb-patches

On 11/11/2013 06:37 AM, Joel Brobecker wrote:
> Hello,
> 
> I noticed this when I tried to #include "language.h" from mi-parse.c,
> when I got compilation errors from missing type declarations:
> 
> In addition to the fact that language.h depends on a number of struct
> types declared in symtab.h, language.h also depends on an enumerated
> type (domain_enum). So language.h should #include "symtab.h".
> 
> gdb/ChangeLog:
> 
>         * language.h: Add "symtab.h" #include.
> 
> Tested on x86_64-linux.  I think this patch should go in on its own.

Agreed.

I wonder what people think of something like the patch below.

$ make check-headers CHECK_HEADERS="language.h"
Checking headers.
for i in language.h ; do \
        gcc -g3 -O0   -I. -I../../src/gdb -I../../src/gdb/common -I../../src/gdb/config -DLOCALEDIR="\"/usr/local/share/locale\"" -DHAVE_CONFIG_H -I../../src/gdb/../include/opcode -I../../src/gdb/../opcodes/.. -I../../src/gdb/../readline/.. -I../bfd -I../../src/gdb/../bfd -I../../src/gdb/../include -I../libdecnumber -I../../src/gdb/../libdecnumber  -I../../src/gdb/gnulib/import -Ibuild-gnulib/import   -DTUI=1  -I/usr/include/python2.7 -I/usr/include/python2.7 -Wall -Wdeclaration-after-statement -Wpointer-arith -Wpointer-sign -Wno-unused -Wunused-value -Wunused-function -Wno-switch -Wno-char-subscripts -Wmissing-prototypes -Wdeclaration-after-statement -Wempty-body -Wmissing-parameter-type -Wold-style-declaration -Wold-style-definition -Wformat-nonliteral -Werror -c -o check-headers -Wno-error -fno-strict-aliasing -DNDEBUG -fwrapv -include defs.h \
                ../../src/gdb/$i -o /dev/null ; \
done
../../src/gdb/language.h:200:17: warning: ‘struct symbol’ declared inside parameter list [enabled by default]
../../src/gdb/language.h:200:17: warning: its scope is only this definition or declaration, which is probably not what you want [enabled by default]
../../src/gdb/language.h:242:13: warning: ‘struct symbol’ declared inside parameter list [enabled by default]
../../src/gdb/language.h:263:14: warning: type defaults to ‘int’ in declaration of ‘domain_enum’ [-Wimplicit-int]
../../src/gdb/language.h:263:8: warning: ‘struct block’ declared inside parameter list [enabled by default]
../../src/gdb/language.h:294:5: error: expected specifier-qualifier-list before ‘VEC’
make: *** [check-headers] Error 1

After your patch, the above comes out clean.

-------------
New "make check-headers" rule.

Tries to compile each header in isolation, thus ensuring headers are
self-contained.

Defaults to checking all $HFILES_NO_SRCDIR headers.

Do:

  make check-headers CHECK_HEADERS="header.h list.h"

to check specific headers.

gdb/
2013-11-13  Pedro Alves  <palves@redhat.com>

        * Makefile.in (COMPILE.pre): Rename to ...
        (NODEPS_COMPILE.pre): ... this.
        (COMPILE.post): Rename to ...
        (NODEPS_COMPILE.post): ... this.
        (COMPILE): Rename to ...
        (NODEPS_COMPILE): ... this.
        (COMPILE.pre, COMPILE.post, COMPILE): Reimplement as wrappers
        around the NODEPS variants.
        (CHECK_HEADERS): New.
        (check-headers:): New rule.
---

 gdb/Makefile.in |   32 +++++++++++++++++++++++++++++---
 1 file changed, 29 insertions(+), 3 deletions(-)

diff --git a/gdb/Makefile.in b/gdb/Makefile.in
index 975edbf..d39b5b4 100644
--- a/gdb/Makefile.in
+++ b/gdb/Makefile.in
@@ -86,11 +86,15 @@ DEPMODE = @CCDEPMODE@
 DEPDIR = @DEPDIR@
 depcomp = $(SHELL) $(srcdir)/../depcomp
 
+NODEPS_COMPILE.pre = $(CC)
+NODEPS_COMPILE.post = -c -o $@
+NODEPS_COMPILE = $(NODEPS_COMPILE.pre) $(INTERNAL_CFLAGS) $(NODEPS_COMPILE.post)
+
 # Note that these are overridden by GNU make-specific code below if
 # GNU make is used.  The overrides implement dependency tracking.
-COMPILE.pre = $(CC)
-COMPILE.post = -c -o $@
-COMPILE = $(COMPILE.pre) $(INTERNAL_CFLAGS) $(COMPILE.post)
+COMPILE.pre = $(NODEPS_COMPILE.pre)
+COMPILE.post = $(NODEPS_COMPILE.post)
+COMPILE = $(NODEPS_COMPILE)
 POSTCOMPILE = @true
 
 # Directory containing source files.
@@ -1050,6 +1054,28 @@ check//%: force
 	    "$$target"; \
 	else true; fi
 
+# The set of headers checked by 'check-headers' by default.
+CHECK_HEADERS = $(HFILES_NO_SRCDIR)
+
+# Try to compile each header in isolation, thus ensuring headers are
+# self-contained.
+#
+# Defaults to checking all $HFILES_NO_SRCDIR headers.
+#
+# Do:
+#
+#    make check-headers CHECK_HEADERS="header.h list.h"
+#
+# to check specific headers.
+#
+check-headers:
+	@echo Checking headers.
+	for i in $(CHECK_HEADERS) ; do \
+		$(NODEPS_COMPILE) -Wno-error $(PYTHON_CFLAGS) -include defs.h \
+			$(srcdir)/$$i -o /dev/null ; \
+	done
+.PHONY: check-headers
+
 info install-info clean-info dvi pdf install-pdf html install-html: force
 	@$(MAKE) $(FLAGS_TO_PASS) DO=$@ "DODIRS=$(SUBDIRS)" subdir_do
 

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [RFA 2/3] New function cli-utils.c:extract_arg_const
  2013-11-11  6:39 ` [RFA 2/3] New function cli-utils.c:extract_arg_const Joel Brobecker
@ 2013-11-13 19:35   ` Pedro Alves
  2013-11-14 10:55     ` pushed: " Joel Brobecker
  0 siblings, 1 reply; 15+ messages in thread
From: Pedro Alves @ 2013-11-13 19:35 UTC (permalink / raw)
  To: Joel Brobecker; +Cc: gdb-patches

On 11/11/2013 06:37 AM, Joel Brobecker wrote:
> Hello,
> 
> This function provides the exact same functionality as extract_arg,
> except that it takes a "const char**" instead of a "char **". This
> will be useful in the context of my next patch.
> 
> gdb/ChangeLog:
> 
>         * cli/cli-utils.h (extract_arg_const): Add declaration.
>         * cli/cli-utils.c (extract_arg_const): New function.
> 
> Tested on x86_64-linux. OK to push?

Instead of essentially duplicating the code, how about we reimplement
one on top of the other?  See below.  It compiles, but it's otherwise
untested.

-----------------
 gdb/cli/cli-utils.c |   25 +++++++++++++++++--------
 gdb/cli/cli-utils.h |    7 +++++++
 2 files changed, 24 insertions(+), 8 deletions(-)

diff --git a/gdb/cli/cli-utils.c b/gdb/cli/cli-utils.c
index f74e6b1..316cf4f 100644
--- a/gdb/cli/cli-utils.c
+++ b/gdb/cli/cli-utils.c
@@ -261,30 +261,39 @@ remove_trailing_whitespace (const char *start, char *s)
 /* See documentation in cli-utils.h.  */
 
 char *
-extract_arg (char **arg)
+extract_arg_const (const char **arg)
 {
-  char *result, *copy;
+  const char *result;
 
   if (!*arg)
     return NULL;
 
   /* Find the start of the argument.  */
-  *arg = skip_spaces (*arg);
+  *arg = skip_spaces_const (*arg);
   if (!**arg)
     return NULL;
   result = *arg;
 
   /* Find the end of the argument.  */
-  *arg = skip_to_space (*arg + 1);
+  *arg = skip_to_space_const (*arg + 1);
 
   if (result == *arg)
     return NULL;
 
-  copy = xmalloc (*arg - result + 1);
-  memcpy (copy, result, *arg - result);
-  copy[*arg - result] = '\0';
+  return savestring (result, *arg - result);
+}
+
+/* See documentation in cli-utils.h.  */
+
+char *
+extract_arg (char **arg)
+{
+  const char *arg_const = *arg;
+  char *result;
 
-  return copy;
+  result = extract_arg_const (&arg_const);
+  *arg += arg_const - *arg;
+  return result;
 }
 
 /* See documentation in cli-utils.h.  */
diff --git a/gdb/cli/cli-utils.h b/gdb/cli/cli-utils.h
index 152fb89..ebae2d2 100644
--- a/gdb/cli/cli-utils.h
+++ b/gdb/cli/cli-utils.h
@@ -118,6 +118,13 @@ extern char *remove_trailing_whitespace (const char *start, char *s);
 
 extern char *extract_arg (char **arg);
 
+/* A const-correct version of "extract_arg".
+
+   Since the returned value is xmalloc'd, it eventually needs to be
+   xfree'ed, which prevents us from making it const as well.  */
+
+extern char *extract_arg_const (const char **arg);
+
 /* A helper function that looks for an argument at the start of a
    string.  The argument must also either be at the end of the string,
    or be followed by whitespace.  Returns 1 if it finds the argument,

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [RFC 3/3] GDB/MI: Add new "--language LANG" command option.
  2013-11-11  6:55 ` [RFC 3/3] GDB/MI: Add new "--language LANG" command option Joel Brobecker
  2013-11-11  7:02   ` Joel Brobecker
  2013-11-11 16:29   ` Eli Zaretskii
@ 2013-11-13 19:49   ` Pedro Alves
  2013-11-14 11:17     ` Joel Brobecker
  2 siblings, 1 reply; 15+ messages in thread
From: Pedro Alves @ 2013-11-13 19:49 UTC (permalink / raw)
  To: Joel Brobecker; +Cc: gdb-patches

On 11/11/2013 06:37 AM, Joel Brobecker wrote:
> Hello,
> 
> Frontend sometimes need to evaluate expressions that are
> language-specific. For instance, Eclipse uses the following
> expression to determine the size of an address on the target:
> 
>     -data-evaluate-expression "sizeof (void*)"
> 
> Unfortunately, if the main of the program being debugged is not C,
> this may not work. For instance, if the main is in Ada, you get...
> 
>     -data-evaluate-expression "sizeof (void*)"
>     ^error,msg="No definition of \"sizeof\" in current context."
> 
> ... and apparently decides to stop the debugging session as a result.
> The  recommendation sent was to specifically set the language to C
> before trying to evaluate the expression.  Something such as:
> 
>     1. save current language
>     2. set language c
>     3. -data-evaluate-expression "sizeof (void*)"
>     4. Restore language
> 
> This has the same disadvantages as the ones outlined in the "Context
> Management" section of the GDB/MI documentation regarding setting
> the current thread or the current frame, thus recommending the use of
> general command-line switches such as --frame, or --thread instead.
> 
> This patch follows the same steps for the language, adding a similar
> new command option: --language LANG. Example of use:

Makes sense to me.  I skimmed the patch and it looked fine.

> 
>     -data-evaluate-expression --language c "sizeof (void*)"
>     ^done,value="4"
> 
> gdb/ChangeLog:
> 
>         * mi/mi-parse.h (struct mi_parse) [language]: New field.

[] is used for conditionally compiled code (#if FOO).  For specifying
context, use <>.

>         * mi/mi-main.c (mi_cmd_execute): Temporarily set language to
>         PARSE->LANGUAGE during command execution, if set.
>         * mi/mi-parse.c: Add "language.h" #include.
>         (mi_parse): Add parsing of "--language" command option.

-- 
Pedro Alves

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH] New "make check-headers" rule. (was: Re: [RFA/commit 1/3] language.h: Add "symtab.h" #include)
  2013-11-13 19:34 ` [PATCH] New "make check-headers" rule. (was: Re: [RFA/commit 1/3] language.h: Add "symtab.h" #include) Pedro Alves
@ 2013-11-14 10:00   ` Joel Brobecker
  2013-11-14 21:32   ` [PATCH] New "make check-headers" rule Tom Tromey
  1 sibling, 0 replies; 15+ messages in thread
From: Joel Brobecker @ 2013-11-14 10:00 UTC (permalink / raw)
  To: Pedro Alves; +Cc: gdb-patches

> I wonder what people think of something like the patch below.
> 
> $ make check-headers CHECK_HEADERS="language.h"
> Checking headers.
> for i in language.h ; do \
>         gcc -g3 -O0   -I. -I../../src/gdb -I../../src/gdb/common -I../../src/gdb/config -DLOCALEDIR="\"/usr/local/share/locale\"" -DHAVE_CONFIG_H -I../../src/gdb/../include/opcode -I../../src/gdb/../opcodes/.. -I../../src/gdb/../readline/.. -I../bfd -I../../src/gdb/../bfd -I../../src/gdb/../include -I../libdecnumber -I../../src/gdb/../libdecnumber  -I../../src/gdb/gnulib/import -Ibuild-gnulib/import   -DTUI=1  -I/usr/include/python2.7 -I/usr/include/python2.7 -Wall -Wdeclaration-after-statement -Wpointer-arith -Wpointer-sign -Wno-unused -Wunused-value -Wunused-function -Wno-switch -Wno-char-subscripts -Wmissing-prototypes -Wdeclaration-after-statement -Wempty-body -Wmissing-parameter-type -Wold-style-declaration -Wold-style-definition -Wformat-nonliteral -Werror -c -o check-headers -Wno-error -fno-strict-aliasing -DNDEBUG -fwrapv -include defs.h \
>                 ../../src/gdb/$i -o /dev/null ; \
> done
> ../../src/gdb/language.h:200:17: warning: ‘struct symbol’ declared inside parameter list [enabled by default]
> ../../src/gdb/language.h:200:17: warning: its scope is only this definition or declaration, which is probably not what you want [enabled by default]
> ../../src/gdb/language.h:242:13: warning: ‘struct symbol’ declared inside parameter list [enabled by default]
> ../../src/gdb/language.h:263:14: warning: type defaults to ‘int’ in declaration of ‘domain_enum’ [-Wimplicit-int]
> ../../src/gdb/language.h:263:8: warning: ‘struct block’ declared inside parameter list [enabled by default]
> ../../src/gdb/language.h:294:5: error: expected specifier-qualifier-list before ‘VEC’
> make: *** [check-headers] Error 1

Pretty cool! Once it comes out clean for all header files, we could even
add this to the "nightly" stuff we do, after sources have been packaged.
We'd be sending an email when failures are detected.

-- 
Joel

^ permalink raw reply	[flat|nested] 15+ messages in thread

* pushed: [RFA 2/3] New function cli-utils.c:extract_arg_const
  2013-11-13 19:35   ` Pedro Alves
@ 2013-11-14 10:55     ` Joel Brobecker
  0 siblings, 0 replies; 15+ messages in thread
From: Joel Brobecker @ 2013-11-14 10:55 UTC (permalink / raw)
  To: gdb-patches

[-- Attachment #1: Type: text/plain, Size: 698 bytes --]

> Instead of essentially duplicating the code, how about we reimplement
> one on top of the other?  See below.  It compiles, but it's otherwise
> untested.

Indeed! I actually had started with something very similar, and
backtracked, because other units failed to compile. But that was
because I had also made the return value a "const" which, in
hindsight, was no reason for me to backtrack the whole way!

Attached is what I ended up pushing.

gdb/ChangeLog:

        * cli/cli-utils.h (extract_arg_const): Add declaration.
        * cli/cli-utils.c (extract_arg_const): New function.
        (extract_arg): Reimplement using extract_arg_const.

Tested on x86_64-linux.

Thanks, Pedro.

-- 
Joel

[-- Attachment #2: 0001-New-function-cli-utils.c-extract_arg_const.patch --]
[-- Type: text/x-diff, Size: 3328 bytes --]

From b5be8ce022f894831b133b3b424238d8058eb29e Mon Sep 17 00:00:00 2001
From: Joel Brobecker <brobecker@adacore.com>
Date: Mon, 11 Nov 2013 09:19:32 +0400
Subject: [PATCH 1/2] New function cli-utils.c:extract_arg_const

This function provides the exact same functionality as extract_arg,
except that it takes a "const char**" instead of a "char **".
It allows us also to re-implement extract_arg almost as a simple
wrapper around the new function.

gdb/ChangeLog:

        Pedro Alves  <palves@redhat.com>
        Joel Brobecker  <brobecker@adacore.com>

        * cli/cli-utils.h (extract_arg_const): Add declaration.
        * cli/cli-utils.c (extract_arg_const): New function.
        (extract_arg): Reimplement using extract_arg_const.
---
 gdb/ChangeLog       |  7 +++++++
 gdb/cli/cli-utils.c | 25 +++++++++++++++++--------
 gdb/cli/cli-utils.h |  7 +++++++
 3 files changed, 31 insertions(+), 8 deletions(-)

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index 64adfd2..2dc6cca 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,3 +1,10 @@
+2013-11-14  Pedro Alves  <palves@redhat.com>
+	    Joel Brobecker  <brobecker@adacore.com>
+
+	* cli/cli-utils.h (extract_arg_const): Add declaration.
+	* cli/cli-utils.c (extract_arg_const): New function.
+	(extract_arg): Reimplement using extract_arg_const.
+
 2013-11-14  Joel Brobecker  <brobecker@adacore.com>
 
 	* language.h: Add "symtab.h" #include.
diff --git a/gdb/cli/cli-utils.c b/gdb/cli/cli-utils.c
index f74e6b1..316cf4f 100644
--- a/gdb/cli/cli-utils.c
+++ b/gdb/cli/cli-utils.c
@@ -261,30 +261,39 @@ remove_trailing_whitespace (const char *start, char *s)
 /* See documentation in cli-utils.h.  */
 
 char *
-extract_arg (char **arg)
+extract_arg_const (const char **arg)
 {
-  char *result, *copy;
+  const char *result;
 
   if (!*arg)
     return NULL;
 
   /* Find the start of the argument.  */
-  *arg = skip_spaces (*arg);
+  *arg = skip_spaces_const (*arg);
   if (!**arg)
     return NULL;
   result = *arg;
 
   /* Find the end of the argument.  */
-  *arg = skip_to_space (*arg + 1);
+  *arg = skip_to_space_const (*arg + 1);
 
   if (result == *arg)
     return NULL;
 
-  copy = xmalloc (*arg - result + 1);
-  memcpy (copy, result, *arg - result);
-  copy[*arg - result] = '\0';
+  return savestring (result, *arg - result);
+}
+
+/* See documentation in cli-utils.h.  */
+
+char *
+extract_arg (char **arg)
+{
+  const char *arg_const = *arg;
+  char *result;
 
-  return copy;
+  result = extract_arg_const (&arg_const);
+  *arg += arg_const - *arg;
+  return result;
 }
 
 /* See documentation in cli-utils.h.  */
diff --git a/gdb/cli/cli-utils.h b/gdb/cli/cli-utils.h
index 152fb89..ebae2d2 100644
--- a/gdb/cli/cli-utils.h
+++ b/gdb/cli/cli-utils.h
@@ -118,6 +118,13 @@ extern char *remove_trailing_whitespace (const char *start, char *s);
 
 extern char *extract_arg (char **arg);
 
+/* A const-correct version of "extract_arg".
+
+   Since the returned value is xmalloc'd, it eventually needs to be
+   xfree'ed, which prevents us from making it const as well.  */
+
+extern char *extract_arg_const (const char **arg);
+
 /* A helper function that looks for an argument at the start of a
    string.  The argument must also either be at the end of the string,
    or be followed by whitespace.  Returns 1 if it finds the argument,
-- 
1.8.1.2


^ permalink raw reply	[flat|nested] 15+ messages in thread

* pushed: [RFA/commit 1/3] language.h: Add "symtab.h" #include
  2013-11-11  6:38 [RFA/commit 1/3] language.h: Add "symtab.h" #include Joel Brobecker
                   ` (2 preceding siblings ...)
  2013-11-13 19:34 ` [PATCH] New "make check-headers" rule. (was: Re: [RFA/commit 1/3] language.h: Add "symtab.h" #include) Pedro Alves
@ 2013-11-14 10:55 ` Joel Brobecker
  3 siblings, 0 replies; 15+ messages in thread
From: Joel Brobecker @ 2013-11-14 10:55 UTC (permalink / raw)
  To: gdb-patches

> gdb/ChangeLog:
> 
>         * language.h: Add "symtab.h" #include.
> 
> Tested on x86_64-linux.  I think this patch should go in on its own.

For the record, this patch is now in.

-- 
Joel

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [RFC 3/3] GDB/MI: Add new "--language LANG" command option.
  2013-11-13 19:49   ` Pedro Alves
@ 2013-11-14 11:17     ` Joel Brobecker
  0 siblings, 0 replies; 15+ messages in thread
From: Joel Brobecker @ 2013-11-14 11:17 UTC (permalink / raw)
  To: Pedro Alves; +Cc: gdb-patches

> > This patch follows the same steps for the language, adding a similar
> > new command option: --language LANG. Example of use:
> 
> Makes sense to me.  I skimmed the patch and it looked fine.
> 
> > 
> >     -data-evaluate-expression --language c "sizeof (void*)"
> >     ^done,value="4"
> > 
> > gdb/ChangeLog:
> > 
> >         * mi/mi-parse.h (struct mi_parse) [language]: New field.
> 
> [] is used for conditionally compiled code (#if FOO).  For specifying
> context, use <>.
> 
> >         * mi/mi-main.c (mi_cmd_execute): Temporarily set language to
> >         PARSE->LANGUAGE during command execution, if set.
> >         * mi/mi-parse.c: Add "language.h" #include.
> >         (mi_parse): Add parsing of "--language" command option.

Thanks! I made the correction you pointed out, and pushed the commit.

-- 
Joel

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH] New "make check-headers" rule.
  2013-11-13 19:34 ` [PATCH] New "make check-headers" rule. (was: Re: [RFA/commit 1/3] language.h: Add "symtab.h" #include) Pedro Alves
  2013-11-14 10:00   ` Joel Brobecker
@ 2013-11-14 21:32   ` Tom Tromey
  2014-01-13 19:42     ` Pedro Alves
  1 sibling, 1 reply; 15+ messages in thread
From: Tom Tromey @ 2013-11-14 21:32 UTC (permalink / raw)
  To: Pedro Alves; +Cc: Joel Brobecker, gdb-patches

>>>>> "Pedro" == Pedro Alves <palves@redhat.com> writes:

Pedro> New "make check-headers" rule.

It seems like a good idea to me.

Pedro> +check-headers:
Pedro> +	@echo Checking headers.
Pedro> +	for i in $(CHECK_HEADERS) ; do \
Pedro> +		$(NODEPS_COMPILE) -Wno-error $(PYTHON_CFLAGS) -include defs.h \
Pedro> +			$(srcdir)/$$i -o /dev/null ; \
Pedro> +	done

I'm curious why -Wno-error.
Also why not -fsyntax-only?

... neither of which I think are blockers.
It's fine to refine this later.

Tom

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH] New "make check-headers" rule.
  2013-11-14 21:32   ` [PATCH] New "make check-headers" rule Tom Tromey
@ 2014-01-13 19:42     ` Pedro Alves
  0 siblings, 0 replies; 15+ messages in thread
From: Pedro Alves @ 2014-01-13 19:42 UTC (permalink / raw)
  To: Tom Tromey; +Cc: Joel Brobecker, gdb-patches

On 11/14/2013 09:21 PM, Tom Tromey wrote:
>>>>>> "Pedro" == Pedro Alves <palves@redhat.com> writes:
> 
> Pedro> New "make check-headers" rule.
> 
> It seems like a good idea to me.
> 
> Pedro> +check-headers:
> Pedro> +	@echo Checking headers.
> Pedro> +	for i in $(CHECK_HEADERS) ; do \
> Pedro> +		$(NODEPS_COMPILE) -Wno-error $(PYTHON_CFLAGS) -include defs.h \
> Pedro> +			$(srcdir)/$$i -o /dev/null ; \
> Pedro> +	done
> 
> I'm curious why -Wno-error.

No good reason, I'm afraid.

> Also why not -fsyntax-only?

It hadn't crossed my radar.  

Looking at it, I wasn't sure it was the exact same -- e.g.,
whether it expanded C++ templates, but google convinced me it
does.  I looked over gcc's codebase (grep for flag_syntax_only)
and I saw things like folding and optimization possibly behaving
differently when that flag in enabled.  I was going to prefer
not using the flag on those grounds, but then I timed it:

make check-headers w/ -o /dev/null

 real    1m35.383s
 user    0m44.862s
 sys     0m8.140s

make check-headers w/ -fsyntax-only:

 real    0m42.648s
 user    0m32.563s
 sys     0m7.891s

That's a considerable difference, so that convinced me.
Nice one.

If that flag indeed causes a difference of visible
behavior wrt to errors detected, I guess I'll call it
a gcc bug.

One extra thing we discussed offline is that with
-fsyntax-only, gcc needs to be told the language being
compiled explicitly ("-x c"), otherwise we get:

$ gcc -c -fsyntax-only (...)
cc1: error: output filename specified twice

Below's what I pushed.

Thanks.

----------
New "make check-headers" rule.

Tries to compile each header in isolation, thus ensuring headers are
self-contained.

Defaults to checking all $HFILES_NO_SRCDIR headers.

Do:

  make check-headers CHECK_HEADERS="header.h list.h"

to check specific headers.

gdb/
2014-01-13  Pedro Alves  <palves@redhat.com>

        * Makefile.in (CHECK_HEADERS): New variable.
        (check-headers:): New rule.
---
 gdb/ChangeLog   |  5 +++++
 gdb/Makefile.in | 22 ++++++++++++++++++++++
 2 files changed, 27 insertions(+)

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index 6cdafcc..18ed4d3 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,3 +1,8 @@
+2014-01-13  Pedro Alves  <palves@redhat.com>
+
+        * Makefile.in (CHECK_HEADERS): New variable.
+        (check-headers:): New rule.
+
 2014-01-13  Tom Tromey  <tromey@redhat.com>
 
 	* cli/cli-setshow.c (do_set_command): Update.
diff --git a/gdb/Makefile.in b/gdb/Makefile.in
index 824b26b..9811cbe 100644
--- a/gdb/Makefile.in
+++ b/gdb/Makefile.in
@@ -1050,6 +1050,28 @@ check//%: force
 	    "$$target"; \
 	else true; fi
 
+# The set of headers checked by 'check-headers' by default.
+CHECK_HEADERS = $(HFILES_NO_SRCDIR)
+
+# Try to compile each header in isolation, thus ensuring headers are
+# self-contained.
+#
+# Defaults to checking all $HFILES_NO_SRCDIR headers.
+#
+# Do:
+#
+#    make check-headers CHECK_HEADERS="header.h list.h"
+#
+# to check specific headers.
+#
+check-headers:
+	@echo Checking headers.
+	for i in $(CHECK_HEADERS) ; do \
+		$(CC) -x c -c -fsyntax-only $(INTERNAL_CFLAGS) \
+			-include defs.h $(srcdir)/$$i ; \
+	done
+.PHONY: check-headers
+
 info install-info clean-info dvi pdf install-pdf html install-html: force
 	@$(MAKE) $(FLAGS_TO_PASS) DO=$@ "DODIRS=$(SUBDIRS)" subdir_do
 
-- 
1.7.11.7


^ permalink raw reply	[flat|nested] 15+ messages in thread

end of thread, other threads:[~2014-01-13 19:42 UTC | newest]

Thread overview: 15+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2013-11-11  6:38 [RFA/commit 1/3] language.h: Add "symtab.h" #include Joel Brobecker
2013-11-11  6:39 ` [RFA 2/3] New function cli-utils.c:extract_arg_const Joel Brobecker
2013-11-13 19:35   ` Pedro Alves
2013-11-14 10:55     ` pushed: " Joel Brobecker
2013-11-11  6:55 ` [RFC 3/3] GDB/MI: Add new "--language LANG" command option Joel Brobecker
2013-11-11  7:02   ` Joel Brobecker
2013-11-11 17:20     ` Eli Zaretskii
2013-11-11 16:29   ` Eli Zaretskii
2013-11-13 19:49   ` Pedro Alves
2013-11-14 11:17     ` Joel Brobecker
2013-11-13 19:34 ` [PATCH] New "make check-headers" rule. (was: Re: [RFA/commit 1/3] language.h: Add "symtab.h" #include) Pedro Alves
2013-11-14 10:00   ` Joel Brobecker
2013-11-14 21:32   ` [PATCH] New "make check-headers" rule Tom Tromey
2014-01-13 19:42     ` Pedro Alves
2013-11-14 10:55 ` pushed: [RFA/commit 1/3] language.h: Add "symtab.h" #include Joel Brobecker

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).