public inbox for gdb-patches@sourceware.org
 help / color / mirror / Atom feed
* GDB 7.99.91 available for testing
@ 2017-05-04 19:44 Joel Brobecker
  2017-05-08 14:53 ` Eli Zaretskii
                   ` (3 more replies)
  0 siblings, 4 replies; 56+ messages in thread
From: Joel Brobecker @ 2017-05-04 19:44 UTC (permalink / raw)
  To: gdb-patches


Hello,

I have just finished creating the gdb-7.99.91 pre-release.
It is available for download at the following location:

    ftp://sourceware.org/pub/gdb/snapshots/branch/gdb-7.99.91.tar.xz

A gzip'ed version is also available: gdb-7.99.91.tar.gz.

Please give it a test if you can and report any problems you might find.

On behalf of all the GDB contributors, thank you!
-- 
Joel Brobecker

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

* Re: GDB 7.99.91 available for testing
  2017-05-04 19:44 GDB 7.99.91 available for testing Joel Brobecker
@ 2017-05-08 14:53 ` Eli Zaretskii
  2017-05-17 11:36   ` Yao Qi
  2017-05-08 15:00 ` GDB 7.99.91 MinGW compilation error in cli-script.c Eli Zaretskii
                   ` (2 subsequent siblings)
  3 siblings, 1 reply; 56+ messages in thread
From: Eli Zaretskii @ 2017-05-08 14:53 UTC (permalink / raw)
  To: Joel Brobecker; +Cc: gdb-patches

> From: Joel Brobecker <brobecker@adacore.com>
> Date: Thu,  4 May 2017 12:44:42 -0700 (PDT)
> 
> I have just finished creating the gdb-7.99.91 pre-release.
> It is available for download at the following location:
> 
>     ftp://sourceware.org/pub/gdb/snapshots/branch/gdb-7.99.91.tar.xz
> 
> A gzip'ed version is also available: gdb-7.99.91.tar.gz.
> 
> Please give it a test if you can and report any problems you might find.

I've built this with MinGW and bumped into a few problems.  Two
problems related to GDB sources, the rest to libiberty and readline.
I will report the GDB problems in separate messages.  As to problems
with libiberty and readline (all of them minor), what should I do to
fix them in GDB in addition to reporting them to their respective
upstream maintainers?  Should I push the fixes to the GDB repository,
master and branch?

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

* Re: GDB 7.99.91 MinGW compilation error in cli-script.c
  2017-05-04 19:44 GDB 7.99.91 available for testing Joel Brobecker
  2017-05-08 14:53 ` Eli Zaretskii
@ 2017-05-08 15:00 ` Eli Zaretskii
  2017-05-14  3:19   ` Simon Marchi
  2017-05-08 15:02 ` GDB 7.99.91 MinGW compilation warning in tui.c Eli Zaretskii
  2017-05-15 21:11 ` GDB 7.99.91 available for testing Simon Marchi
  3 siblings, 1 reply; 56+ messages in thread
From: Eli Zaretskii @ 2017-05-08 15:00 UTC (permalink / raw)
  To: gdb-patches

The error manifests itself as follows:

       g++ -std=gnu++11 -O2 -gdwarf-4 -g3    -I. -I. -I./common -I./config  -DLOCALEDIR="\"d:/usr/share/locale\"" -DHAVE_CONFIG_H -I./../include/opcode -I./../opcodes/.. -I./../readline/..   -I../bfd -I./../bfd -I./../include -I../libdecnumber -I./../libdecnumber   -I./gnulib/import -Ibuild-gnulib/import   -DTUI=1   -Id:/usr/include -Id:/usr/include/guile/2.0 -Id:/usr/include   -Id:/usr/Python26/include -Id:/usr/Python26/include -Wall -Wpointer-arith -Wno-unused -Wunused-value -Wunused-function -Wno-switch -Wno-char-subscripts -Wempty-body -Wunused-but-set-parameter -Wunused-but-set-variable -Wno-sign-compare -Wno-narrowing -Wno-format  -c -o cli-script.o -MT cli-script.o -MMD -MP  -MF .deps/cli-script.Tpo cli/cli-script.c
       cli/cli-script.c: In member function 'std::__cxx11::string user_args::insert_args(const char*) const':
       cli/cli-script.c:809:16: error: 'to_string' is not a member of 'std'
	   new_line += std::to_string (m_args.size ());
		       ^
       Makefile:1888: recipe for target `cli-script.o' failed
       make[2]: *** [cli-script.o] Error 1

The reason is that std::to_string is guarded by the symbol
_GLIBCXX_HAVE_BROKEN_VSWPRINTF, which mingw.org's MinGW defines in its
os_defines.h, because msvcrt.dll's implementation of vswprintf is
incompatible with what C++11 expects.  So GDB assumes here without
testing that this method is available, which is not true at least on
one platform.

How best to solve this?  I worked around by providing my own
implementation based on std::ostringstream, but I'm not sure this is
TRT.  An alternative would be to use some less problematic API, since
currently cli-script.c is the only user of this method, and its needs
are quite modest.  And if we do provide a replacement for to_string,
should the configure script probe for it, or should we condition it
specifically on MinGW and _GLIBCXX_HAVE_BROKEN_VSWPRINTF?

Thoughts?

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

* Re: GDB 7.99.91 MinGW compilation warning in tui.c
  2017-05-04 19:44 GDB 7.99.91 available for testing Joel Brobecker
  2017-05-08 14:53 ` Eli Zaretskii
  2017-05-08 15:00 ` GDB 7.99.91 MinGW compilation error in cli-script.c Eli Zaretskii
@ 2017-05-08 15:02 ` Eli Zaretskii
  2017-05-09 10:17   ` Yao Qi
  2017-05-15 21:11 ` GDB 7.99.91 available for testing Simon Marchi
  3 siblings, 1 reply; 56+ messages in thread
From: Eli Zaretskii @ 2017-05-08 15:02 UTC (permalink / raw)
  To: gdb-patches

The warning manifests itself as follows:

       g++ -std=gnu++11 -O2 -gdwarf-4 -g3    -I. -I. -I./common -I./config  -DLOCALEDIR="\"d:/usr/share/locale\"" -DHAVE_CONFIG_H -I./../include/opcode -I./../opcodes/.. -I./../readline/..   -I../bfd -I./../bfd -I./../include -I../libdecnumber -I./../libdecnumber   -I./gnulib/import -Ibuild-gnulib/import   -DTUI=1   -Id:/usr/include -Id:/usr/include/guile/2.0 -Id:/usr/include   -Id:/usr/Python26/include -Id:/usr/Python26/include -Wall -Wpointer-arith -Wno-unused -Wunused-value -Wunused-function -Wno-switch -Wno-char-subscripts -Wempty-body -Wunused-but-set-parameter -Wunused-but-set-variable -Wno-sign-compare -Wno-narrowing -Wno-format  -c -o tui.o -MT tui.o -MMD -MP  -MF .deps/tui.Tpo tui/tui.c
       tui/tui.c: In function 'void tui_enable()':
       tui/tui.c:430:39: warning: ISO C++ forbids converting a string constant to 'char*' [-Wwrite-strings]
	 s = newterm ("unknown", stdout, stdin);
					      ^
I fixed this by explicitly casting "unknown" to 'char *' -- is that
the right fix?  Is it OK to push that fix?

Thanks.

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

* Re: GDB 7.99.91 MinGW compilation warning in tui.c
  2017-05-08 15:02 ` GDB 7.99.91 MinGW compilation warning in tui.c Eli Zaretskii
@ 2017-05-09 10:17   ` Yao Qi
  2017-05-13  8:12     ` Eli Zaretskii
  0 siblings, 1 reply; 56+ messages in thread
From: Yao Qi @ 2017-05-09 10:17 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: gdb-patches

On Mon, May 8, 2017 at 4:01 PM, Eli Zaretskii <eliz@gnu.org> wrote:
>        tui/tui.c: In function 'void tui_enable()':
>        tui/tui.c:430:39: warning: ISO C++ forbids converting a string constant to 'char*' [-Wwrite-strings]
>          s = newterm ("unknown", stdout, stdin);
>                                               ^
> I fixed this by explicitly casting "unknown" to 'char *' -- is that
> the right fix?  Is it OK to push that fix?
>

Yes, I think so, we've done this before
https://sourceware.org/ml/gdb-patches/2017-04/msg00032.html

-- 
Yao (齐尧)

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

* Re: GDB 7.99.91 MinGW compilation warning in tui.c
  2017-05-09 10:17   ` Yao Qi
@ 2017-05-13  8:12     ` Eli Zaretskii
  2017-05-17 16:26       ` Yao Qi
  0 siblings, 1 reply; 56+ messages in thread
From: Eli Zaretskii @ 2017-05-13  8:12 UTC (permalink / raw)
  To: Yao Qi; +Cc: gdb-patches

> From: Yao Qi <qiyaoltc@gmail.com>
> Date: Tue, 9 May 2017 11:17:25 +0100
> Cc: "gdb-patches@sourceware.org" <gdb-patches@sourceware.org>
> 
> On Mon, May 8, 2017 at 4:01 PM, Eli Zaretskii <eliz@gnu.org> wrote:
> >        tui/tui.c: In function 'void tui_enable()':
> >        tui/tui.c:430:39: warning: ISO C++ forbids converting a string constant to 'char*' [-Wwrite-strings]
> >          s = newterm ("unknown", stdout, stdin);
> >                                               ^
> > I fixed this by explicitly casting "unknown" to 'char *' -- is that
> > the right fix?  Is it OK to push that fix?
> >
> 
> Yes, I think so, we've done this before
> https://sourceware.org/ml/gdb-patches/2017-04/msg00032.html

Thanks, pushed.

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

* Re: GDB 7.99.91 MinGW compilation error in cli-script.c
  2017-05-08 15:00 ` GDB 7.99.91 MinGW compilation error in cli-script.c Eli Zaretskii
@ 2017-05-14  3:19   ` Simon Marchi
  2017-05-14 14:13     ` Eli Zaretskii
  0 siblings, 1 reply; 56+ messages in thread
From: Simon Marchi @ 2017-05-14  3:19 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: gdb-patches

On 2017-05-08 11:00, Eli Zaretskii wrote:
> The reason is that std::to_string is guarded by the symbol
> _GLIBCXX_HAVE_BROKEN_VSWPRINTF, which mingw.org's MinGW defines in its
> os_defines.h, because msvcrt.dll's implementation of vswprintf is
> incompatible with what C++11 expects.  So GDB assumes here without
> testing that this method is available, which is not true at least on
> one platform.
> 
> How best to solve this?  I worked around by providing my own
> implementation based on std::ostringstream, but I'm not sure this is
> TRT.  An alternative would be to use some less problematic API, since
> currently cli-script.c is the only user of this method, and its needs
> are quite modest.  And if we do provide a replacement for to_string,
> should the configure script probe for it, or should we condition it
> specifically on MinGW and _GLIBCXX_HAVE_BROKEN_VSWPRINTF?
> 
> Thoughts?

I think the best solution would be a check at configure time.  I think 
it's a function that can be quite handy, so it would be unfortunate if 
we had to avoid using it, especially if it's easy to implement ourselves 
for MinGW.  A configure check would be more robust than checking for 
MinGW or _GLIBCXX_HAVE_BROKEN_VSWPRINTF specifically, in case another 
platform needs the replacement too, or if the define changes at some 
point.

Note that you'll need to check for the specific overload of the function 
that we use, the one that accepts a parameter of 
std::vector<T>::size_type (what .size() returns).  In practice, I think 
we can consider that it will always correspond to size_t.

Thanks,

Simon

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

* Re: GDB 7.99.91 MinGW compilation error in cli-script.c
  2017-05-14  3:19   ` Simon Marchi
@ 2017-05-14 14:13     ` Eli Zaretskii
  2017-05-17 14:31       ` Joel Brobecker
  0 siblings, 1 reply; 56+ messages in thread
From: Eli Zaretskii @ 2017-05-14 14:13 UTC (permalink / raw)
  To: Simon Marchi; +Cc: gdb-patches

> Date: Sat, 13 May 2017 23:19:06 -0400
> From: Simon Marchi <simon.marchi@polymtl.ca>
> Cc: gdb-patches@sourceware.org
> 
> > How best to solve this?  I worked around by providing my own
> > implementation based on std::ostringstream, but I'm not sure this is
> > TRT.  An alternative would be to use some less problematic API, since
> > currently cli-script.c is the only user of this method, and its needs
> > are quite modest.  And if we do provide a replacement for to_string,
> > should the configure script probe for it, or should we condition it
> > specifically on MinGW and _GLIBCXX_HAVE_BROKEN_VSWPRINTF?
> > 
> > Thoughts?
> 
> I think the best solution would be a check at configure time.  I think 
> it's a function that can be quite handy, so it would be unfortunate if 
> we had to avoid using it, especially if it's easy to implement ourselves 
> for MinGW.  A configure check would be more robust than checking for 
> MinGW or _GLIBCXX_HAVE_BROKEN_VSWPRINTF specifically, in case another 
> platform needs the replacement too, or if the define changes at some 
> point.

I've meanwhile learned that the latest release 5.0 of MinGW runtime
solves this problem.

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

* Re: GDB 7.99.91 available for testing
  2017-05-04 19:44 GDB 7.99.91 available for testing Joel Brobecker
                   ` (2 preceding siblings ...)
  2017-05-08 15:02 ` GDB 7.99.91 MinGW compilation warning in tui.c Eli Zaretskii
@ 2017-05-15 21:11 ` Simon Marchi
  2017-05-16 13:51   ` Simon Marchi
  2017-05-16 14:28   ` GDB 7.99.91 available for testing Joel Brobecker
  3 siblings, 2 replies; 56+ messages in thread
From: Simon Marchi @ 2017-05-15 21:11 UTC (permalink / raw)
  To: Joel Brobecker; +Cc: gdb-patches

On 2017-05-04 15:44, Joel Brobecker wrote:
> Hello,
> 
> I have just finished creating the gdb-7.99.91 pre-release.
> It is available for download at the following location:
> 
>     ftp://sourceware.org/pub/gdb/snapshots/branch/gdb-7.99.91.tar.xz
> 
> A gzip'ed version is also available: gdb-7.99.91.tar.gz.
> 
> Please give it a test if you can and report any problems you might 
> find.
> 
> On behalf of all the GDB contributors, thank you!

Hi Joel,

