Skip to content
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

Mark code that uses improper lists as generated #556

Merged

Conversation

al2o3cr
Copy link
Contributor

@al2o3cr al2o3cr commented Jul 6, 2021

Addresses #549 by marking code that creates improper lists as generated: true.

If it's preferable to explicitly opt-out of the specific Dialyzer warnings (using @dialyzer), that's an easy change.

Addresses [elixir-ecto#549](elixir-ecto#549) by
marking code that creates improper lists as `generated: true`.
@@ -6,7 +6,7 @@ defmodule Postgrex.Extensions.BitString do
def init(opts), do: Keyword.fetch!(opts, :decode_binary)

def encode(_) do
quote location: :keep do
quote location: :keep, generated: true do
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is a common pattern: the code explicitly makes a definitely improper list; this is fine for iodata and saves a cons cell.

@@ -333,7 +333,7 @@ defmodule Postgrex.TypeModule do
end

defp decode_rows(dispatch, rest, acc, rem, full, rows) do
quote location: :keep do
quote location: :keep, generated: true do
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This one covers clauses like:

      defp decode_rows(<<?D, rest::binary>>, _, _, rows) do
        {:more, [?D | rest], rows, 0}
      end

in the code; is there a performance difference between creating an improper list which is immediately turned back into a binary and passing along the binary directly (maybe related to "small piece of a big buffer" issues?)

      defp decode_rows(<<?D, rest::binary>> = whole_thing, _, _, rows) do
        {:more, whole_thing, rows, 0}
      end

@@ -464,7 +464,7 @@ defmodule Postgrex.TypeModule do
decode_tuple_dispatch(extension, format, rest, oids, n, acc)
end

quote do
quote generated: true do
Copy link
Contributor Author

@al2o3cr al2o3cr Jul 6, 2021

Choose a reason for hiding this comment

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

This covers the code starting at line 500:

        case Postgrex.Types.fetch(oid, types) do
          {:ok, {:binary, type}} ->
            unquote(oids) = rem - 1

            case [type | types] do
              unquote(dispatch)
            end

That case [type | types] do code is tricky - it looks like a boring "prepend to list" operation, but AFAIK types is never a list (it's a Postgrex.Types.state!) so it's always making an improper list.

@josevalim josevalim merged commit 3892e45 into elixir-ecto:master Jul 6, 2021
@josevalim
Copy link
Member

💚 💙 💜 💛 ❤️

josevalim pushed a commit that referenced this pull request Jul 27, 2021
Addresses [#549](#549) by
marking code that creates improper lists as `generated: true`.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants