[PATCH 4/5] Implement D primitive types in GDB

classic Classic list List threaded Threaded
8 messages Options
Reply | Threaded
Open this post in threaded view
|

[PATCH 4/5] Implement D primitive types in GDB

Iain Buclaw
D has it's own type system separate from C.  This defines all
primitive types all found in D.

2014-01-09  Iain Buclaw  <[hidden email]>

    gdb/

        * d-lang.h (struct builtin_d_type): New data type.
        (builtin_d_type): Add declaration.
        * d-lang.c (d_language_arch_info, build_d_types)
        (builtin_d_type): New functions.
        (enum d_primitive_types): New data type.
        (d_language_defn): Change c_language_arch_info to
        d_language_arch_info.
        (d_type_data): New static variable.
        (_initialize_d_language): Initialize d_type_data.

    gdb/testsuite/

        * gdb.dlang/primitive-types.exp: New file.

---

dlang-p4.patch (15K) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH 4/5] Implement D primitive types in GDB

Tom Tromey
>>>>> "Iain" == Iain Buclaw <[hidden email]> writes:

Iain> D has it's own type system separate from C.  This defines all
Iain> primitive types all found in D.

Thanks.  I think the guts of this patch are fine, just some nits around
the edges.

 
Iain> +enum d_primitive_types {

Comment before the new type.

Iain> +  d_primitive_type_cent,
Iain> +  d_primitive_type_ucent,

I don't think we need a comment for each enum constant, but it would be
nice for the ones that are "not obvious to C developers".  Subjective I
realize; but at least here I have no idea what "cent" means.

Iain> +  d_primitive_type_ifloat,
Iain> +  d_primitive_type_idouble,
Iain> +  d_primitive_type_ireal,

Or what the "i" prefix means.

Iain> +  d_primitive_type_cfloat,
Iain> +  d_primitive_type_cdouble,
Iain> +  d_primitive_type_creal,

"c" means complex maybe?

Iain> +static void
Iain> +d_language_arch_info (struct gdbarch *gdbarch,
Iain> +      struct language_arch_info *lai)
Iain> +{

All new functions need intro comments.
It's fine if they are short; in a case like this where the new function
is the implementation of a language method, it can just mention which
method it implements.

Iain> +static void *
Iain> +build_d_types (struct gdbarch *gdbarch)
Iain> +{

Comment.

Iain> +const struct builtin_d_type *
Iain> +builtin_d_type (struct gdbarch *gdbarch)
Iain> +{

Comment.
 
Iain> +struct builtin_d_type
Iain> +{

Comment.  I think it's fine to refer readers back to the enum for the
meaning of fields here.

Iain> +# NOTE: The tests here intentionally do not require a go compiler.

Cut-and-pasto :)

Tom
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH 4/5] Implement D primitive types in GDB

Iain Buclaw
On 9 January 2014 18:33, Tom Tromey <[hidden email]> wrote:

>>>>>> "Iain" == Iain Buclaw <[hidden email]> writes:
>
> Iain> D has it's own type system separate from C.  This defines all
> Iain> primitive types all found in D.
>
> Thanks.  I think the guts of this patch are fine, just some nits around
> the edges.
>
>
> Iain> +enum d_primitive_types {
>
> Comment before the new type.
>
> Iain> +  d_primitive_type_cent,
> Iain> +  d_primitive_type_ucent,
>
> I don't think we need a comment for each enum constant, but it would be
> nice for the ones that are "not obvious to C developers".  Subjective I
> realize; but at least here I have no idea what "cent" means.
>
> Iain> +  d_primitive_type_ifloat,
> Iain> +  d_primitive_type_idouble,
> Iain> +  d_primitive_type_ireal,
>
> Or what the "i" prefix means.
>
> Iain> +  d_primitive_type_cfloat,
> Iain> +  d_primitive_type_cdouble,
> Iain> +  d_primitive_type_creal,
>
> "c" means complex maybe?
>

Sure, not a problem.  For reference, cent/ucent are 128bit types - not
actually implemented at all, but they have been kept around as
keywords for possible future use.  The "i" prefix is for Imaginary
numbers, and the "c" prefix is for Complex numbers.


>
> Iain> +# NOTE: The tests here intentionally do not require a go compiler.
>
> Cut-and-pasto :)
>

Aww..... :-)
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH 4/5] Implement D primitive types in GDB

Iain Buclaw
In reply to this post by Tom Tromey
On 9 January 2014 18:33, Tom Tromey <[hidden email]> wrote:
>>>>>> "Iain" == Iain Buclaw <[hidden email]> writes:
>
> Iain> D has it's own type system separate from C.  This defines all
> Iain> primitive types all found in D.
>
> Thanks.  I think the guts of this patch are fine, just some nits around
> the edges.
>

Comments duly noted and rebased patch.  Though I have a feeling you
may ask to comment non-static functions in their declared headers. :)

dlang-p4.patch (16K) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH 4/5] Implement D primitive types in GDB

Tom Tromey
>>>>> "Iain" == Iain Buclaw <[hidden email]> writes:

Iain> Comments duly noted and rebased patch.  Though I have a feeling you
Iain> may ask to comment non-static functions in their declared headers. :)

That's like the one thing gdb doesn't have a pedantic standard for.
Yet :)  At some point Stan will presumably push in his doxygen change
and then we'll start converting to that.

Iain> +/* Mapping of all D basic data types into the language vector.  */
Iain> +enum d_primitive_types {

Space between the comment and the definition.
Yeah, that pedantic.

Iain> +  d_primitive_type_cent,    /* signed 128 bit integer */

Comments have to start with an upper-case letter and end with a period
and two spaces.

Iain> +/* Implements the la_language_arch_info language_defn routine
Iain> +   for language D.  */
Iain> +static void

Blank line here and elsewhere.

This is ok with those nits fixed.

Tom
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH 4/5] Implement D primitive types in GDB

Iain Buclaw
On 10 January 2014 21:18, Tom Tromey <[hidden email]> wrote:
>
> Blank line here and elsewhere.
>
> This is ok with those nits fixed.
>

Third time a charm.  Re-done with your nits fixed.

Regards
Iain
---

dlang-p4.patch (15K) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH 4/5] Implement D primitive types in GDB

Tom Tromey
>>>>> "Iain" == Iain Buclaw <[hidden email]> writes:

Iain> Third time a charm.  Re-done with your nits fixed.

Thanks, this is ok.

Tom
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH 4/5] Implement D primitive types in GDB

Iain Buclaw
On 13 January 2014 20:03, Tom Tromey <[hidden email]> wrote:
>>>>>> "Iain" == Iain Buclaw <[hidden email]> writes:
>
> Iain> Third time a charm.  Re-done with your nits fixed.
>
> Thanks, this is ok.
>
> Tom

Thanks, have now pushed this in.

Regards
Iain