litwindow-users

RE: [litwindow-users] m_current as a pointer

[yrs90 <yr...@ya...>]

> I wonder if m_current as a pointer into the underlying data is
> causing a circular reference somewhere during change resolution.

I see now.  When m_current is made a pointer that points into the underlying
data, the notification process does enter a death loop.  Of course, the nice
thing about the originally implementation is precisely that it was an
independent object being monitored for changes, just like any other object
for which a rule is defined.

I considered several different options:
a) change my container to permit copying
b) add a specializable assignment template like type_traits
c) create m_current with a copy constructor version of create_object()
d) figure out how to break the recursive loop when m_current points into a
container.

The first three options use m_current as defined originally.  The last
option changes m_current into a pointer to an accessor.

a) change my container to permit copying

I considered several ways to do this, but I can't see a way to accomplish
this. Overriding the key alone isn't sufficient because it's still going to
be a constant.  I would like to override pair<>, but I don't know how to get
map<> to use the pair<> derivative unless I reimplement map<> (not
desirable).  I can't just specialize pair<>::operator= (say, with a delete
and recreate using the copy constructor) because it doesn't have an
operator=() for me to override in pair<>.

b) add a specializable assignment template like type_traits

This doesn't get around the inability to copy a constant member.  However,
this would keep the framework general while still allowing the user to
specialize the assignment so that, in my case, perhaps I only copy the value
and not the key.

This would require that I define my adapter as permitting copy.  I presume
this is okay, but I am concerned that somewhere in the framework it might
copy data into the container rather.  If it's only going to be used in
m_current then a partial copy won't be problem.

c) create m_current with a copy constructor version of create_object()

This seems like a more complete work-around for the need to use the
copy-constructor for constant member initialization.  On the other hand, I
don't know if it actually increases or decreases compatibility by moving the
requirement from operator= to copy-constructor being defined.

d) figure out how to break the recursive loop when m_current points into a
container.

This sounds like a great solution, but I don't understand the code well
enough to pursue this yet.


So what I actually pursued to completion was (b), just adding an
assign<Type>::operator()(dst,src) that defaults to dst=src, but can be
overridden to provide a partial copy of pair<>.  Now however, I'm finding
that when current is reassigned for a second time (by clicking different
list entries) that I'm getting some type of memory violation.  I must be
overlooking something.  This should have been a minimal change.


Aside: While looking through the code, I noticed that in dataadapterimp.h,
the function implementation for converter_abstract_base::destroy() throws an
exception claiming to be clone.  Perhaps that body should belong to the
function converter_abstract_base::clone(), which is declared directly before
destroy()?

Regards,
Joel


---
[This E-mail scanned for viruses by Declude Virus]

[litwindow-users] Re: m_current as a pointer

[Hajo K. <mai...@ha...>]

yrs90 wrote:
>>I wonder if m_current as a pointer into the underlying data is
>>causing a circular reference somewhere during change resolution.
> 
> 
> I see now.  When m_current is made a pointer that points into the underlying
> data, the notification process does enter a death loop.  Of course, the nice

Can you post the relevant code? m_current declaration, and where you 
call NotifyChanged and other relevant places.


> Aside: While looking through the code, I noticed that in dataadapterimp.h,
> the function implementation for converter_abstract_base::destroy() throws an
> exception claiming to be clone.  Perhaps that body should belong to the
> function converter_abstract_base::clone(), which is declared directly before
> destroy()?

No. Thats a cut&paste error. It should read "not_implemented("destroy 
(abstract class)". The clone body is at dataadapter.h, line 871.

I'll correct and commit it. Thanks for finding.

BTW, I don't know if you have stumbled across create, clone and destroy 
yet as they are still undocumented. Together with a couple of other 
methods they implement an object factory, giving you the ability to 
create and clone objects dynamically by type, similar to the wxWidgets 
runtime type system.

Comes in useful when you want to write a generic insert method for your 
control.

Have a look at objectfactorytests.cpp for more information.

Regards

Hajo

RE: [litwindow-users] Re: m_current as a pointer

[yrs90 <yr...@ya...>]

To answer this email I'm returning to the pointer implementation I tried.
Maybe it will turn out to be a better solution.

Currently, though, I'm trying to figure out why, when using m_current as
originally designed, I can follow a particular click-sequence and watch what
starts out as an iterator pointing to correct contents translate into
corrupted data during the assignment to m_current.  Not only that but
m_current's original contents coming into the assignment look suspect too.

Actually, my investigation is hampered because I can't find the data or a
data pointer in the accessor data structure.  Could you tell me where to
find the data that the accessor is pointing to?  I can find it in the
iterator but lose it once it is dereferenced and becomes an accessor. To
find the data in the iterator 'i' I can look in
i.it.container_iterator.i._Tree._Ptr._Myval

So on (or back?) to m_current as a pointer.  I probably should have renamed
the variable to avoid confusion...

> Can you post the relevant code? m_current declaration, and where you 
> call NotifyChanged and other relevant places.

Let's see.  This is a little hard to boil down. I presume (perhaps wrongly)
that you prefer the narrative explanation over a diff.

lwListAdapterBase header was changed so
     mutable accessor    m_current;
became
     mutable accessor    *m_current;
     mutable accessor     m_empty;

m_current was initialized to &m_empty in lwListAdapterBase constructor.
m_empty.destroy() is called in the lwListAdapterBase destructor.

The biggest changes were to CalcCurrent where empty is assigned an empty
object if not !is_valid and m_current either points to the the_item or
m_empty accessor depending on whether the container index indicated an item
was selected.

