From: Kevin A. M. <ka...@da...> - 2007-09-24 17:38:14
|
David Piepgrass wrote: > >> -----Original Message----- >> From: Kevin A. Mitchell [mailto:ka...@da...] >> Sent: Friday, September 21, 2007 3:24 PM >> To: David Piepgrass >> Cc: swi...@li... >> Subject: RE: C++ "properties" to members in SWIG >> >> > >> One thing that I'm still chasing down is that the generated get/set >> routines don't seem to be respecting %naturalvar and so my >> > std::wstrings > >> are being treated as pointers...and I'm getting the scope problems I >> > had > >> before: >> > > I don't really understand what %naturalvar does, but if you need to get > rid of the braces ({ ... }) around the block, you can use %{ ... %} > instead of { } in your typemap. > > %naturalvar says to treat the type as if it was a natural variable, using a reference rather than a pointer. std::string and std::wstring are set to have this property. I'd still have difficulty with the scope of result_ref when I turn exception support back on. I turned it off to make the example easier to understand. My property getters can throw. The real problem is that if the member variable is due to extend, then it's accessed with a function call, which means it's not really part of the object. If the function call returns a value, then a const reference is bound to a temporary, and then we take the pointer to an object that is destroyed before we use it. I'm tinkering with a patch to Swig_wrapped_member_var_type(). Basically, the patch is that if we're using C++, and the member variable is due to %extend, and is a %naturalvar, use a value type instead of a const reference type. It may be more interesting to have a variation on %naturalvar that states a preference to use values instead of references. In my problem case, std::wstring is a class, but really makes more sense when treated as a value. Smart pointers may fall into the same category of being used as values instead of references. Taking this to swig-devel because it's turning into an internals discussion. Kevin |
From: David P. <dpi...@me...> - 2007-09-24 18:39:46
|
> I'm tinkering with a patch to Swig_wrapped_member_var_type(). Basically, > the patch is that if we're using C++, and the member variable is due to > %extend, and is a %naturalvar, use a value type instead of a const > reference type. Hmm, I notice there is no problem when SWIG is wrapping a function that returns a string. Do you happen to know why that is? I'm not sure your proposal would be acceptable in all cases. Consider cases like this (basically, the following is what %attribute_ref is for): %ignore Foo::X() %extend Foo { string x; } #define Foo_x_get(obj) obj->X() #define Foo_x_set(obj, value) obj->X() =3D value class Foo {=20 string _x; public: int& X() { return _x; } } Although "x" is added by %extend, it actually does refer to a variable inside the class (_x). =20 > It may be more interesting to have a variation on %naturalvar that > states a preference to use values instead of references. To generalize the solution, I suspect there should be a typemap to control $1_ltype (as explained in the manual section 26.7.3: "All arguments and local variables in wrapper functions are declared using this type so that their values can be properly assigned.") |
From: Kevin A. M. <ka...@da...> - 2007-09-24 21:15:45
|
David Piepgrass wrote: >> I'm tinkering with a patch to Swig_wrapped_member_var_type(). >> > Basically, > >> the patch is that if we're using C++, and the member variable is due >> > to > >> %extend, and is a %naturalvar, use a value type instead of a const >> reference type. >> > > Hmm, I notice there is no problem when SWIG is wrapping a function that > returns a string. Do you happen to know why that is? > Because the return type is (unqualified) string. So the type of the result variable in the wrapper function is std::string, it uses the std::string typemap, and it's all good. When referring to a member variable in C++, SWIG uses std::string*, unless %naturalvar is set, in which case it uses const std::string&. > I'm not sure your proposal would be acceptable in all cases. Consider > cases like this (basically, the following is what %attribute_ref is > for): > > %ignore Foo::X() > %extend Foo { > string x; > } > #define Foo_x_get(obj) obj->X() > #define Foo_x_set(obj, value) obj->X() = value > > class Foo { > string _x; > public: int& X() { return _x; } > } > > Although "x" is added by %extend, it actually does refer to a variable > inside the class (_x). > Thank you, the counterexample has me rethinking my solution. > > >> It may be more interesting to have a variation on %naturalvar that >> states a preference to use values instead of references. >> > > To generalize the solution, I suspect there should be a typemap to > control $1_ltype (as explained in the manual section 26.7.3: "All > arguments and local variables in wrapper functions are declared using > this type so that their values can be properly assigned.") > > $1_ltype is defined as the "lvalue type" of $type. I guess a big part of the trouble is that SWIG considers the lvalue type of a reference to be a pointer. It works to take the address of an object, as long as the object persists (if it was real). Because they aren't assignable, references don't really have lvalues, and therein lies some of the impedence mismatch, I think. Now I'm thinking more along the lines of adding a feature to mark an extended property as being a value type. Kevin |
From: William S F. <ws...@fu...> - 2007-09-25 21:24:11
|
Kevin A. Mitchell wrote: > > David Piepgrass wrote: >>> I'm tinkering with a patch to Swig_wrapped_member_var_type(). >>> >> Basically, >> >>> the patch is that if we're using C++, and the member variable is due >>> >> to >> >>> %extend, and is a %naturalvar, use a value type instead of a const >>> reference type. >>> >> Hmm, I notice there is no problem when SWIG is wrapping a function that >> returns a string. Do you happen to know why that is? >> > Because the return type is (unqualified) string. So the type of the > result variable in the wrapper function is std::string, it uses the > std::string typemap, and it's all good. > > When referring to a member variable in C++, SWIG uses std::string*, > unless %naturalvar is set, in which case it uses const std::string&. >> I'm not sure your proposal would be acceptable in all cases. Consider >> cases like this (basically, the following is what %attribute_ref is >> for): >> >> %ignore Foo::X() >> %extend Foo { >> string x; >> } >> #define Foo_x_get(obj) obj->X() >> #define Foo_x_set(obj, value) obj->X() = value >> >> class Foo { >> string _x; >> public: int& X() { return _x; } >> } >> >> Although "x" is added by %extend, it actually does refer to a variable >> inside the class (_x). >> > Thank you, the counterexample has me rethinking my solution. >> >> >>> It may be more interesting to have a variation on %naturalvar that >>> states a preference to use values instead of references. >>> >> To generalize the solution, I suspect there should be a typemap to >> control $1_ltype (as explained in the manual section 26.7.3: "All >> arguments and local variables in wrapper functions are declared using >> this type so that their values can be properly assigned.") >> >> > $1_ltype is defined as the "lvalue type" of $type. I guess a big part of > the trouble is that SWIG considers the lvalue type of a reference to be > a pointer. It works to take the address of an object, as long as the > object persists (if it was real). > > Because they aren't assignable, references don't really have lvalues, > and therein lies some of the impedence mismatch, I think. > > Now I'm thinking more along the lines of adding a feature to mark an > extended property as being a value type. > I don't think these extensions are warranted. See my other email on this thread on swig-user and let me know if that works for you. Also the types need to be marshalled by pointer/reference and not by value for reason outlined in the docs: SWIG.html#SWIG_structure_data_members. William |
From: David P. <dpi...@me...> - 2007-10-03 15:34:05
|
Sending to swig-devel for some developer feedback. The problem: %include "attribute.i" %include "std_wstring.i" %attribute(Foo, std::wstring, FileName, GetFileName); %inline { class Foo { std::wstring GetFileName(); }; } Produces wrapper code that assumes wstring can be treated as a reference, which it cannot: { std::wstring const &_result_ref =3D Foo_FileName_get(arg1); result =3D (std::wstring *) &_result_ref; } I think the problem here is that attributes (aka member variables) use the logic designed for references rather than the logic for rvalues. Sometimes it is better to use the logic designed for references, but if the attribute is returned as an rvalue then it is *crucial* for it to be treated as such. If SWIG is ignoring %naturalvar then that is definitely part of the problem, but not the entire problem because even if a type is not %naturalvar, it still ought to be treated differently if it is an rvalue. But the root problem might be that SWIG treats attributes added with %extend the same way it treats variables, which is just not correct in our situation. It would be nice if we could tell swig: "treat this attribute as wstring, not as wstring& like you do for variables!" Maybe a new %feature could be introduced for such a purpose, either that the syntax could be changed (but it would break compatibility): %extend MyClass { std::wstring& X; // treat as a reference std::wstring X; // DO NOT treat as a reference } Alternatively, a typemap could come to the rescue (the disadvantage of a typemap being that they it would apply to ALL attributes and variables). But recall that the "_result_ref" thing is hard-coded in the Swig_cresult() function, and that function calls SwigType_type() to decide what to do. The interesting thing is the description of SwigType_type: /* ----------------------------------------------------------------------- * SwigType_type() * * Returns an integer code describing the datatype. This is only used for * compatibility with SWIG1.1 language modules and is likely to go away once * everything is based on typemaps. * -----------------------------------------------------------------------* / Does this suggest that some lead developer intended to change Swig_cresult to use typemaps eventually? A typemap would certainly allow us to solve our little problem. Thanks, - David |