-
Notifications
You must be signed in to change notification settings - Fork 3.7k
Fix incorrect narrowing in mixed type for into #15952
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -634,7 +634,10 @@ defmodule Module.Types.Expr do | |
| # is ok for now because we only check for bitstring if the type | ||
| # is a subset of empty_list() or bitstring(), but we may want to | ||
| # relax in the future. | ||
| if empty?(intersection) do | ||
| # | ||
| # 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 | ||
| error = {:badbitbody, block_type, block, context} | ||
| {error_type(), error(__MODULE__, error, meta, stack, context)} | ||
| else | ||
|
|
@@ -864,12 +867,16 @@ defmodule Module.Types.Expr do | |
| # only bitstring/list, even if a dynamic with something else is given. | ||
| if subtype?(type, @into_compile) do | ||
| 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) -> | ||
|
Comment on lines
869
to
+873
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Let's convert this into a case |
||
| # The collectable may be a list, which accepts any element, | ||
| # so we cannot restrict the body to bitstrings. | ||
| {[:bitstring, :non_empty_list], opt_union(binary(), type), term(), context} | ||
|
|
||
| bitstring_type?(type) -> | ||
| kinds = if empty_list_type?(type), do: [:bitstring, :non_empty_list], else: [:bitstring] | ||
| # 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. | ||
| {kinds, opt_union(binary(), type), bitstring(), context} | ||
| {[:bitstring], opt_union(binary(), type), bitstring(), context} | ||
|
|
||
| empty_list_type?(type) -> | ||
| {[:non_empty_list], type, maybe_list_hd_or_term(expected), context} | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -3586,6 +3586,42 @@ defmodule Module.Types.ExprTest do | |
| dynamic( | ||
| opt_union(opt_union(bitstring(), empty_list()), list(bitstring_no_binary())) | ||
| ) | ||
|
|
||
| # The list path accepts any element, so the body is not restricted to bitstrings | ||
| assert typecheck!( | ||
| [flag], | ||
| ( | ||
| into = if flag, do: [], else: "" | ||
| value = if flag, do: :ok, else: "ok" | ||
| for(_ <- [1], do: value, into: into) | ||
|
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 |
||
| ) | ||
| ) == opt_union(binary(), list(opt_union(atom([:ok]), binary()))) | ||
|
|
||
| assert typedyn!( | ||
| [flag], | ||
| ( | ||
| into = if flag, do: [], else: "" | ||
| value = if flag, do: :ok, else: "ok" | ||
| for(_ <- [1], do: value, into: into) | ||
| ) | ||
| ) == dynamic(opt_union(binary(), list(opt_union(atom([:ok]), binary())))) | ||
|
|
||
| assert typecheck!( | ||
| [flag], | ||
| ( | ||
| into = if flag, do: [], else: "" | ||
| for(_ <- [1], do: :ok, into: into) | ||
|
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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. |
||
| ) | ||
| ) == opt_union(binary(), list(atom([:ok]))) | ||
|
|
||
| assert typecheck!( | ||
| [flag, value], | ||
| ( | ||
| into = if flag, do: [], else: "" | ||
| for(_ <- [1], do: value, into: into) | ||
| value | ||
| ) | ||
| ) == dynamic() | ||
| end | ||
|
|
||
| test ":into bitstrings" do | ||
|
|
||
There was a problem hiding this comment.
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!