Mirror of the gdb-patches mailing list
 help / color / mirror / Atom feed
From: Pedro Alves <palves@redhat.com>
To: Tom Tromey <tom@tromey.com>
Cc: gdb-patches@sourceware.org
Subject: Re: [RFA] Convert observers to C++
Date: Mon, 12 Feb 2018 18:37:00 -0000	[thread overview]
Message-ID: <be7542fe-a9ab-c72e-6c68-18a9e5e1e806@redhat.com> (raw)
In-Reply-To: <87zi4haeak.fsf@tromey.com>

Hi Tom,

Thanks!

On 02/10/2018 12:17 AM, Tom Tromey wrote:

> 
>>> +    m_observers.erase (m_observers.begin () + f);
> 
> Pedro> Hmm, this looks incorrect.  "attach" returns an index
> Pedro> as token.  And then detach uses "std::vector::erase()",
> Pedro> which removes the element at the index and then shifts
> Pedro> the rest of the elements left to fill in the space
> Pedro> left over by the removed element(s).  So that invalidates
> Pedro> all token after the element erased.
> 
> Yes, thanks for catching this.
> 
> I figure that observers aren't very space-sensitive, so I just put a
> counter into the observer and then put the value into the vector, like:
> 
>   std::vector<std::pair<size_t, func_type>> m_observers;

My initial reaction was "but counter of what?", but I took a
look at the patch, and I see what you mean.  I'll say more in
response to your new patch.

> 
> Pedro> This could be constexpr and then defined in-class, right?
> 
> Did it.
> 
> Pedro> (I wonder about splitting core/lib from uses of the core.
> Pedro> I.e., defining the struct observer core mechanism in one
> Pedro> header, and then define the actual observers in a separate header.
> Pedro> I'm basically pondering about e.g., using observers under common/
> Pedro> or in gdbserver/.  Like e.g., "common/observer.h" and then
> Pedro> "gdb/observers.h" and "gdbserver/observers.h", etc.  But we can cross
> Pedro> that bridge when we get to it, too.)
> 
> I did this.  Also I realized that calling the class "observer" is
> somewhat wrong, as the callback is the observer -- so I made
> common/observable.h and I renamed the class to "class observable".

Yeah, the current documentation in master calls them "subjects":

/* An observer is an entity who is interested in being notified when GDB
   reaches certain states, or certain events occur in GDB.  The entity being
   observed is called the Subject.  To receive notifications, the observer
   attaches a callback to the subject.  One subject can have several
   observers.

though "observable" does seem better to me.  

Thanks,
Pedro Alves


      parent reply	other threads:[~2018-02-12 18:37 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-11-05  1:33 Tom Tromey
2017-11-07 15:48 ` Pedro Alves
2018-02-10  0:17   ` Tom Tromey
2018-02-10  3:38     ` Tom Tromey
2018-02-12 18:43       ` Pedro Alves
2018-02-12 18:37     ` Pedro Alves [this message]

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=be7542fe-a9ab-c72e-6c68-18a9e5e1e806@redhat.com \
    --to=palves@redhat.com \
    --cc=gdb-patches@sourceware.org \
    --cc=tom@tromey.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