Re: GDB 7.99.91 MinGW compilation error in cli-script.c

[Pedro Alves <palves@redhat.com>]

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