-
Notifications
You must be signed in to change notification settings - Fork 4.5k
Fix trivial inference tests for Python 3.12 support #31170
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
426b2b7
67073dd
24c3b40
c260ef1
45af07c
cc26125
3f23c5b
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,100 @@ | ||
| # | ||
| # Licensed to the Apache Software Foundation (ASF) under one or more | ||
| # contributor license agreements. See the NOTICE file distributed with | ||
| # this work for additional information regarding copyright ownership. | ||
| # The ASF licenses this file to You under the Apache License, Version 2.0 | ||
| # (the "License"); you may not use this file except in compliance with | ||
| # the License. You may obtain a copy of the License at | ||
| # | ||
| # http://www.apache.org/licenses/LICENSE-2.0 | ||
| # | ||
| # Unless required by applicable law or agreed to in writing, software | ||
| # distributed under the License is distributed on an "AS IS" BASIS, | ||
| # WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. | ||
| # See the License for the specific language governing permissions and | ||
| # limitations under the License. | ||
| # | ||
|
|
||
| """Defines the actions intrinsic bytecodes have on the frame. | ||
|
|
||
| Each function here corresponds to a bytecode documented in | ||
| https://docs.python.org/3/library/dis.html . The first argument is a (mutable) | ||
| FrameState object, the second the integer opcode argument. | ||
|
|
||
| Bytecodes with more complicated behavior (e.g. modifying the program counter) | ||
| are handled inline rather than here. | ||
|
|
||
| For internal use only; no backwards-compatibility guarantees. | ||
| """ | ||
| # pytype: skip-file | ||
|
|
||
| from . import opcodes | ||
|
|
||
|
|
||
| def intrinsic_1_invalid(state, arg): | ||
| pass | ||
|
|
||
|
|
||
| def intrinsic_print(state, arg): | ||
| pass | ||
|
|
||
|
|
||
| def intrinsic_import_star(state, arg): | ||
| pass | ||
|
|
||
|
|
||
| def intrinsic_stopiteration_error(state, arg): | ||
| pass | ||
|
|
||
|
|
||
| def intrinsic_async_gen_wrap(state, arg): | ||
| pass | ||
|
|
||
|
|
||
| def intrinsic_unary_positive(state, arg): | ||
| opcodes.unary_positive(state, arg) | ||
| pass | ||
|
|
||
|
|
||
| def intrinsic_list_to_tuple(state, arg): | ||
| opcodes.list_to_tuple(state, arg) | ||
| pass | ||
|
|
||
|
|
||
| def intrinsic_typevar(state, arg): | ||
| pass | ||
|
|
||
|
|
||
| def intrinsic_paramspec(state, arg): | ||
| pass | ||
|
|
||
|
|
||
| def intrinsic_typevartuple(state, arg): | ||
| pass | ||
|
|
||
|
|
||
| def intrinsic_subscript_generic(state, arg): | ||
| pass | ||
|
|
||
|
|
||
| def intrinsic_typealias(state, arg): | ||
| pass | ||
|
|
||
|
|
||
| # The order of operations in the table of the intrinsic one operations is | ||
| # defined in https://docs.python.org/3/library/dis.html#opcode-CALL_INTRINSIC_1 | ||
| # and may change between minor versions. | ||
| INT_ONE_OPS = tuple([ | ||
| intrinsic_1_invalid, | ||
| intrinsic_print, | ||
| intrinsic_import_star, | ||
| intrinsic_stopiteration_error, | ||
| intrinsic_async_gen_wrap, | ||
| intrinsic_unary_positive, | ||
| intrinsic_list_to_tuple, | ||
| intrinsic_typevar, | ||
| intrinsic_paramspec, | ||
| intrinsic_typevartuple, | ||
| intrinsic_subscript_generic, | ||
| intrinsic_typealias | ||
| ]) |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,40 @@ | ||
| # | ||
| # Licensed to the Apache Software Foundation (ASF) under one or more | ||
| # contributor license agreements. See the NOTICE file distributed with | ||
| # this work for additional information regarding copyright ownership. | ||
| # The ASF licenses this file to You under the Apache License, Version 2.0 | ||
| # (the "License"); you may not use this file except in compliance with | ||
| # the License. You may obtain a copy of the License at | ||
| # | ||
| # http://www.apache.org/licenses/LICENSE-2.0 | ||
| # | ||
| # Unless required by applicable law or agreed to in writing, software | ||
| # distributed under the License is distributed on an "AS IS" BASIS, | ||
| # WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. | ||
| # See the License for the specific language governing permissions and | ||
| # limitations under the License. | ||
| # | ||
|
|
||
| """Tests for apache_beam.typehints.intrinsic_one_ops.""" | ||
|
|
||
| # pytype: skip-file | ||
|
|
||
| import dis | ||
| import sys | ||
| import unittest | ||
|
|
||
| from apache_beam.typehints import intrinsic_one_ops | ||
|
|
||
|
|
||
| class IntrinsicOneOpsTest(unittest.TestCase): | ||
| def test_unary_intrinsic_ops_are_in_the_same_order_as_in_cpython(self): | ||
| if sys.version_info >= (3, 12): | ||
| dis_order = dis.__dict__['_intrinsic_1_descs'] | ||
| beam_ops = [fn.__name_upper() for fn in intrinsic_one_ops.INT_ONE_OPS] | ||
| for fn in intrinsic_one_ops.INT_ONE_OPS: | ||
| beam_ops.append(fn.__name__.upper()) | ||
| self.assertListEqual(dis_order, beam_ops) | ||
|
|
||
|
|
||
| if __name__ == '__main__': | ||
| unittest.main() |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -359,6 +359,7 @@ def infer_return_type_func(f, input_types, debug=False, depth=0): | |
| dis.dis(f) | ||
| from . import opcodes | ||
| simple_ops = dict((k.upper(), v) for k, v in opcodes.__dict__.items()) | ||
| from . import intrinsic_one_ops | ||
|
|
||
| co = f.__code__ | ||
| code = co.co_code | ||
|
|
@@ -432,6 +433,10 @@ def infer_return_type_func(f, input_types, debug=False, depth=0): | |
| elif op in dis.haslocal: | ||
| print('(' + co.co_varnames[arg] + ')', end=' ') | ||
| elif op in dis.hascompare: | ||
| if (sys.version_info.major, sys.version_info.minor) >= (3, 12): | ||
| # In 3.12 this arg was bit-shifted. Shifting it back avoids an | ||
| # out-of-index. | ||
| arg = arg >> 4 | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. for my knowledge, where does this come from? Perhaps there is a link you could add in a comment.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This one is funny in that it was actually an undocumented change as best as I could tell. The comparison operations are similar to the intrinsic functions here in that the arg for a comparison op is an integer index into a list of operations that you index into. The output is always a boolean here anyway so we don't necessarily care what the operation itself is, but for debugging purposes we get the operation to print anyway. The problem was that these indices got bit shifted (a trick this package really likes to do for operations in different versions), so our print statement here was failing with an out-of-range index. I'll try to add some comments around some of these changes since they're a little arbitrary, but for the most part it really just breaks down to "this argument's representation was changed in the package so we have to change it here for this version" |
||
| print('(' + dis.cmp_op[arg] + ')', end=' ') | ||
| elif op in dis.hasfree: | ||
| if free is None: | ||
|
|
@@ -578,7 +583,13 @@ def infer_return_type_func(f, input_types, debug=False, depth=0): | |
| state = None | ||
| elif opname in ('POP_JUMP_IF_TRUE', 'POP_JUMP_IF_FALSE'): | ||
| state.stack.pop() | ||
| jmp = arg * jump_multiplier | ||
| # The arg was changed to be a relative delta instead of an absolute | ||
| # in 3.11, and became a full instruction instead of a | ||
| # pseudo-instruction in 3.12 | ||
| if (sys.version_info.major, sys.version_info.minor) >= (3, 12): | ||
| jmp = pc + arg * jump_multiplier | ||
| else: | ||
| jmp = arg * jump_multiplier | ||
| jmp_state = state.copy() | ||
| elif opname in ('POP_JUMP_FORWARD_IF_TRUE', 'POP_JUMP_FORWARD_IF_FALSE'): | ||
| state.stack.pop() | ||
|
|
@@ -608,6 +619,10 @@ def infer_return_type_func(f, input_types, debug=False, depth=0): | |
| state.stack.pop() | ||
| elif opname == 'FOR_ITER': | ||
| jmp = pc + arg * jump_multiplier | ||
| if sys.version_info >= (3, 12): | ||
| # The jump is relative to the next instruction after a cache call, | ||
| # so jump 4 more bytes. | ||
| jmp += 4 | ||
| jmp_state = state.copy() | ||
| jmp_state.stack.pop() | ||
| state.stack.append(element_type(state.stack[-1])) | ||
|
|
@@ -641,6 +656,19 @@ def infer_return_type_func(f, input_types, debug=False, depth=0): | |
| # No-op introduced in 3.11. Without handling this some | ||
| # instructions have functionally > 2 byte size. | ||
| pass | ||
| elif opname == 'RETURN_CONST': | ||
| # Introduced in 3.12. Handles returning constants directly | ||
| # instead of having a LOAD_CONST before a RETURN_VALUE. | ||
| returns.add(state.const_type(arg)) | ||
| state = None | ||
| elif opname == 'CALL_INTRINSIC_1': | ||
| # Introduced in 3.12. The arg is an index into a table of | ||
| # operations reproduced in INT_ONE_OPS. Not all ops are | ||
| # relevant for our type checking infrastructure. | ||
| int_op = intrinsic_one_ops.INT_ONE_OPS[arg] | ||
| if debug: | ||
| print("Executing intrinsic one op", int_op.__name__.upper()) | ||
| int_op(state, arg) | ||
|
|
||
| else: | ||
| raise TypeInferenceError('unable to handle %s' % opname) | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For my understanding, what is the interpretation of
out = element_type(base)?does this say: take a container, and return the type of individual element that lives in the container? Wouldn't a slice always return a container as opposed to individual elements?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hmm that's actually an implementation interpretation I hadn't thought of. The documentation for the op is here - https://docs.python.org/3/library/dis.html#opcode-BINARY_SLICE and I was adapting the BINARY_SUBSCR code (which is doing similar work with a single index arg instead of a slice.) I think most cases will be caught by the indexable type constraint (so the type of the container is what we return) but that else case might not be handling things correctly. I'll see if I can get a bit more of an understanding on this op code specifically.