`Module.delete_attribute/2` doesn’t reset `accumulate: true`

[ImNotAVirus]

I'm not sure if it's a bug but if it's I can do a PR to fix it.

Current behavior

In summary the steps are:

• Create a module attribute with the accumulate: true option
• Delete the module attribute
• Recreate it with the accumulate: false option
• Try to assign values, they will still be accumulated

Steps to reproduce

Here is a small example.

defmodule ModuleDeleteTest do
  # 1/ Define a module attribute with accumulate: true
  Module.register_attribute(__MODULE__, :attribute, accumulate: true)
  
  # 2/ Then delete and recreate the module attribute but with accumulate: false
  Module.delete_attribute(__MODULE__, :attribute)
  Module.register_attribute(__MODULE__, :attribute, accumulate: false)

  # Unfortunately, the accumulate: true flag is still present
  @attribute :foo
  @attribute :bar
  
  # > attribute value: [:bar, :foo]
  IO.inspect(@attribute, label: "attribute value")
end

This code displays attribute value: [:bar, :foo] when compiled.

Expected behavior

I expect the code to display attribute value: :bar.

How to fix

The bug come from theses lines:

elixir/lib/elixir/lib/module.ex

Lines 1529 to 1530 in 1e4ed32

	[{_, _, :accumulate}] ->
	reverse_values(:ets.take(bag, {:accumulate, key}), [])

Here, when the module attribute has the flag accumulate: true, the bag is emptied but not the set which always contains {attribute, [], :accumulate}. So when register_attribute/3 is then called with accumulate: true, this line https://github.com/elixir-lang/elixir/blob/main/lib/elixir/lib/module.ex#L1592 will do nothing.

Here is a quick woking example:

defmodule ModuleFix do
  def delete_attribute(module, key) do
    {set, _bag} = :elixir_module.data_tables(module)
    
    # Here bag is clean by Module.delete_attribute/2
    Module.delete_attribute(module, key)
    
    # But you also need to clean the set or the module attribute
    # still have the :accumulate flag set
    :ets.delete(set, key)
  end
end

defmodule ModuleDeleteTestFix do
  # 1/ Define a module attribute with accumulate: true
  Module.register_attribute(__MODULE__, :attribute, accumulate: true)
  
  # 2/ Then delete and recreate the module attribute but with accumulate: false
  ModuleFix.delete_attribute(__MODULE__, :attribute)
  Module.register_attribute(__MODULE__, :attribute, accumulate: false)

  # Now, module attribute is properly reset
  @attribute :foo
  @attribute :bar
  
  # > field value: :bar
  IO.inspect(@attribute, label: "attribute value")
end

Environment

• Elixir & Erlang/OTP versions (elixir --version):

elixir --version
Erlang/OTP 24 [erts-12.2] [source] [64-bit] [smp:8:8] [ds:8:8:10] [async-threads:1]

Elixir 1.13.2 (compiled with Erlang/OTP 24)

• Operating system: Windows & Linux Debian

[josevalim]

Yes, this is intentional. It is the way to clear the accumulated attributes if you want to start accumulating from scratch. :)

[ImNotAVirus]

Personally I find it strange to have a function called delete_attribute which doesn't actually delete the module attribute.
Also, I haven't found any information in the documentation explaining this behavior.

Do you think it would be possible to rename this function clear_attribute and create a new function delete_attribute which would completely remove the attribute from the module?

What I'm trying to do is create an attribute with accumulate: true, assign values to it, then reverse that list and reassign it to the same attribute. But it doesn't seem feasible at the moment.

EDIT: Or maybe verify in register_attribute(_, _, accumulate: false) if the attribute was previously set to accumulate: true but currently the bag is empty then the attribute becomes accumulate: false.

[josevalim]

Any of those changes could be considered a breaking change. Perhaps the best is to introduce an unregister_attribute that deletes and removes any existing configuration, but then we should probably discuss the use cases as having an attribute change from accumulating to non-accumulating can be confusing. :)