GH-145247: Implement _PyTuple_FromPair

[sergey-miryanov]

• Issue: Add PyTuple_FromPair #145247

[eendebakpt]

To minor comments on the test, but overall this looks good!

[eendebakpt]

The _testinternalcapi._tuple_from_pair_steal that wraps _PyTuple_FromPairSteal increments references to the arguments. For that reason there is no way to make a distinction between the _testinternalcapi._tuple_from_pair and _testinternalcapi._tuple_from_pair._tuple_from_pair_steal in the tests)

I would either change the name of _tuple_from_pair_steal or add a comment to make this clear.

[sergey-miryanov]

I'm not sure I understand what you mean. Could you elaborate please?
You are right that tests seem equals from point of view, but we need to test both functions to check a) their behavior and b) absence of the leak if we run it with -R option.

[eendebakpt]

The test is good indeed.

What I meant is that _testinternalcapi._tuple_from_pair_steal is not stealing references (since the arguments are incremented before passing to _PyTuple_FromPairSteal. So a (way too long) descriptive name would be _testinternalcapi._wraps_tuple_from_pair_steal_but_does_not_actually_steal.

[vstinner]

I don't think that the _ underscore prefix is useful here. Other _testinternalcapi functions don't use an _ underscore prefix.

[vstinner]

I would prefer a more flat structure:

    def check_tuple_from_pair(self, from_pair):
        ...

    def test_tuple_from_pair(self):
        # Test _PyTuple_FromPair()
        from_pair = _testinternalcapi._tuple_from_pair
        self.check_tuple_from_pair(from_pair)

    def test_tuple_from_pair_steal(self):
        # Test _PyTuple_FromPairSteal()
        from_pair = _testinternalcapi._tuple_from_pair_steal
        self.check_tuple_from_pair(from_pair)

[vstinner]

nitpick: i suggest moving these checks at the end.

[vstinner]

You may reuse self._tracked() and self._not_tracked() methods for these checks.