On Tuesday 23 April 2013 19:11:29 William S Fulton wrote:

> On 19/04/13 18:41, Geert Janssens wrote:

> > Hi,

> >

> > I'm working through the failing testcases for guile. One testcase fails

> > with a compilation error in some generated code based on std_map and

> > std_pair. I know exactly where this generated code comes from, but my

> > C++ knowledge is insufficient to understand the error, let alone remedy

> > it. I gather some kind of const violation, but that's all I can read

> > from it :( My hope is that someone used to working with stl will more

> > easily understand it.

> >

> > So, if someone can help me understand the error and what the fix would

> > be in C++, I can fix the corresponding .i file.

>

> > This is the code snippet the causes the error:

> <snip>

>

> > /usr/lib/gcc/i686-redhat-linux/4.7.2/../../../../include/c++/4.7.2/bits/st

> > l_pair.h:88:12: error: non-static const member ‘const std::pair<int, A*>

> > std::pair<int, const std::pair<int, A*> >::second’, can’t use default

> > assignment operator

> This is the main problem - it is saying there is no default assignment

> operator (because one of the members in pair is const). The solution is

> to avoid assignment. I've attached a patch to do this plus some error

> message corrections.

>

> William

 

I have tested your patch on both my platforms (Fedora/Guile 1.8 and Ubuntu/Guile 2.0) and it fixes the testcase twice. Great and thank you for helping out !

 

I notice that you do more than just strictly fixing the invalid const assignment. I would guess those are small changes that come from experience and best-practice programming.

 

Would you mind shortly describing why you did the following changes, so (and perhaps others on the list) can learn from them ?

 

 

- %typemap(in) pair<T,U> (std::pair<T,U>* m) {

+ %typemap(in) pair<T,U> %{

 

Here I notice you choose to change from {} delimiters to %{ %}. I read in the docs this means that the code block will not be parsed by the swig preprocessor. The code seemed to work ok before as well (other than the const assignment bug). So what made you change it ?

 

You also remove the (std::pair<T,U>* m) part. I don't know why it was there, or why you removed it.

 

 

- }

- %typemap(in) const pair<T,U>& (std::pair<T,U> temp,

- std::pair<T,U>* m),

- const pair<T,U>* (std::pair<T,U> temp,

- std::pair<T,U>* m) {

+ %}

+ %typemap(in) const pair<T,U>& (std::pair<T,U> *temp = 0),

+ const pair<T,U>* (std::pair<T,U> *temp = 0) %{

 

Here you do something similar. I'm mostly interested in why you changed

(std::pair<T,U> temp, std::pair<T,U>* m) into (std::pair<T,U> *temp = 0) ?

 

%typemap(argout) const pair<T,U>&, const pair<T,U>* %{ delete temp$argnum; %}

 

I gather this is added because you allocated memory using new, which has to be freed. While reading the docs to get to understand this, I also found the "freearg" typemap. With my limited knowledge I interpreted that typemap for this use case. Can you clarify my confusion here ?

 

I hope you won't find my questions too basic... I really feel I have much to learn still.

 

Oh, and lastly, what do I do with your patch ? Do I add it to my branch (and then how do I attribute you for it) ? Or will you apply it yourself after my patches have been merged in ?

 

Geert