Skip to content

Inline in empty list in erlang pass#15413

Open
sabiwara wants to merge 5 commits into
elixir-lang:mainfrom
sabiwara:in-empty-list
Open

Inline in empty list in erlang pass#15413
sabiwara wants to merge 5 commits into
elixir-lang:mainfrom
sabiwara:in-empty-list

Conversation

@sabiwara
Copy link
Copy Markdown
Contributor

  • x in [] is expanded at :lists.member(x, [])
  • then later the erlang pass inlines it as false

Same approach as 34a5000

Closes #15411

assert typecheck!(
[x],
x in []
) == boolean()
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.

It feels we should be able to narrow it to false (in which case we'd still have a warning, just more helpful), but right now it's consistent with :foo in [:bar] not warning.

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.

On apply.ex, you can replace this:

{:lists, :member, [{[term(), list(term())], boolean()}]},

with this:

{:lists, :member, [{[term(), empty_list()], atom([false])}, {[term(), non_empty_list(term())], boolean()}]},

Comment thread lib/elixir/src/elixir_erl_pass.erl Outdated
]}, VS};
translate_remote(lists, member, Meta, [_Expr, []], S) ->
Ann = ?ann(Meta),
{{atom, Ann, false}, S};
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.

We need to execute the expression in case of side-effects, so it has to be something like before:

Suggested change
{{atom, Ann, false}, S};
{{block, Ann, [{match, Ann, {var, Ann, '_'}, Expr}, {atom, Ann, false}]}, S};

Shall we add a test to the test/erlang/* files? I think we test other optimizations there already.

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.

Or even better, perhaps a test for Kernel.in/2 like this:

false = send(self(), :hello) in []
assert_received :hello

Copy link
Copy Markdown
Member

@josevalim josevalim left a comment

Choose a reason for hiding this comment

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

I dropped some comments.

We also need to replace this in types/helpers.ex:

      {{:., _, [:lists, :member]}, meta, [expr, [_ | _] = args]} = call ->
        if Enum.any?(args, &match?({:|, _, [_, _]}, &1)) do
          call
        else
          {:in, meta, [expr, args]}
        end

by this:

      {{:., _, [:lists, :member]}, meta, [expr, args]} = call when is_list(args) ->
        if Enum.any?(args, &match?({:|, _, [_, _]}, &1)) do
          call
        else
          {:in, meta, [expr, args]}
        end

You can add some tests to types/helper_test.exs for in (currently no coverage).

@sabiwara
Copy link
Copy Markdown
Contributor Author

Thanks for all the pointers @josevalim 💜

OK, with 58a96ee we're back with a warning but a readable one:

Screenshot 2026-05-23 at 18 52 15

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.

Confusing type warning when checking with true <- x in []

2 participants