troycurtisjr_at_apache.org wrote on Sat, 23 Dec 2017 04:43 +0000:
> Author: troycurtisjr
> Date: Sat Dec 23 04:43:26 2017
> New Revision: 1819110
> URL: http://svn.apache.org/viewvc?rev=1819110&view=rev
> On branch swig-py3: Replace hasattr check for a method with try-except.
> * subversion/bindings/swig/include/proxy.swg
> (_assert_valid_deep): Replace hasattr check for the 'assert_valid' method
> with a try-except block as some class instances can have the method but return
> False to hasattr().
I'm not too familiar with this code; shouldn't we be fixing the original
problem, of hasattr() wrongly returning False? I assume it predates the
While reviewing this I also noticed that svn_swig_py_convert_ptr() also
does this hasattr() check — not sure whether it needs to change too? —
and also has an "x | 0" construct, which seems suspicious (isn't it a
> Modified: subversion/branches/swig-py3/subversion/bindings/swig/include/
> --- subversion/branches/swig-py3/subversion/bindings/swig/include/
> proxy.swg (original)
> +++ subversion/branches/swig-py3/subversion/bindings/swig/include/
> proxy.swg Sat Dec 23 04:43:26 2017
> @@ -57,8 +57,11 @@
> # Ensure that the passed in value isn't a type, which could have an
> # assert_valid attribute, but it can not be called without an
> - elif type(value) != type and hasattr(value, "assert_valid"):
> + elif type(value) != type:
> + try:
> + except AttributeError:
> + pass
Hmm. Strictly speaking, the equivalent form would be:
since we don't want to mask any AttributeErrors inside assert_valid().
I'm not sure how careful we are about this distinction, though.
> /* Default code for all wrapped proxy classes in Python.
Received on 2017-12-23 06:11:11 CET