pschanely / CrossHair

An analysis tool for Python that blurs the line between testing and type systems.
Other
996 stars 47 forks source link

Weird failures using sets #297

Closed Zac-HD closed 1 month ago

Zac-HD commented 1 month ago

Expected vs actual behavior The code below runs without error in CPython, and fails with TypeError: BadCompare.__eq__() takes 1 positional argument but 2 were given under Crosshair.

(I was originally debugging a RecursionError using https://github.com/DRMacIver/shrinkray, but my test script was only conditioned on 'fails on crosshair, passes on hypothesis backend' and so here we are. I'll make another attempt with a more specific condition too.)

To Reproduce

class BadCompare:
    def __eq__(self):
        pass

    def __hash__(self):
        return 0

def test_repro(x: bool):
    """post: True"""
    for _ in set([BadCompare]) | {BadCompare()}:
        pass
Zac-HD commented 1 month ago

Turns out that the RecursionError is even more trivial - it doesn't even need any elements in the set!

def test_repro(x: bool):
    """post: True"""
    s = set()
    copy = set(s)
    s |= s
    s != copy
pschanely commented 1 month ago

Thanks!!!

The first case is tricky. Was it intentional to not instantiate BadCompare in set([BadCompare])? The type is unlikely to hash to zero bucket, so I don't think normal Python attempts the equality check. CrossHair however, avoids hashing and jumps to an exploding equality comparison. So far, my gut says to accept this unsoundness, and encourage people to use other tests to ensure their __eq__ and __hash__ methods are well-formed, pure, and consistent with each other. BUT, I've toyed with a variety of alternative ideas now and then (that I won't attempt to describe right this second).

The second case is a plain old bug, and an important one! Fix is in progress.

Zac-HD commented 1 month ago

The first case was just what shrinkray gave me when I wasn't careful enough about the condition (reducers are fuzzers!) - I don't have any real reason to care about it, but figured I might as well report that too while the second one was processing 🙂

pschanely commented 1 month ago

The first case was just what shrinkray gave me when I wasn't careful enough about the condition (reducers are fuzzers!) - I don't have any real reason to care about it, but figured I might as well report that too while the second one was processing 🙂

Got it! I've hopefully fixed the set-related recursion errors in 0.0.67. And I've started a discussion about the larger question brought up by your first example in #299