>>>>> "Trevor" == Trevor Saunders <tsaund...@mozilla.com> writes:
Trevor> I'm curious, what is the reason you choose not to write this in C++11 or
Trevor> later? Its distributed with gcc, so the only case where you aren't
Trevor> building with the in tree compiler and libraries is when your cross
Trevor> compiling gcc, and it doesn't seem particularly important to support
Trevor> building the plugin or library in that configuration. So istm you could
Trevor> go all the way and assume you are being built with trunk gcc and
Trevor> libraries.
The plugin has to be ABI compatible with GCC itself, and my
understanding was that C++11 and "however GCC is built" are not
necessarily compatible.
Switching to C++11 would be an improvement -- variadic templates would
simplify the RPC code (with a complicated caveat). So if it is possible
I am interested.
Trevor> I'm going to use this as an excuse to bring up something I've wanted to
Trevor> discuss for a while.
Trevor> So can we add C++ stuff to libiberty and allow building
Trevor> libiberty without it for binutils / gdb, or can we do something
Trevor> else to avoid this kind of stuff?
One way would be to just make a new top-level directory for a new
library.
Trevor> This question also arises in the case of templating splay_tree, and I
Trevor> imagine if gdb switches to C++ some day they'll want to reuse vec.h.
While I would like that to happen, I think the odds are very long now.
>> + connection (int fd)
>> + : m_fd (fd),
>> + m_aux_fd (-1),
>> + m_callbacks ()
Trevor> Personally I'd leave that to the compiler to write, but I guess there's
Trevor> something to be said for being explicit.
I can't recall if I did this in response to a warning or if it was just
because I wanted to be explicit.
I'm inclined to leave it, but I suppose only out of inertia.
>> + void print (const char *buf)
Trevor> explicitly mark it as virtual?
Good idea, done.
>> +// This is a wrapper function that is called by the RPC system and
>> +// that then forwards the call to the library user. Note that the
>> +// return value is not used; the type cannot be 'void' due to
>> +// limitations in our simple RPC.
>> +gcc_address
Trevor> looks like this one probably is used?
Thanks, fixed.
>> + char **argv = new (std::nothrow) char *[self->args.size () + 1];
Trevor> What's the point of making this no throw? you don't null check it so
Trevor> you'll crash anyway afaict.
Thanks. I changed it to do a NULL check. It's nothrow because nothing
in libcc1 or gdb is prepared for a C++ exception. While I like
exceptions (gdb uses its own longjmp-based exception system
extensively), my understanding is that they aren't currently used in
gcc.
>> +cc1_plugin::status
>> +cc1_plugin::unmarshall (connection *conn, char **result)
>> +{
>> + unsigned long long len;
>> +
>> + if (!conn->require ('s'))
>> + return FAIL;
>> + if (!conn->get (&len, sizeof (len)))
>> + return FAIL;
>> +
>> + if (len == -1ULL)
>> + {
>> + *result = NULL;
>> + return OK;
>> + }
>> +
>> + char *str = new (std::nothrow) char[len + 1];
Trevor> It'd be really nice if the type of the out arg forced the caller to deal
Trevor> with deleting the string like unique_ptr<char>, it would be even nicer
Trevor> if you could stick a random buffer in a std::string, but I guess you
Trevor> can't :(
Yeah, it's all quite simplistic. I suppose it could be upgraded, there
just didn't seem to be a need.
Trevor> Also where does this array get deleted?
The unmarshalling methods are generally called via argument_wrappers.
An example is in connection.cc:
// Use an argument_wrapper here to simplify management
// of the string's lifetime.
argument_wrapper<char *> method_name;
if (!method_name.unmarshall (this))
return FAIL;
Then in rpc.hh:
// Specialization for string types.
template<>
class argument_wrapper<const char *>
{
public:
argument_wrapper () : m_object (NULL) { }
~argument_wrapper ()
{
delete[] m_object;
}
Tom
