This is the mail archive of the
gdb-patches@sourceware.org
mailing list for the GDB project.
[RFA] Typedef'd method parameters [0/4]
- From: Keith Seitz <keiths at redhat dot com>
- To: gdb-patches at sourceware dot org
- Date: Thu, 21 Apr 2011 14:15:24 -0700
- Subject: [RFA] Typedef'd method parameters [0/4]
Hi,
I've been working on fixing c++/12266 and c++/12506 in which users are
unable to specify linespecs for class methods which have typedef'd
parameters, e.g., klass:method(my_typedef). This is an attempt to fix
this shortcoming. [This is not a dwarf2_physname regression.]
I've attempted to split this up into four different patches to help
facilitate review, but to be honest, I'm not sure how helpful this split
is going to be, but I'll leave it to maintainers to ask for whatever
would help them digest this easiest.
Back to the problem. Physnames (similar to linkage names) are used to
store symbols in the symbol tables -- this is the whole dwarf2_physname
patchset from last year. These physnames should represent the most basic
"name" of a symbol.
The inability to use typedef'd parameter types arises from the fact that
we do fairly literal lookups in the symbol table from decode_line_1 et
al. So for example, if the user types "break klass::method(my_typedef)",
decode_compound will attempt to look for a method of "klass" with the
parameter type "my_typedef". Alas, as I've already mentioned, the symbol
tables do not contain this information -- the contain whatever the
typedef resolves to (klass::method(int), for example).
The proposed solution in this patchset is to use the C++ name parser to
find these method parameter types, look them up, and if they are
typedefs, change the name in the tree to physname of the typedef.
A note on the test suite: I propose to get these all approved and commit
in one go. If done this way, no test suite regressions should occur. Or
at least they don't show up here. :-)
There are several (mechanical) problems that arise from this approach
that need to be dealt with first. First of all,
cp_demangled_name_to_comp can not be called recursively because the
storage for the resulting parse tree is rewritten with every call.
This can happen before debuginfo is read and the user attempts to
specify a linespec; when the typedef is looked up, the dwarf reader will
call cp_canonicalize_string, destroying the result. So the first of
these patches addresses this. It is pretty much orthogonal to the other
patches.
Second, c_type_print_args (and several functions in c-typeprint.c)
needed to be taught the difference between linkage names and print
names. [Reminder: Both skip artificial parameters; linkage names always
resolve typedefs.] This is addressed in the second patch.
The third patch is actually the "guts" of the patch, which walks the
name parse tree and converts typedefs. The main function of interest
here is cp_canonicalize_string_no_typedefs, which is largely a clone of
cp_canonicalize_string, except that it resolves any typedefs found in
the name (as if you couldn't guess that).
The final/fourth patch is test cases for this. I have really tried to
hit a bunch of corner cases, but it is likely that users have thought of
some way to express something in C++ that I have not.
Just a side note for maintainers, since I know this is going to come up:
I have tried my best to consolidate name canonicalization, but there are
a bunch of problems that arise from attempting to do this. I believe the
solution proposed here is the most consistent from an API perspective --
everything still acts the way it should (and has). The alternative is to
canonicalize_no_typedefs before anyone calls lookup_symbol or
lookup_symbol_in_language. I rejected this approach.
In one specific case, we have no choice but to re-canonicalize the input
anyway, and this isn't particularly time-critical code, since it is only
called from decode_line_1, i.e., from user input. It does not get called
from dwarf2read.c.
Keith
PS. I've attached the whole patch as a single large piece for those
interested in skipping the piecemeal evaluation of the patchset. [See
ChangeLogs in subsequent messages.]
Attachment:
12266-whole.patch
Description: Text document