Skip to content

Fix incorrect narrowing in mixed type for into - #15952

Open
josevalim wants to merge 1 commit into
elixir-lang:mainfrom
lukaszsamson:ls-mixed-into
Open

josevalim wants to merge 1 commit into
elixir-lang:mainfrom
lukaszsamson:ls-mixed-into

Conversation

@josevalim

Copy link
Copy Markdown
Member

No description provided.

#
# If the collectable may also be a list, the body may be valid
# on that path, so we only error when the list path is impossible.
if empty?(intersection) and :non_empty_list not in into_kinds do

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

I think we can remove the TODO above because I believe it is talking about exactly this case!

Comment on lines 869 to +873
cond do
# A comprehension may concatenate the block an arbitrary number of times.
# Even if both the initial value and each block are unaligned bitstrings,
# repeated concatenation may eventually produce an aligned binary.
bitstring_type?(type) and empty_list_type?(type) ->

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Let's convert this into a case {bitstring_type?(type), empty_list_type?(type)}.

[flag],
(
into = if flag, do: [], else: ""
for(_ <- [1], do: :ok, into: into)

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

This should not type check in the static version. It would type check in the dynamic one.

(
into = if flag, do: [], else: ""
value = if flag, do: :ok, else: "ok"
for(_ <- [1], do: value, into: into)

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

I don't think this should type check in the static version either. Because if you write it as value = if !flag, it will fail, and the type system would accept it.

This branch has not been deployed

No deployments
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