gh-129463: gh-128593: Simplify ForwardRef

[JelleZijlstra]

Fixes #129463. Fixes #128593.

• Issue: ForwardRef: remove __hash__ and __eq__ #129463

[sobolevn]

We can remove this else branch and just write: try: return self.__cell__.cell_contents

[sobolevn]

We try really hard to unwire different test cases, if possible - maybe it would be better to import this object from somewhere else?

[leycec]

Ho, ho... ho. @beartype maintainer @leycec has been summoned via #129463. As a born contrarian living in a cabin in the Canadian woods with too much time and too little sense, I have a lot of tiresome things to say about this and every other topic. Thankfully, nobody wants to hear it. I'll just pontificate about type hint smells instead.

So Even Type Hints Smell Now, Huh?

A type hint smell is the typing equivalent of a code smell. All existing PEP-compliant type hints (including typing.ForwardRef) currently satisfy this simple maxim. In fact, all existing PEP-noncompliant third-party type hints supported by @beartype (like numpy.typing.NDArray[...]) also satisfy this simple maxim.

It's simple. Thus, it's maximally supported:

from typing import TypeForm

def is_hints_smelly(hint_a: TypeForm, hint_b: TypeForm) -> bool:
    '''
    :data:`True` only if the passed type hints **smell** (i.e.,
    are poorly designed in a manner suggesting catastrophic
    failure by end users).
    '''

    return not (
        (repr(hint_a) == repr(hint_b)) is
        (     hint_a  ==      hint_b)
    )

There should thus exist a one-to-one correspondence between:

• The string representation (i.e., repr()) of a type hint.
• The identity (i.e., is) or equality (i.e., ==) of that type hint against other hints.

Two type hints that share the same string representation should thus either literally be the same type hint or compare equal to one another. Makes sense, right?

In fact, this maxim makes so much sense that we can pretty much extend it to most object-oriented APIs. An API that violates this smell test isn't necessarily bad per say, but it is suggestive of badness.

Nobody Cares About Your Suggestions, Though

In the case of type hints, this isn't quite a "suggestion."

@beartype really wants this smell test to hold across all type hints. @beartype aggressively memoizes all internal calls on the basis of type hints and their string representations. Since all existing type hints satisfy the above smell test, they also memoize well out-of-the-box with respect to @beartype.

The proof is in the tedious pudding. Pretend this matters:

# Prove that the "list[int]" type hint isn't smelly.
>>> is_hints_smelly(list[int], list[int])
False  # <-- good!

# Prove that the "Literal['smells', 'good']" type hint isn't smelly.
>>> is_hints_smelly(Literal['smells', 'good'], Literal['smells', 'good'])
False  # <-- good!

# Prove that the "ForwardRef('muh_package.MuhType')" type hint isn't smelly.
>>> is_hints_smelly(ForwardRef('muh_package.MuhType'), ForwardRef('muh_package.MuhType'))
False  # <-- good!

So. We're agreed. Existing type hints don't smell. That's good... isn't it? 🤔 💭 💥

Would You Please Stop Talking Already

No. I've warned you that I would pontificate! I intend to do just that.

The proposed refactoring does simplify the existing implementation of typing.ForwardRef, which is to be applauded. Let us ease the burden of CPython volunteer devs. They have suffered too much already.

The proposed refactoring also violates the above smell test. After merging this PR, typing.ForwardRef will now produce smelly type hints. Two forward references to the same referent will still share the same string representations but no longer compare as equal (and certainly won't be identical objects).

So. You Admit You Are Willing to Do Something Then?

No. I'm super-lazy! I just pontificate without writing code. Mostly.

Actually, I lie. I casually inspected the typing.ForwardRef implementation for the first time a hot minute ago and... it looks pretty good, actually! It's nearly there.

Sure. CPython could diminish the real-world utility of forward references by gutting the __eq__() and __hash__() dunder methods. That's not hard. Anyone can destroy something useful. But why bother? Am I right? Deep down, you know I'm probably right. Let us preserve useful things instead.

I have a trivial proposal. It is trivial, because I just did it. If I can do it, literally anyone can do it. I live in a cabin in the woods, people. Let's just resolve the issue in the __hash__() dunder method by aligning the implementation of that method with that of the existing __eq__() dunder method: e.g.,

    # This is what I'm a-sayin', folks.
    def __hash__(self):
        return hash(
            (self.__forward_arg__, self.__forward_value__)
            if self.__forward_evaluated__ else
            (self.__forward_arg__, self.__forward_module__)
        )

Literally just two more lines of code. That's it. Of course...

Python 3.14: It Is Hell

I acknowledge that Python 3.14 is coming. Indeed, Python 3.14 is almost here. typing.ForwardRef is mutating into a shambolic monstrosity under Python 3.14, isn't it? Sure. It probably is.

Preserving the fragrant non-smelliness of typing.ForwardRef type hints is still useful, though – Python 3.14 or no Python 3.14. How many more fields could Python 3.14 possibly be adding to typing.ForwardRef, anyway? Surely, somebody who is not me wants to extend the above issue resolution to cover those Python 3.14 "improvements."

Somebody? Anybody? Hellllllllllo? 😮‍💨

tl;dr: Let Us Fix a Thing Rather than Break a Thing

Thus spake @leycec. He don't know much, but he know a thing. This might be that thing.

[Viicos]

@leycec, keeping the caching mechanism through __forward_evaluated__ is not going to fix #128593, and I think removing this mechanism altogether is the best way to go (or else we should avoid caching _GenericAlias.__getitem__ calls with string values).

Also, I'm not sure trying to hash __forward_value__ is safe:

class A:
    __hash__ = None

hash(Annotated[int, A()])
# TypeError

---

Two type hints that share the same string representation should thus either literally be the same type hint or compare equal to one another. Makes sense, right? [...] https://github.com/beartype aggressively memoizes all internal calls on the basis of type hints and their string representations.

It seems to be that this was a bad assumption on your end, that is now causing an issue. I wouldn't rely on string representations for caching, even though it works for most type hints.

[JelleZijlstra]

I feel we should get rid of the value caching mechanism (__forward_evaluated__, __forward_value__); that mechanism provides little if any value and it's bound to be a source of confusion and bugs.

I'm more open to keeping __hash__/__eq__, though if we keep them we should make them look at all attributes on the ForwardRef object. If nothing else this would make testing with ForwardRef objects easier (see the test changes in this PR); it might also alleviate some of @leycec's concerns.