Mirror of the gdb-patches mailing list
 help / color / mirror / Atom feed
From: Tom Tromey <tromey@redhat.com>
To: xgsa <xgsa@yandex.ru>
Cc: gdb-patches@sourceware.org
Subject: Re: set print object on should affect MI varobjs (PR 13393)
Date: Tue, 20 Dec 2011 17:47:00 -0000	[thread overview]
Message-ID: <m3mxanjiy0.fsf@fleche.redhat.com> (raw)
In-Reply-To: <4ED92C05.9080803@yandex.ru> (xgsa@yandex.ru's message of "Fri,	02 Dec 2011 21:50:29 +0200")

>>>>> ">" == xgsa  <xgsa@yandex.ru> writes:

>> (it seems that my previous message about this patch was lost somehow,
>> so I resend it again; sorry, if I sent it twice)

No problem.  Thanks for working on this.

>> 2011-12-02  Anton Gorenkov <xgsa@yandex.ru>

>>     PR 13393

I think it has to mention the category for the commit->bz scanner to
work.

Your patch got wrapped by your mailer, making it harder to read.

>> -static struct type *
>> +struct type *
>>  make_qualified_type (struct type *type, int new_flags,
>>               struct type *storage)

I'd prefer not to export this.
I think you can just use make_cv_type instead.

>> +  else if (TYPE_CODE(type) == TYPE_CODE_PTR)

Missing space before the open paren here.  This occurs a few times.


>> +struct type *
>> +actual_type (struct value *value, int resolve_simple_types)
>> +{

This should have an introductory comment saying to look at value.h.

>> +      struct type *real_type;
>> +      real_type = value_rtti_target_type ( value, 0, 0, 0 );

Blank line between declaration and code.
This has an extra space after "(" and before ")".

>> +/* Returns value_type() or value_enclosing_type () depending on

Remove the '()'s.

>> +   value_print_options.objectprint.
>> +
>> +   If RESOLVE_SIMPLE_TYPES is 0 the enclosing type will be resolved
>> +   only for pointers and references, else it will be returned also
>> +   all the types (e.g. for structures). This option is useful
>> +   to prevent retrieving enclosing type for the base classes fields */

Sentences should end with a period followed by 2 spaces.
The last sentence needs this too.

>> +      struct type *enclosing_type;
>> +      var->type = value_type (value);

Blank line.

>> +      enclosing_type = actual_type(value, 0);

Missing space.

>> +      if (check_typedef (enclosing_type) != check_typedef (var->type))
>> +        {
>> +          var->type = enclosing_type;
>> +          value = value_cast (enclosing_type, value);

Will this always do the right thing?
It seems questionable.

>> +  // Access a real type of the value (if necessary and possible)

GDB is C90 and can't use // comments.

>> +      enclosing_type = actual_type(*value, 1);

I don't understand why this one call has a '1' here.

>> +      // It is necessary to access a real type (via RTTI)
>> +      if (opts.objectprint)
>> +        {
>> +          value = var->value;
>> +          lookup_actual_type = TYPE_CODE (var->type) == TYPE_CODE_REF
>> || TYPE_CODE (var->type) == TYPE_CODE_PTR;

This line has to be reformatted per the GNU coding standards.

Do you have copyright assignment papers filed with the FSF?
If not, contact me off-list to get started with this.
We can't put anything in until the paperwork is finished.

Tom


  parent reply	other threads:[~2011-12-20 16:40 UTC|newest]

Thread overview: 47+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2011-12-02 19:50 xgsa
2011-12-08  8:15 ` xgsa
2011-12-19 20:34   ` xgsa
2011-12-20 17:47 ` Tom Tromey [this message]
2011-12-21 19:01   ` set print object on should affect MI varobjs (PR gdb/13393) xgsa
2011-12-21 19:37     ` Jan Kratochvil
2011-12-22 10:17       ` set print object on should affect MI varobjs (PR mi/13393) xgsa
2011-12-24  1:55         ` Jan Kratochvil
2011-12-24 13:56           ` xgsa
2012-01-02  2:22             ` Jan Kratochvil
2012-01-06 15:47               ` RTTI type improvement for (was: "Re: set print object on should affect MI varobjs (PR mi/13393)") xgsa
2012-01-09 14:41                 ` Jan Kratochvil
2012-01-11 21:58                   ` RTTI type improvement for Tom Tromey
2012-01-12 11:25                     ` xgsa
2012-02-06 21:45                 ` Tom Tromey
2012-02-08 18:34                   ` xgsa
2012-02-10 20:13                     ` Tom Tromey
2012-02-19 18:46                       ` set print object on should affect MI varobjs (PR mi/13393) xgsa
2012-02-23  4:58                         ` xgsa
2012-03-18 16:28                         ` xgsa
2012-03-18 20:41                           ` Eli Zaretskii
2012-03-19  7:10                             ` xgsa
2012-03-19 17:41                               ` Eli Zaretskii
2012-03-23 17:09                                 ` xgsa
2012-03-26 19:08                                   ` xgsa
2012-03-30 17:51                                   ` Tom Tromey
2012-03-30 18:01                                     ` Eli Zaretskii
2012-03-30 20:11                                     ` xgsa
2012-03-30 18:01                                   ` Eli Zaretskii
2012-03-30 20:25                                     ` xgsa
2012-03-30 20:52                                       ` Eli Zaretskii
2012-03-30 21:26                                         ` xgsa
2012-03-31  5:54                                           ` Eli Zaretskii
2012-03-31  6:57                                             ` xgsa
2012-03-31  9:33                                               ` xgsa
2012-04-03  0:54                                                 ` Doug Evans
2012-04-03 13:27                                                   ` xgsa
2012-04-06 17:11                                                     ` xgsa
2012-04-13  8:07                                                       ` xgsa
2012-04-13 12:22                                                         ` Eli Zaretskii
2012-04-13 12:34                                                           ` xgsa
2012-04-13 17:23                                                             ` Tom Tromey
2012-04-14 23:35                                                               ` xgsa
2012-04-16 18:35                                                                 ` Jan Kratochvil
2012-07-23 17:21                                                                   ` Ulrich Weigand
2012-08-06  7:26                                                                     ` xgsa
2012-02-21 14:15                       ` RTTI type improvement for xgsa

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=m3mxanjiy0.fsf@fleche.redhat.com \
    --to=tromey@redhat.com \
    --cc=gdb-patches@sourceware.org \
    --cc=xgsa@yandex.ru \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox