Skip to content

Fix bug in the codegen with a logical guard clauses.#5255

Draft
vyacheslavhere wants to merge 2 commits intogleam-lang:mainfrom
vyacheslavhere:fix-wrong-logical-clase-guard-codegen
Draft

Fix bug in the codegen with a logical guard clauses.#5255
vyacheslavhere wants to merge 2 commits intogleam-lang:mainfrom
vyacheslavhere:fix-wrong-logical-clase-guard-codegen

Conversation

@vyacheslavhere
Copy link
Contributor

Closes #5254

@vyacheslavhere vyacheslavhere changed the title Fix bug in the codegen with an logical guard clauses. Fix bug in the codegen with a logical guard clauses. Jan 2, 2026
Copy link
Member

@lpil lpil left a comment

Choose a reason for hiding this comment

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

Thank you! I've left some notes inline 🙏

in `BitArray`s' `size` option to be `>= 1.12.0`.
([Giacomo Cavalieri](https://github.com/giacomocavalieri))

- Clause guard with logical expression now compiles correctly in JS.
Copy link
Member

Choose a reason for hiding this comment

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

Be more precise with the description please, this sounds like it never worked 🙏

----- COMPILED JAVASCRIPT
export function main(x, y) {
if (!y && !x) {
if ((!y && !x)) {
Copy link
Member

Choose a reason for hiding this comment

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

It would be good to not have these parens here and in other places they are not needed.

}
"
)
}
Copy link
Member

Choose a reason for hiding this comment

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

This test is quite complex, so it's hard to understand. Could you simplify it please 🙏

Is this testing what the issue reported? The title of the issue says it's related to instanceof, but this one doesn't use it? I'm finding it hard to understand this due to the complex test.

Copy link
Contributor Author

@vyacheslavhere vyacheslavhere Jan 4, 2026

Choose a reason for hiding this comment

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

Thanks for the feedback, I'll work on it when I have time.

@lpil lpil marked this pull request as draft January 4, 2026 16:06
@vyacheslavhere
Copy link
Contributor Author

vyacheslavhere commented Jan 30, 2026

I tried to come up with a new solution within the current system. It may be rough, so I am open to suggestions, corrections, etc.

@vyacheslavhere vyacheslavhere requested a review from lpil January 30, 2026 16:12
@vyacheslavhere
Copy link
Contributor Author

Just a heads up, the test hasn't been simplified yet.

@lpil
Copy link
Member

lpil commented Feb 7, 2026

Hi @vyacheslavhere, please mark this as ready-for-review and tag me if you want a review.

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.

case with if can be wrong when JavaScript codegen has instanceof

2 participants