Repository: python-attrs/attrs

Problem statement:
auto_exc's hashing behavior and its documentation do not match
The [documentation](https://github.com/python-attrs/attrs/blob/25a02bbc7b5309acebda20101125372d42c978db/src/attr/_make.py#L846) for `auto_exc` says:
```
  - the values for *cmp* and *hash* are ignored and the instances compare
          and hash by the instance's ids (N.B. ``attrs`` will *not* remove
          existing implementations of ``__hash__`` or the equality methods. It
          just won't add own ones.),
```

However, the test for `auto_exc` [says](https://github.com/python-attrs/attrs/blob/25a02bbc7b5309acebda20101125372d42c978db/tests/test_dark_magic.py#L519):
```
    Classes with auto_exc=True have a Exception-style __str__, are neither
        comparable nor hashable, and store the fields additionally in
        self.args.
```

and [tests](https://github.com/python-attrs/attrs/blob/25a02bbc7b5309acebda20101125372d42c978db/tests/test_dark_magic.py#L558):

```
            with pytest.raises(TypeError):
                hash(e)
```

The documentation and the code should be made consistent.  I noticed this because `unittest` complained that the exception type was unhashable when thrown from a test and I see some [references to `logging` expecting hashable exceptions](https://github.com/schematics/schematics/issues/452), so the documented behavior is probably better than the implemented behavior.


Hints:
Oh god, are there any docs on Exception hashability? This looks bonkers:

```python
In [7]: hash(Exception()) == hash(Exception())
Out[7]: True

In [8]: hash(Exception(1)) == hash(Exception(1))
Out[8]: True

In [9]: hash(Exception(1)) == hash(Exception(2))
Out[9]: True

In [10]: hash(TypeError(1)) == hash(TypeError(2))
Out[10]: True
```
@hynek: This confused me for quite a while, but I think I have tracked it down, helped by some further investigation by @berquist.

Note you don't get the same error if you first assign the exceptions to variables before doing the comparison.  I think what is going on is:

1. `Exception` and `TypeError` have hash and equality by `id()`.
2.  The the first exception is created and its hash is taken from its memory location.  Then it is no longer referenced, so it is deleted from memory.  The second exception is created in the same memory location formerly occupied by the first exception and therefore has an identical `id()`.  See https://github.com/satwikkansal/wtfpython#-deep-down-were-all-the-same- .

For the life of me I can't find any documentation of the requirements or conventions for `__eq__` and `__hash__` with respect to exceptions.   There is definitely nothing about it in [the obvious](https://docs.python.org/3.7/tutorial/errors.html) [places](https://docs.python.org/3.7/library/exceptions.html) in the Python documentation.  This is unfortunately an extremely difficult thing to Google for, and I can't turn up anything else useful.  Also, what appears to be [the tests for the base exception classes in CPython](https://github.com/python/cpython/blob/8f4ef3b019ce380022018587571b0f970e668de3/Lib/test/test_exceptions.py) have no tests related to either hashing or equality. 

The actual behavior of the built-in exceptions seems to match what the `attrs` documentation says it will do (hashing and equality by `id()`), so it seems reasonable to switch the implementation to match the documentation.


Patch:
diff --git a/src/attr/_make.py b/src/attr/_make.py
--- a/src/attr/_make.py
+++ b/src/attr/_make.py
@@ -912,20 +912,20 @@ def wrap(cls):
             raise TypeError(
                 "Invalid value for hash.  Must be True, False, or None."
             )
-        elif hash is False or (hash is None and cmp is False):
+        elif hash is False or (hash is None and cmp is False) or is_exc:
+            # Don't do anything. Should fall back to __object__'s __hash__
+            # which is by id.
             if cache_hash:
                 raise TypeError(
                     "Invalid value for cache_hash.  To use hash caching,"
                     " hashing must be either explicitly or implicitly "
                     "enabled."
                 )
-        elif (
-            hash is True
-            or (hash is None and cmp is True and frozen is True)
-            and is_exc is False
-        ):
+        elif hash is True or (hash is None and cmp is True and frozen is True):
+            # Build a __hash__ if told so, or if it's safe.
             builder.add_hash()
         else:
+            # Raise TypeError on attempts to hash.
             if cache_hash:
                 raise TypeError(
                     "Invalid value for cache_hash.  To use hash caching,"


Test patch:
diff --git a/tests/test_dark_magic.py b/tests/test_dark_magic.py
--- a/tests/test_dark_magic.py
+++ b/tests/test_dark_magic.py
@@ -515,9 +515,8 @@ class C(object):
     @pytest.mark.parametrize("frozen", [True, False])
     def test_auto_exc(self, slots, frozen):
         """
-        Classes with auto_exc=True have a Exception-style __str__, are neither
-        comparable nor hashable, and store the fields additionally in
-        self.args.
+        Classes with auto_exc=True have a Exception-style __str__, compare and
+        hash by id, and store the fields additionally in self.args.
         """
 
         @attr.s(auto_exc=True, slots=slots, frozen=frozen)
@@ -545,21 +544,28 @@ class FooError(Exception):
         assert FooErrorMade(1, "foo") != FooErrorMade(1, "foo")
 
         for cls in (FooError, FooErrorMade):
-            with pytest.raises(cls) as ei:
+            with pytest.raises(cls) as ei1:
                 raise cls(1, "foo")
 
-            e = ei.value
+            with pytest.raises(cls) as ei2:
+                raise cls(1, "foo")
+
+            e1 = ei1.value
+            e2 = ei2.value
 
-            assert e is e
-            assert e == e
-            assert "(1, 'foo')" == str(e)
-            assert (1, "foo") == e.args
+            assert e1 is e1
+            assert e1 == e1
+            assert e2 == e2
+            assert e1 != e2
+            assert "(1, 'foo')" == str(e1) == str(e2)
+            assert (1, "foo") == e1.args == e2.args
 
-            with pytest.raises(TypeError):
-                hash(e)
+            hash(e1) == hash(e1)
+            hash(e2) == hash(e2)
 
             if not frozen:
-                deepcopy(e)
+                deepcopy(e1)
+                deepcopy(e2)
 
     @pytest.mark.parametrize("slots", [True, False])
     @pytest.mark.parametrize("frozen", [True, False])