stop suppressing unrelated exceptions in PyAny::hasattr

[davidhewitt]

I realised today that our hasattr implementation suppresses all exceptions, which is inconsistent with hasattr, which only suppresses AttributeError.

>>> class Foo:
...     def __getattribute__(self, *args):
...         raise ValueError("oh no")
...
>>> hasattr(Foo(), "bar")
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "<stdin>", line 3, in __getattribute__
ValueError: oh no

See also capi-workgroup/problems#51

TODO: Before merging, I should push a test for this.

[adamreichold]

This does imply an unnecessary increment-then-decrement of the reference count, doesn't it? Also why GetItem instead of GetAttr as suggested by the docs?

If we do eat the reference count modifications, couldn't this just call getattr(name) and match on the error?

[davidhewitt]

This does imply an unnecessary increment-then-decrement of the reference count, doesn't it?

Sadly yes, though this was always the case, just abstracted away in PyObject_HasAttr.

Also why GetItem instead of GetAttr as suggested by the docs?

A bug, should be GetAttr, this is exactly why I need to push a test 😄

[davidhewitt]

Changed the implementation to go via getattr as suggested (and pushed tests).

[adamreichold]

Just to make sure I understand it correctly: Calling _getattr instead of getattr avoids the into_ref(py) which involves the pool? So when we drop the pool and getattr returns PyAny<'py> this distinction can be removed?

[davidhewitt]

Yes that's exactly right. Avoiding the pool means that we don't end up holding a reference to whatever we checked for in hasattr in the pool - so it's just a micro-optimization for now that should reduce away soon.