Skip to content

Remove redundant line empty when we eliminate negations#15196

Merged
josevalim merged 2 commits intomainfrom
jv-remove-redundant-line-empty
Apr 13, 2026
Merged

Remove redundant line empty when we eliminate negations#15196
josevalim merged 2 commits intomainfrom
jv-remove-redundant-line-empty

Conversation

@josevalim
Copy link
Copy Markdown
Member

No description provided.

@josevalim josevalim requested a review from gldubc March 14, 2026 18:04
@josevalim josevalim added this to the v1.20 milestone Mar 31, 2026
@gldubc
Copy link
Copy Markdown
Member

gldubc commented Apr 13, 2026

Removing tuple_dnf_union in tuple_eliminate_single_negation is fine because tuple_elim_size(n, m, tag, elements, neg_tag) and tuple_elim_content([], tag, elements, neg_elements) generate
tuples of different sizes, so they are disjoint.

But once tuple_bdd_to_dnf_no_negations/1 no longer unions the generated lines, different BDD lines can produce structural duplicates. That is semantically fine, but it can regress singleton?/1, because each_singleton?(:tuple, bdd) expects the DNF to contain exactly one tuple line for a singleton. I put an example of that.

So I think we should keep the optimization inside tuple_eliminate_single_negation, but still deduplicate at the final no-negations DNF boundary (via Enum.uniq()). An alternative is to do it directly in each_singleton(:tuple, bdd).

@josevalim
Copy link
Copy Markdown
Member Author

@gldubc we can do a simple dedup that deduplicate only the next immediate element!

@gldubc
Copy link
Copy Markdown
Member

gldubc commented Apr 13, 2026

Sounds good!

@josevalim josevalim merged commit 03b45e5 into main Apr 13, 2026
26 checks passed
@josevalim josevalim deleted the jv-remove-redundant-line-empty branch April 13, 2026 19:41
@josevalim
Copy link
Copy Markdown
Member Author

💚 💙 💜 💛 ❤️

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

Labels

None yet

Development

Successfully merging this pull request may close these issues.

2 participants