Comparison operators could benefit from constant folding

[WolframAlph]

Feature or enhancement

Proposal:

Constant expresisons like:

1 + 1

are folded in AST optimizer, but constant comparisons are not. For instance:

1 in (1, 2)
3 > 2 > 1

etc...

Proposing to add constant comparison folding. Linking draft PR. No tests yet in case proposal is rejected. cc @Eclips4 as codeowner and AST specialist.

Has this already been discussed elsewhere?

No response given

Links to previous discussion of this feature:

No response

Linked PRs

• gh-128706: Evaluate constant comparisons in fold_compare #128705

[Eclips4]

Hello!
Firstly, thanks for your efforts, Yan!

Unfortunately, I do think that we can't do this at the moment.
You can see in your PR that CI is failing (there are some failing tests, but let's concentrate on test_grammar):

======================================================================
FAIL: test_comparison_is_literal (test.test_grammar.GrammarTests.test_comparison_is_literal)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/Users/runner/work/cpython/cpython/Lib/test/test_grammar.py", line 1435, in test_comparison_is_literal
    check('None is 1', '"is" with \'int\' literal')
    ~~~~~^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/Users/runner/work/cpython/cpython/Lib/test/test_grammar.py", line 1425, in check
    self.check_syntax_warning(test, msg)
    ~~~~~~~~~~~~~~~~~~~~~~~~~^^^^^^^^^^^
  File "/Users/runner/work/cpython/cpython/Lib/test/support/warnings_helper.py", line 23, in check_syntax_warning
    testcase.assertEqual(len(warns), 1, warns)
    ~~~~~~~~~~~~~~~~~~~~^^^^^^^^^^^^^^^^^^^^^^
AssertionError: 0 != 1 : []

This happens because at the codegen stage None is 1 is already folded into a LOAD_CONST False, so Python/codegen.c::check_is_arg isn't working as intended and can't raise a warning about the usage of is operator with constants.

Also please see the #126835. Later, we can add folding for constant comparisons (where's both operands are constants) in the CFG.

[WolframAlph]

Yes, I expected and saw some tests fail, but skipped fixing them as I did not know if this will be rejected or not.

Also please see the #126835.

Interesting. So ast optimizer will be removed and all folding will happen in CFG? I did not know that.
I will leave it up to you then whether to close this one or keep for later and link this issue.
Additionally, would you accept some help with #126835 ?

[serhiy-storchaka]

It was already proposed several times (see for example #70909, there were more). The reason why it was not implemented in first place is that the optimizer only performs the most useful practical optimizations. 24*60*60, 2**32-1, 1/3, b'A'[0] -- are pretty common use cases, and there is a benefit of calculating them at the compile time. The benefit/cost ration for constant folding of comparison operations is not so great.

So I suggest to close this issue as all previous attempts.

[picnixz]

I've read the tests in the PR and all the expressions that are used for testing are actually expressions I don't think someone is ever going to use in production. Constant folding for arithmetic operations is relevant as it improves readability most of the time, but constant folding on comparisons (which only return True or False in the end) are not needed in production IMO.

[WolframAlph]

I would expect folding of every constant expression in a programming language. But this is my personal opinion. Additionally, if constant propagation is somehow implemented in future, wouldn't constant folding be useful in this scenario as well?

[sobolevn]

I agree that this feature is not quite useful. Real code does not usually have expressions like 1 > 2 or 1 in (1, 2). Linters should catch cases like this, because they are most likely just typos.

But, I am not strongly -1, more like -0.

In any case, thanks a lot for your PR!

[erlend-aasland]

I agree that there is no real benefit of this. Count me as -1. Suggesting to close as wont-implement.