The hash and equals methods of code objects are large, complex and unnecessary. #101346

Activity

1. markshannon

   added

   performancePerformance or resource usagePerformance or resource usage

   on Jan 26, 2023
2. markshannon commented on Jan 26, 2023

   

   markshannon

   on Jan 26, 2023

   MemberAuthor

   More actions

   This will involves a slight change in semantics:
   Currently:

   >>> def (): pass
   >>> c = f.__code__
   >>> c.replace() == c
   True

   With this change:

   >>> c.replace() == c
   False
3. Eclips4 commented on Feb 2, 2023

   

   Eclips4

   on Feb 2, 2023

   Member

   More actions

   It's a good idea.
   What's stopping you from implementing this?

   How i understand, we need to delete code_hash and code_richcompare from Objects/codeobject.c(and ofc, set tp_hash and tp_richcompare to 0 in PyCode_Type
   Does it breaks backwards compatibility?
4. carljm commented on Mar 18, 2023

   

   carljm

   on Mar 18, 2023

   Member

   More actions

   We should just compare code objects by value.

   I assume this meant to say "by identity."

   This will involves a slight change in semantics

   So do we need a SC ruling on whether this needs a deprecation process? I guess technically we could raise a DeprecationWarning anytime an __eq__ comparison returns True for different code objects.
5. carljm

   added a commit that references this issue on Apr 5, 2023

   pythongh-101346: compare and hash code objects by identity

   5d7dc64
6. carljm commented on Apr 6, 2023

   

   carljm

   on Apr 6, 2023

   Last edited by carljm

   Member

   More actions

   I put up a pull request for this, and looking at the resulting test failures in the CPython test suite alone has changed my mind about whether this is a good idea (or at the very least, whether we can do it without a deprecation process.)

   Dynamically compiling Python source strings to code objects is not uncommon, and in that scenario it is a) quite likely that the same input will result in the "same" output code object, position information and all, and b) quite likely that the test suite for such dynamic-compilation code may be using code object equality. And in fact this case does occur in our own test suite (e.g. in test_codeop and elsewhere), which makes me think it's reasonably likely to be occurring in third-party test suites also.

   I didn't see any reasonable way to fix test_codeop other than to reimplement the entire current definition of code object equality as an assertion method in the codeop tests. This is actually quite difficult to do given the unavailability of _PyCode_ConstantKey to managed code and the fact that code objects can occur nested arbitrarily deep in co_consts. And the fact that we would need to do it at all suggests that the current definition of code object equality is in fact sometimes useful.
7. markshannon commented on Apr 6, 2023

   

   markshannon

   on Apr 6, 2023

   MemberAuthor

   More actions

   We have lots of tests that test artifacts of the implementation, rather than any spec.
   It may not be as bad as it appears.
8. carljm commented on Apr 6, 2023

   

   carljm

   on Apr 6, 2023

   Member

   More actions

   Indeed, I hoped initially it was not as bad as it appeared! But when I dug into trying to fix test_codeop, it did lead me to question whether we should do this, for the reasons I mentioned above. Happy to hear counter-arguments, though! Ideally specific proposals for what we should do with test_codeop, and what we would tell any third-party libraries that do dynamic compilation if they have similar code in their test suite.
9. markshannon commented on Apr 6, 2023

   

   markshannon

   on Apr 6, 2023

   MemberAuthor

   More actions

   What is test_codeop supposed to be testing? I'm pretty sure it isn't the notion of equality of code objects.

   Whatever notion of equivalence it has should be spelled, rather than using == on code objects.
   I suspect that testing that the bytecode, names and constants were the same would be enough.
10. markshannon commented on Apr 6, 2023

    

    markshannon

    on Apr 6, 2023

    MemberAuthor

    More actions

    any third-party libraries that do dynamic compilation

    Given the bytecode changes from one version to the next, I suspect a change in code object equality will be the least of their problems. We have changed the behavior of code object equality several times in the past.
11. carljm commented on Apr 6, 2023

    

    carljm

    on Apr 6, 2023

    Member

    More actions

    I think in principle what test_codeop wants to test is "would any consumer of this code object observe any behavior difference from this other code object," which does sound a lot like "code object equality."

    We have changed the behavior of code object equality several times in the past.

    Has it ever been defined in a way that would mean that compile("some code") == compile("some code") would ever not have been true? This is the scenario that I think (in somewhat more complex disguise) may not be uncommon in test suites for libraries doing dynamic compilation.

    I suspect that testing that the bytecode, names and constants were the same would be enough.

    Constants make this non-trivial, since you have to check the types of constants and, if they are code objects, recursively use whatever definition of pseudo-code-object equality you have chosen on those nested code objects too, instead of just comparing them. self.assertEqual(c1.co_consts, c2.co_consts) doesn't work. This is of course doable in any particular case, but it seems to me like it may be a pretty noticeable burden to impose on anyone who wants to ask "is method 1 of compiling this code leading to the 'same' results as method 2."
12. iritkatriel

    added

    interpreter-core(Objects, Python, Grammar, and Parser dirs)(Objects, Python, Grammar, and Parser dirs)

    on Nov 27, 2023