Skip to content

#5478: rename bool branch selector voids so if results do not capture left and right#5497

Open
morphqdd wants to merge 2 commits into
objectionary:masterfrom
morphqdd:5478
Open

#5478: rename bool branch selector voids so if results do not capture left and right#5497
morphqdd wants to merge 2 commits into
objectionary:masterfrom
morphqdd:5478

Conversation

@morphqdd

Copy link
Copy Markdown
Contributor

Closes #5478.

Root cause found. The result of every if. is a copy of the branch selector formation from true.eo and false.eo:

bool > false
  right > [left right]

The selector's voids are named left and right. Attribute lookup finds them before the phi fallthrough, so any dispatch of .left or .right on an if. result hits the selector's own void instead of the method of the chosen branch. For the repro from the issue: magnitude.right 1 takes the void right (the already bound else branch, a bytes object) and then applies 1 to it, which lands in the already set data void of that bytes. That is exactly the reported error: This void attribute "data" is already set, can't reset. Since left and right are the shift methods of bytes, every piece of integer arithmetic that first normalizes a value with if. and then shifts it was broken. This is what killed the composed pure-EO arithmetic in #5467, #5471 and the hash-code-of failure on #5475, with Expect replacing this error by the generic type message.

The names were introduced on 2026-06-09 in 8314d70 (the Church-encoded bool for #5189); nothing there suggests the capture was intended.

The fix renames the selector voids to left-branch and right-branch. All callers bind them positionally, so nothing else changes. Two regression tests in bool.eo pin the behavior: .right and .left on an if. result now reach the chosen branch.

Note for a follow-up: this is one instance of a general hazard. Any formation's void shadows the same-named attribute of the object it evaluates to, so every helper like [condition body] > while plants the same trap for .condition and .body. A WPA lint that flags a void whose name collides with an attribute of the object it returns would catch the whole class. I can file that separately.

Verification: full eo-runtime build with lints and the whole test suite: 1505 tests, 0 failures, 0 errors; qulice clean.

@github-actions

github-actions Bot commented Jul 14, 2026

Copy link
Copy Markdown
Contributor

⚠️ Benchmark Comparison Unavailable

Unfortunately, one of the benchmarks is missing, and we couldn't generate a performance comparison report.

Please ensure that both the base and PR benchmark results are available for analysis.

@morphqdd

Copy link
Copy Markdown
Contributor Author

@yegor256, please look

@yegor256

Copy link
Copy Markdown
Member

@maxonfjvipon look at this. It seems that we should get rid of the bool object, to fix this problem.

@maxonfjvipon

Copy link
Copy Markdown
Member

@yegor256 no, we just need other names instead of left and right

@maxonfjvipon

Copy link
Copy Markdown
Member

@yegor256 that's a known problem and should be caught by type inference

@0crat

0crat commented Jul 15, 2026

Copy link
Copy Markdown

@maxonfjvipon Thanks for the review! You've earned +4 points for this: +12 as a basis; -8 for absolutely no comments posted; -4 for too few (14) hits-of-code; +4 to give you at least something. Your running score is +633; don't forget to check your Zerocracy account too).

@morphqdd

Copy link
Copy Markdown
Contributor Author

@maxonfjvipon @yegor256 this PR already does exactly that: it renames the selector voids from left/right to left-branch/right-branch in true.eo and false.eo, so an if. result no longer shadows the left/right methods of the value it returns. All callers bind them positionally, so nothing else changes, and the full runtime suite is green (1505 tests). The bool object itself stays.

On the "should be caught by type inference" point: I filed #5498 for exactly that follow-up (a lint that flags a formation whose void name collides with an attribute of the object it returns), since the same trap exists in other helpers like while with [condition body].

@sonarqubecloud

Copy link
Copy Markdown

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Runtime failure on composing i64 arithmetic in pure-EO and masking of actual exception by Expect

4 participants