void lwListAdapterBase::CalcCurrent(int containerIndex) const {
    if (m_items.is_valid() && m_items.is_container()) {
        // the const_accessor points to the end item which is invalid
        // but the type of the item is valid.
        container c=m_items.get_container();
        container::iterator i=containerIndex>=0 ?
m_items.get_container().at(containerIndex) : m_items.get_container().end();
        accessor the_item=*i;
        prop_t type=the_item.get_type();
        if (!m_empty.is_valid())
            m_empty=create_object(type);
        if (i!=c.end())
            m_current = &the_item;
        else {
            m_current = &m_empty;
        }
        g_rapidUI->ValueChanged(*m_current, true);
    }
}


Commands to destroy m_current were removed.  All ValueChanged() calls in
wxListObject were changed to reference *m_current rather than m_current.

> BTW, I don't know if you have stumbled across create, clone and 
> destroy yet as they are still undocumented. Together with a couple of 
> other methods they implement an object factory, giving you the ability 
> to create and clone objects dynamically by type, similar to the 
> wxWidgets runtime type system.
>
> Comes in useful when you want to write a generic insert method for 
> your control.
>
> Have a look at objectfactorytests.cpp for more information.

I'll take a closer look.  Thanks.

Best regards,
Joel



---
[This E-mail scanned for viruses by Declude Virus]

RE: [litwindow-users] Re: m_current as a pointer

[yrs90 <yr...@ya...>]

Oh, I didn't explain the failure mode for the m_current as pointer
implementation.  I have to paraphrase this because I don't have the stack
trace at hand.  When there was a rule defined using Current and data changed
(I presume this involved something like NotifyChanged("m_List");) then I
would see function calls related to find_scope that were sometimes seeking
...Current... and sometimes seeking the underlying data m_List.  Anyway I
stepped through watching iterations of find_scope until I got a heap
failure.

I rationalized this by thinking that recursing m_current and getting an
m_List member must have triggered Current to be evaluated and so on.
However, I didn't stay with it long to pinpoint the propagation mechanism.
(Lots of loops...) Instead I did a combination of tracing and sitting on
breakpoints until I confirmed I had localized the failure.

Regards,
Joel


-----Original Message-----
Sent: Thursday, April 28, 2005 4:46 AM
To: lit...@li...
Subject: RE: [litwindow-users] Re: m_current as a pointer

> Can you post the relevant code? m_current declaration, and where you 
> call NotifyChanged and other relevant places.

Let's see.  This is a little hard to boil down. I presume (perhaps wrongly)
that you prefer the narrative explanation over a diff.

lwListAdapterBase header was changed so
     mutable accessor    m_current;
became
     mutable accessor    *m_current;
     mutable accessor     m_empty;

m_current was initialized to &m_empty in lwListAdapterBase constructor.
m_empty.destroy() is called in the lwListAdapterBase destructor.

The biggest changes were to CalcCurrent where empty is assigned an empty
object if not !is_valid and m_current either points to the the_item or
m_empty accessor depending on whether the container index indicated an item
was selected.

void lwListAdapterBase::CalcCurrent(int containerIndex) const {
    if (m_items.is_valid() && m_items.is_container()) {
        // the const_accessor points to the end item which is invalid
        // but the type of the item is valid.
        container c=m_items.get_container();
        container::iterator i=containerIndex>=0 ?
m_items.get_container().at(containerIndex) : m_items.get_container().end();
        accessor the_item=*i;
        prop_t type=the_item.get_type();
        if (!m_empty.is_valid())
            m_empty=create_object(type);
        if (i!=c.end())
            m_current = &the_item;
        else {
            m_current = &m_empty;
        }
        g_rapidUI->ValueChanged(*m_current, true);
    }
}


Commands to destroy m_current were removed.  All ValueChanged() calls in
wxListObject were changed to reference *m_current rather than m_current.



---
[This E-mail scanned for viruses by Declude Virus]

RE: [litwindow-users] Re: m_current as a pointer

[yrs90 <yr...@ya...>]

Earlier I wrote:
> Could you tell me where to find the data that the accessor is pointing to?


I found it right in front of me in const_accessor.this_ptr.  I looked at the
memory this pointed to several times but I was expecting to see some string
data rather than just pointers to wxString.  It's obvious now, but I was
confused... sorry.

> Currently, though, I'm trying to figure out why, when using m_current as
> originally designed, I can follow a particular click-sequence and watch
> what starts out as an iterator pointing to correct contents translate into
> corrupted data during the assignment to m_current.  Not only that but
> m_current's original contents coming into the assignment look suspect too.

I figured this out too.  I wonder if this was a danger I should have been
sensitive to. (It vaguely sounds familiar.)  Here's my explanation.

A class that manages some allocated memory.

class A
{
  char *data;
  int size;
public:
  A():data(0),size(0){}
  ~A(){delete[]data;}

  char *getdata() { return data; }
  void setdata(int sz, char* src) 
  { 
    if(data) delete [] data;
    data = new char[sz];
    if(data) 
    {
      memcpy(data,src,sz);
      size = sz;
    }
  }

  int getsize() {return size;}

  A &operator(const A& a)
  {
    setdata(a.getsize(), a.getdata());
  }
}

And now a function from class typed_const_accessor:
const Value get() const
 {
     Value v;
     get(v);
     return v;
 }

Function get(v) will copy an instance of A into v using the operator=.  No
problem.

On return from get(void) I seem to see another instance of A being created,
but /data/ will have the same pointer value as v.data!  Then v gets
destroyed freeing the memory that /data/ references.

Perhaps I needed a copy constructor?  Anyway, I resolved by managing my data
in a new object that I embed in A.  Now it works!  <sigh>

Regards,
Joel



---
[This E-mail scanned for viruses by Declude Virus]