Skip to content

numberize - rewrite only positive leaves, add map value numberization - #15801

Draft
lukaszsamson wants to merge 2 commits into
elixir-lang:mainfrom
lukaszsamson:ls-numberize-map-domains
Draft

numberize - rewrite only positive leaves, add map value numberization#15801
lukaszsamson wants to merge 2 commits into
elixir-lang:mainfrom
lukaszsamson:ls-numberize-map-domains

Conversation

@lukaszsamson

Copy link
Copy Markdown
Contributor

Addresses 1 and 2 from #15799
Extracted from #15800

AssistedBy: Claude Fable 5, GPT 5.6 Sol

@lukaszsamson lukaszsamson mentioned this pull request Aug 27, 2026
@josevalim
josevalim requested a review from gldubc August 28, 2026 09:06
case fun.(leaf) do
^leaf -> bdd_node_new(leaf, left, union, right)
new_leaf when right == :bdd_bot -> bdd_node_new(new_leaf, left, union, :bdd_bot)
new_leaf -> bdd_union(bdd_intersection(new_leaf, left), bdd_union(union, right))

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Now I am thinking... isn't our bdd_map inherently wrong because it may rewrite the leafs which means they have a different ordering? I am thinking everything needs to be written as in this formula... which would also make numberize too expensive...

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I recall one of AI reviews flagged the order not being preserved as an issue. I wasn’t able to produce code where it would surface as an evident bug. I dismissed the proposed fix with BDD rebuilds everywhere as a bad tradeoff

@josevalim

Copy link
Copy Markdown
Member

@lukaszsamson I am thinking a better solution here would be have a field in the context called numberize. So when we assemble the type here:

  defp custom_compare(name, arg, literal, expected, expr, stack, context, of_fun) do
    case booleaness(expected) do
      booleaness when booleaness in [:maybe_both, :none] ->
        compare(name, arg, literal, false, expr, stack, context, of_fun)

      {boolean, _maybe_or_always} ->
        {type, context} = of_fun.(literal, term(), expr, stack, context) # <=======

We set this value in the stack and we interpret all integers/floats in the literal as numbers. This makes it correct by construction and we can remove numberize.

We still have to address 3 from 15799 though... perhaps we have a version of disjoint that ignores numbers... but that may be complicated.

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