On Sun, Aug 3, 2014 at 11:12 AM, Iain Buclaw <ibuc...@gdcproject.org> wrote:
>
> This adds a demangler for the D programming language to libiberty,
> intended to be used in GDB and Binutils. GDB already has a trimmed
> down implementation of this, but have been advised that here would be
> a better location to house it.
>
> Notes that I think are of interest / questions I have about how I've done
> this.
>
> - The implementation is some 1200 SLOC (and may grow), so I've put it
> in a new file, as opposed included cplus-dem.c. Is this reasonable?
Yes.
> - This borrows and extends the mini string package in cplus-dem.c,
> because it was the simplest to use when writing this. The GDB
> implementation uses obstack, and I'm aware of dyn_string, but I can't
> say I'm a fan of using either.
One thing we discovered for the C++ demangler is that sometimes it's
nice to be able to invoke it from locations where the memory allocator
is not available, such as when the program is crashing. That is why
the C++ demangler has a cplus_demangle_v3_callback entry point. For
ordinary demangling the code uses growable_string and
d_growable_string_callback_adapter. It's not important for now but
you may want to consider doing something like that in the future.
If you do choose to follow that path, because of namespace
considerations I think the right approach would be to move the
growable_string code and the adapter into a .h file in libiberty that
defines the functions as static. Then both demanglers could #include
that file. That would let them share the code without adding
unacceptable new symbols to libstdc++.
> - GDB has a testsuite that provides most of the coverage for what this
> code should be doing.
That is nice but we need a small testsuite in libiberty too--see
libiberty/testsuite/test-demangle.c and demangle-expected. Ideally
you should be able to just add some entries to demangle-expected.
> - List of functions in d-demangle.c can be added to the ChangeLog upon
> request.
Not necessary.
> - I haven't signed any copyright assignments to GCC. But I have
> papers from Donald ready to send across.
Definitely necessary before we can consider this. Please get this
squared away before proceeding with this patch. Let us know if you
need any help with this.
> +#ifdef HAVE_STDLIB_H
> +#include <stdlib.h>
> +#else
> +long strtol (const char *nptr, char **endptr, int base);
> +long double strtold (const char *nptr, char **endptr);
Use an explicit "extern".
> +#define ASCII2HEX(c) \
> + (('a' <= (c) && (c) <= 'f') ? \
> + ((c) - 'a' + 10) \
> + : (('A' <= (c) && (c) <= 'F') ? \
> + ((c) - 'A' + 10) \
> + : (('0' <= (c) && (c) <= '9') ? \
> + ((c) - '0') \
> + : 0 \
> + ) \
> + ) \
> + )
Use a function, not a macro that multiple-evaluates its argument.
> +/* Prototypes for D demangling functions */
Current style is to only forward declare functions when necessary.
> +const char *
> +dlang_call_convention (string *decl, const char *mangled)
This should be explicitly static. Same applies to many of the
functions below.
Ian
