fix: unbound-name after assignment to singleton attribute ; unexpected unbound-name error on second walrus conditional assignment for same variable #1940
Conversation
tannguyencse19
changed the title
| // - It was defined before the loop (base_has_value), OR | ||
| // - It's defined in all loop body branches (since the loop definitely runs at least once) | ||
| // For regular loops and other merges, a name is always defined if it's in all branches. | ||
| let is_name_exists_in_all_branch_flow = match merge_style { |
There was a problem hiding this comment.
This block of code was inserted between the comment above & the piece of code it describes below. I think it should be moved above the comment
The comment might also need to be updated since you made changes below
There was a problem hiding this comment.
I pushed new commit: cdbed60
I moved the new logic so it sits directly under the comment it describes, and updated the comment to match the final behavior
|
This says #1397 is fixed, but I don't see the test cases for that. Do you mind adding the test case from the original issue as well as the one from the comment below? |
pyrefly/lib/binding/scope.rs
Outdated
| let n_branch_flow_infos = flow_infos.len(); | ||
| // Track if base has a value for this name (for LoopDefinitelyRuns init check) | ||
| let base_has_value = merge_item.base.as_ref().is_some_and(|b| b.value.is_some()); | ||
| let base_has_name = merge_item.base.is_some(); |
There was a problem hiding this comment.
Are we able to do this by modifying base_has_value? Perhaps instead of b.value.is_some() we should check taht b.value.is_some() || b.narrow.is_some() and if one of them is Some, its style shouldn't be FlowStyle::Uninitialized
There was a problem hiding this comment.
I pushed new commit: cdbed60
Yes — I removed base_has_name and switched to a single base_has_binding = base.value.is_some() || base.narrow.is_some() so loop merges treat names that only have narrows as still present/defined.
yangdanny97
left a comment
yangdanny97
left a comment
There was a problem hiding this comment.
Thanks for the PR, back to you with some comments
I'm not 100% sure on the logic either, but see if you can get it to work without adding base_has_name. If we absolutely have to we can, but it might be simpler if we don't
|
@yangdanny97 Okay. I will try |
2 participants
Summary
Updated the flow-merge logic so names that are present in all flows (even if only via attribute “narrows”) are treated as defined after a loop
fix #1378
fix #1397
Test Plan