You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@subversion.apache.org by Peter Samuelson <pe...@p12n.org> on 2008/01/30 01:04:57 UTC

Re: More info on Swig 1.31 vs 1.33

Roderich, thanks for all your hard work!

Kouhei, Joe, * - can anyone have a look at this compatibility issue
with svn + ruby + swig 1.3.33?  Roderich Schupp has figured out exactly
what the problem is, but I have no idea how to address it.

His analysis applies to 1.4.x.  Apologies if this has already been
fixed on trunk - I can't easily check, as 'make check-swig-rb' has been
failing for some time for unrelated reasons, like
"subversion/bindings/swig/ruby/svn/error.rb:68: uninitialized constant
Svn::Error::WcMismatchedChangelist (NameError)".

Thanks,
Peter


[Roderich Schupp writes...]

Hi,

I got my shovel and dug some more into these
"TypeError: Expected argument 1 of type ..., but got Array []" errors.
Here's what I found (warning lengthy explanation ahead).

(1)  In general, Swig tries to deliver output parameters (as viewed from
the C level) as multiple return values in the wrapper (for target languages
that support multiple return values, like Python, Perl or Ruby). If the
C function also has a return value, this counts as the zeroeth output
parameter.

(2) The means to map an output parameter from C to the target
language is an argout typemap. Now almost all argout typemaps
for Ruby look like this:

 %typemap(ruby, argout, fragment="output_helper") some_type
 {
    /* marshal $1 (some_type) into Ruby as variable foo */

    $result = output_helper($result, foo);
 }

where the final line

    $result = output_helper($result, foo);

does the following (note that $return is a Swig meta variable
and stands for the actual name of the variable that will
finally be returned from the generated function wrapper):

   if $esult == Nil then
      # the current output parameter is the first output parameter
      $result = foo
   elsif !($result is a Ruby array)
      # the current output parameter is the second output parameter
      $result = new_ruby_array($result, foo)
   else
      # the current output parameter is the third or higher output parameter
      push $result, foo

(3) There's one problem with this: it doesn't work if foo is already
a Ruby array AND foo is the first output parameter AND there are
other output parameters. In that case the wrapper should return
a Ruby array with foo as its first element and the other output parameters
as the rest of the elements. But it will actually return foo itself,
with the other output parameters appended to foo.

The following change tries to address this problem:
http://swig.svn.sourceforge.net/viewvc/swig/trunk/Source/Modules/ruby.cxx?r1=9721&r2=9746&pathrev=9746
It counts the number of output parameters in a first pass and if this
is > 1 inserts the line

  vresult = rb_ary_new();           #  ***

just before the code rom the argout typemaps is emitted.
This fix is correct, Swig is not to blame. Now over to Subversion.

(4) If you look at the Ruby wrapper generated by Swig
for a Subversion function like

void svn_opt_format_option(const char **string,
                      const apr_getopt_option_t *opt,
                      svn_boolean_t doc,
                      apr_pool_t *pool);

(and compare code generated with Swig 1.31 and 1.33)
you see that Swig 1.33 just adds the above line ***. This will
result in an error "expected string, but got Array". But why
does Swig think that this function has more than one output parameter -
obviously there's only one (string). But Swig just does what
it's been told because Subversion (line 350 in
subversion/bindings/swig/include/svn_types.swg)
specifies an argout typemap for parameters of type apr_pool_t*

%typemap(ruby, argout) apr_pool_t *pool
{
   svn_swig_rb_set_pool($result, _global_svn_swig_rb_pool);
   svn_swig_rb_pop_pool(_global_svn_swig_rb_pool);
}

This isn't really output parameter processing (hint: it doesn't
assign to $result), it is merely abusing an argout typemap
to apply some final processing AFTER all "real"
output parameters have been processed (and accumulated
in $result), but BEFORE the final "return $result".
This works since apr_pool_t* parameters
are generally the last parameter in SVN and APR functions.

