[RFC] Class names matching field names

[RFC] Class names matching field names

[Stan Shebs]

This one seems conceptually shaky to me, so I'm hoping others can shed 
some light on what to do.  The problem is with a C++ class that has a 
constructor, and using the class name to qualify a static field, in the 
context of a method of the class:

<in main>
(gdb) print class1::s_field3
$1 = 46
(gdb) c
<stop in method>
(gdb) print class1::s_field3
A syntax error in expression, near `s_field3'.
(gdb) print 'class1::s_field3'
$2 = 46
(gdb)

While adding the single quotes is an almost-acceptable workaround for 
printing, it becomes maddening when setting up conditions or collections 
for tracepoints, because the context for those correspond to the 
location of the tracepoint, not one's current location, and you get 
parse errors for no apparent reason - or worse, an error for one 
tracepoint, and no complaint for the exact same expression used in a 
different tracepoint!

The root cause is that lookup_symbol_aux goes thumbing through the 
fields of a class looking for matches, and if it finds the class name 
(as would happen if there was a constructor), it declares victory, but 
then the remainder of the qualified name is left unhandled.  The obvious 
patch is to reject such matches, and in fact the patch has the desired 
effect.  At the same time, it seems like it's letting an obscure point 
of C++ leak into generic symbol lookup, and that seems wrong.

So I'm interested in people's thoughts on whether this patch is a good 
idea or not.

Stan

2010-01-29  Stan Shebs  <stan@codesourcery.com>

    * symtab.c (lookup_symbol_aux): Force a class name to be interpreted
    as such, even if there is a field of the same name.


Index: gdb/symtab.c
===================================================================
*** gdb/symtab.c    (revision 274234)
--- gdb/symtab.c    (revision 274235)
*************** lookup_symbol_aux (const char *name, con
*** 1331,1337 ****
          error (_("Internal error: `%s' is not an aggregate"),
             langdef->la_name_of_this);
 
!       if (check_field (t, name))
          {
            *is_a_field_of_this = 1;
            return NULL;
--- 1331,1343 ----
          error (_("Internal error: `%s' is not an aggregate"),
             langdef->la_name_of_this);
 
!       if (check_field (t, name)
!           /* Assume that a field name matching the name of the
!          type is an accidental match on a constructor, and
!          reject it, which allows our lookup to go on to search
!          in global areas.  */
!           && !(TYPE_NAME (t)
!            && strcmp (TYPE_NAME (t), name) == 0))
          {
            *is_a_field_of_this = 1;
            return NULL;

Re: [RFC] Class names matching field names

[Tom Tromey]

>>>>> "Stan" == Stan Shebs <stan@codesourcery.com> writes:

Stan> (gdb) print class1::s_field3
Stan> A syntax error in expression, near `s_field3'.

Stan> While adding the single quotes is an almost-acceptable workaround for
Stan> printing

FWIW, I think requiring single quotes when perfectly ordinary C++ syntax
would do is a bug.  I understand the state of things and how this can be
difficult to lift, but we'll get there eventually...

Stan> The root cause is that lookup_symbol_aux goes thumbing through the
Stan> fields of a class looking for matches, and if it finds the class name
Stan> (as would happen if there was a constructor), it declares victory, but
Stan> then the remainder of the qualified name is left unhandled.  The
Stan> obvious patch is to reject such matches, and in fact the patch has the
Stan> desired effect.  At the same time, it seems like it's letting an
Stan> obscure point of C++ leak into generic symbol lookup, and that seems
Stan> wrong.

Stan> So I'm interested in people's thoughts on whether this patch is a good
Stan> idea or not.

All things being equal, I'd lean toward not.  My reasoning is both what
you say here, and additionally that such changes are hard to back out
(or even find) once they go in.

It seems like it should be possible to fix this in the C++ parser (well,
lexer, given some previous hacks), perhaps in conjunction with some new
symbol lookup API (not sure).

Tom