troycurti...@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
