m_current as a pointer

[yrs90]

It turns out that the pair<> issue isn't a copy constructor or operator=.
The problem is that map<A,B> creates pairs pair<const A, B>. The "const" is
what breaks assignability and keeps copy from working.

To overcome this I changed the list adapter implementation so that m_current
is accessor*, a pointer. This eliminates the need to copy. I created an
m_empty list-adapter member that m_current points to when there isn't an
item selected. This seems to work. I can add to the list, delete selected
items, etc.

Now, however, I am running into problems with rules that reference Current.
Since Current is a pair<> it is necessary to use ".second" in order to
access the data. Frequently, the rule that includes Current can be seen to
work correctly before an exception (for example it can put the selected item
in the title bar), but an exception will be thrown whenever some state
changes (insert, selection, etc).

I think it has something to do with the structure walking process. When I
was looking at this earlier, I think it was encountering null schema ?type?,
but now I seem to be getting some type of heap allocation error. I wonder
if m_current as a pointer into the underlying data is causing a circular
reference somewhere during change resolution.

I'll try to compose an explanation. First I'll try using a vector instead
of a map and see if a RULE using Current behaves any differently.



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



-------------------------------------------------------
SF.Net email is sponsored by: Tell us your software development plans!
Take this survey and enter to win a one-year sub to SourceForge.net
Plus IDC's 2005 look-ahead and a copy of this survey
Click here to start! http://www.idcswdc.com/cgi-bin/survey?id=105hix

[yrs90]

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]



-------------------------------------------------------
SF.Net email is sponsored by: Tell us your software development plans!
Take this survey and enter to win a one-year sub to SourceForge.net
Plus IDC's 2005 look-ahead and a copy of this survey
Click here to start! http://www.idcswdc.com/cgi-bin/survey?id=105hix

[Hajo Kirchhoff]

Post by yrs90

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.

Post by yrs90
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



-------------------------------------------------------
SF.Net email is sponsored by: Tell us your software development plans!
Take this survey and enter to win a one-year sub to SourceForge.net
Plus IDC's 2005 look-ahead and a copy of this survey
Click here to start! http://www.idcswdc.com/cgi-bin/survey?id=105hix

[yrs90]

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...

Post by Hajo Kirchhoff
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.

Post by Hajo Kirchhoff
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]



-------------------------------------------------------
SF.Net email is sponsored by: Tell us your software development plans!
Take this survey and enter to win a one-year sub to SourceForge.net
Plus IDC's 2005 look-ahead and a copy of this survey
Click here to start! http://www.idcswdc.com/cgi-bin/survey?id=105hix

[yrs90]

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: litwindow-users-5NWGOfrQmneRv+***@public.gmane.org
Subject: RE: [litwindow-users] Re: m_current as a pointer

Post by Hajo Kirchhoff
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]



-------------------------------------------------------
SF.Net email is sponsored by: Tell us your software development plans!
Take this survey and enter to win a one-year sub to SourceForge.net
Plus IDC's 2005 look-ahead and a copy of this survey
Click here to start! http://www.idcswdc.com/cgi-bin/survey?id=105hix

[yrs90]

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]



-------------------------------------------------------
SF.Net email is sponsored by: Tell us your software development plans!
Take this survey and enter to win a one-year sub to SourceForge.net
Plus IDC's 2005 look-ahead and a copy of this survey
Click here to start! http://www.idcswdc.com/cgi-bin/survey?id=105hix