Re: [PATCH]: Pascal language support : New files

[Stan Shebs <shebs at apple dot com>]

Pierre Muller wrote:
> 
>   This is a new application of my patch for adding Pascal Language support to
> GDB.

Overall, it looks pretty good to me, just need to remove the code
that is really specific to C/C++, so we don't have to maintain
lots of dead code forever.

> --- /dev/null
> +++ p-exp.y

> 
> /* C++.  */
> exp     :       THIS
>                         { write_exp_elt_opcode (OP_THIS);
>                           write_exp_elt_opcode (OP_THIS); }
>         ;
>
> /* end of C++.  */

Do you want this?  In any case, the C++ comment would be confusing.

>                               /* C++: it hangs off of `this'.  Must
>                                  not inadvertently convert from a method call
>                                  to data ref.  */

Similarly.

> /* We used to try to recognize more pointer to member types here, but
>    that didn't work (shift/reduce conflicts meant that these rules never
>    got executed).  The problem is that
>      int (foo::bar::baz::bizzle)
>    is a function type but
>      int (foo::bar::baz::bizzle::*)
>    is a pointer to member type.  Stroustrup loses again!  */

And here (although Pascal advocates might appreciate the Stroustrup criticism anyway :-) ).

> typebase  /* Implements (approximately): (type-qualifier)* type-specifier */
>         :       TYPENAME
>                         { $$ = $1.type; }
>         |       STRUCT name
>                         { $$ = lookup_struct (copy_name ($2),
>                                               expression_context_block); }
>         |       CLASS name
>                         { $$ = lookup_struct (copy_name ($2),
>                                               expression_context_block); }
>         /* "const" and "volatile" are curently ignored.  A type qualifier
>            after the type is handled in the ptype rule.  I think these could
>            be too.  */
>         ;

Probably should whack the class part of the rule?

> --- /dev/null
> +++ p-lang.c

> extern void _initialize_pas_language PARAMS ((void));

Personally, I think it would be better to either use just "p", or
else spell out "pascal" for clarity.  "pas" seems wishy-washy, neither
aggressively brief like "p" nor totally accurate like "pascal".

But it's not that important.

>   /* If the string was not truncated due to `set print elements', and
>      the last byte of it is a null, we don't print that, in traditional C
>      style.  */
>   if ((!force_ellipses) && length > 0 && string[length - 1] == '\0')
>     length--;

Is that desirable for Pascal?

>     /* C++  apparently removed from GDB since 4.18
>        {"::", BINOP_SCOPE, PREC_PREFIX, 0}, */

Remove this, can only confuse.

> --- /dev/null
> +++ p-typeprint.c

> 
> /* If TYPE is a derived type, then print out derivation information.
>    Print only the actual base classes of this type, not the base classes
>    of the base classes.  I.E.  for the derivation hierarchy:
> 
>    class A { int a; };
>    class B : public A {int b; };
>    class C : public B {int c; };
> 
>    Print the type of class C as:
> 
>    class C : public B {
>    int c;
>    }
> 
>    Not as the following (like gdb used to), which is not legal C++ syntax for
>    derived types and may be confused with the multiple inheritance form:
> 
>    class C : public B : public A {
>    int c;
>    }
> 
>    In general, gdb should try to print the types as closely as possible to
>    the form that they appear in the source code. */

This comment is completely misleading, and presumably the code following
is not needed.  It's important to remove now, otherwise later maintainers
might think it's necessary to keep it.

> static void
> pcpas_type_print_derivation_info (stream, type)
> [...]

> --- /dev/null
> +++ p-valprint.c

>               /* If this is a virtual function table, print the 0th
>                  entry specially, and the rest of the members normally.  */
>               if (pas_cp_is_vtbl_ptr_type (elttype))
>                 {
>                   i = 1;
>                   fprintf_filtered (stream, "%d vtable entries", len - 1);
>                 }
>               else
>                 {
>                   i = 0;
>                 }

No vtables in Pascal, right?

>       if (vtblprint && pas_cp_is_vtbl_ptr_type (type))
>         {
>           /* Print the unmangled name if desired.  */
>           /* Print vtable entry - we only get here if we ARE using
>              -fvtable_thunks.  (Otherwise, look under TYPE_CODE_STRUCT.) */
>           print_address_demangle (extract_address (valaddr + embedded_offset, TYPE_LENGTH (type)),
>                                   stream, demangle);
>           break;
>         }
>       elttype = check_typedef (TYPE_TARGET_TYPE (type));
>       if (TYPE_CODE (elttype) == TYPE_CODE_METHOD)
>         {
>           pas_cp_print_class_method (valaddr + embedded_offset, type, stream);
>         }
>       else if (TYPE_CODE (elttype) == TYPE_CODE_MEMBER)
>         {
>           pas_cp_print_class_member (valaddr + embedded_offset,
>                                  TYPE_DOMAIN_TYPE (TYPE_TARGET_TYPE (type)),
>                                      stream, "&");
>         }

Ditto.  There are lots more chunks in the following code.  Again,
please remove all unnecessary code.

> void
> _initialize_pas_valprint ()
> {
>   add_show_from_set
>     (add_set_cmd ("pas_static-members", class_support, var_boolean,
>                   (char *) &pas_static_field_print,
>                   "Set printing of pascal static members.",
>                   &setprintlist),
>      &showprintlist);
>   /* Turn on printing of static fields.  */
>   pas_static_field_print = 1;
> 
> }

What's a "pascal static member"?  Presumably this is a leftover from
the C++ value printing code, so this variable and code should go away too.

Stan