Branko Čibej wrote:
> For anyone who wants to work on updating the bindings: note that we have
> a lot of language-specific stuff in there that's a consequence of Swig 1
> not knowing any better back in the day. Most of those typemaps can be
> made language-independent (thus reducing the size of the Swig files)
> with features from Swig 2 and especially 3, which introduced built-in
> constructs for handling various kinds of data structures that we're
> currently mapping separately for each target language.
Hooray! I was hoping that might be the case.
Another thought. Some comments and code in the current bindings seem
to indicate that a few of our C APIs are written in a way that makes
it harder to bind in a 'natural' way. We could consider changing those
C APIs to use idioms that bind better.
Here is something that at first seemed a very trivial update: adding
'const' to apr_array_header_t input parameters. I added 'const' across
all instances in our C API many years ago (and I see just one that lacks
it now: svn_repos_freeze()), so we can update the typemaps and remove
the obsolete comment seen in the following patch:
[[[
Index: subversion/bindings/swig/include/svn_containers.swg
===================================================================
--- subversion/bindings/swig/include/svn_containers.swg (revision 1826615)
+++ subversion/bindings/swig/include/svn_containers.swg (working copy)
@@ -639,26 +639,18 @@
}
#endif
-/* svn_delta_path_driver() mutates its 'paths' argument (by sorting it),
- despite the fact that it is notionally an input parameter - hence, the
- lack of 'const' in that one case.
-
- svn_wc_get_update_editor3() and svn_wc_get_switch_editor3() aren't changing
- their 'preserved_exts' argument, but it is forwarded to
- svn_cstring_match_glob_list which also doesn't modify it, but does not have
- const in its prototype. */
%apply const apr_array_header_t *STRINGLIST {
const apr_array_header_t *args,
const apr_array_header_t *diff_options,
- apr_array_header_t *paths,
- apr_array_header_t *revprops,
+ const apr_array_header_t *paths,
+ const apr_array_header_t *revprops,
const apr_array_header_t *targets,
- apr_array_header_t *preserved_exts
+ const apr_array_header_t *preserved_exts
};
#if defined(SWIGPERL) || defined(SWIGRUBY)
%apply const apr_array_header_t *STRINGLIST_MAY_BE_NULL {
- apr_array_header_t *revprops
+ const apr_array_header_t *revprops
};
#endif
]]]
But then I went on through that file doing the same thing until I came to
one where both input and output typemaps are applied to the same pattern
(in Ruby):
[[[
#ifdef SWIGRUBY
%typemap(in) apr_array_header_t *PROP_LIST
{
VALUE rb_pool;
apr_pool_t *pool;
svn_swig_rb_get_pool(argc, argv, self, &rb_pool, &pool);
$1 = svn_swig_rb_to_apr_array_prop($input, pool);
}
%typemap(out) apr_array_header_t *PROP_LIST
{
%append_output(svn_swig_rb_prop_apr_array_to_hash_prop($1));
}
[...]
%apply apr_array_header_t *PROP_LIST {
apr_array_header_t *wcprop_changes,
apr_array_header_t *propchanges
};
]]]
Now, a 'wcprop_changes' identifier appears not only as function arguments
(with 'const') but also as a field in 'svn_client_commit_item2_t' (without).
At this point I am for the time being out of my depth -- I don't know enough
to know where to add 'const' and where not.
I'll try to learn more Swig, sooner or later.
- Julian