Note that this approach is rather specific to Ruby. Python does
the equivalent processing in a freearg typemap (line 319).
(freearg typemap code is emitted at roughly the same place
as the code for the final argout parameter.)
But Ruby probably can't just switch to a freearg typemap,
because it requires access to $result which isn't available
in freearg typemaps (though I don't understand what
svn_swig_rb_set_pool does with $result).


Cheers, Roderich

Re: More info on Swig 1.31 vs 1.33

Posted by Kouhei Sutou <ko...@cozmixng.org>.
Hi,

In <a7...@mail.gmail.com>
  "Re: More info on Swig 1.31 vs 1.33" on Thu, 31 Jan 2008 13:32:58 +0900,
  "Kouhei Sutou" <ko...@cozmixng.org> wrote:

> Solution:
> 1) use 'vresult' instead of $result. :<
> 2) ask for reverting the SWIG change and mark SWIG 1.3.33 as 'not supported'.
> 3) come up with any other implementation idea.

I used 1) in r29201 because I've already used 'vresult'.


Thanks,
--
kou

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org

Re: More info on Swig 1.31 vs 1.33

Posted by Kouhei Sutou <ko...@cozmixng.org>.
Hi,

2008/1/30, Peter Samuelson <pe...@p12n.org>:

>  %typemap(ruby, argout) apr_pool_t *pool
>  {
>    svn_swig_rb_set_pool($result, _global_svn_swig_rb_pool);
>    svn_swig_rb_pop_pool(_global_svn_swig_rb_pool);
>  }
>
>  This isn't really output parameter processing (hint: it doesn't
>  assign to $result), it is merely abusing an argout typemap
>  to apply some final processing AFTER all "real"
>  output parameters have been processed (and accumulated
>  in $result), but BEFORE the final "return $result".
>  This works since apr_pool_t* parameters
>  are generally the last parameter in SVN and APR functions.
>
>  Note that this approach is rather specific to Ruby. Python does
>  the equivalent processing in a freearg typemap (line 319).
>  (freearg typemap code is emitted at roughly the same place
>  as the code for the final argout parameter.)
>  But Ruby probably can't just switch to a freearg typemap,
>  because it requires access to $result which isn't available
>  in freearg typemaps (though I don't understand what
>  svn_swig_rb_set_pool does with $result).

This is for GC. The ruby bindings create a pool for each method call.
The pool is owned (referenced) by $result not self. If self ownes the pool,
the pool is never GCed until self is GCed even if $result that uses memory
in the pool is GCed. If $result ownes the pool, the pool will be GCed when
$result is GCed even if self isn't GCed.

Solution:
1) use 'vresult' instead of $result. :<
2) ask for reverting the SWIG change and mark SWIG 1.3.33 as 'not supported'.
3) come up with any other implementation idea.


Thanks,
--
kou

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org

Re: More info on Swig 1.31 vs 1.33

Posted by Joe Swatosh <jo...@gmail.com>.
Hi Peter,

On Jan 29, 2008 5:04 PM, Peter Samuelson <pe...@p12n.org> wrote:
>
> Roderich, thanks for all your hard work!
>
> Kouhei, Joe, * - can anyone have a look at this compatibility issue
> with svn + ruby + swig 1.3.33?  Roderich Schupp has figured out exactly
> what the problem is, but I have no idea how to address it.
>
> His analysis applies to 1.4.x.  Apologies if this has already been
> fixed on trunk - I can't easily check, as 'make check-swig-rb' has been
> failing for some time for unrelated reasons, like
> "subversion/bindings/swig/ruby/svn/error.rb:68: uninitialized constant
> Svn::Error::WcMismatchedChangelist (NameError)".
>

>
>

I'm not sure how much help I can be.  The Ruby bindings never worked on
Windows for 1.4, but I've downloaded SWIG 1.3.33 and after I get the
bindings tests passing again I will try it out as time allows.

--
Joe

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org