[svn.haxx.se] · SVN Dev · SVN Users · SVN Org · TSVN Dev · TSVN Users · Subclipse Dev · Subclipse Users · this month's index

Re: parameter name of svn_fs_paths_changed2() -- was: Re: SWIG (Perl) prospective patch

From: Neels Janosch Hofmeyr <neels_at_elego.de>
Date: Wed, 17 Jun 2009 05:24:06 +0200

C. Michael Pilato wrote:
> Stefan Sperling wrote:
>> On Tue, Jun 16, 2009 at 04:07:26PM +0200, Neels Janosch Hofmeyr wrote:
>>> That said, I see how the bindings sort of *need* it to be a different name.
>>>
>>> [[[
>>> %hash_argout_typemap(changed_paths_p, svn_fs_path_change_t *)
>>> +%hash_argout_typemap(changed_paths2_p, svn_fs_path_change2_t *)
>>> ]]]
>>>
>>> The bindings code shouldn't be like that. after all, variable names can be
>>> similar and hash tables can contain anything.
>> Somehow this doesn't make sense to me...
>> I must be missing something.
>>
>> Why do bindings depend on the name of variables, instead of the type?
>> E.g. it's legal in C to omit variable names from function prototypes
>> altogether. What would the bindings do then?
>>
>> If the type of values in a hash table changes, can't the bindings
>> deal with this by making changes internally to the function,
>> just like we do in C?
>>
>> I admit that I don't know much about how SWIG works, but relying
>> on the names of variables seems really wrong. And from the short
>> time I played with SWIG a long time ago I can't recall it having
>> such a requirement.
>
> The bindings *can* rely on the type, when the type is distinct enough. But
> it's really hard to write a function that converts an apr_hash_t * of
> arbitrary C data type keys and arbitrary C data type values into a Python
> dictionary of Python object keys and Python object values. You rather need
> to know the specifics of the data types inside the hash/dictionary. There
> are two ways to do this: make a new typedef in C for every possible flavor
> of apr_hash_t so the bindings can distinguish between them, or use
> variable-name-based hints to pull that off.
>

So you're saying, since we don't create a separate typedef for each hash
table, the bindings will never work unless we rename that parameter? In that
case, +1 for

[[[
--- subversion/include/svn_fs.h
+++ subversion/include/svn_fs.h
@@ -1135,11 +1135,11 @@
  * Use @c pool for all allocations, including the hash and its values.
  *
  * @since New in 1.6.
  */
 svn_error_t *
-svn_fs_paths_changed2(apr_hash_t **changed_paths_p,
+svn_fs_paths_changed2(apr_hash_t **changed_paths2_p,
                       svn_fs_root_t *root,
                       apr_pool_t *pool);

 /** Same as svn_fs_paths_changed2(), only with @c svn_fs_path_change_t * values
]]]

~Neels

------------------------------------------------------
http://subversion.tigris.org/ds/viewMessage.do?dsForumId=462&dsMessageId=2362676

Received on 2009-06-17 05:25:11 CEST

This is an archived mail posted to the Subversion Dev mailing list.

This site is subject to the Apache Privacy Policy and the Apache Public Forum Archive Policy.