Somebody reported on IRC that the command "tty" (an alias for "set 
inferior-tty") didn't work anymore.  git bisect found one of my commits 
as the culprit:

   b593ecca856860a8b38deb808493bba4beef3aee
   Makefiles: Flatten and sort file lists

This is likely due to the _initialize functions being called in a 
different order.  I think this should block the 8.0 release.  I'll have 
time to investigate this tonight or tomorrow.

Thanks,

Simon

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

* Re: GDB 7.99.91 available for testing
  2017-05-15 21:11 ` GDB 7.99.91 available for testing Simon Marchi
@ 2017-05-16 13:51   ` Simon Marchi
  2017-05-16 20:50     ` Yao Qi
  2017-05-16 14:28   ` GDB 7.99.91 available for testing Joel Brobecker
  1 sibling, 1 reply; 56+ messages in thread
From: Simon Marchi @ 2017-05-16 13:51 UTC (permalink / raw)
  To: Joel Brobecker; +Cc: gdb-patches

On 2017-05-15 17:11, Simon Marchi wrote:
> Hi Joel,
> 
> Somebody reported on IRC that the command "tty" (an alias for "set
> inferior-tty") didn't work anymore.  git bisect found one of my
> commits as the culprit:
> 
>   b593ecca856860a8b38deb808493bba4beef3aee
>   Makefiles: Flatten and sort file lists
> 
> This is likely due to the _initialize functions being called in a
> different order.  I think this should block the 8.0 release.  I'll
> have time to investigate this tonight or tomorrow.
> 
> Thanks,
> 
> Simon

Hi,

So indeed, the problem is due to the ordering of the _initialize 
functions that changed, _initialize_printcmd used to be called before 
_initialize_infcmd, now it's the reverse.

When it works, the timeline of events to be able to get the tty alias 
working is the following:

1. The "set" command is registered as a prefix command in printcmd.c, 
which adds it to the global command list (cmdlist).  setlist is the list 
of its subcommands.
2. The "set inferior-tty" command is added as a child of "set" (i.e. 
it's inserted into setlist) in infcmd.c.
3. The "tty" alias is created for "set inferior-tty" in infcmd.c.  This 
looks up "set" in cmdlist, then "inferior-tty" in the subcommands of 
"set".  It's found and everyone is happy.

With the _initialize functions called in a different order, steps are 
executed in the order 2-3-1.  When we try to create the alias, the "set" 
command has not been created and is therefore not part of cmdlist.  We 
can't find the "set inferior-tty" command, so the alias is not created.

I think the core issue is that it's currently possible to create 
subcommands for prefixes that have not been created yet.  We need some 
way of ensuring some kind of ordering.  Here are some ideas:


1. Force some kind of ordering through init.c

It's already done for at least one file, if you look at gdb/Makefile.in, 
you'll see that the gdbtypes file is put at the beginning, so that it's 
called first.  We could do the same with printcmd.  This is not a 
solution I would like in the long term, but maybe it's good for the 8.0 
release, as it would minimize the required changes.

2. Add dependencies between _initialize_functions

We could have a way for _initialize functions to call each other.  For 
example, since we know that _initialize_printcmd must be called before 
_initialize_infcmd, the latter could call the former.  A static flag in 
each function could ensure that each is called only once.

I don't like this one very much, since the dependencies between 
functions have to be manually deduced, they can become outdated and it's 
easy to forget some.

3. Create prefix commands on demand

Right now, if you want to create a prefix command under "set", you just 
refer to &setlist, regardless of whether the "set" command has been 
created or not yet.  Instead of referring directly to setlist, we could 
call something like set_prefix_list, which would create the "set" 
command if it hasn't been created yet, and return setlist.  Something 
like that:

   cmd_list_element **set_prefix_list ()
   {
     static cmd_list_element *setlist = NULL;
     bool initialized = false;

     if (!initialized)
       {
         // Create the set command
         add_prefix_cmd ("set", ..., &setlist, ...);
       }

     return &setlist;
   }

References to &setlist would be changed to set_prefix_list ().  This 
encodes the dependencies directly in the code: if want to add a 
subcommand to "set", you need to call set_prefix_list to obtain the list 
of "set"'s children, ensuring that "set" has been created.  So the deps 
can't get stale, or you can't miss one by mistake.

So far this is the approach I like the most for the master branch.  
There are many ways the code could be improved further (C++, better 
encapsulation), but this would be a manageable first step.  But for the 
immediate needs of the imminent 8.0 release, I think #1 would be better.

Any comments?

Simon

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

* Re: GDB 7.99.91 available for testing
  2017-05-15 21:11 ` GDB 7.99.91 available for testing Simon Marchi
  2017-05-16 13:51   ` Simon Marchi
@ 2017-05-16 14:28   ` Joel Brobecker
  1 sibling, 0 replies; 56+ messages in thread
From: Joel Brobecker @ 2017-05-16 14:28 UTC (permalink / raw)
  To: Simon Marchi; +Cc: gdb-patches

> Somebody reported on IRC that the command "tty" (an alias for "set
> inferior-tty") didn't work anymore.  git bisect found one of my commits as
> the culprit:
> 
>   b593ecca856860a8b38deb808493bba4beef3aee
>   Makefiles: Flatten and sort file lists
> 
> This is likely due to the _initialize functions being called in a different
> order.  I think this should block the 8.0 release.  I'll have time to
> investigate this tonight or tomorrow.

Thanks for letting me know! I've updated the release's wiki page
to include this one.

-- 
Joel

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

* Re: GDB 7.99.91 available for testing
  2017-05-16 13:51   ` Simon Marchi
@ 2017-05-16 20:50     ` Yao Qi
  2017-05-16 21:22       ` Simon Marchi
  0 siblings, 1 reply; 56+ messages in thread
From: Yao Qi @ 2017-05-16 20:50 UTC (permalink / raw)
  To: Simon Marchi; +Cc: Joel Brobecker, gdb-patches

On 17-05-16 09:51:42, Simon Marchi wrote:
> 
> So indeed, the problem is due to the ordering of the _initialize functions
> that changed, _initialize_printcmd used to be called before
> _initialize_infcmd, now it's the reverse.
> 
> When it works, the timeline of events to be able to get the tty alias
> working is the following:
> 
> 1. The "set" command is registered as a prefix command in printcmd.c, which
> adds it to the global command list (cmdlist).  setlist is the list of its
> subcommands.
> 2. The "set inferior-tty" command is added as a child of "set" (i.e. it's
> inserted into setlist) in infcmd.c.
> 3. The "tty" alias is created for "set inferior-tty" in infcmd.c.  This
> looks up "set" in cmdlist, then "inferior-tty" in the subcommands of "set".
> It's found and everyone is happy.
> 
> With the _initialize functions called in a different order, steps are
> executed in the order 2-3-1.  When we try to create the alias, the "set"
> command has not been created and is therefore not part of cmdlist.  We can't
> find the "set inferior-tty" command, so the alias is not created.

When we try to create the alias (in step 3), setlist is already created
in step 2, but it is not linked with cmdlist.  The command "set
inferior-tty" is created in step 2 too.  Since step 2 and step 3 are
close to each other (in _initialize_infcmd), we can pass the "set
inferior-tty" cmd_list_element to the place we create alias, like this,

  cmd_name = "inferior-tty";
  c = lookup_cmd (&cmd_name, setlist, "", -1, 1);
  gdb_assert (c != NULL);
  add_alias_cmd ("tty", c, class_alias, 0, &cmdlist);

this makes sure we add alias "tty" under cmdlist, and it is alias to
"set inferior-tty".  Step 1 just links setlist and cmdlist together.

> 
> I think the core issue is that it's currently possible to create subcommands
> for prefixes that have not been created yet.  We need some way of ensuring
> some kind of ordering.  Here are some ideas:
> 

How is the patch below?  I run regress tests yet, and it still needs
some comments.

-- 
Yao


diff --git a/gdb/cli/cli-decode.c b/gdb/cli/cli-decode.c
index d45733e..bec5e36 100644
--- a/gdb/cli/cli-decode.c
+++ b/gdb/cli/cli-decode.c
@@ -284,16 +284,10 @@ deprecate_cmd (struct cmd_list_element *cmd, const char *replacement)
 }
 
 struct cmd_list_element *
-add_alias_cmd (const char *name, const char *oldname, enum command_class theclass,
-	       int abbrev_flag, struct cmd_list_element **list)
+add_alias_cmd (const char *name, cmd_list_element *old,
+	       enum command_class theclass, int abbrev_flag,
+	       struct cmd_list_element **list)
 {
-  const char *tmp;
-  struct cmd_list_element *old;
-  struct cmd_list_element *c;
-
-  tmp = oldname;
-  old = lookup_cmd (&tmp, *list, "", 1, 1);
-
   if (old == 0)
     {
       struct cmd_list_element *prehook, *prehookee, *posthook, *posthookee;
@@ -307,7 +301,7 @@ add_alias_cmd (const char *name, const char *oldname, enum command_class theclas
       return 0;
     }
 
-  c = add_cmd (name, theclass, NULL, old->doc, list);
+  struct cmd_list_element *c = add_cmd (name, theclass, NULL, old->doc, list);
 
   /* If OLD->DOC can be freed, we should make another copy.  */
   if (old->doc_allocated)
@@ -330,6 +324,21 @@ add_alias_cmd (const char *name, const char *oldname, enum command_class theclas
   return c;
 }
 
+struct cmd_list_element *
+add_alias_cmd (const char *name, const char *oldname, enum command_class theclass,
+	       int abbrev_flag, struct cmd_list_element **list)
+{
+  const char *tmp;
+  struct cmd_list_element *old;
+  struct cmd_list_element *c;
+
+  tmp = oldname;
+  old = lookup_cmd (&tmp, *list, "", 1, 1);
+
+  return add_alias_cmd (name, old, theclass, abbrev_flag, list);
+}
+
+
 /* Like add_cmd but adds an element for a command prefix: a name that
    should be followed by a subcommand to be looked up in another
    command list.  PREFIXLIST should be the address of the variable
diff --git a/gdb/command.h b/gdb/command.h
index ae20021..aa179e9 100644
--- a/gdb/command.h
+++ b/gdb/command.h
@@ -141,6 +141,12 @@ extern struct cmd_list_element *add_alias_cmd (const char *, const char *,
 					       enum command_class, int,
 					       struct cmd_list_element **);
 
+extern struct cmd_list_element *add_alias_cmd (const char *,
+					       cmd_list_element *,
+					       enum command_class, int,
+					       struct cmd_list_element **);
+
+
 extern struct cmd_list_element *add_prefix_cmd (const char *, enum command_class,
 						cmd_cfunc_ftype *fun,
 						const char *,
diff --git a/gdb/infcmd.c b/gdb/infcmd.c
index f42c6d1..09060b5 100644
--- a/gdb/infcmd.c
+++ b/gdb/infcmd.c
@@ -3210,7 +3210,10 @@ is restored."),
 				     set_inferior_tty_command,
 				     show_inferior_tty_command,
 				     &setlist, &showlist);
-  add_com_alias ("tty", "set inferior-tty", class_alias, 0);
+  cmd_name = "inferior-tty";
+  c = lookup_cmd (&cmd_name, setlist, "", -1, 1);
+  gdb_assert (c != NULL);
+  add_alias_cmd ("tty", c, class_alias, 0, &cmdlist);
 
   cmd_name = "args";
   add_setshow_string_noescape_cmd (cmd_name, class_run,

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

* Re: GDB 7.99.91 available for testing
  2017-05-16 20:50     ` Yao Qi
@ 2017-05-16 21:22       ` Simon Marchi
  2017-05-16 21:40         ` Yao Qi
  0 siblings, 1 reply; 56+ messages in thread
From: Simon Marchi @ 2017-05-16 21:22 UTC (permalink / raw)
  To: Yao Qi; +Cc: Joel Brobecker, gdb-patches

On 2017-05-16 16:49, Yao Qi wrote:
> On 17-05-16 09:51:42, Simon Marchi wrote:
>> 
>> So indeed, the problem is due to the ordering of the _initialize 
>> functions
>> that changed, _initialize_printcmd used to be called before
>> _initialize_infcmd, now it's the reverse.
>> 
>> When it works, the timeline of events to be able to get the tty alias
>> working is the following:
>> 
>> 1. The "set" command is registered as a prefix command in printcmd.c, 
>> which
>> adds it to the global command list (cmdlist).  setlist is the list of 
>> its
>> subcommands.
>> 2. The "set inferior-tty" command is added as a child of "set" (i.e. 
>> it's
>> inserted into setlist) in infcmd.c.
>> 3. The "tty" alias is created for "set inferior-tty" in infcmd.c.  
>> This
>> looks up "set" in cmdlist, then "inferior-tty" in the subcommands of 
>> "set".
>> It's found and everyone is happy.
>> 
>> With the _initialize functions called in a different order, steps are
>> executed in the order 2-3-1.  When we try to create the alias, the 
>> "set"
>> command has not been created and is therefore not part of cmdlist.  We 
>> can't
>> find the "set inferior-tty" command, so the alias is not created.
> 
> When we try to create the alias (in step 3), setlist is already created
> in step 2, but it is not linked with cmdlist.  The command "set
> inferior-tty" is created in step 2 too.  Since step 2 and step 3 are
> close to each other (in _initialize_infcmd), we can pass the "set
> inferior-tty" cmd_list_element to the place we create alias, like this,
> 
>   cmd_name = "inferior-tty";
>   c = lookup_cmd (&cmd_name, setlist, "", -1, 1);
>   gdb_assert (c != NULL);
>   add_alias_cmd ("tty", c, class_alias, 0, &cmdlist);
> 
> this makes sure we add alias "tty" under cmdlist, and it is alias to
> "set inferior-tty".  Step 1 just links setlist and cmdlist together.
> 
>> 
>> I think the core issue is that it's currently possible to create 
>> subcommands
>> for prefixes that have not been created yet.  We need some way of 
>> ensuring
>> some kind of ordering.  Here are some ideas:
>> 
> 
> How is the patch below?  I run regress tests yet, and it still needs
> some comments.

It's very clean, I like it.  I think it's good for master and 8.0.  Do 
you want to finish it or do you want me to pick it up?

I have enhanced gdb.base/set-inferior-tty.exp to exercise the alias, it 
could be added to your patch:

https://github.com/simark/binutils-gdb/commit/1a8bcb5dbc0dde3d8a4d3f73d0a057b3831902dd#diff-2f94d545bf202185ae6a6022ebfa93bc

Thanks,

Simon

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

* Re: GDB 7.99.91 available for testing
  2017-05-16 21:22       ` Simon Marchi
@ 2017-05-16 21:40         ` Yao Qi
  2017-05-17 11:31           ` [PATCH master/8.0] Add alias command to cmd_list_element Yao Qi
  0 siblings, 1 reply; 56+ messages in thread
From: Yao Qi @ 2017-05-16 21:40 UTC (permalink / raw)
  To: Simon Marchi; +Cc: Joel Brobecker, gdb-patches

On 17-05-16 17:22:21, Simon Marchi wrote:
> It's very clean, I like it.  I think it's good for master and 8.0.  Do you
> want to finish it or do you want me to pick it up?
> 
> I have enhanced gdb.base/set-inferior-tty.exp to exercise the alias, it
> could be added to your patch:
> 
> https://github.com/simark/binutils-gdb/commit/1a8bcb5dbc0dde3d8a4d3f73d0a057b3831902dd#diff-2f94d545bf202185ae6a6022ebfa93bc
>

I'll finish it tomorrow.

-- 
Yao

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

* [PATCH master/8.0] Add alias command to cmd_list_element
  2017-05-16 21:40         ` Yao Qi
@ 2017-05-17 11:31           ` Yao Qi
  2017-05-17 12:16             ` Simon Marchi
  0 siblings, 1 reply; 56+ messages in thread
From: Yao Qi @ 2017-05-17 11:31 UTC (permalink / raw)
  To: gdb-patches; +Cc: simon.marchi

When we add alias command, we call add_alias_cmd and pass the alias name
and command name.  This implicitly requires the command and its prefix
commands are already added to cmdlist.  This may not be true, for example,

  add_com_alias ("tty", "set inferior-tty", class_alias, 0);

"inferior-tty" command is added to setlist, but setlist may not be added
to cmdlist (It depends on the order of related _initialize_XXX functions
called) so that we can't find "set inferior-tty" from cmdlist.

This patch fixes this problem by pass cmd_list_element of "inferior-tty"
to add_alias_cmd, so that cmd_list_element of "inferior-tty" doesn't have
to be reachable from cmdlist at that moment.

Regression tested on x86_64-linux.

gdb:

2017-05-17  Yao Qi  <yao.qi@linaro.org>

	* cli/cli-decode.c (add_alias_cmd): New function.
	* command.h (add_alias_cmd): Declare.
	* infcmd.c (_initialize_infcmd): Don't call add_com_alias,
	instead call add_alias_cmd.

gdb/testsuite:

2017-05-17  Simon Marchi  <simon.marchi@ericsson.com>

	* gdb.base/set-inferior-tty.exp (test_set_inferior_tty): Add
	argument command.
	(top-level): Invoke test_set_inferior_tty.
---
 gdb/cli/cli-decode.c                        | 30 +++++++++++++++++++----------
 gdb/command.h                               |  6 ++++++
 gdb/infcmd.c                                |  5 ++++-
 gdb/testsuite/gdb.base/set-inferior-tty.exp | 10 ++++++----
 4 files changed, 36 insertions(+), 15 deletions(-)

diff --git a/gdb/cli/cli-decode.c b/gdb/cli/cli-decode.c
index d45733e..e1d257a 100644
--- a/gdb/cli/cli-decode.c
+++ b/gdb/cli/cli-decode.c
@@ -284,16 +284,10 @@ deprecate_cmd (struct cmd_list_element *cmd, const char *replacement)
 }
 
 struct cmd_list_element *
-add_alias_cmd (const char *name, const char *oldname, enum command_class theclass,
-	       int abbrev_flag, struct cmd_list_element **list)
+add_alias_cmd (const char *name, cmd_list_element *old,
+	       enum command_class theclass, int abbrev_flag,
+	       struct cmd_list_element **list)
 {
-  const char *tmp;
-  struct cmd_list_element *old;
-  struct cmd_list_element *c;
-
-  tmp = oldname;
-  old = lookup_cmd (&tmp, *list, "", 1, 1);
-
   if (old == 0)
     {
       struct cmd_list_element *prehook, *prehookee, *posthook, *posthookee;
@@ -307,7 +301,7 @@ add_alias_cmd (const char *name, const char *oldname, enum command_class theclas
       return 0;
     }
 
-  c = add_cmd (name, theclass, NULL, old->doc, list);
+  struct cmd_list_element *c = add_cmd (name, theclass, NULL, old->doc, list);
 
   /* If OLD->DOC can be freed, we should make another copy.  */
   if (old->doc_allocated)
@@ -330,6 +324,22 @@ add_alias_cmd (const char *name, const char *oldname, enum command_class theclas
   return c;
 }
 
+struct cmd_list_element *
+add_alias_cmd (const char *name, const char *oldname,
+	       enum command_class theclass, int abbrev_flag,
+	       struct cmd_list_element **list)
+{
+  const char *tmp;
+  struct cmd_list_element *old;
+  struct cmd_list_element *c;
+
+  tmp = oldname;
+  old = lookup_cmd (&tmp, *list, "", 1, 1);
+
+  return add_alias_cmd (name, old, theclass, abbrev_flag, list);
+}
+
+
 /* Like add_cmd but adds an element for a command prefix: a name that
    should be followed by a subcommand to be looked up in another
    command list.  PREFIXLIST should be the address of the variable
diff --git a/gdb/command.h b/gdb/command.h
index ae20021..aa179e9 100644
--- a/gdb/command.h
+++ b/gdb/command.h
@@ -141,6 +141,12 @@ extern struct cmd_list_element *add_alias_cmd (const char *, const char *,
 					       enum command_class, int,
 					       struct cmd_list_element **);
 
+extern struct cmd_list_element *add_alias_cmd (const char *,
+					       cmd_list_element *,
+					       enum command_class, int,
+					       struct cmd_list_element **);
+
+
 extern struct cmd_list_element *add_prefix_cmd (const char *, enum command_class,
 						cmd_cfunc_ftype *fun,
 						const char *,
diff --git a/gdb/infcmd.c b/gdb/infcmd.c
index f42c6d1..09060b5 100644
--- a/gdb/infcmd.c
+++ b/gdb/infcmd.c
@@ -3210,7 +3210,10 @@ is restored."),
 				     set_inferior_tty_command,
 				     show_inferior_tty_command,
 				     &setlist, &showlist);
-  add_com_alias ("tty", "set inferior-tty", class_alias, 0);
+  cmd_name = "inferior-tty";
+  c = lookup_cmd (&cmd_name, setlist, "", -1, 1);
+  gdb_assert (c != NULL);
+  add_alias_cmd ("tty", c, class_alias, 0, &cmdlist);
 
   cmd_name = "args";
   add_setshow_string_noescape_cmd (cmd_name, class_run,
diff --git a/gdb/testsuite/gdb.base/set-inferior-tty.exp b/gdb/testsuite/gdb.base/set-inferior-tty.exp
index d84d4a3..480cffa 100644
--- a/gdb/testsuite/gdb.base/set-inferior-tty.exp
+++ b/gdb/testsuite/gdb.base/set-inferior-tty.exp
@@ -21,20 +21,22 @@ if {[build_executable $testfile.exp $testfile ${srcfile} ${compile_options}] ==
     return -1
 }
 
-proc test_set_inferior_tty { } {
+proc test_set_inferior_tty { command } {
     global binfile
 
     clean_restart ${binfile}
 
-    gdb_test_no_output "set inferior-tty hello" "set inferior-tty to hello"
+    gdb_test_no_output "$command hello" "set inferior-tty to hello"
     gdb_test "show inferior-tty" \
 	     "Terminal for future runs of program being debugged is \"hello\"." \
 	     "show inferior-tty shows hello"
 
-    gdb_test_no_output "set inferior-tty" "set inferior-tty to empty"
+    gdb_test_no_output "$command" "set inferior-tty to empty"
     gdb_test "show inferior-tty" \
 	     "Terminal for future runs of program being debugged is \"\"." \
 	     "show inferior-tty shows empty"
 }
 
-test_set_inferior_tty
+foreach_with_prefix command {"set inferior-tty" "tty"} {
+    test_set_inferior_tty $command
+}
-- 
1.9.1

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

* Re: GDB 7.99.91 available for testing
  2017-05-08 14:53 ` Eli Zaretskii
@ 2017-05-17 11:36   ` Yao Qi
  2017-05-17 14:39     ` Eli Zaretskii
  0 siblings, 1 reply; 56+ messages in thread
From: Yao Qi @ 2017-05-17 11:36 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: Joel Brobecker, gdb-patches, sergiodj

Eli Zaretskii <eliz@gnu.org> writes:

> I've built this with MinGW and bumped into a few problems.  Two
> problems related to GDB sources, the rest to libiberty and readline.
> I will report the GDB problems in separate messages.  As to problems
> with libiberty and readline (all of them minor), what should I do to
> fix them in GDB in addition to reporting them to their respective
> upstream maintainers?  Should I push the fixes to the GDB repository,
> master and branch?

Hi Eli,
Can we add a MinGW buildslave to our buildbot?  We can install the
desired MinGW cross-toolchain on some Linux machine in gcc compile farm,
and start a buildslave instance there?  We only do the build.  This is
very helpful to find such build issues.

-- 
Yao (齐尧)

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

* Re: [PATCH master/8.0] Add alias command to cmd_list_element
  2017-05-17 11:31           ` [PATCH master/8.0] Add alias command to cmd_list_element Yao Qi
@ 2017-05-17 12:16             ` Simon Marchi
  2017-05-17 13:36               ` Yao Qi
  0 siblings, 1 reply; 56+ messages in thread
From: Simon Marchi @ 2017-05-17 12:16 UTC (permalink / raw)
  To: Yao Qi; +Cc: gdb-patches, simon.marchi

On 2017-05-17 07:31, Yao Qi wrote:
> When we add alias command, we call add_alias_cmd and pass the alias 
> name
> and command name.  This implicitly requires the command and its prefix
> commands are already added to cmdlist.  This may not be true, for 
> example,
> 
>   add_com_alias ("tty", "set inferior-tty", class_alias, 0);
> 
> "inferior-tty" command is added to setlist, but setlist may not be 
> added
> to cmdlist (It depends on the order of related _initialize_XXX 
> functions
> called) so that we can't find "set inferior-tty" from cmdlist.
> 
> This patch fixes this problem by pass cmd_list_element of 
> "inferior-tty"

"by passing"

> to add_alias_cmd, so that cmd_list_element of "inferior-tty" doesn't 
> have
> to be reachable from cmdlist at that moment.
> 
> Regression tested on x86_64-linux.
> 
> gdb:
> 
> 2017-05-17  Yao Qi  <yao.qi@linaro.org>
> 
> 	* cli/cli-decode.c (add_alias_cmd): New function.
> 	* command.h (add_alias_cmd): Declare.
> 	* infcmd.c (_initialize_infcmd): Don't call add_com_alias,
> 	instead call add_alias_cmd.
> 
> gdb/testsuite:
> 
> 2017-05-17  Simon Marchi  <simon.marchi@ericsson.com>
> 
> 	* gdb.base/set-inferior-tty.exp (test_set_inferior_tty): Add
> 	argument command.
> 	(top-level): Invoke test_set_inferior_tty.
> ---
>  gdb/cli/cli-decode.c                        | 30 
> +++++++++++++++++++----------
>  gdb/command.h                               |  6 ++++++
>  gdb/infcmd.c                                |  5 ++++-
>  gdb/testsuite/gdb.base/set-inferior-tty.exp | 10 ++++++----
>  4 files changed, 36 insertions(+), 15 deletions(-)
> 
> diff --git a/gdb/cli/cli-decode.c b/gdb/cli/cli-decode.c
> index d45733e..e1d257a 100644
> --- a/gdb/cli/cli-decode.c
> +++ b/gdb/cli/cli-decode.c
> @@ -284,16 +284,10 @@ deprecate_cmd (struct cmd_list_element *cmd,
> const char *replacement)
>  }
> 
>  struct cmd_list_element *
> -add_alias_cmd (const char *name, const char *oldname, enum
> command_class theclass,
> -	       int abbrev_flag, struct cmd_list_element **list)
> +add_alias_cmd (const char *name, cmd_list_element *old,
> +	       enum command_class theclass, int abbrev_flag,
> +	       struct cmd_list_element **list)
>  {
> -  const char *tmp;
> -  struct cmd_list_element *old;
> -  struct cmd_list_element *c;
> -
> -  tmp = oldname;
> -  old = lookup_cmd (&tmp, *list, "", 1, 1);
> -
>    if (old == 0)
>      {
>        struct cmd_list_element *prehook, *prehookee, *posthook, 
> *posthookee;
> @@ -307,7 +301,7 @@ add_alias_cmd (const char *name, const char
> *oldname, enum command_class theclas
>        return 0;
>      }
> 
> -  c = add_cmd (name, theclass, NULL, old->doc, list);
> +  struct cmd_list_element *c = add_cmd (name, theclass, NULL, 
> old->doc, list);
> 
>    /* If OLD->DOC can be freed, we should make another copy.  */
>    if (old->doc_allocated)
> @@ -330,6 +324,22 @@ add_alias_cmd (const char *name, const char
> *oldname, enum command_class theclas
>    return c;
>  }
> 
> +struct cmd_list_element *
> +add_alias_cmd (const char *name, const char *oldname,
> +	       enum command_class theclass, int abbrev_flag,
> +	       struct cmd_list_element **list)
> +{
> +  const char *tmp;
> +  struct cmd_list_element *old;
> +  struct cmd_list_element *c;

c is now unused.

Otherwise, LGTM.  Thanks!

Simon

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

* Re: [PATCH master/8.0] Add alias command to cmd_list_element
  2017-05-17 12:16             ` Simon Marchi
@ 2017-05-17 13:36               ` Yao Qi
  0 siblings, 0 replies; 56+ messages in thread
From: Yao Qi @ 2017-05-17 13:36 UTC (permalink / raw)
  To: Simon Marchi; +Cc: gdb-patches, simon.marchi

Simon Marchi <simon.marchi@polymtl.ca> writes:

Hi Simon,
Thanks for the quick review.

>> This patch fixes this problem by pass cmd_list_element of
>> "inferior-tty"
>
> "by passing"
>

Fixed.

>> +struct cmd_list_element *
>> +add_alias_cmd (const char *name, const char *oldname,
>> +	       enum command_class theclass, int abbrev_flag,
>> +	       struct cmd_list_element **list)
>> +{
>> +  const char *tmp;
>> +  struct cmd_list_element *old;
>> +  struct cmd_list_element *c;
>
> c is now unused.

Removed.

>
> Otherwise, LGTM.  Thanks!

Patch below is pushed to both master and 8.0 branch.

-- 
Yao (齐尧)

From: Yao Qi <yao.qi@linaro.org>
Date: Wed, 17 May 2017 14:22:04 +0100
Subject: [PATCH] Add alias command to cmd_list_element

When we add alias command, we call add_alias_cmd and pass the alias name
and command name.  This implicitly requires the command and its prefix
commands are already added to cmdlist.  This may not be true, for example,

  add_com_alias ("tty", "set inferior-tty", class_alias, 0);

"inferior-tty" command is added to setlist, but setlist may not be added
to cmdlist (It depends on the order of related _initialize_XXX functions
called) so that we can't find "set inferior-tty" from cmdlist.

This patch fixes this problem by passing cmd_list_element of "inferior-tty"
to add_alias_cmd, so that cmd_list_element of "inferior-tty" doesn't have
to be reachable from cmdlist at that moment.

gdb:

2017-05-17  Yao Qi  <yao.qi@linaro.org>

	* cli/cli-decode.c (add_alias_cmd): New function.
	* command.h (add_alias_cmd): Declare.
	* infcmd.c (_initialize_infcmd): Don't call add_com_alias,
	instead call add_alias_cmd.

gdb/testsuite:

2017-05-17  Simon Marchi  <simon.marchi@ericsson.com>

	* gdb.base/set-inferior-tty.exp (test_set_inferior_tty): Add
	argument command.
	(top-level): Invoke test_set_inferior_tty.

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index f2068d2..fdc2a40 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,3 +1,10 @@
+2017-05-17  Yao Qi  <yao.qi@linaro.org>
+
+	* cli/cli-decode.c (add_alias_cmd): New function.
+	* command.h (add_alias_cmd): Declare.
+	* infcmd.c (_initialize_infcmd): Don't call add_com_alias,
+	instead call add_alias_cmd.
+
 2017-05-17  Pedro Alves  <palves@redhat.com>
 
 	* Makefile.in (nat_extra_makefile_frag): Rename to ...
diff --git a/gdb/cli/cli-decode.c b/gdb/cli/cli-decode.c
index d45733e..d386d02 100644
--- a/gdb/cli/cli-decode.c
+++ b/gdb/cli/cli-decode.c
@@ -284,16 +284,10 @@ deprecate_cmd (struct cmd_list_element *cmd, const char *replacement)
 }
 
 struct cmd_list_element *
-add_alias_cmd (const char *name, const char *oldname, enum command_class theclass,
-	       int abbrev_flag, struct cmd_list_element **list)
+add_alias_cmd (const char *name, cmd_list_element *old,
+	       enum command_class theclass, int abbrev_flag,
+	       struct cmd_list_element **list)
 {
-  const char *tmp;
-  struct cmd_list_element *old;
-  struct cmd_list_element *c;
-
-  tmp = oldname;
-  old = lookup_cmd (&tmp, *list, "", 1, 1);
-
   if (old == 0)
     {
       struct cmd_list_element *prehook, *prehookee, *posthook, *posthookee;
@@ -307,7 +301,7 @@ add_alias_cmd (const char *name, const char *oldname, enum command_class theclas
       return 0;
     }
 
-  c = add_cmd (name, theclass, NULL, old->doc, list);
+  struct cmd_list_element *c = add_cmd (name, theclass, NULL, old->doc, list);
 
   /* If OLD->DOC can be freed, we should make another copy.  */
   if (old->doc_allocated)
@@ -330,6 +324,21 @@ add_alias_cmd (const char *name, const char *oldname, enum command_class theclas
   return c;
 }
 
+struct cmd_list_element *
+add_alias_cmd (const char *name, const char *oldname,
+	       enum command_class theclass, int abbrev_flag,
+	       struct cmd_list_element **list)
+{
+  const char *tmp;
+  struct cmd_list_element *old;
+
+  tmp = oldname;
+  old = lookup_cmd (&tmp, *list, "", 1, 1);
+
+  return add_alias_cmd (name, old, theclass, abbrev_flag, list);
+}
+
+
 /* Like add_cmd but adds an element for a command prefix: a name that
    should be followed by a subcommand to be looked up in another
    command list.  PREFIXLIST should be the address of the variable
diff --git a/gdb/command.h b/gdb/command.h
index ae20021..aa179e9 100644
--- a/gdb/command.h
+++ b/gdb/command.h
@@ -141,6 +141,12 @@ extern struct cmd_list_element *add_alias_cmd (const char *, const char *,
 					       enum command_class, int,
 					       struct cmd_list_element **);
 
+extern struct cmd_list_element *add_alias_cmd (const char *,
+					       cmd_list_element *,
+					       enum command_class, int,
+					       struct cmd_list_element **);
+
+
 extern struct cmd_list_element *add_prefix_cmd (const char *, enum command_class,
 						cmd_cfunc_ftype *fun,
 						const char *,
diff --git a/gdb/infcmd.c b/gdb/infcmd.c
index f42c6d1..09060b5 100644
--- a/gdb/infcmd.c
+++ b/gdb/infcmd.c
@@ -3210,7 +3210,10 @@ is restored."),
 				     set_inferior_tty_command,
 				     show_inferior_tty_command,
 				     &setlist, &showlist);
-  add_com_alias ("tty", "set inferior-tty", class_alias, 0);
+  cmd_name = "inferior-tty";
+  c = lookup_cmd (&cmd_name, setlist, "", -1, 1);
+  gdb_assert (c != NULL);
+  add_alias_cmd ("tty", c, class_alias, 0, &cmdlist);
 
   cmd_name = "args";
   add_setshow_string_noescape_cmd (cmd_name, class_run,
diff --git a/gdb/testsuite/ChangeLog b/gdb/testsuite/ChangeLog
index b672df3..91712e2 100644
--- a/gdb/testsuite/ChangeLog
+++ b/gdb/testsuite/ChangeLog
@@ -1,3 +1,9 @@
+2017-05-17  Simon Marchi  <simon.marchi@ericsson.com>
+
+	* gdb.base/set-inferior-tty.exp (test_set_inferior_tty): Add
+	argument command.
+	(top-level): Invoke test_set_inferior_tty.
+
 2017-05-04  Pedro Alves  <palves@redhat.com>
 
 	* gdb.python/py-record-btrace-threads.exp (check_insn_for_thread):
diff --git a/gdb/testsuite/gdb.base/set-inferior-tty.exp b/gdb/testsuite/gdb.base/set-inferior-tty.exp
index d84d4a3..480cffa 100644
--- a/gdb/testsuite/gdb.base/set-inferior-tty.exp
+++ b/gdb/testsuite/gdb.base/set-inferior-tty.exp
@@ -21,20 +21,22 @@ if {[build_executable $testfile.exp $testfile ${srcfile} ${compile_options}] ==
     return -1
 }
 
-proc test_set_inferior_tty { } {
+proc test_set_inferior_tty { command } {
     global binfile
 
     clean_restart ${binfile}
 
-    gdb_test_no_output "set inferior-tty hello" "set inferior-tty to hello"
+    gdb_test_no_output "$command hello" "set inferior-tty to hello"
     gdb_test "show inferior-tty" \
 	     "Terminal for future runs of program being debugged is \"hello\"." \
 	     "show inferior-tty shows hello"
 
-    gdb_test_no_output "set inferior-tty" "set inferior-tty to empty"
+    gdb_test_no_output "$command" "set inferior-tty to empty"
     gdb_test "show inferior-tty" \
 	     "Terminal for future runs of program being debugged is \"\"." \
 	     "show inferior-tty shows empty"
 }
 
-test_set_inferior_tty
+foreach_with_prefix command {"set inferior-tty" "tty"} {
+    test_set_inferior_tty $command
+}

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

* Re: GDB 7.99.91 MinGW compilation error in cli-script.c
  2017-05-14 14:13     ` Eli Zaretskii
@ 2017-05-17 14:31       ` Joel Brobecker
  2017-05-17 16:01         ` Eli Zaretskii
  0 siblings, 1 reply; 56+ messages in thread
From: Joel Brobecker @ 2017-05-17 14:31 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: Simon Marchi, gdb-patches

> > I think the best solution would be a check at configure time.  I think 
> > it's a function that can be quite handy, so it would be unfortunate if 
> > we had to avoid using it, especially if it's easy to implement ourselves 
> > for MinGW.  A configure check would be more robust than checking for 
> > MinGW or _GLIBCXX_HAVE_BROKEN_VSWPRINTF specifically, in case another 
> > platform needs the replacement too, or if the define changes at some 
> > point.
> 
> I've meanwhile learned that the latest release 5.0 of MinGW runtime
> solves this problem.

Version 5.0 was first release Oct of 2016, so about 6 months ago.
Is that sufficient to consider this issue as being resolved, or
do we consider it as blocking for the 8.0 release?

-- 
Joel

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

* Re: GDB 7.99.91 available for testing
  2017-05-17 11:36   ` Yao Qi
@ 2017-05-17 14:39     ` Eli Zaretskii
  0 siblings, 0 replies; 56+ messages in thread
From: Eli Zaretskii @ 2017-05-17 14:39 UTC (permalink / raw)
  To: Yao Qi; +Cc: brobecker, gdb-patches, sergiodj

> From: Yao Qi <qiyaoltc@gmail.com>
> Cc: Joel Brobecker <brobecker@adacore.com>,  gdb-patches@sourceware.org, sergiodj@redhat.com
> Date: Wed, 17 May 2017 12:36:46 +0100
> 
> Hi Eli,
> Can we add a MinGW buildslave to our buildbot?

I'm not sure how can I answer this question.  I certainly don't object
to this, and would love to see it coming on-line.  But I don't think I
can help doing this in any way, except encouraging and perhaps helping
by advice with some initial build issues.

> We can install the desired MinGW cross-toolchain on some Linux
> machine in gcc compile farm, and start a buildslave instance there?
> We only do the build.  This is very helpful to find such build
> issues.

I agree.  There's one complication: there are 2 divergent MinGW
distributions; most (all?) Linux distributions pick up the one that is
different from the one I use.  So it could be that the problems I find
will not be discovered even by such a bot.  Still, I think doing that
would be useful.

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

* Re: GDB 7.99.91 MinGW compilation error in cli-script.c
  2017-05-17 14:31       ` Joel Brobecker
@ 2017-05-17 16:01         ` Eli Zaretskii
  2017-05-19  9:10           ` Eli Zaretskii
  0 siblings, 1 reply; 56+ messages in thread
From: Eli Zaretskii @ 2017-05-17 16:01 UTC (permalink / raw)
  To: Joel Brobecker; +Cc: simon.marchi, gdb-patches

> Date: Wed, 17 May 2017 07:31:36 -0700
> From: Joel Brobecker <brobecker@adacore.com>
> Cc: Simon Marchi <simon.marchi@polymtl.ca>, gdb-patches@sourceware.org
> 
> > I've meanwhile learned that the latest release 5.0 of MinGW runtime
> > solves this problem.
> 
> Version 5.0 was first release Oct of 2016, so about 6 months ago.
> Is that sufficient to consider this issue as being resolved, or
> do we consider it as blocking for the 8.0 release?

No, I don't think this should block.  I will probably post later a
workaround patch for older versions, but that's for master.  I see no
reason to delay the release or to have the fix on the branch.

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

* Re: GDB 7.99.91 MinGW compilation warning in tui.c
  2017-05-13  8:12     ` Eli Zaretskii
@ 2017-05-17 16:26       ` Yao Qi
  2017-05-17 16:45         ` Eli Zaretskii
  0 siblings, 1 reply; 56+ messages in thread
From: Yao Qi @ 2017-05-17 16:26 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: gdb-patches

Eli Zaretskii <eliz@gnu.org> writes:

> Thanks, pushed.

Hi Eli,
Did you push it to 8.0 branch?  I think we need it in 8.0 branch.

-- 
Yao (齐尧)

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

* Re: GDB 7.99.91 MinGW compilation warning in tui.c
  2017-05-17 16:26       ` Yao Qi
@ 2017-05-17 16:45         ` Eli Zaretskii
  2017-05-17 18:21           ` Joel Brobecker
  0 siblings, 1 reply; 56+ messages in thread
From: Eli Zaretskii @ 2017-05-17 16:45 UTC (permalink / raw)
  To: Yao Qi; +Cc: gdb-patches

> From: Yao Qi <qiyaoltc@gmail.com>
> Cc: gdb-patches@sourceware.org
> Date: Wed, 17 May 2017 17:26:35 +0100
> 
> Eli Zaretskii <eliz@gnu.org> writes:
> 
> > Thanks, pushed.
> 
> Hi Eli,
> Did you push it to 8.0 branch?  I think we need it in 8.0 branch.

I pushed it to master.  Is it really serious enough to have a Bugzilla
report for it, have it listed on the wiki, etc.?

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

* Re: GDB 7.99.91 MinGW compilation warning in tui.c
  2017-05-17 16:45         ` Eli Zaretskii
@ 2017-05-17 18:21           ` Joel Brobecker
  2017-05-19  8:02             ` Eli Zaretskii
  0 siblings, 1 reply; 56+ messages in thread
From: Joel Brobecker @ 2017-05-17 18:21 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: Yao Qi, gdb-patches

> > Did you push it to 8.0 branch?  I think we need it in 8.0 branch.
> 
> I pushed it to master.  Is it really serious enough to have a Bugzilla
> report for it, have it listed on the wiki, etc.?

You only need that for the .1 (or any later release on that branch).
For the .0, it's OK to just push to the branch without any of the above.

Just to provide a reminder for those who haven't seen why we have
this procedure in place: It's to give users a way to assess the fixes
that were introduced between two minor releases on a given branch.
Eg: I have 8.0 installed on my machine, and today 8.1 just came out;
is there any point in me upgrading? For that, I would look at the
announcement, which would list all the PRs that were part of this
corrective release, look at the PRs that looks potentially interesting,
and would then be in a better position to decide. It also helps
me (as release manager) automatically generate the announcement for
the .1, making sure I don't miss some changes by accident.

For the .0, it's a major release, so the information we provide
about the release is coarser. I normally write a summary of
the NEWS file with a reference to it for more information. Also,
several contributors still decide that it's worth creating PRs
and then set the milestone to the correct release once fixed. This
gives more information about the specific problems they fixed.
But that part is optional at this point.

-- 
Joel

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

* Re: GDB 7.99.91 MinGW compilation warning in tui.c
  2017-05-17 18:21           ` Joel Brobecker
@ 2017-05-19  8:02             ` Eli Zaretskii
  2017-05-19  9:54               ` Pedro Alves
  2017-05-19 17:50               ` DJ Delorie
  0 siblings, 2 replies; 56+ messages in thread
From: Eli Zaretskii @ 2017-05-19  8:02 UTC (permalink / raw)
  To: Joel Brobecker; +Cc: qiyaoltc, gdb-patches

> Date: Wed, 17 May 2017 11:21:11 -0700
> From: Joel Brobecker <brobecker@adacore.com>
> Cc: Yao Qi <qiyaoltc@gmail.com>, gdb-patches@sourceware.org
> 
> > > Did you push it to 8.0 branch?  I think we need it in 8.0 branch.
> > 
> > I pushed it to master.  Is it really serious enough to have a Bugzilla
> > report for it, have it listed on the wiki, etc.?
> 
> You only need that for the .1 (or any later release on that branch).
> For the .0, it's OK to just push to the branch without any of the above.

Right, sorry for forgetting (again).  Thanks for reminding me.

I've now cherry-picked the patch to the 8.0 branch.

Btw, what should I do about the issues I reported against libiberty?
I got no responses for my reports.  Should I just go ahead and fix our
copy of libiberty?

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

* Re: GDB 7.99.91 MinGW compilation error in cli-script.c
  2017-05-17 16:01         ` Eli Zaretskii
@ 2017-05-19  9:10           ` Eli Zaretskii
  2017-05-19  9:49             ` Pedro Alves
  0 siblings, 1 reply; 56+ messages in thread
From: Eli Zaretskii @ 2017-05-19  9:10 UTC (permalink / raw)
  To: brobecker, simon.marchi; +Cc: gdb-patches

> Date: Wed, 17 May 2017 19:01:23 +0300
> From: Eli Zaretskii <eliz@gnu.org>
> CC: simon.marchi@polymtl.ca, gdb-patches@sourceware.org
> 
> I will probably post later a workaround patch for older versions,
> but that's for master.  I see no reason to delay the release or to
> have the fix on the branch.

Is the below OK?  It's a bit ugly, and a small fix is needed even for
the latest MinGW runtime.

Contrary to what I wrote, maybe it's also should be considered for the
branch.  Let me know what you think.

diff --git a/gdb/cli/cli-script.c b/gdb/cli/cli-script.c
index e0e27ef..ec33a37 100644
--- a/gdb/cli/cli-script.c
+++ b/gdb/cli/cli-script.c
@@ -17,6 +17,23 @@
    You should have received a copy of the GNU General Public License
    along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
 
+#if defined __MINGW32__
+#  include <_mingw.h>
+#  ifndef __MINGW64_VERSION_MAJOR
+// For versions of mingw.org's MinGW runtime before 5.0, make sure
+// libstdc++ headers don't omit portions that require C99.
+#    if __MINGW32_MAJOR_VERSION < 5
+#      ifndef _GLIBCXX_USE_C99
+#        define _GLIBCXX_USE_C99 1
+#      endif
+#    else
+// This is required with mingwrt-5.0 to avoid linking errors due to
+// multiple definitions of vsnprintf.
+#      undef __USE_MINGW_ANSI_STDIO
+#    endif
+#  endif
+#endif
+
 #include "defs.h"
 #include "value.h"
 #include "language.h"		/* For value_true */

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

* Re: GDB 7.99.91 MinGW compilation error in cli-script.c
  2017-05-19  9:10           ` Eli Zaretskii
@ 2017-05-19  9:49             ` Pedro Alves
  2017-05-19 11:17               ` Eli Zaretskii
  0 siblings, 1 reply; 56+ messages in thread
From: Pedro Alves @ 2017-05-19  9:49 UTC (permalink / raw)
  To: Eli Zaretskii, brobecker, simon.marchi; +Cc: gdb-patches

On 05/19/2017 10:10 AM, Eli Zaretskii wrote:
>> Date: Wed, 17 May 2017 19:01:23 +0300
>> From: Eli Zaretskii <eliz@gnu.org>
>> CC: simon.marchi@polymtl.ca, gdb-patches@sourceware.org
>>
>> I will probably post later a workaround patch for older versions,
>> but that's for master.  I see no reason to delay the release or to
>> have the fix on the branch.
> 
> Is the below OK?  It's a bit ugly, and a small fix is needed even for
> the latest MinGW runtime.

Messing with libstdc++ internal macros and changing what libstdc++
defines is recipe for undefined behavior and trouble.  I'd prefer
avoiding it if possible.

How about going back to the original gdb::to_string, implemented
in terms of stringstring:

 https://sourceware.org/ml/gdb-patches/2016-10/msg00535.html

... with a tweak to only use if it the host compiler doesn't support
std::to_string, like the patchlet below.  (It's functionally equivalent,
it's just that std::to_string is potentially cheaper.)   We'd use
gdb::to_string instead of std::to_string until compilers that miss
std::to_string are phased out.

From bf3b8c9199965d15ddf8f0435fab6b6af3b08bda Mon Sep 17 00:00:00 2001
From: Pedro Alves <palves@redhat.com>
Date: Fri, 19 May 2017 10:34:51 +0100
Subject: [PATCH] to_string

---
 gdb/cli/cli-script.c      |  2 +-
 gdb/common/common-utils.h | 25 +++++++++++++++++++++++++
 2 files changed, 26 insertions(+), 1 deletion(-)

diff --git a/gdb/cli/cli-script.c b/gdb/cli/cli-script.c
index e0e27ef..3a2ae15 100644
--- a/gdb/cli/cli-script.c
+++ b/gdb/cli/cli-script.c
@@ -806,7 +806,7 @@ user_args::insert_args (const char *line) const
 
       if (p[4] == 'c')
 	{
-	  new_line += std::to_string (m_args.size ());
+	  new_line += gdb::to_string (m_args.size ());
 	  line = p + 5;
 	}
       else
diff --git a/gdb/common/common-utils.h b/gdb/common/common-utils.h
index c331f0d..2c35de8 100644
--- a/gdb/common/common-utils.h
+++ b/gdb/common/common-utils.h
@@ -22,6 +22,7 @@
 
 #include <string>
 #include <vector>
+#include <sstream>
 
 /* If possible, define FUNCTION_NAME, a macro containing the name of
    the function being defined.  Since this macro may not always be
@@ -63,6 +64,30 @@ int xsnprintf (char *str, size_t size, const char *format, ...)
 std::string string_printf (const char* fmt, ...)
   ATTRIBUTE_PRINTF (1, 2);
 
+/* Returns a string representation of VAL.  Replacement for C++11
+   std::to_string on hosts that miss it.  */
+
+namespace gdb {
+
+#if 0 // some check here
+
+template <class T>
+inline std::string
+to_string (const T &val)
+{
+  std::stringstream ss;
+
+  ss << val;
+  return ss.str ();
+}
+
+#else
+
+using std::to_string;
+
+#endif
+
+}
 /* Make a copy of the string at PTR with LEN characters
    (and add a null character at the end in the copy).
    Uses malloc to get the space.  Returns the address of the copy.  */
-- 
2.5.5


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

* Re: GDB 7.99.91 MinGW compilation warning in tui.c
  2017-05-19  8:02             ` Eli Zaretskii
@ 2017-05-19  9:54               ` Pedro Alves
  2017-05-19 11:14                 ` Eli Zaretskii
  2017-05-19 17:50               ` DJ Delorie
  1 sibling, 1 reply; 56+ messages in thread
From: Pedro Alves @ 2017-05-19  9:54 UTC (permalink / raw)
  To: Eli Zaretskii, Joel Brobecker; +Cc: qiyaoltc, gdb-patches

On 05/19/2017 09:01 AM, Eli Zaretskii wrote:

> Btw, what should I do about the issues I reported against libiberty?
> I got no responses for my reports. 

Someone needs to turn the reports into patches.

> Should I just go ahead and fix our copy of libiberty?

I think you should send patches to gcc first.

Thanks,
Pedro Alves

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

* Re: GDB 7.99.91 MinGW compilation warning in tui.c
  2017-05-19  9:54               ` Pedro Alves
@ 2017-05-19 11:14                 ` Eli Zaretskii
  2017-05-19 11:25                   ` Pedro Alves
  0 siblings, 1 reply; 56+ messages in thread
From: Eli Zaretskii @ 2017-05-19 11:14 UTC (permalink / raw)
  To: Pedro Alves; +Cc: brobecker, qiyaoltc, gdb-patches

> Cc: qiyaoltc@gmail.com, gdb-patches@sourceware.org
> From: Pedro Alves <palves@redhat.com>
> Date: Fri, 19 May 2017 10:54:00 +0100
> 
> On 05/19/2017 09:01 AM, Eli Zaretskii wrote:
> 
> > Btw, what should I do about the issues I reported against libiberty?
> > I got no responses for my reports. 
> 
> Someone needs to turn the reports into patches.

I did.

> > Should I just go ahead and fix our copy of libiberty?
> 
> I think you should send patches to gcc first.

I did.

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

* Re: GDB 7.99.91 MinGW compilation error in cli-script.c
  2017-05-19  9:49             ` Pedro Alves
@ 2017-05-19 11:17               ` Eli Zaretskii
  2017-05-19 11:23                 ` Pedro Alves
  0 siblings, 1 reply; 56+ messages in thread
From: Eli Zaretskii @ 2017-05-19 11:17 UTC (permalink / raw)
  To: Pedro Alves; +Cc: brobecker, simon.marchi, gdb-patches

> Cc: gdb-patches@sourceware.org
> From: Pedro Alves <palves@redhat.com>
> Date: Fri, 19 May 2017 10:48:57 +0100
> 
> On 05/19/2017 10:10 AM, Eli Zaretskii wrote:
> >> Date: Wed, 17 May 2017 19:01:23 +0300
> >> From: Eli Zaretskii <eliz@gnu.org>
> >> CC: simon.marchi@polymtl.ca, gdb-patches@sourceware.org
> >>
> >> I will probably post later a workaround patch for older versions,
> >> but that's for master.  I see no reason to delay the release or to
> >> have the fix on the branch.
> > 
> > Is the below OK?  It's a bit ugly, and a small fix is needed even for
> > the latest MinGW runtime.
> 
> Messing with libstdc++ internal macros and changing what libstdc++
> defines is recipe for undefined behavior and trouble.  I'd prefer
> avoiding it if possible.
> 
> How about going back to the original gdb::to_string, implemented
> in terms of stringstring:
> 
>  https://sourceware.org/ml/gdb-patches/2016-10/msg00535.html
> 
> ... with a tweak to only use if it the host compiler doesn't support
> std::to_string, like the patchlet below.

Fine with me, but Someone(TM) should figure out what to use instead of
"#if 0".

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

* Re: GDB 7.99.91 MinGW compilation error in cli-script.c
  2017-05-19 11:17               ` Eli Zaretskii
@ 2017-05-19 11:23                 ` Pedro Alves
  2017-05-21 15:33                   ` Eli Zaretskii
  0 siblings, 1 reply; 56+ messages in thread
From: Pedro Alves @ 2017-05-19 11:23 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: brobecker, simon.marchi, gdb-patches

On 05/19/2017 12:17 PM, Eli Zaretskii wrote:

> Fine with me, but Someone(TM) should figure out what to use instead of
> "#if 0".
> 

Something around
__MINGW32__ / _GLIBCXX_USE_C99 / _GLIBCXX_HAVE_BROKEN_VSWPRINTF ?

Thanks,
Pedro Alves

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

* Re: GDB 7.99.91 MinGW compilation warning in tui.c
  2017-05-19 11:14                 ` Eli Zaretskii
@ 2017-05-19 11:25                   ` Pedro Alves
  2017-05-19 12:51                     ` Eli Zaretskii
  0 siblings, 1 reply; 56+ messages in thread
From: Pedro Alves @ 2017-05-19 11:25 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: brobecker, qiyaoltc, gdb-patches

On 05/19/2017 12:14 PM, Eli Zaretskii wrote:
>> Cc: qiyaoltc@gmail.com, gdb-patches@sourceware.org
>> From: Pedro Alves <palves@redhat.com>
>> Date: Fri, 19 May 2017 10:54:00 +0100
>>
>> On 05/19/2017 09:01 AM, Eli Zaretskii wrote:
>>
>>> Btw, what should I do about the issues I reported against libiberty?
>>> I got no responses for my reports. 
>>
>> Someone needs to turn the reports into patches.
> 
> I did.
> 
>>> Should I just go ahead and fix our copy of libiberty?
>>
>> I think you should send patches to gcc first.
> 
> I did.
> 

I'm not seeing any patches here:

 https://gcc.gnu.org/ml/gcc-patches/2017-05/msg00555.html
 https://gcc.gnu.org/ml/gcc-patches/2017-05/msg00556.html
 https://gcc.gnu.org/ml/gcc-patches/2017-05/msg00557.html

Am I missing some other emails?

Thanks,
Pedro Alves

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

* Re: GDB 7.99.91 MinGW compilation warning in tui.c
  2017-05-19 11:25                   ` Pedro Alves
@ 2017-05-19 12:51                     ` Eli Zaretskii
  2017-05-19 13:58                       ` Pedro Alves
  0 siblings, 1 reply; 56+ messages in thread
From: Eli Zaretskii @ 2017-05-19 12:51 UTC (permalink / raw)
  To: Pedro Alves; +Cc: brobecker, qiyaoltc, gdb-patches

> Cc: brobecker@adacore.com, qiyaoltc@gmail.com, gdb-patches@sourceware.org
> From: Pedro Alves <palves@redhat.com>
> Date: Fri, 19 May 2017 12:25:25 +0100
> 
> >> I think you should send patches to gcc first.
> > 
> > I did.
> > 
> 
> I'm not seeing any patches here:
> 
>  https://gcc.gnu.org/ml/gcc-patches/2017-05/msg00555.html
>  https://gcc.gnu.org/ml/gcc-patches/2017-05/msg00556.html
>  https://gcc.gnu.org/ml/gcc-patches/2017-05/msg00557.html
> 
> Am I missing some other emails?

No.  The patches are there, they are just not in the Diff format.  I
don't have write access to the GCC repository anyway, so describing a
bunch of simple patches sounded appropriate.  And since no one
responded, I'm not sure it's worth my while to make any further
efforts.

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

* Re: GDB 7.99.91 MinGW compilation warning in tui.c
  2017-05-19 12:51                     ` Eli Zaretskii
@ 2017-05-19 13:58                       ` Pedro Alves
  2017-05-19 14:40                         ` Eli Zaretskii
  0 siblings, 1 reply; 56+ messages in thread
From: Pedro Alves @ 2017-05-19 13:58 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: brobecker, qiyaoltc, gdb-patches

On 05/19/2017 01:51 PM, Eli Zaretskii wrote:

> No.  The patches are there, they are just not in the Diff format.

A description of a patch is not a patch, in my book.

>  I
> don't have write access to the GCC repository anyway, so describing a
> bunch of simple patches sounded appropriate.  And since no one
> responded, I'm not sure it's worth my while to make any further
> efforts.

"A picture is worth a thousand words" kind of applies to patches too.
You seem to assume that someone would take the descriptions you
gave, turn them into actual code, make sure things still compile
on their favorite host plus that it actually fixes what it's supposed
to fix on MinGW, write some ChangeLogs, etc.  And maybe what you
describe as simple turns out to not be so simple, etc.  Others will be
wondering whether it'll be worth their efforts to engage too, especially
those that don't use Windows which I think is the vast majority in
these circles.  If you attach/post a real diff/patch, showing exactly
and unambiguously what it is that what you want to get in, then I
expect you'd get quicker attention.

You've asked

  "Should I just go ahead and fix our copy of libiberty?"

So you've already implied that you'd do the work to
actually write the patch and the ChangeLogs.  

So I don't understand the reluctance of posting an actual diff
to gcc-patches.

Thanks,
Pedro Alves

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

* Re: GDB 7.99.91 MinGW compilation warning in tui.c
  2017-05-19 13:58                       ` Pedro Alves
@ 2017-05-19 14:40                         ` Eli Zaretskii
  0 siblings, 0 replies; 56+ messages in thread
From: Eli Zaretskii @ 2017-05-19 14:40 UTC (permalink / raw)
  To: Pedro Alves; +Cc: brobecker, qiyaoltc, gdb-patches

> Cc: brobecker@adacore.com, qiyaoltc@gmail.com, gdb-patches@sourceware.org
> From: Pedro Alves <palves@redhat.com>
> Date: Fri, 19 May 2017 14:58:54 +0100
> 
> You seem to assume that someone would take the descriptions you
> gave, turn them into actual code, make sure things still compile
> on their favorite host plus that it actually fixes what it's supposed
> to fix on MinGW, write some ChangeLogs, etc.

It's not unheard of.  E.g., I do that for Emacs all the time.  And the
changes are so simple there shouldn't be any doubt about correctness
in  this case.

> So I don't understand the reluctance of posting an actual diff
> to gcc-patches.

The lack of responses is not exactly encouraging.  Would you mind
asking them to pay attention?

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

* Re: GDB 7.99.91 MinGW compilation warning in tui.c
  2017-05-19  8:02             ` Eli Zaretskii
  2017-05-19  9:54               ` Pedro Alves
@ 2017-05-19 17:50               ` DJ Delorie
  2017-05-19 17:55                 ` Eli Zaretskii
  1 sibling, 1 reply; 56+ messages in thread
From: DJ Delorie @ 2017-05-19 17:50 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: brobecker, qiyaoltc, gdb-patches


Eli Zaretskii <eliz@gnu.org> writes:
> Btw, what should I do about the issues I reported against libiberty?
> I got no responses for my reports.  Should I just go ahead and fix our
> copy of libiberty?

They're on my list, just had a busy week!

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

* Re: GDB 7.99.91 MinGW compilation warning in tui.c
  2017-05-19 17:50               ` DJ Delorie
@ 2017-05-19 17:55                 ` Eli Zaretskii
  0 siblings, 0 replies; 56+ messages in thread
From: Eli Zaretskii @ 2017-05-19 17:55 UTC (permalink / raw)
  To: DJ Delorie; +Cc: brobecker, qiyaoltc, gdb-patches

> From: DJ Delorie <dj@redhat.com>
> Cc: brobecker@adacore.com, qiyaoltc@gmail.com, gdb-patches@sourceware.org
> Date: Fri, 19 May 2017 13:50:41 -0400
> 
> Eli Zaretskii <eliz@gnu.org> writes:
> > Btw, what should I do about the issues I reported against libiberty?
> > I got no responses for my reports.  Should I just go ahead and fix our
> > copy of libiberty?
> 
> They're on my list, just had a busy week!

Sorry for my impatience, and thanks in advance.

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

* Re: GDB 7.99.91 MinGW compilation error in cli-script.c
  2017-05-19 11:23                 ` Pedro Alves
@ 2017-05-21 15:33                   ` Eli Zaretskii
  2017-05-22 15:26                     ` Pedro Alves
  0 siblings, 1 reply; 56+ messages in thread
From: Eli Zaretskii @ 2017-05-21 15:33 UTC (permalink / raw)
  To: Pedro Alves; +Cc: brobecker, simon.marchi, gdb-patches

> Cc: brobecker@adacore.com, simon.marchi@polymtl.ca, gdb-patches@sourceware.org
> From: Pedro Alves <palves@redhat.com>
> Date: Fri, 19 May 2017 12:23:06 +0100
> 
> On 05/19/2017 12:17 PM, Eli Zaretskii wrote:
> 
> > Fine with me, but Someone(TM) should figure out what to use instead of
> > "#if 0".
> > 
> 
> Something around
> __MINGW32__ / _GLIBCXX_USE_C99 / _GLIBCXX_HAVE_BROKEN_VSWPRINTF ?

Here's what I came up with.  OK to commit?

--- gdb/common/common-utils.h~0	2017-04-17 17:09:57.000000000 +0300
+++ gdb/common/common-utils.h	2017-05-21 10:01:22.219168100 +0300
@@ -22,6 +22,7 @@
 
 #include <string>
 #include <vector>
+#include <sstream>
 
 /* If possible, define FUNCTION_NAME, a macro containing the name of
    the function being defined.  Since this macro may not always be
@@ -63,6 +64,42 @@
 std::string string_printf (const char* fmt, ...)
   ATTRIBUTE_PRINTF (1, 2);
 
+/* Returns a string representation of VAL.  Replacement for C++11
+   std::to_string for hosts that lack it.  */
+
+namespace gdb {
+
+#define REPLACE_TO_STRING 0
+
+#ifdef __MINGW32__
+# include <_mingw.h>
+# ifndef _GLIBCXX_USE_C99
+#  undef REPLACE_TO_STRING
+#  define REPLACE_TO_STRING 1
+# endif
+#endif
+
+#if REPLACE_TO_STRING
+
+template <class T>
+inline std::string
+to_string (const T &val)
+{
+  std::stringstream ss;
+
+  ss << val;
+  return ss.str ();
+}
+
+#else /* !REPLACE_TO_STRING */
+
+using std::to_string;
+
+#endif
+
+#undef REPLACE_TO_STRING
+}
+
 /* Make a copy of the string at PTR with LEN characters
    (and add a null character at the end in the copy).
    Uses malloc to get the space.  Returns the address of the copy.  */
--- gdb/cli/cli-script.c~0	2017-04-17 17:09:57.000000000 +0300
+++ gdb/cli/cli-script.c	2017-05-21 09:36:17.610252500 +0300
@@ -806,7 +818,7 @@ user_args::insert_args (const char *line
 
       if (p[4] == 'c')
 	{
-	  new_line += std::to_string (m_args.size ());
+	  new_line += gdb::to_string (m_args.size ());
 	  line = p + 5;
 	}
       else

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

* Re: GDB 7.99.91 MinGW compilation error in cli-script.c
  2017-05-21 15:33                   ` Eli Zaretskii
@ 2017-05-22 15:26                     ` Pedro Alves
  2017-05-22 18:43                       ` Eli Zaretskii
  0 siblings, 1 reply; 56+ messages in thread
From: Pedro Alves @ 2017-05-22 15:26 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: brobecker, simon.marchi, gdb-patches

On 05/21/2017 04:33 PM, Eli Zaretskii wrote:
>> Cc: brobecker@adacore.com, simon.marchi@polymtl.ca, gdb-patches@sourceware.org
>> From: Pedro Alves <palves@redhat.com>
>> Date: Fri, 19 May 2017 12:23:06 +0100
>>
>> On 05/19/2017 12:17 PM, Eli Zaretskii wrote:
>>
>>> Fine with me, but Someone(TM) should figure out what to use instead of
>>> "#if 0".
>>>
>>
>> Something around
>> __MINGW32__ / _GLIBCXX_USE_C99 / _GLIBCXX_HAVE_BROKEN_VSWPRINTF ?
> 
> Here's what I came up with.  OK to commit?

Is there still a reason for the "#include <_mingw.h>"?
I thought that was needed in the previous version for
__MINGW64_VERSION_MAJOR, but this version of the patch doesn't
use that symbol.

Otherwise, if

- older broken mingw releases get the replacement
- newer fixed mingw releases don't get the replacement
- mingw-w64 doesn't get the replacement (as it doesn't need one IIUC)

then it's fine with me.

It'd be useful for the archives if you expanded on which mingw versions
and compilers you tested this on.  Also likewise a short comment to
the effect in the code would be likewise handy for future readers
(please use /**/ style comments).

Thanks,
Pedro Alves

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

* Re: GDB 7.99.91 MinGW compilation error in cli-script.c
  2017-05-22 15:26                     ` Pedro Alves
@ 2017-05-22 18:43                       ` Eli Zaretskii
  2017-05-23  9:53                         ` Pedro Alves
  0 siblings, 1 reply; 56+ messages in thread
From: Eli Zaretskii @ 2017-05-22 18:43 UTC (permalink / raw)
  To: Pedro Alves; +Cc: brobecker, simon.marchi, gdb-patches

> Cc: brobecker@adacore.com, simon.marchi@polymtl.ca, gdb-patches@sourceware.org
> From: Pedro Alves <palves@redhat.com>
> Date: Mon, 22 May 2017 16:26:19 +0100
> 
> Is there still a reason for the "#include <_mingw.h>"?

Yes, that's where _GLIBCXX_USE_C99 is defined.

> Otherwise, if
> 
> - older broken mingw releases get the replacement
> - newer fixed mingw releases don't get the replacement
> - mingw-w64 doesn't get the replacement (as it doesn't need one IIUC)
> 
> then it's fine with me.

Hmmm... I see that MinGW64 doesn't define _GLIBCXX_USE_C99 in its
system headers, so I guess I will have to add the
__MINGW64_VERSION_MAJOR guard as well.

> It'd be useful for the archives if you expanded on which mingw versions
> and compilers you tested this on.  Also likewise a short comment to
> the effect in the code would be likewise handy for future readers

OK, will do.

> (please use /**/ style comments).

I thought I did...

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

* Re: GDB 7.99.91 MinGW compilation error in cli-script.c
  2017-05-22 18:43                       ` Eli Zaretskii
@ 2017-05-23  9:53                         ` Pedro Alves
  2017-05-24  2:44                           ` Eli Zaretskii
  2017-05-24 18:28                           ` Eli Zaretskii
  0 siblings, 2 replies; 56+ messages in thread
From: Pedro Alves @ 2017-05-23  9:53 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: brobecker, simon.marchi, gdb-patches

On 05/22/2017 07:42 PM, Eli Zaretskii wrote:
>> Cc: brobecker@adacore.com, simon.marchi@polymtl.ca, gdb-patches@sourceware.org
>> From: Pedro Alves <palves@redhat.com>
>> Date: Mon, 22 May 2017 16:26:19 +0100
>>
>> Is there still a reason for the "#include <_mingw.h>"?
> 
> Yes, that's where _GLIBCXX_USE_C99 is defined.

Urgh...

> 
>> Otherwise, if
>>
>> - older broken mingw releases get the replacement
>> - newer fixed mingw releases don't get the replacement
>> - mingw-w64 doesn't get the replacement (as it doesn't need one IIUC)
>>
>> then it's fine with me.
> 
> Hmmm... I see that MinGW64 doesn't define _GLIBCXX_USE_C99 in its
> system headers, 

I'm surprised mingw does this, because that's a libstdc++
internal symbol...

> so I guess I will have to add the
> __MINGW64_VERSION_MAJOR guard as well.

Please also take a look at the fix for:

 https://gcc.gnu.org/bugzilla/show_bug.cgi?id=58393

which suggests to me that newer compilers against older mingw
might actually be fixed, independently of the _GLIBCXX_USE_C99 hack?

> 
>> It'd be useful for the archives if you expanded on which mingw versions
>> and compilers you tested this on.  Also likewise a short comment to
>> the effect in the code would be likewise handy for future readers
> 
> OK, will do.
> 
>> (please use /**/ style comments).
> 
> I thought I did...

The comments you had added in the original patch used // style:

 +// For versions of mingw.org's MinGW runtime before 5.0, make sure
 +// libstdc++ headers don't omit portions that require C99.

Thanks,
Pedro Alves

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

* Re: GDB 7.99.91 MinGW compilation error in cli-script.c
  2017-05-23  9:53                         ` Pedro Alves
@ 2017-05-24  2:44                           ` Eli Zaretskii
  2017-05-25 10:05                             ` Pedro Alves
  2017-05-24 18:28                           ` Eli Zaretskii
  1 sibling, 1 reply; 56+ messages in thread
From: Eli Zaretskii @ 2017-05-24  2:44 UTC (permalink / raw)
  To: Pedro Alves; +Cc: brobecker, simon.marchi, gdb-patches

> Cc: brobecker@adacore.com, simon.marchi@polymtl.ca, gdb-patches@sourceware.org
> From: Pedro Alves <palves@redhat.com>
> Date: Tue, 23 May 2017 10:53:47 +0100
> 
> I'm surprised mingw does this, because that's a libstdc++
> internal symbol...

I'm guessing that was done because releases of MinGW runtime and the
MinGW port of GCC are not in sync.  So people who have a GCC
installation and upgrade to a later MinGW runtime expect to have this
problem solved, but the way you seem to assume tjis to happen is by
them having to build an updated GCC or wait for an update of the GCC
distribution.

> Please also take a look at the fix for:
> 
>  https://gcc.gnu.org/bugzilla/show_bug.cgi?id=58393
> 
> which suggests to me that newer compilers against older mingw
> might actually be fixed, independently of the _GLIBCXX_USE_C99 hack?

mingw.org's MinGW currently doesn't offer GCC newer than 5.3.0, so
this solution is not yet available to MinGW users.  Maybe soon it will
be.

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

* Re: GDB 7.99.91 MinGW compilation error in cli-script.c
  2017-05-23  9:53                         ` Pedro Alves
  2017-05-24  2:44                           ` Eli Zaretskii
@ 2017-05-24 18:28                           ` Eli Zaretskii
  2017-05-24 19:37                             ` Joel Brobecker
  1 sibling, 1 reply; 56+ messages in thread
From: Eli Zaretskii @ 2017-05-24 18:28 UTC (permalink / raw)
  To: Pedro Alves; +Cc: brobecker, simon.marchi, gdb-patches

> Cc: brobecker@adacore.com, simon.marchi@polymtl.ca, gdb-patches@sourceware.org
> From: Pedro Alves <palves@redhat.com>
> Date: Tue, 23 May 2017 10:53:47 +0100
> 
> >> It'd be useful for the archives if you expanded on which mingw versions
> >> and compilers you tested this on.  Also likewise a short comment to
> >> the effect in the code would be likewise handy for future readers
> > 
> > OK, will do.

Here's the final version; okay to commit?

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index a96e71f..d3a251b 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,3 +1,10 @@
+2017-05-24  Eli Zaretskii  <eliz@gnu.org>
+
+	* common/common-utils.h (REPLACE_TO_STRING) [__MINGW32__]: Define
+	to 1 if std::to_string is not available.
+	(std::to_string) [REPLACE_TO_STRING]: Provide a replacement
+	implementation.
+
 2017-05-24  Yao Qi  <yao.qi@linaro.org>
 
 	* rs6000-tdep.c (gdb_print_insn_powerpc): Remove.
diff --git a/gdb/cli/cli-script.c b/gdb/cli/cli-script.c
index e0e27ef..2e5e397 100644
--- a/gdb/cli/cli-script.c
+++ b/gdb/cli/cli-script.c
@@ -806,7 +818,7 @@ user_args::insert_args (const char *line) const
 
       if (p[4] == 'c')
 	{
-	  new_line += std::to_string (m_args.size ());
+	  new_line += gdb::to_string (m_args.size ());
 	  line = p + 5;
 	}
       else
diff --git a/gdb/common/common-utils.h b/gdb/common/common-utils.h
index c331f0d..e3cd66e 100644
--- a/gdb/common/common-utils.h
+++ b/gdb/common/common-utils.h
@@ -22,6 +22,7 @@
 
 #include <string>
 #include <vector>
+#include <sstream>
 
 /* If possible, define FUNCTION_NAME, a macro containing the name of
    the function being defined.  Since this macro may not always be
@@ -63,6 +64,47 @@ int xsnprintf (char *str, size_t size, const char *format, ...)
 std::string string_printf (const char* fmt, ...)
   ATTRIBUTE_PRINTF (1, 2);
 
+/* Returns a string representation of VAL.  Replacement for C++11
+   std::to_string for hosts that lack it.  */
+
+namespace gdb {
+
+#define REPLACE_TO_STRING 0
+
+#ifdef __MINGW32__
+/* mingw.org's MinGW runtime before version 5.0 needs the replacement
+   below.  Tested with mingwrt-3.22.2 and mingwrt-5.0, and with
+   libstdc++ included with MinGW GCC 5.3.0.  */
+# include <_mingw.h>
+/* We test __MINGW64_VERSION_MAJOR to exclude MinGW64, which doesn't
+   need this replacement.  */
+# if !defined __MINGW64_VERSION_MAJOR && !defined _GLIBCXX_USE_C99
+#  undef REPLACE_TO_STRING
+#  define REPLACE_TO_STRING 1
+# endif
+#endif
+
+#if REPLACE_TO_STRING
+
+template <class T>
+inline std::string
+to_string (const T &val)
+{
+  std::stringstream ss;
+
+  ss << val;
+  return ss.str ();
+}
+
+#else /* !REPLACE_TO_STRING */
+
+using std::to_string;
+
+#endif
+
+#undef REPLACE_TO_STRING
+}
+
 /* Make a copy of the string at PTR with LEN characters
    (and add a null character at the end in the copy).
    Uses malloc to get the space.  Returns the address of the copy.  */

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

* Re: GDB 7.99.91 MinGW compilation error in cli-script.c
  2017-05-24 18:28                           ` Eli Zaretskii
@ 2017-05-24 19:37                             ` Joel Brobecker
  2017-05-25 10:12                               ` Pedro Alves
  0 siblings, 1 reply; 56+ messages in thread
From: Joel Brobecker @ 2017-05-24 19:37 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: Pedro Alves, simon.marchi, gdb-patches

> Here's the final version; okay to commit?
> 
> diff --git a/gdb/ChangeLog b/gdb/ChangeLog
> index a96e71f..d3a251b 100644
> --- a/gdb/ChangeLog
> +++ b/gdb/ChangeLog
> @@ -1,3 +1,10 @@
> +2017-05-24  Eli Zaretskii  <eliz@gnu.org>
> +
> +	* common/common-utils.h (REPLACE_TO_STRING) [__MINGW32__]: Define
> +	to 1 if std::to_string is not available.
> +	(std::to_string) [REPLACE_TO_STRING]: Provide a replacement
> +	implementation.

Did we consider the option of perhaps only pushing this patch
to the 8.0 branch, and require MinGW 5.x for the current master?
If we did it that way, it would allow us to avoid remembering
that we need to use gdb::to_string instead of std::to_string.


-- 
Joel

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

* Re: GDB 7.99.91 MinGW compilation error in cli-script.c
  2017-05-24  2:44                           ` Eli Zaretskii
@ 2017-05-25 10:05                             ` Pedro Alves
  2017-05-26  7:57                               ` Eli Zaretskii
  0 siblings, 1 reply; 56+ messages in thread
From: Pedro Alves @ 2017-05-25 10:05 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: brobecker, simon.marchi, gdb-patches

On 05/24/2017 03:44 AM, Eli Zaretskii wrote:
>> Cc: brobecker@adacore.com, simon.marchi@polymtl.ca, gdb-patches@sourceware.org
>> From: Pedro Alves <palves@redhat.com>
>> Date: Tue, 23 May 2017 10:53:47 +0100
>>
>> I'm surprised mingw does this, because that's a libstdc++
>> internal symbol...
> 
> I'm guessing that was done because releases of MinGW runtime and the
> MinGW port of GCC are not in sync.  So people who have a GCC
> installation and upgrade to a later MinGW runtime expect to have this
> problem solved, but the way you seem to assume tjis to happen is by
> them having to build an updated GCC or wait for an update of the GCC
> distribution.

Your use of "have to wait" above doesn't make sense to me, to be
honest.  mingw.org maintainers chose to update the runtime and not
the compiler.  It's their choice.  Users have to update some component to
fix this, but the decision that the component that needs updating is
the runtime is mingw.org maintainer's.  It would have been equally possible to
provide an updated GCC release of the exact same GCC version / vintage
that defined _GLIBCXX_USE_C99 itself (or some other similar localized
hack or even a backport of the proper fix upstream) instead of
defining a libstdc++ internal symbol in the runtime and potentially
causing trouble when/if folks update to newer GCCs that
treat _GLIBCXX_USE_C99 differently, exactly because, as you say,
the runtime and the compiler are not released in sync.

> 
>> Please also take a look at the fix for:
>>
>>  https://gcc.gnu.org/bugzilla/show_bug.cgi?id=58393
>>
>> which suggests to me that newer compilers against older mingw
>> might actually be fixed, independently of the _GLIBCXX_USE_C99 hack?
> 
> mingw.org's MinGW currently doesn't offer GCC newer than 5.3.0, so
> this solution is not yet available to MinGW users.  Maybe soon it will
> be.

The point was that the compiler fix will make std::to_string available
even when _GLIBCXX_USE_C99 ends up not defined, AFAICS.  Meaning that with
newer compilers we'll replace std::to_string when it's not necessary,
AFAICS, given:

 +#ifdef __MINGW32__
 +# include <_mingw.h>
 +# ifndef _GLIBCXX_USE_C99
 +#  undef REPLACE_TO_STRING
 +#  define REPLACE_TO_STRING 1
 +# endif
 +#endif

From the ChangeLog entry seen on that bugzilla url, I'd think that
skipping the replacement when GLIBCXX_USE_C99_STDIO is defined
would work.

But guess we'll just revisit when/if someone ever notices [to_string
replaced when it didn't need to], or when we stop supporting compilers
that had this broken, whichever comes first.

I'll go look at your newer patch.

Thanks,
Pedro Alves

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

* Re: GDB 7.99.91 MinGW compilation error in cli-script.c
  2017-05-24 19:37                             ` Joel Brobecker
@ 2017-05-25 10:12                               ` Pedro Alves
  2017-05-26  7:47                                 ` Eli Zaretskii
  0 siblings, 1 reply; 56+ messages in thread
From: Pedro Alves @ 2017-05-25 10:12 UTC (permalink / raw)
  To: Joel Brobecker, Eli Zaretskii; +Cc: simon.marchi, gdb-patches

On 05/24/2017 08:37 PM, Joel Brobecker wrote:

>> +2017-05-24  Eli Zaretskii  <eliz@gnu.org>
>> +
>> +	* common/common-utils.h (REPLACE_TO_STRING) [__MINGW32__]: Define
>> +	to 1 if std::to_string is not available.

This:

>> +	(std::to_string) [REPLACE_TO_STRING]: Provide a replacement
>> +	implementation.

Should really be:

	(gdb::to_string): Define.

and you need an entry for the cli/cli-script.c change, like:

        * cli/cli-script.c (user_args::insert_args): Use it.

Otherwise this version of the patch is OK with me.

> Did we consider the option of perhaps only pushing this patch
> to the 8.0 branch, and require MinGW 5.x for the current master?
> If we did it that way, it would allow us to avoid remembering
> that we need to use gdb::to_string instead of std::to_string.

That'd be fine with me, FWIW.

Thanks,
Pedro Alves

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

* Re: GDB 7.99.91 MinGW compilation error in cli-script.c
  2017-05-25 10:12                               ` Pedro Alves
@ 2017-05-26  7:47                                 ` Eli Zaretskii
  2017-05-26 10:54                                   ` Pedro Alves
  0 siblings, 1 reply; 56+ messages in thread
From: Eli Zaretskii @ 2017-05-26  7:47 UTC (permalink / raw)
  To: Pedro Alves; +Cc: brobecker, simon.marchi, gdb-patches

> Cc: simon.marchi@polymtl.ca, gdb-patches@sourceware.org
> From: Pedro Alves <palves@redhat.com>
> Date: Thu, 25 May 2017 11:12:22 +0100
> 
> This:
> 
> >> +	(std::to_string) [REPLACE_TO_STRING]: Provide a replacement
> >> +	implementation.
> 
> Should really be:
> 
> 	(gdb::to_string): Define.

The code says std::to_string, though.  So it sounds like some coding
conventions are being applied here of which I wasn't aware, and
neither is Emacs.  Are these conventions described somewhere?

> and you need an entry for the cli/cli-script.c change, like:
> 
>         * cli/cli-script.c (user_args::insert_args): Use it.

I added that, but once again, the convention to put the
fully-qualified symbol name in the log entry should be documented, if
it isn't already, because Emacs doesn't do that, at least not by
default.  Is this convention applied consistently across the project?

> > Did we consider the option of perhaps only pushing this patch
> > to the 8.0 branch, and require MinGW 5.x for the current master?
> > If we did it that way, it would allow us to avoid remembering
> > that we need to use gdb::to_string instead of std::to_string.
> 
> That'd be fine with me, FWIW.

I wasn't sure what it means "to require MinGW 5.x", in practice, but I
pushed only to the branch for now.

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

* Re: GDB 7.99.91 MinGW compilation error in cli-script.c
  2017-05-25 10:05                             ` Pedro Alves
@ 2017-05-26  7:57                               ` Eli Zaretskii
  2017-05-26 11:52                                 ` Pedro Alves
  0 siblings, 1 reply; 56+ messages in thread
From: Eli Zaretskii @ 2017-05-26  7:57 UTC (permalink / raw)
  To: Pedro Alves; +Cc: brobecker, simon.marchi, gdb-patches

> Cc: brobecker@adacore.com, simon.marchi@polymtl.ca, gdb-patches@sourceware.org
> From: Pedro Alves <palves@redhat.com>
> Date: Thu, 25 May 2017 11:05:00 +0100
> 
> >> I'm surprised mingw does this, because that's a libstdc++
> >> internal symbol...
> > 
> > I'm guessing that was done because releases of MinGW runtime and the
> > MinGW port of GCC are not in sync.  So people who have a GCC
> > installation and upgrade to a later MinGW runtime expect to have this
> > problem solved, but the way you seem to assume tjis to happen is by
> > them having to build an updated GCC or wait for an update of the GCC
> > distribution.
> 
> Your use of "have to wait" above doesn't make sense to me, to be
> honest.  mingw.org maintainers chose to update the runtime and not
> the compiler.  It's their choice.  Users have to update some component to
> fix this, but the decision that the component that needs updating is
> the runtime is mingw.org maintainer's.  It would have been equally possible to
> provide an updated GCC release of the exact same GCC version / vintage
> that defined _GLIBCXX_USE_C99 itself (or some other similar localized
> hack or even a backport of the proper fix upstream) instead of
> defining a libstdc++ internal symbol in the runtime and potentially
> causing trouble when/if folks update to newer GCCs that
> treat _GLIBCXX_USE_C99 differently, exactly because, as you say,
> the runtime and the compiler are not released in sync.

I think there's some misunderstanding here.  The MinGW runtime needed
to be updated in any case, because the root cause of not defining
_GLIBCXX_USE_C99 for MinGW was a deficiency in the MinGW runtime: a
specific stdio function in the MS runtime wasn't behaving in
ANSI-compliant way, and needed a replacement for it to be added to
MinGW.  The _GLIBCXX_USE_C99 macro was undefined in the libstdc++
headers because of that problem, and the decision whether to define it
is made at libstdc++ build time, based on the MinGW runtime that was
present at that time.

Once that change in the MinGW runtime was made, there were two
possible ways of fixing the to_string problem in libstdc++:

  . build and release a new distribution of GCC, and tell everyone
    that they must upgrade their GCC, together with MinGW runtime; or

  . force _GLIBCXX_USE_C99 to be defined in the MinGW runtime headers

The MinGW maintainers decided to do the latter, most probably because
it is easier on both users and the maintainers.

> > mingw.org's MinGW currently doesn't offer GCC newer than 5.3.0, so
> > this solution is not yet available to MinGW users.  Maybe soon it will
> > be.
> 
> The point was that the compiler fix will make std::to_string available
> even when _GLIBCXX_USE_C99 ends up not defined, AFAICS.  Meaning that with
> newer compilers we'll replace std::to_string when it's not necessary,
> AFAICS, given:
> 
>  +#ifdef __MINGW32__
>  +# include <_mingw.h>
>  +# ifndef _GLIBCXX_USE_C99
>  +#  undef REPLACE_TO_STRING
>  +#  define REPLACE_TO_STRING 1
>  +# endif
>  +#endif
> 
> >From the ChangeLog entry seen on that bugzilla url, I'd think that
> skipping the replacement when GLIBCXX_USE_C99_STDIO is defined
> would work.

I'm okay with adding the newer symbols to the patch I already
installed, if that's what you want.  the latest GCC that's currently
available for MinGW is 5.3.0, where these newer symbols don't yet
appear.

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

* Re: GDB 7.99.91 MinGW compilation error in cli-script.c
  2017-05-26  7:47                                 ` Eli Zaretskii
@ 2017-05-26 10:54                                   ` Pedro Alves
  2017-05-26 13:03                                     ` Eli Zaretskii
  0 siblings, 1 reply; 56+ messages in thread
From: Pedro Alves @ 2017-05-26 10:54 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: brobecker, simon.marchi, gdb-patches

On 05/26/2017 08:46 AM, Eli Zaretskii wrote:
>> Cc: simon.marchi@polymtl.ca, gdb-patches@sourceware.org
>> From: Pedro Alves <palves@redhat.com>
>> Date: Thu, 25 May 2017 11:12:22 +0100
>>
>> This:
>>
>>>> +	(std::to_string) [REPLACE_TO_STRING]: Provide a replacement
>>>> +	implementation.
>>
>> Should really be:
>>
>> 	(gdb::to_string): Define.
> 
> The code says std::to_string, though.  So it sounds like some coding
> conventions are being applied here of which I wasn't aware, and
> neither is Emacs.  Are these conventions described somewhere?

Just the standard GNU conventions.  The code is defining a new
function template called gdb::to_string.  Simplified:

 namespace gdb {
   template <class T> std::string to_string (const T &val);
 }

There are two implementations of that, one for mingw, written
as a new function template in place.  And another which is
importing std::to_string into the gdb namespace.  But whatever
the implementations, it's implementation detail of gdb::to_string.

So gdb::to_string is meant as a std::to_string replacement, yes.
But that's not "what", that's "why".

This is really the same as if the patch looked like this:

+ namespace gdb {
+   std::string tostr(int i)
+   {
+ #ifdef MINGW
+     // something using sprintf;
+ #else
+     return std::to_string (i);
+ #endif
+   }
+ }

The ChangeLog entry would be something like:

 	(gdb::tostr): Define.

too, not:

 	(std::to_string): Provide a replacement.

And the entry is not conditional on MINGW, since we're
adding both #ifdef and #ifndef implementations at the same
time.

> 
>> and you need an entry for the cli/cli-script.c change, like:
>>
>>         * cli/cli-script.c (user_args::insert_args): Use it.
> 
> I added that, 

Thank you.

> but once again, the convention to put the
> fully-qualified symbol name in the log entry should be documented, if
> it isn't already, because Emacs doesn't do that, at least not by
> default.  

I can't see how what Emacs does has any bearing here, since AFAIK,
Emacs isn't written in C++.  Here there's no namespace at all,
and "insert_args" is a method of the user_args class.  So I find
the above a natural a concise way to write the symbol's name.

> Is this convention applied consistently across the project?

Sure, grep ChangeLog for "::".

Thanks,
Pedro Alves

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

* Re: GDB 7.99.91 MinGW compilation error in cli-script.c
  2017-05-26  7:57                               ` Eli Zaretskii
@ 2017-05-26 11:52                                 ` Pedro Alves
  2017-05-26 12:57                                   ` Eli Zaretskii
  0 siblings, 1 reply; 56+ messages in thread
From: Pedro Alves @ 2017-05-26 11:52 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: brobecker, simon.marchi, gdb-patches

On 05/26/2017 08:57 AM, Eli Zaretskii wrote:
>> Cc: brobecker@adacore.com, simon.marchi@polymtl.ca, gdb-patches@sourceware.org
>> From: Pedro Alves <palves@redhat.com>
>> Date: Thu, 25 May 2017 11:05:00 +0100
>>
>>>> I'm surprised mingw does this, because that's a libstdc++
>>>> internal symbol...
>>>
>>> I'm guessing that was done because releases of MinGW runtime and the
>>> MinGW port of GCC are not in sync.  So people who have a GCC
>>> installation and upgrade to a later MinGW runtime expect to have this
>>> problem solved, but the way you seem to assume tjis to happen is by
>>> them having to build an updated GCC or wait for an update of the GCC
>>> distribution.
>>
>> Your use of "have to wait" above doesn't make sense to me, to be
>> honest.  mingw.org maintainers chose to update the runtime and not
>> the compiler.  It's their choice.  Users have to update some component to
>> fix this, but the decision that the component that needs updating is
>> the runtime is mingw.org maintainer's.  It would have been equally possible to
>> provide an updated GCC release of the exact same GCC version / vintage
>> that defined _GLIBCXX_USE_C99 itself (or some other similar localized
>> hack or even a backport of the proper fix upstream) instead of
>> defining a libstdc++ internal symbol in the runtime and potentially
>> causing trouble when/if folks update to newer GCCs that
>> treat _GLIBCXX_USE_C99 differently, exactly because, as you say,
>> the runtime and the compiler are not released in sync.
> 
> I think there's some misunderstanding here.  

Maybe.

> The MinGW runtime needed
> to be updated in any case, because the root cause of not defining
> _GLIBCXX_USE_C99 for MinGW was a deficiency in the MinGW runtime: a
> specific stdio function in the MS runtime wasn't behaving in
> ANSI-compliant way, and needed a replacement for it to be added to
> MinGW.  

My understanding of the issue is that libstdc++ had a too-coarse way
to tell whether the runtime supports C99, and that ended up disabling
std::to_string because some unrelated (to std::to_string) bits of C99
support are missing in mingw.org.

Specifically, wide string functions are not really necessary
for std::to_string.  Having non-conforming C99 math-related bits also
disabled std::to_string (see bug I pointed at), again
because _GLIBCXX_USE_C99 is too coarse-grained.

So forcing _GLIBCXX_USE_C99 exposes std::to_string, but also
exposes other C99 broken / non-conforming bits.

> The _GLIBCXX_USE_C99 macro was undefined in the libstdc++
> headers because of that problem, and the decision whether to define it
> is made at libstdc++ build time, based on the MinGW runtime that was
> present at that time.

Yes.

> 
> Once that change in the MinGW runtime was made, there were two
> possible ways of fixing the to_string problem in libstdc++:
> 
>   . build and release a new distribution of GCC, and tell everyone
>     that they must upgrade their GCC, together with MinGW runtime; or
> 
>   . force _GLIBCXX_USE_C99 to be defined in the MinGW runtime headers
> 
> The MinGW maintainers decided to do the latter, most probably because
> it is easier on both users and the maintainers.

I don't think the fix on the GCC side really requires a mingw runtime
update.  I may well be wrong, of course.

>>> mingw.org's MinGW currently doesn't offer GCC newer than 5.3.0, so
>>> this solution is not yet available to MinGW users.  Maybe soon it will
>>> be.
>>
>> The point was that the compiler fix will make std::to_string available
>> even when _GLIBCXX_USE_C99 ends up not defined, AFAICS.  Meaning that with
>> newer compilers we'll replace std::to_string when it's not necessary,
>> AFAICS, given:
>>
>>  +#ifdef __MINGW32__
>>  +# include <_mingw.h>
>>  +# ifndef _GLIBCXX_USE_C99
>>  +#  undef REPLACE_TO_STRING
>>  +#  define REPLACE_TO_STRING 1
>>  +# endif
>>  +#endif
>>
>> >From the ChangeLog entry seen on that bugzilla url, I'd think that
>> skipping the replacement when GLIBCXX_USE_C99_STDIO is defined
>> would work.
> 
> I'm okay with adding the newer symbols to the patch I already
> installed, if that's what you want.  the latest GCC that's currently
> available for MinGW is 5.3.0, where these newer symbols don't yet
> appear.
> 

I cloned the current mingw sources to see what's been done
over there, and here's what's there now (on the 5.0-active branch):

#if _ISOC99_SOURCE && __cplusplus >= 201103L && __GNUC__ < 6
 /* Due to a configuration defect in GCC versions prior to GCC-6, when
  * compiling C++11 code, the ISO-C99 functions may not be incorporated
  * into the appropriate namespace(s); we may be able to mitigate this,
  * by ensuring that these GCC configuration macros are defined.
  */
# define _GLIBCXX_USE_C99       1
# define _GLIBCXX_HAVE_WCSTOF   1
#endif

I would have helped a lot if I had been shown this.  Note the 
"__GNUC__ < 6" check.  So with newer compilers (yes, there's no
official binaries, but other people can and do build their own
compilers) _GLIBCXX_USE_C99 won't be hacked in.
So I think we'll define the replacement replacement when we needn't.
But in any case, I'll just ignore that.
If you're happy with the gdb fix in place now, I'm happy enough
with it too.

Thanks,
Pedro Alves

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

* Re: GDB 7.99.91 MinGW compilation error in cli-script.c
  2017-05-26 11:52                                 ` Pedro Alves
@ 2017-05-26 12:57                                   ` Eli Zaretskii
  2017-05-26 13:58                                     ` Pedro Alves
  0 siblings, 1 reply; 56+ messages in thread
From: Eli Zaretskii @ 2017-05-26 12:57 UTC (permalink / raw)
  To: Pedro Alves; +Cc: brobecker, simon.marchi, gdb-patches

> Cc: brobecker@adacore.com, simon.marchi@polymtl.ca, gdb-patches@sourceware.org
> From: Pedro Alves <palves@redhat.com>
> Date: Fri, 26 May 2017 12:52:27 +0100
> 
> My understanding of the issue is that libstdc++ had a too-coarse way
> to tell whether the runtime supports C99, and that ended up disabling
> std::to_string because some unrelated (to std::to_string) bits of C99
> support are missing in mingw.org.

That's another problem, but it doesn't matter in the case of MinGW,
because the root cause was indeed affecting std::to_string, AFAIU.

> I don't think the fix on the GCC side really requires a mingw runtime
> update.  I may well be wrong, of course.

I think you are wrong.  In the libstdc++ 5.3.0 distribution, the
offending defines are in include/c++/mingw32/bits/c++config.h, which
AFAIU is generated at libstdc++ build time.

> I cloned the current mingw sources to see what's been done
> over there, and here's what's there now (on the 5.0-active branch):

You could have just downloaded the MinGW runtime from here:

  https://sourceforge.net/projects/mingw/files/MinGW/Base/mingwrt/mingwrt-5.0/libmingwex-5.0-mingw32-dev.tar.xz/download

> #if _ISOC99_SOURCE && __cplusplus >= 201103L && __GNUC__ < 6
>  /* Due to a configuration defect in GCC versions prior to GCC-6, when
>   * compiling C++11 code, the ISO-C99 functions may not be incorporated
>   * into the appropriate namespace(s); we may be able to mitigate this,
>   * by ensuring that these GCC configuration macros are defined.
>   */
> # define _GLIBCXX_USE_C99       1
> # define _GLIBCXX_HAVE_WCSTOF   1
> #endif
> 
> I would have helped a lot if I had been shown this.  Note the 
> "__GNUC__ < 6" check.

Hey, that's unfair!  You expect me to look at the MinGW64 headers and
at related Bugzilla bugs, but you yourself cannot look in the MinGW
headers?

> If you're happy with the gdb fix in place now, I'm happy enough
> with it too.

I'm happy.  Any problems with MinGW GCC 6.x don't matter as long as
there's no such GCC version on the MinGW site.

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

* Re: GDB 7.99.91 MinGW compilation error in cli-script.c
  2017-05-26 10:54                                   ` Pedro Alves
@ 2017-05-26 13:03                                     ` Eli Zaretskii
  2017-05-26 14:10                                       ` Pedro Alves
  0 siblings, 1 reply; 56+ messages in thread
From: Eli Zaretskii @ 2017-05-26 13:03 UTC (permalink / raw)
  To: Pedro Alves; +Cc: brobecker, simon.marchi, gdb-patches

> Cc: brobecker@adacore.com, simon.marchi@polymtl.ca, gdb-patches@sourceware.org
> From: Pedro Alves <palves@redhat.com>
> Date: Fri, 26 May 2017 11:54:03 +0100
> 
> > The code says std::to_string, though.  So it sounds like some coding
> > conventions are being applied here of which I wasn't aware, and
> > neither is Emacs.  Are these conventions described somewhere?
> 
> Just the standard GNU conventions.  The code is defining a new
> function template called gdb::to_string.  Simplified:
> 
>  namespace gdb {
>    template <class T> std::string to_string (const T &val);
>  }
> 
> There are two implementations of that, one for mingw, written
> as a new function template in place.  And another which is
> importing std::to_string into the gdb namespace.  But whatever
> the implementations, it's implementation detail of gdb::to_string.

So the convention is to include the full qualifier of every method?
Including the namespace of the class?  Does that include the whole
chain of namespaces up to the root?  If not, where does one stop?

> > but once again, the convention to put the
> > fully-qualified symbol name in the log entry should be documented, if
> > it isn't already, because Emacs doesn't do that, at least not by
> > default.  
> 
> I can't see how what Emacs does has any bearing here, since AFAIK,
> Emacs isn't written in C++.

I use Emacs commands, such as "C-x 4 a", to generate ChangeLog
entries.  Don't you do the same?  Emacs add-log commands strip the
class and namespace qualifiers.

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

* Re: GDB 7.99.91 MinGW compilation error in cli-script.c
  2017-05-26 12:57                                   ` Eli Zaretskii
@ 2017-05-26 13:58                                     ` Pedro Alves
  0 siblings, 0 replies; 56+ messages in thread
From: Pedro Alves @ 2017-05-26 13:58 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: brobecker, simon.marchi, gdb-patches

On 05/26/2017 01:57 PM, Eli Zaretskii wrote:

>> I would have helped a lot if I had been shown this.  Note the 
>> "__GNUC__ < 6" check.
> 
> Hey, that's unfair!  You expect me to look at the MinGW64 headers and
> at related Bugzilla bugs, but you yourself cannot look in the MinGW
> headers?

Sorry, I guess I'm a bit frustrated that we're spending a lot more time
on this than I think it's warranted.  Note I didn't ask you to search
for the bugs yourself, I took the time to research them, and provided
direct links which you can just open.  I didn't have that header handy,
and I had to go hunt for where to get it.

>> If you're happy with the gdb fix in place now, I'm happy enough
>> with it too.
> 
> I'm happy.  Any problems with MinGW GCC 6.x don't matter as long as
> there's no such GCC version on the MinGW site.

OK.

Thanks,
Pedro Alves

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

* Re: GDB 7.99.91 MinGW compilation error in cli-script.c
  2017-05-26 13:03                                     ` Eli Zaretskii
@ 2017-05-26 14:10                                       ` Pedro Alves
  2017-05-26 14:35                                         ` Eli Zaretskii
  0 siblings, 1 reply; 56+ messages in thread
From: Pedro Alves @ 2017-05-26 14:10 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: brobecker, simon.marchi, gdb-patches

On 05/26/2017 02:02 PM, Eli Zaretskii wrote:
>> Cc: brobecker@adacore.com, simon.marchi@polymtl.ca, gdb-patches@sourceware.org
>> From: Pedro Alves <palves@redhat.com>
>> Date: Fri, 26 May 2017 11:54:03 +0100
>>
>>> The code says std::to_string, though.  So it sounds like some coding
>>> conventions are being applied here of which I wasn't aware, and
>>> neither is Emacs.  Are these conventions described somewhere?
>>
>> Just the standard GNU conventions.  The code is defining a new
>> function template called gdb::to_string.  Simplified:
>>
>>  namespace gdb {
>>    template <class T> std::string to_string (const T &val);
>>  }
>>
>> There are two implementations of that, one for mingw, written
>> as a new function template in place.  And another which is
>> importing std::to_string into the gdb namespace.  But whatever
>> the implementations, it's implementation detail of gdb::to_string.
> 
> So the convention is to include the full qualifier of every method?
> Including the namespace of the class?  Does that include the whole
> chain of namespaces up to the root?  If not, where does one stop?

I admit that I'm puzzled about you having an issue with
gdb::to_string, when your original entry said "(std::to_string)", 
not "(to_string)".

I think the convention is to provide enough qualifiers to
to make it convenient to search/see what function was changed.
For class methods, I think we should include the class name,
otherwise it can get ambiguous.  I'm fine with not mentioning
the namespace when otherwise it's obvious or redundant.  E.g.,
if all of gdb was in namespace gdb [it's not], it'd probably not
make much sense to keep putting "gdb::" everywhere.  But when
referring to a symbol outside gdb's namespace, I'd think it wise to
mention the namespace, so that it's clearer what the entry
is referring to.

> 
>>> but once again, the convention to put the
>>> fully-qualified symbol name in the log entry should be documented, if
>>> it isn't already, because Emacs doesn't do that, at least not by
>>> default.  
>>
>> I can't see how what Emacs does has any bearing here, since AFAIK,
>> Emacs isn't written in C++.
> 
> I use Emacs commands, such as "C-x 4 a", to generate ChangeLog
> entries.  Don't you do the same?  Emacs add-log commands strip the
> class and namespace qualifiers.

Ah, I thought you were talking about your experience as emacs
maintainer.  Sorry.

I never managed to get used to using emacs commands to write the ChangeLog.
I write ChangeLog entries manually, directly in the git commit log,
while scrolling through the diff, as last "self review" event, and
then copy over to the ChangeLog file at push time.

Thanks,
Pedro Alves

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

* Re: GDB 7.99.91 MinGW compilation error in cli-script.c
  2017-05-26 14:10                                       ` Pedro Alves
@ 2017-05-26 14:35                                         ` Eli Zaretskii
  2017-05-26 14:45                                           ` Pedro Alves
  0 siblings, 1 reply; 56+ messages in thread
From: Eli Zaretskii @ 2017-05-26 14:35 UTC (permalink / raw)
  To: Pedro Alves; +Cc: brobecker, simon.marchi, gdb-patches

> Cc: brobecker@adacore.com, simon.marchi@polymtl.ca, gdb-patches@sourceware.org
> From: Pedro Alves <palves@redhat.com>
> Date: Fri, 26 May 2017 15:10:01 +0100
> 
> > So the convention is to include the full qualifier of every method?
> > Including the namespace of the class?  Does that include the whole
> > chain of namespaces up to the root?  If not, where does one stop?
> 
> I admit that I'm puzzled about you having an issue with
> gdb::to_string, when your original entry said "(std::to_string)", 
> not "(to_string)".

I did that because of the different namespaces involved in that
particular change.  But that was an exception: I normally don't look
too hard at what Emacs's add-log commands produce in the parentheses,
I just trust it to DTRT.  Having to manually correct that in each case
is a complication.

> I think the convention is to provide enough qualifiers to
> to make it convenient to search/see what function was changed.
> For class methods, I think we should include the class name,
> otherwise it can get ambiguous.  I'm fine with not mentioning
> the namespace when otherwise it's obvious or redundant.  E.g.,
> if all of gdb was in namespace gdb [it's not], it'd probably not
> make much sense to keep putting "gdb::" everywhere.  But when
> referring to a symbol outside gdb's namespace, I'd think it wise to
> mention the namespace, so that it's clearer what the entry
> is referring to.

Well, I suggest to document these conventions somewhere, like the
Wiki.  I don't think they are self-evident, and standards.texi doesn't
say anything about qualifying method names, it just tells to use the
Emacs add-log commands and the related VC features.

> I never managed to get used to using emacs commands to write the ChangeLog.
> I write ChangeLog entries manually, directly in the git commit log,
> while scrolling through the diff, as last "self review" event, and
> then copy over to the ChangeLog file at push time.

You will find some advice for a better workflow in the CONTRIBUTE file
in the Emacs repository
(http://git.savannah.gnu.org/cgit/emacs.git/tree/CONTRIBUTE).

I've just asked the CC Mode maintainer to allow the add-log commands
to not strip the class qualifiers.  But namespace qualifiers will
still have to be added by hand, I'm afraid, which is a bit of a
nuisance, IMO.

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

* Re: GDB 7.99.91 MinGW compilation error in cli-script.c
  2017-05-26 14:35                                         ` Eli Zaretskii
@ 2017-05-26 14:45                                           ` Pedro Alves
  0 siblings, 0 replies; 56+ messages in thread
From: Pedro Alves @ 2017-05-26 14:45 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: brobecker, simon.marchi, gdb-patches

On 05/26/2017 03:35 PM, Eli Zaretskii wrote:

> Well, I suggest to document these conventions somewhere, like the
> Wiki.  I don't think they are self-evident, and standards.texi doesn't
> say anything about qualifying method names, it just tells to use the
> Emacs add-log commands and the related VC features.

OK.  I see plenty of entries on gcc's ChangeLogs using the same
convention, and I never thought that others would ever want to skip a
method's class name.  Agreed about wiki.  Though maybe GCC already
documents it somewhere, and we just need to point at it.  I wasn't aware
about this emacs issue.

>> I never managed to get used to using emacs commands to write the ChangeLog.
>> I write ChangeLog entries manually, directly in the git commit log,
>> while scrolling through the diff, as last "self review" event, and
>> then copy over to the ChangeLog file at push time.
> 
> You will find some advice for a better workflow in the CONTRIBUTE file
> in the Emacs repository
> (http://git.savannah.gnu.org/cgit/emacs.git/tree/CONTRIBUTE).
> 
> I've just asked the CC Mode maintainer to allow the add-log commands
> to not strip the class qualifiers.  But namespace qualifiers will
> still have to be added by hand, I'm afraid, which is a bit of a
> nuisance, IMO.
> 

Thanks.

-- 
Pedro Alves

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

end of thread, other threads:[~2017-05-26 14:45 UTC | newest]

Thread overview: 56+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-05-04 19:44 GDB 7.99.91 available for testing Joel Brobecker
2017-05-08 14:53 ` Eli Zaretskii
2017-05-17 11:36   ` Yao Qi
2017-05-17 14:39     ` Eli Zaretskii
2017-05-08 15:00 ` GDB 7.99.91 MinGW compilation error in cli-script.c Eli Zaretskii
2017-05-14  3:19   ` Simon Marchi
2017-05-14 14:13     ` Eli Zaretskii
2017-05-17 14:31       ` Joel Brobecker
2017-05-17 16:01         ` Eli Zaretskii
2017-05-19  9:10           ` Eli Zaretskii
2017-05-19  9:49             ` Pedro Alves
2017-05-19 11:17               ` Eli Zaretskii
2017-05-19 11:23                 ` Pedro Alves
2017-05-21 15:33                   ` Eli Zaretskii
2017-05-22 15:26                     ` Pedro Alves
2017-05-22 18:43                       ` Eli Zaretskii
2017-05-23  9:53                         ` Pedro Alves
2017-05-24  2:44                           ` Eli Zaretskii
2017-05-25 10:05                             ` Pedro Alves
2017-05-26  7:57                               ` Eli Zaretskii
2017-05-26 11:52                                 ` Pedro Alves
2017-05-26 12:57                                   ` Eli Zaretskii
2017-05-26 13:58                                     ` Pedro Alves
2017-05-24 18:28                           ` Eli Zaretskii
2017-05-24 19:37                             ` Joel Brobecker
2017-05-25 10:12                               ` Pedro Alves
2017-05-26  7:47                                 ` Eli Zaretskii
2017-05-26 10:54                                   ` Pedro Alves
2017-05-26 13:03                                     ` Eli Zaretskii
2017-05-26 14:10                                       ` Pedro Alves
2017-05-26 14:35                                         ` Eli Zaretskii
2017-05-26 14:45                                           ` Pedro Alves
2017-05-08 15:02 ` GDB 7.99.91 MinGW compilation warning in tui.c Eli Zaretskii
2017-05-09 10:17   ` Yao Qi
2017-05-13  8:12     ` Eli Zaretskii
2017-05-17 16:26       ` Yao Qi
2017-05-17 16:45         ` Eli Zaretskii
2017-05-17 18:21           ` Joel Brobecker
2017-05-19  8:02             ` Eli Zaretskii
2017-05-19  9:54               ` Pedro Alves
2017-05-19 11:14                 ` Eli Zaretskii
2017-05-19 11:25                   ` Pedro Alves
2017-05-19 12:51                     ` Eli Zaretskii
2017-05-19 13:58                       ` Pedro Alves
2017-05-19 14:40                         ` Eli Zaretskii
2017-05-19 17:50               ` DJ Delorie
2017-05-19 17:55                 ` Eli Zaretskii
2017-05-15 21:11 ` GDB 7.99.91 available for testing Simon Marchi
2017-05-16 13:51   ` Simon Marchi
2017-05-16 20:50     ` Yao Qi
2017-05-16 21:22       ` Simon Marchi
2017-05-16 21:40         ` Yao Qi
2017-05-17 11:31           ` [PATCH master/8.0] Add alias command to cmd_list_element Yao Qi
2017-05-17 12:16             ` Simon Marchi
2017-05-17 13:36               ` Yao Qi
2017-05-16 14:28   ` GDB 7.99.91 available for testing 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).