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
> Log:
> 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
branch, though?
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
no-op?).
> Modified:
> subversion/branches/swig-py3/subversion/bindings/swig/include/proxy.swg
>
> Modified: subversion/branches/swig-py3/subversion/bindings/swig/include/
> proxy.swg
> URL:
> http://svn.apache.org/viewvc/subversion/branches/swig-py3/subversion/bindings/swig/include/proxy.swg?rev=1819110&r1=1819109&r2=1819110&view=diff
> ==============================================================================
> --- 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 @@
> _assert_valid_deep(v)
> # 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
> instance.
> - elif type(value) != type and hasattr(value, "assert_valid"):
> + elif type(value) != type:
> + try:
> value.assert_valid()
> + except AttributeError:
> + pass
Hmm. Strictly speaking, the equivalent form would be:
try:
value.assert_valid
except AttributeError:
pass
else:
value.assert_valid()
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.
>
>
Cheers,
Daniel
Received on 2017-12-23 06:11:11 CET