[issue39223] Fold constant slicing with slices

Serhiy Storchaka added the comment: Then I suggest to focus on optimizing real code. -- resolution: -> rejected stage: patch review -> resolved status: open -> closed ___ Python tracker

[issue39223] Fold constant slicing with slices

Pablo Galindo Salgado added the comment: > Even if there are cases, we need to prove that they are common enough I don't think cases for an extended slicing are common enough. In addition to stdlib, Django and flask (checked by Batuhan) I checked many popular 3rd party packages and there is

[issue39223] Fold constant slicing with slices

Serhiy Storchaka added the comment: The optimization for constant indexes is handy for things like b'A'[0]. I do not know cases for constant slices of constant strings. Even if there are cases, we need to prove that they are common enough. The compiler does not perform all possible constant

[issue39223] Fold constant slicing with slices

Batuhan added the comment: It looks like we have some usage for normal indexing (not this PR, the original optimization); cpython/cpython/Lib/base64.py:382 cpython/cpython/Lib/base64.py:382 cpython/cpython/Lib/base64.py:393 cpython/cpython/Lib/base64.py:397 cpython/cpython/Lib/pickle.py:306 c

[issue39223] Fold constant slicing with slices

Pablo Galindo Salgado added the comment: The one in cpython/Tools/clinic/clinic.py:1185 is sort of an anti-pattern as it should be using '''\ Something something ''' instead of skipping the newline. -- ___ Python tracker

[issue39223] Fold constant slicing with slices

Pablo Galindo Salgado added the comment: I was reading the pull request and I have to say that I find using a switch instead of a very crowded "if" ward a bit more readable. I would be +0 on this case. -- ___ Python tracker

[issue39223] Fold constant slicing with slices

Batuhan added the comment: I've scanned stdlib, django and flask and no usage so far. There is one in the test (which basically verifies behavior of slice) and one in the argument clinic which passes the newline for a multiline string. cpython/Tools/clinic/clinic.py:1185 cpython/Lib/test/te

[issue39223] Fold constant slicing with slices

Batuhan added the comment: > Then I'd suggest removing the optimization for "abcde"[2], too. I've never > seen this in real code, and I can't come up with a scenario where it would be > performance critical. Remember, one of the goals for CPython is to be > reasonable readable, not maximally

[issue39223] Fold constant slicing with slices

Eric V. Smith added the comment: Then I'd suggest removing the optimization for "abcde"[2], too. I've never seen this in real code, and I can't come up with a scenario where it would be performance critical. Remember, one of the goals for CPython is to be reasonable readable, not maximally t

[issue39223] Fold constant slicing with slices

Batuhan added the comment: We already have a folding operation for Index access ("abcde"[2]) and there was a todo (left by @methane I guess) about supporting other slicings. I think this would look inconsistent if one is support and not the other one. --

[issue39223] Fold constant slicing with slices

Pablo Galindo Salgado added the comment: Specially because you need to do the mental calculation to know what the indexes are for the letters you want, and at that point you better write that string anyways, no? -- ___ Python tracker

[issue39223] Fold constant slicing with slices

Pablo Galindo Salgado added the comment: I am with Eric. Why would anyone write: "abcde"[2:4] Instead of just cd ? -- nosy: +pablogsal ___ Python tracker ___

[issue39223] Fold constant slicing with slices

Eric V. Smith added the comment: Does this occur often enough that it's worth the added code? -- nosy: +eric.smith ___ Python tracker ___ _

[issue39223] Fold constant slicing with slices

Change by Batuhan : -- keywords: +patch pull_requests: +17265 stage: -> patch review pull_request: https://github.com/python/cpython/pull/17838 ___ Python tracker ___

[issue39223] Fold constant slicing with slices

New submission from Batuhan : >>> def g(): "abcde"[2:4] ... >>> g.__code__.co_consts (None, 'abcde', 2, 4) to >>> def g(): "abcde"[2:4] ... >>> g.__code__.co_consts (None, 'cd') (I have a patch) -- components: Interpreter Core messages: 359350 nosy: BTaskaya priority: normal sever