Skip to content

Remove redundant line empty when we eliminate negations#15196

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

Remove redundant line empty when we eliminate negations#15196
josevalim merged 2 commits into
mainfrom
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

gldubc commented Apr 13, 2026

Copy link
Copy Markdown
Member

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

gldubc commented Apr 13, 2026

Copy link
Copy Markdown
Member

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