Mirror of the gdb-patches mailing list
 help / color / mirror / Atom feed
From: Tom Tromey <tromey@redhat.com>
To: sami wagiaalla <swagiaal@redhat.com>
Cc: Eli Zaretskii <eliz@gnu.org>, gdb-patches@sourceware.org
Subject: Re: [patch 2/2] Fix overload resolution of int* vs void*
Date: Wed, 13 Oct 2010 15:49:00 -0000	[thread overview]
Message-ID: <m34ocqcctf.fsf@fleche.redhat.com> (raw)
In-Reply-To: <4CB5CD25.7080803@redhat.com> (sami wagiaalla's message of "Wed,	13 Oct 2010 11:15:49 -0400")

>>>>> "Sami" == sami wagiaalla <swagiaal@redhat.com> writes:

Sami> I added one. The test attempts to make the conversion and fails as
Sami> expected.

Thanks for adding the tests, and for the explanations.

Sami> Having typed this out, I think POINTER_CONVERSION_BADNESS should be
Sami> renamed to BOOL_PTR_CONVERSION_BADNESS and given a rank of 3. And all
Sami> other conversions outlawed. I'll do this in and test it another
Sami> patch. This patch really only deals with the first clauses of the
Sami> nested switch statements.

Thanks.

Tom> GDB generally ignores access protection.  It seems like it ought to here
Tom> as well.

Sami> Hmm, I am worried that this will make overload resolution more
Sami> complicated, and lead to incorrect resolution.

Yeah, that makes sense to me.  But I still think ignoring it is probably
the best thing to do here.

Consider if you are stopped in a method or friend function of a derived
class.  In this case you do have access to the base -- so overload
resolution should work.  But with this patch, it will not.

Sami> I have no objection to
Sami> calling the inaccessible base function if the user casts the the type:

Casting is only ok if the base is accessible :)

Access bits are a real problem.  I don't know what to do in the long
run.  Consider something like "print obj.field".  If we strictly follow
C++, that can only work when "field" is accessible -- but in a debugger
that is basically useless.

Maybe we could come up with some heuristic for when it would be
acceptable and when it would not be.  And maybe overloading should be on
the "respect access" list -- but I think for the time being we should
still ignore it, until we have a good idea of what we want and the
capability to implement it correctly.

Aside from this final issue the patch looks ok to me.

Tom


  reply	other threads:[~2010-10-13 15:49 UTC|newest]

Thread overview: 18+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2010-08-30 15:24 [patch] " sami wagiaalla
2010-08-30 16:13 ` sami wagiaalla
2010-08-30 20:01 ` Tom Tromey
2010-10-08 18:39   ` [patch 1/2] " sami wagiaalla
2010-10-08 18:54     ` Tom Tromey
2010-10-08 19:35       ` sami wagiaalla
2010-10-08 21:30         ` Tom Tromey
2010-10-08 19:05   ` [patch 2/2] " sami wagiaalla
2010-10-08 20:46     ` Eli Zaretskii
2010-10-08 21:10       ` sami wagiaalla
2010-10-08 22:54         ` Tom Tromey
2010-10-12 20:01           ` sami wagiaalla
2010-10-12 20:41             ` Tom Tromey
2010-10-13 15:16               ` sami wagiaalla
2010-10-13 15:49                 ` Tom Tromey [this message]
2010-10-13 18:29                   ` sami wagiaalla
2010-10-15 14:46                   ` sami wagiaalla
2010-10-15 22:48                     ` Tom Tromey

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=m34ocqcctf.fsf@fleche.redhat.com \
    --to=tromey@redhat.com \
    --cc=eliz@gnu.org \
    --cc=gdb-patches@sourceware.org \
    --cc=swagiaal@redhat.com \
    /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