Skip to content

Fix if-branch pruning for Final bool constants#3571

Open
LeSingh1 wants to merge 5 commits into
facebook:mainfrom
LeSingh1:fix-final-bool-control-flow
Open

Fix if-branch pruning for Final bool constants#3571
LeSingh1 wants to merge 5 commits into
facebook:mainfrom
LeSingh1:fix-final-bool-control-flow

Conversation

@LeSingh1
Copy link
Copy Markdown
Contributor

Summary

Fixes #3537.

When a variable is declared Final and assigned a bool literal (False or True), control-flow tests like if flag: now prune unreachable branches the same way as literal if False: / if True:.

Before this change, both arms of an if/else could contribute to merged types even when the condition was statically known to be always false via Final = False.

Test plan

  • Added test_final_bool_unreachable_if_branchFinal = False → else-only assignment type
  • Added test_final_bool_unreachable_else_branchFinal = True → if-only assignment type
  • cargo test test_final_bool_unreachable

When a name is annotated Final and assigned a bool literal, treat
if/elif tests on that name like literal True/False so unreachable
arms are pruned during binding. Fixes incorrect union types after
if/else when the condition is always false or always true.
@github-actions github-actions Bot added size/m and removed size/m labels May 24, 2026
@Arths17
Copy link
Copy Markdown
Contributor

Arths17 commented May 24, 2026

The idea here makes sense, but the CI failures suggest this change may be leaking beyond Final-declared constants into general boolean control-flow inference.

I’d double-check the branch pruning condition — it should ideally only trigger on Final-backed literals, not inferred constant truthiness.

Might be worth adding a regression test for:

non-Final bool literal assignment
mixed union bool narrowing in if/else joins

to ensure no accidental over-pruning.

SysInfo derives Copy + Dupe (it's a thin Intern handle), so the
clippy::trivially_copy_pass_by_ref lint flagged the &SysInfo
parameter as needlessly indirect at deny-level. Switch the helper
to take SysInfo by value and update the single call site.

Fixes the deny-level clippy error blocking the pyrefly CI checks
on this PR; no behavior change.
@github-actions github-actions Bot added size/m and removed size/m labels May 24, 2026
Commit e1416af ("Remove redundant pyrefly/rust-toolchain file") deleted
pyrefly/rust-toolchain but left the pyrefly_wasm/rust-toolchain symlink
which pointed at it. The dangling symlink fails 'stat' during Ubuntu CI
setup with:

  Error: ENOENT: no such file or directory, stat
  '/home/runner/work/pyrefly/pyrefly/pyrefly_wasm/rust-toolchain'

The root rust-toolchain.toml (channel = "stable") already takes
precedence for the wasm build, per the rationale in e1416af, so the
symlink served no purpose after that deletion. Remove it so this branch
(and any other PR branched off current main) can finish CI setup.
@github-actions github-actions Bot added size/m and removed size/m labels May 24, 2026
@github-actions github-actions Bot added size/m and removed size/m labels May 25, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

always True & always False variables are incorrectly treated as if they could be either

2 participants