Skip to content
This repository has been archived by the owner on Jan 24, 2024. It is now read-only.

[Compat][3.11] enable test_11_jumps.py #361

Merged
merged 12 commits into from
Aug 31, 2023

Conversation

zrr1999
Copy link
Member

@zrr1999 zrr1999 commented Aug 30, 2023

  1. 给 VariableBase 添加 is_none 方法。
  2. 优化 PyCodeGen 的 gen_rot_n 方法,针对不同 Python版本,采用不同的字节码。
  3. 重构 POP_JUMP 相关字节码的实现,提高代码的复用性,并且补齐了 Python3.11 中相关的新字节码。
  4. 增加更多单测,提高代码稳定性。

@paddle-bot
Copy link

paddle-bot bot commented Aug 30, 2023

Thanks for your contribution!

@paddle-bot paddle-bot bot added the contributor External developers label Aug 30, 2023
@@ -13,7 +13,6 @@ py311_skiped_tests=(
# ./test_04_list.py There are some case need to be fixed
# ./test_05_dict.py There are some case need to be fixed
./test_10_build_unpack.py
./test_11_jumps.py
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

部分完成的像上面加一个注释吧

self.assert_results(pop_jump_if_not_none, True, a)

@unittest.skipIf(
sys.version_info >= (3, 11), "Python 3.11+ is not supported yet."
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

这里说明是不支持 breakgraph 吧,其他单测里应该有 "Python 3.11+ not support breakgraph"

Comment on lines 84 to 85
self.assert_results(pop_jump_if_none, True, a)
self.assert_results(pop_jump_if_not_none, True, a)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

这里的话,测试「None」和「非 None」两种情况吧,共四个

@@ -56,7 +81,13 @@ def test_simple(self):
self.assert_results(jump_if_true_or_pop, False, a)
self.assert_results(pop_jump_if_true, True, False, a)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

这里和上面是不是有一部分重复了?按理说两个单测 case 应该测不同的分支

@zrr1999 zrr1999 requested a review from SigureMo August 31, 2023 07:25
Copy link
Member

@SigureMo SigureMo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTMeow 🐾

@@ -459,7 +459,7 @@ def gen_resume_fn_at(self, index: int, stack_size: int = 0):
gen_instr('LOAD_FAST', argval=stack_arg_str.format(i))
for i in range(stack_size)
]
+ [gen_instr('JUMP_ABSOLUTE', jump_to=self._instructions[index])]
+ [gen_instr('JUMP_FORWARD', jump_to=self._instructions[index])]
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

这里用 JUMP_FORWARD 应该需要确保一点,它是向前跳的

这里应该没问题,因为是函数初始跳转到中间 breakgraph 处

@SigureMo SigureMo merged commit 96715d1 into PaddlePaddle:develop Aug 31, 2023
8 checks passed
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
contributor External developers
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants