elixir-lang/elixir

Compilation warning when using `super` with unused, overridable function via `using` module

Closed this issue · 8 comments

acco commented

Elixir and Erlang/OTP versions

Elixir 1.18.0 (compiled with Erlang/OTP 27)

Operating system

MacOS Sequoia 15.1.1 (24B91)

Current behavior

Thanks for a great release!

When using a library with super, this implementation:

defmodule Sequin.Encrypted.Field do
  use Cloak.Ecto.Binary, vault: Sequin.Vault

  def load(nil), do: {:ok, nil}

  def load(value) do
    super(Base.decode64!(value))
  end
end

Results in a compilation error:

  warning: this clause of defp load (overridable 1)/1 is never used
    │
  8 │   use Cloak.Ecto.Binary, vault: Sequin.Vault
    │   ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
    │
    └─ lib/sequin/encrypted/field.ex:8: Sequin.Encrypted.Field."load (overridable 1)"/1

The compilation error goes away if I call super/1 in the first load/1:

defmodule Sequin.Encrypted.Field do
  use Cloak.Ecto.Binary, vault: Sequin.Vault

  def load(nil), do: super(nil)

  def load(value) do
    super(Base.decode64!(value))
  end
end

The library defines two overridable load/1 functions:

 defmacro __using__(opts) do
    vault = Keyword.fetch!(opts, :vault)
    label = opts[:label]
    closure = !!opts[:closure]

    quote location: :keep do
    
    
      @doc false
      @impl Ecto.Type
      def load(nil) do
        {:ok, nil}
      end

      def load(value) do
        with {:ok, value} <- decrypt(value) do
          value = after_decrypt(value)

          if unquote(closure) do
            {:ok, fn -> value end}
          else
            {:ok, value}
          end
        else
          _other ->
            :error
        end
      end

So, it appears I need to use both overridable functions in order to not get a compilation warning.

Expected behavior

Because I don't "own" the module I'm using, I'd expect the compiler to not care that one of the clauses is unused?

acco commented

To reproduce:

  1. Clone Sequin
  2. Modify the first load in lib/sequin/encrypted/field.ex:
def load(nil), do: {:ok, nil}
  1. mix compile

Does it work if you change quote location: :keep do to mark the generated functions as generated: true?

acco commented

@josevalim I followed these steps:

  1. Edited deps/cloak_ecto. I changed the using from this:
quote location: :keep do

To this:

quote location: :keep, generated: true do
  1. Ran this:
mix deps.compile cloak_ecto --force

When I run mix compile, the warning persists.

Thank you. We need to decide to not emit these warnings for overriddable functions (that may be the best option) or to generated ones.

@acco I believe you forgot mix compile --force in the last step, that addresses the issue here. We need to decide if that's how we want to move forward though. :)

acco commented

@josevalim Ah, that's right! I was using --force with the dep, but not with my repo. Running mix compile --force after adding generated: true indeed fixes 🙏

Just so you know, we decided to fix this without requiring you to annotate them as :generated. This is because :generated would turn off other warnings.

acco commented

@josevalim Very cool, thank you! ❤️