elixir-lang / elixir

Elixir is a dynamic, functional language for building scalable and maintainable applications
https://elixir-lang.org/
Apache License 2.0
24.55k stars 3.38k forks source link

Support calculations in `size` specifier for binaries #6273

Closed michalmuskala closed 7 years ago

michalmuskala commented 7 years ago

In Erlang, it's possible to create a function like this:

test(X) ->
    << 0:(X * 8) >>.

This is not possible to express in Elixir currently except for setting up a separate variable to do the size computation.

We can't support this directly as:

<<0 :: x * 8>>

since that would be ambiguous with the:

<<0 :: size * unit>>

notation. Fortunately we can support such calculations inside the size specifier:

<<0 :: size(x * 8)>>

Thanks to fowlduck for reporting this initially on IRC.

ggcampinho commented 7 years ago

Hi!

I tried a little bit around with this, maybe I'm doing something wrong but I think there are some limitations on the Erlang side and I don't know how to proceed.

First I added one line this test:

test "bitsyntax variable size" do
    x = 8
    assert <<_, _::size(x)>> = <<?a, ?b>>
    assert <<_, _::size(1 * x)>> = <<?a, ?b>>
    assert (fn <<_, _::size(x)>> -> true end).(<<?a, ?b>>)
  end

Then I got the error:

** (CompileError) lib/elixir/test/elixir/kernel/binary_test.exs:191: size in bitstring expects an integer or a variable as argument, got: :erlang.*(1, x)
    (elixir) src/elixir_bitstring.erl:114: :elixir_bitstring.expand_each_spec/4
    (elixir) src/elixir_bitstring.erl:86: :elixir_bitstring.expand_specs/5
    (elixir) src/elixir_bitstring.erl:29: :elixir_bitstring.expand/6
    (elixir) src/elixir_bitstring.erl:9: :elixir_bitstring.expand/4
    (ex_unit) expanding macro: ExUnit.Assertions.assert/1

Then I changed the validate_spec_arg to allow the expression with any operation on erlang:

validate_spec_arg(_, size, {Var, _, Context}, _) when is_atom(Var) and is_atom(Context) ->
  ok;
validate_spec_arg(_, size, Value, _) when is_integer(Value) ->
  ok;
validate_spec_arg(Meta, size, {{'.', _, [erlang, _]}, _, Context}, E) when is_list(Context) ->
  lists:all(fun (Elem) -> validate_spec_arg(Meta, size, Elem, E) == ok end, Context), ok;
validate_spec_arg(Meta, size, Value, E) ->
  form_error(Meta, ?key(E, file), ?MODULE, {bad_size_argument, Value});
validate_spec_arg(Meta, unit, Value, E) when not is_integer(Value) ->
  form_error(Meta, ?key(E, file), ?MODULE, {bad_unit_argument, Value});
validate_spec_arg(_Meta, _Key, _Value, _E) ->
  ok.

After that I got this error:

** (CompileError) lib/elixir/test/elixir/kernel/binary_test.exs:191: illegal bit size
    (stdlib) lists.erl:1338: :lists.foreach/2
    (elixir) lib/code.ex:376: Code.require_file/2

The representation of my bitstring is:

{{bin,191,
             [{bin_element,191,{var,191,'_'},default,[integer]},
              {bin_element,191,
                           {var,191,'_'},
                           {op,191,'*',{integer,0,1},{var,191,x@1}},
                           [integer]}]}

Checking the Erlang code, the problem is related to having a var inside of the op:

iex(3)> :erl_lint.is_pattern_expr({:op,191,:*,{:integer,0,1},{:var,191,:x@1}}) 
false
iex(4)> :erl_lint.is_pattern_expr({:op,191,:*,{:integer,0,1},{:integer,0,1}}) 
true

I thought about checking the macro env for the var x, but this is happening during compilation time, so I can't calculate the value and send an integer.

josevalim commented 7 years ago

Apologies for the lack of replies, @ggcampinho. It seems this one fell through the cracks.

Thanks for linking to the code. After studying it, it seems that Erlang applies this restriction only for pattern matches. Outside of a pattern, the size can be any expression.

@michalmuskala is it worth relaxing the constraints of the size outside of a pattern given it is constrained inside of a pattern? Maybe we should rather be consistent here? Thoughts?

michalmuskala commented 7 years ago

I wonder what is the reason for this limitation in Erlang. Perhaps we should work towards lifting the limitation there and the supporting it everywhere?

josevalim commented 7 years ago

@michalmuskala patterns are always limited. I don't think we would be able to support any expression in patterns. It gets specially complex if you try to write code like this:

<<x::8, y::size(x - 2)>>

Sure we could support more expressions but it is clear we can't support any expression.

michalmuskala commented 7 years ago

I think it should be possible to support all of the basic arithmetic operations (*, +, - and div), but I haven't looked into that particular part of runtime/compiler yet, so I can't say for sure.

josevalim commented 7 years ago

@michalmuskala in any case, if we add such, it would be at least Erlang 21 which means 2 or 3 years before we can leverage it consistently from Elixir. We need to decide what we want to do today.

michalmuskala commented 7 years ago

Can't we make it transparent and let it fail somewhere deeper in the compiler? Similar to what we do with other features introduced in some erlang version in the middle of Elixir support cycle (map typespecs, optional callbacks, etc).

josevalim commented 7 years ago

Sure but the point is that we need to decide how to act now. Regardless of a feature that may come or not in a year. :) --

José Valimwww.plataformatec.com.br http://www.plataformatec.com.br/Founder and Director of R&D

michalmuskala commented 7 years ago

I think we're able to wait to see if the feature is possible at all, right? Nothing is broken so keeping the status quo, for now, is the "safe" option.

josevalim commented 7 years ago

@michalmuskala no, there are two separate discussions here:

  1. Improve the expressions allowed in a pattern
  2. Decide the allowed expressions in size should be the same outside and inside of pattern

We should focus on 2. If we say they should not be the same, then 1 does not affect this discussion at all.

michalmuskala commented 7 years ago

In an ideal world, I think they should be identical, but we need to verify if that's feasible.

josevalim commented 7 years ago

@michalmuskala no, it is impossible to make them the same. This will never be allowed inside a pattern:

<<integer_key::integer, rest::size(Map.get(map_from_key_to_sizes, integer_key))>> = ...

Or even if you think that is harmless, what about this:

<<integer_key::integer, rest::size(get_size_with_an_over_the_network_call(integer_key))>> = ...

Erlang has always limited the amount of expressions in a pattern and I don't think they would relax this constraint now. And, even if they did, we likely wouldn't accept this an Elixir.

michalmuskala commented 7 years ago

As I said, I was only considering basic arithmetic expressions.

josevalim commented 7 years ago

Hrm, I am not sure how I can make this clearer but:

Decide the allowed expressions in size should be the same outside and inside of pattern

We need to decide if we want support all expressions outside of a pattern, beyond arithmetic expressions, because that is already possible today. If we say that they should be the same, then this issue is closed and when we add arithmetic expressions to patterns we will automatically get those outside patterns. If we say they should be different, then we change now and adding arithmetic expressions to patterns has no effect on general expressions.

A third option, which seems to be what you are hinting to, is to figure out if we can support arithmetic expressions inside pattern and then allow only those outside of a pattern but that's the worst of both words because they are both limited in different ways. And even if that's what you want, it means you have already implicitly chosen that they should be the same.

josevalim commented 7 years ago

Given the x * 8 syntax and the fact calculations cannot be done in pattern, I believe we should rather be consistent with how it works inside and outside patterns, therefore I am closing this issue. However if Erlang changes to support arithmetic expressions, we will support that too.