Require pin variable when accessing variable inside binary size in match, closes #12588

josevalim · josevalim · commit 7454333fd0f8 · 2023-05-26T16:35:36.000+02:00
diff --git a/lib/elixir/lib/base.ex b/lib/elixir/lib/base.ex
@@ -634,7 +634,7 @@ defmodule Base do
 
     defp unquote(name)(string, pad?) do
       segs = div(byte_size(string) + 7, 8) - 1
-      <<main::size(segs)-binary-unit(64), rest::binary>> = string
+      <<main::size(^segs)-binary-unit(64), rest::binary>> = string
 
       main =
         for <<c1::8, c2::8, c3::8, c4::8, c5::8, c6::8, c7::8, c8::8 <- main>>, into: <<>> do
@@ -1107,7 +1107,7 @@ defmodule Base do
 
     defp unquote(name)(string, pad?) do
       segs = div(byte_size(string) + 7, 8) - 1
-      <<main::size(segs)-binary-unit(64), rest::binary>> = string
+      <<main::size(^segs)-binary-unit(64), rest::binary>> = string
 
       main =
         for <<c1::8, c2::8, c3::8, c4::8, c5::8, c6::8, c7::8, c8::8 <- main>>, into: <<>> do
diff --git a/lib/elixir/lib/inspect.ex b/lib/elixir/lib/inspect.ex
@@ -208,7 +208,7 @@ defimpl Inspect, for: BitString do
 
   defp each_bit(bitstring, _counter, opts) do
     size = bit_size(bitstring)
-    <<h::size(size)>> = bitstring
+    <<h::size(^size)>> = bitstring
     concat(Inspect.Integer.inspect(h, opts), "::size(" <> Integer.to_string(size) <> ")")
   end
 
diff --git a/lib/elixir/lib/kernel.ex b/lib/elixir/lib/kernel.ex
@@ -4783,7 +4783,7 @@ defmodule Kernel do
         part
       else
         <<first_byte, rest::binary>> = part
-        for <<_::size(step - 1)-bytes, byte <- rest>>, into: <<first_byte>>, do: <<byte>>
+        for <<_::size(^step - 1)-bytes, byte <- rest>>, into: <<first_byte>>, do: <<byte>>
       end
     else
       ""
diff --git a/lib/elixir/lib/kernel/special_forms.ex b/lib/elixir/lib/kernel/special_forms.ex
@@ -249,7 +249,7 @@ defmodule Kernel.SpecialForms do
   The size can be a variable or any valid guard expression:
 
       iex> name_size = 5
-      iex> <<name::binary-size(name_size), " the ", species::binary>> = <<"Frank the Walrus">>
+      iex> <<name::binary-size(^name_size), " the ", species::binary>> = <<"Frank the Walrus">>
       iex> {name, species}
       {"Frank", "Walrus"}
 
diff --git a/lib/elixir/lib/regex.ex b/lib/elixir/lib/regex.ex
@@ -601,7 +601,7 @@ defmodule Regex do
     new_offset = pos + length
     keep = pos - offset
 
-    <<_::binary-size(offset), part::binary-size(keep), match::binary-size(length), _::binary>> =
+    <<_::binary-size(^offset), part::binary-size(^keep), match::binary-size(^length), _::binary>> =
       string
 
     if keep == 0 and trim do
@@ -618,7 +618,7 @@ defmodule Regex do
     if keep == 0 and trim do
       do_split([h | t], string, new_offset, counter, trim, false)
     else
-      <<_::binary-size(offset), part::binary-size(keep), _::binary>> = string
+      <<_::binary-size(^offset), part::binary-size(^keep), _::binary>> = string
       [part | do_split([h | t], string, new_offset, counter - 1, trim, false)]
     end
   end
@@ -748,12 +748,12 @@ defmodule Regex do
   defp apply_list(whole, string, pos, replacement, [[{mpos, _} | _] | _] = list)
        when mpos > pos do
     length = mpos - pos
-    <<untouched::binary-size(length), rest::binary>> = string
+    <<untouched::binary-size(^length), rest::binary>> = string
     [untouched | apply_list(whole, rest, mpos, replacement, list)]
   end
 
   defp apply_list(whole, string, pos, replacement, [[{pos, length} | _] = head | tail]) do
-    <<_::size(length)-binary, rest::binary>> = string
+    <<_::size(^length)-binary, rest::binary>> = string
     new_data = apply_replace(whole, replacement, head)
     [new_data | apply_list(whole, rest, pos + length, replacement, tail)]
   end
@@ -788,7 +788,7 @@ defmodule Regex do
   end
 
   defp get_index(string, {pos, length}) do
-    <<_::size(pos)-binary, res::size(length)-binary, _::binary>> = string
+    <<_::size(^pos)-binary, res::size(^length)-binary, _::binary>> = string
     res
   end
 
diff --git a/lib/elixir/lib/string.ex b/lib/elixir/lib/string.ex
@@ -673,7 +673,7 @@ defmodule String do
   defp do_split_at(string, position) do
     remaining = byte_size_remaining_at(string, position)
     start = byte_size(string) - remaining
-    <<left::size(start)-binary, right::size(remaining)-binary>> = string
+    <<left::size(^start)-binary, right::size(^remaining)-binary>> = string
     {left, right}
   end
 
@@ -1001,7 +1001,7 @@ defmodule String do
   defp replace_leading(string, match, replacement, prefix_size, suffix_size, acc)
        when suffix_size >= 0 do
     case string do
-      <<prefix::size(prefix_size)-binary, suffix::binary>> when prefix == match ->
+      <<prefix::size(^prefix_size)-binary, suffix::binary>> when prefix == match ->
         replace_leading(
           suffix,
           match,
@@ -1060,7 +1060,7 @@ defmodule String do
   defp replace_trailing(string, match, replacement, prefix_size, suffix_size, acc)
        when prefix_size >= 0 do
     case string do
-      <<prefix::size(prefix_size)-binary, suffix::binary>> when suffix == match ->
+      <<prefix::size(^prefix_size)-binary, suffix::binary>> when suffix == match ->
         replace_trailing(
           prefix,
           match,
@@ -1113,7 +1113,7 @@ defmodule String do
     prefix_size = byte_size(match)
 
     case string do
-      <<prefix::size(prefix_size)-binary, suffix::binary>> when prefix == match ->
+      <<prefix::size(^prefix_size)-binary, suffix::binary>> when prefix == match ->
         prepend_unless_empty(replacement, suffix)
 
       _ ->
@@ -1156,7 +1156,7 @@ defmodule String do
     prefix_size = byte_size(string) - suffix_size
 
     case string do
-      <<prefix::size(prefix_size)-binary, suffix::binary>> when suffix == match ->
+      <<prefix::size(^prefix_size)-binary, suffix::binary>> when suffix == match ->
         append_unless_empty(prefix, replacement)
 
       _ ->
diff --git a/lib/elixir/lib/string_io.ex b/lib/elixir/lib/string_io.ex
@@ -322,13 +322,13 @@ defmodule StringIO do
   end
 
   defp get_chars(input, :latin1, count) do
-    <<chars::binary-size(count), rest::binary>> = input
+    <<chars::binary-size(^count), rest::binary>> = input
     {chars, rest}
   end
 
   defp get_chars(input, :unicode, count) do
     with {:ok, count} <- split_at(input, count, 0) do
-      <<chars::binary-size(count), rest::binary>> = input
+      <<chars::binary-size(^count), rest::binary>> = input
       {chars, rest}
     end
   end
@@ -398,7 +398,7 @@ defmodule StringIO do
   defp get_until(chars, encoding, mod, fun, args, continuation, count) do
     case bytes_until_eol(chars, encoding, 0) do
       {kind, size} when kind in [:split, :replace_split] ->
-        <<line::binary-size(size), rest::binary>> = chars
+        <<line::binary-size(^size), rest::binary>> = chars
 
         case apply(mod, fun, [continuation, binary_to_list(line, encoding) | args]) do
           {:done, result, :eof} ->
diff --git a/lib/elixir/src/elixir.hrl b/lib/elixir/src/elixir.hrl
@@ -8,7 +8,7 @@
 -record(elixir_ex, {
   caller=false,            %% stores if __CALLER__ is allowed
   %% TODO: Remove warn and everywhere it is set in v2.0
-  prematch=warn,           %% {Read, Counter} | warn | raise | pin | {bitsize,PreVars,OriginalVars}
+  prematch=warn,           %% {Read, Counter, {bitsize, Original} | none} | warn | raise | pin
   stacktrace=false,        %% stores if __STACKTRACE__ is allowed
   unused={#{}, 0},         %% a map of unused vars and a version counter for vars
   vars={#{}, false}        %% a tuple with maps of read and optional write current vars
diff --git a/lib/elixir/src/elixir_bitstring.erl b/lib/elixir/src/elixir_bitstring.erl
@@ -262,11 +262,11 @@ expand_spec_arg(Expr, S, _OriginalS, E) when is_atom(Expr); is_integer(Expr) ->
   {Expr, S, E};
 expand_spec_arg(Expr, S, OriginalS, #{context := match} = E) ->
   %% We can only access variables that are either on prematch or not in original
-  #elixir_ex{prematch={PreRead, Counter}} = S,
+  #elixir_ex{prematch={PreRead, PreCounter, _} = OldPre} = S,
   #elixir_ex{vars={OriginalRead, _}} = OriginalS,
-  Prematch = {bitsize, PreRead, OriginalRead},
-  {EExpr, SE, EE} = elixir_expand:expand(Expr, S#elixir_ex{prematch=Prematch}, E#{context := guard}),
-  {EExpr, SE#elixir_ex{prematch={PreRead, Counter}}, EE#{context := match}};
+  NewPre = {PreRead, PreCounter, {bitsize, OriginalRead}},
+  {EExpr, SE, EE} = elixir_expand:expand(Expr, S#elixir_ex{prematch=NewPre}, E#{context := guard}),
+  {EExpr, SE#elixir_ex{prematch=OldPre}, EE#{context := match}};
 expand_spec_arg(Expr, S, OriginalS, E) ->
   elixir_expand:expand(Expr, elixir_env:reset_read(S, OriginalS), E).
 
diff --git a/lib/elixir/src/elixir_clauses.erl b/lib/elixir/src/elixir_clauses.erl
@@ -14,7 +14,7 @@ match(Fun, Expr, AfterS, BeforeS, E) ->
   #elixir_ex{vars={Read, _}, prematch=Prematch} = BeforeS,
 
   CallS = BeforeS#elixir_ex{
-    prematch={Read, Counter},
+    prematch={Read, Counter, none},
     unused=Unused,
     vars=Current
   },
@@ -32,7 +32,7 @@ match(Fun, Expr, AfterS, BeforeS, E) ->
   {EExpr, EndS, EndE}.
 
 def({Meta, Args, Guards, Body}, S, E) ->
-  {EArgs, SA, EA} = elixir_expand:expand_args(Args, S#elixir_ex{prematch={#{}, 0}}, E#{context := match}),
+  {EArgs, SA, EA} = elixir_expand:expand_args(Args, S#elixir_ex{prematch={#{}, 0, none}}, E#{context := match}),
   {EGuards, SG, EG} = guard(Guards, SA#elixir_ex{prematch=raise}, EA#{context := guard}),
   {EBody, SB, EB} = elixir_expand:expand(Body, elixir_env:set_prematch_from_config(SG), EG#{context := nil}),
   elixir_env:check_unused_vars(SB, EB),
diff --git a/lib/elixir/src/elixir_env.erl b/lib/elixir/src/elixir_env.erl
@@ -52,7 +52,7 @@ set_prematch_from_config(#elixir_ex{} = S) ->
 
 env_to_ex(#{context := match, versioned_vars := Vars}) ->
   Counter = map_size(Vars),
-  #elixir_ex{prematch={Vars, Counter}, vars={Vars, false}, unused={#{}, Counter}};
+  #elixir_ex{prematch={Vars, Counter, none}, vars={Vars, false}, unused={#{}, Counter}};
 env_to_ex(#{versioned_vars := Vars}) ->
   set_prematch_from_config(#elixir_ex{vars={Vars, false}, unused={#{}, map_size(Vars)}}).
 
diff --git a/lib/elixir/src/elixir_erl_pass.erl b/lib/elixir/src/elixir_erl_pass.erl
@@ -191,7 +191,7 @@ translate({with, Meta, [_ | _] = Args}, _Ann, S) ->
 
 %% Variables
 
-translate({'^', _, [{Name, VarMeta, Kind}]}, _Ann, #elixir_erl{context=match} = S) when is_atom(Name), is_atom(Kind) ->
+translate({'^', _, [{Name, VarMeta, Kind}]}, _Ann, S) when is_atom(Name), is_atom(Kind) ->
   {Var, VS} = elixir_erl_var:translate(VarMeta, Name, Kind, S),
 
   case S#elixir_erl.extra of
@@ -519,7 +519,7 @@ translate_bitstring(Meta, Args, S) ->
   build_bitstr(Args, ?ann(Meta), S, []).
 
 build_bitstr([{'::', Meta, [H, V]} | T], Ann, S, Acc) ->
-  {Size, Types} = extract_bit_info(V, Meta, S#elixir_erl{context=nil}),
+  {Size, Types} = extract_bit_info(V, Meta, S#elixir_erl{context=nil, extra=nil}),
   build_bitstr(T, Ann, S, Acc, H, Size, Types);
 build_bitstr([], Ann, S, Acc) ->
   {{bin, Ann, lists:reverse(Acc)}, S}.
diff --git a/lib/elixir/src/elixir_expand.erl b/lib/elixir/src/elixir_expand.erl
@@ -270,14 +270,10 @@ expand({super, Meta, Args}, S, E) when is_list(Args) ->
 
 %% Vars
 
-expand({'^', Meta, [Arg]}, S, #{context := match} = E) ->
-  #elixir_ex{prematch={Prematch, _}, vars={_Read, Write}} = S,
-
-  %% We need to rollback to a no match context
-  NoMatchE = E#{context := nil},
+expand({'^', Meta, [Arg]}, #elixir_ex{prematch={Prematch, _, _}, vars={_, Write}} = S, E) ->
   NoMatchS = S#elixir_ex{prematch=pin, vars={Prematch, Write}},
 
-  case expand(Arg, NoMatchS, NoMatchE) of
+  case expand(Arg, NoMatchS, E#{context := nil}) of
     {{Name, _, Kind} = Var, #elixir_ex{unused=Unused}, _} when is_atom(Name), is_atom(Kind) ->
       {{'^', Meta, [Var]}, S#elixir_ex{unused=Unused}, E};
 
@@ -295,7 +291,7 @@ expand({'_', Meta, Kind} = Var, S, #{context := Context} = E) when is_atom(Kind)
 
 expand({Name, Meta, Kind}, S, #{context := match} = E) when is_atom(Name), is_atom(Kind) ->
   #elixir_ex{
-    prematch={_, PrematchVersion},
+    prematch={_, PrematchVersion, _},
     unused={Unused, Version},
     vars={Read, Write}
   } = S,
@@ -335,15 +331,19 @@ expand({Name, Meta, Kind}, S, E) when is_atom(Name), is_atom(Kind) ->
     case Read of
       #{Pair := CurrentVersion} ->
         case Prematch of
-          {bitsize, Pre, Original} ->
+          {Pre, _Counter, {bitsize, Original}} ->
             if
               map_get(Pair, Pre) /= CurrentVersion ->
                 {ok, CurrentVersion};
               is_map_key(Pair, Pre) ->
+                %% TODO: Enable this warning on Elixir v1.19
+                %% TODO: Remove me on Elixir 2.0
+                %% elixir_errors:file_warn(Meta, E, ?MODULE, {unpinned_bitsize_var, Name, Kind}),
                 {ok, CurrentVersion};
               not is_map_key(Pair, Original) ->
                 {ok, CurrentVersion};
-              true -> raise
+              true ->
+                raise
             end;
 
           _ ->
@@ -1132,7 +1132,7 @@ assert_no_match_scope(_Meta, _Kind, _E) -> ok.
 assert_no_guard_scope(Meta, Kind, S, #{context := guard, file := File}) ->
   Key =
     case S#elixir_ex.prematch of
-      {bitsize, _, _}  -> invalid_expr_in_bitsize;
+      {_, _, {bitsize, _}}  -> invalid_expr_in_bitsize;
       _ -> invalid_expr_in_guard
     end,
   file_error(Meta, File, ?MODULE, {Key, Kind});
@@ -1156,7 +1156,7 @@ assert_no_underscore_clause_in_cond(_Other, _E) ->
 
 %% Errors
 
-guard_context(#elixir_ex{prematch={bitsize, _, _}}) -> "bitstring size specifier";
+guard_context(#elixir_ex{prematch={_, _, {bitsize, _}}}) -> "bitstring size specifier";
 guard_context(_) -> "guards".
 
 format_error({useless_literal, Term}) ->
@@ -1317,6 +1317,10 @@ format_error({options_are_not_keyword, Kind, Opts}) ->
                 [Kind, 'Elixir.Macro':to_string(Opts)]);
 format_error({undefined_function, Name, Args}) ->
   io_lib:format("undefined function ~ts/~B (there is no such import)", [Name, length(Args)]);
+format_error({unpinned_bitsize_var, Name, Kind}) ->
+  io_lib:format("the variable \"~ts\"~ts is accessed inside size(...) of a bitstring "
+                "but it was defined outside of the match. You must preceed it with the "
+                "pin operator", [Name, context_info(Kind)]);
 format_error({underscored_var_repeat, Name, Kind}) ->
   io_lib:format("the underscored variable \"~ts\"~ts appears more than once in a "
                 "match. This means the pattern will only match if all \"~ts\" bind "
diff --git a/lib/elixir/test/elixir/kernel/binary_test.exs b/lib/elixir/test/elixir/kernel/binary_test.exs
@@ -83,9 +83,14 @@ defmodule Kernel.BinaryTest do
     assert x == "foo"
 
     size = 3
-    <<x::binary-size(size)>> <> _ = "foobar"
+    <<x::binary-size(^size)>> <> _ = "foobar"
     assert x == "foo"
 
+    size = 3
+    <<size, x::binary-size(^size)>> <> _ = <<10, "foobar">>
+    assert x == "foo"
+    assert size == 10
+
     <<x::6*4-binary>> <> _ = "foobar"
     assert x == "foo"
 
@@ -164,7 +169,7 @@ defmodule Kernel.BinaryTest do
 
   test "pattern match" do
     s = 16
-    assert <<_a, _b::size(s)>> = "foo"
+    assert <<_a, _b::size(^s)>> = "foo"
   end
 
   test "pattern match with splice" do
@@ -240,8 +245,8 @@ defmodule Kernel.BinaryTest do
 
   test "bitsyntax variable size" do
     x = 8
-    assert <<_, _::size(x)>> = <<?a, ?b>>
-    assert (fn <<_, _::size(x)>> -> true end).(<<?a, ?b>>)
+    assert <<_, _::size(^x)>> = <<?a, ?b>>
+    assert (fn <<_, _::size(^x)>> -> true end).(<<?a, ?b>>)
   end
 
   test "bitsyntax size using expressions" do
@@ -256,12 +261,12 @@ defmodule Kernel.BinaryTest do
 
   test "bitsyntax size using guard expressions in match context" do
     x = 8
-    assert <<1::size(x - 5)>> = <<1::3>>
-    assert <<1::size(x - 5)-unit(8)>> = <<1::3*8>>
+    assert <<1::size(^x - 5)>> = <<1::3>>
+    assert <<1::size(^x - 5)-unit(8)>> = <<1::3*8>>
     assert <<1::size(length(~c"abcd"))>> = <<1::4>>
 
     foo = %{bar: 5}
-    assert <<1::size(foo.bar)>> = <<1::5>>
+    assert <<1::size((^foo).bar)>> = <<1::5>>
   end
 
   defmacro signed_16 do
diff --git a/lib/elixir/test/elixir/kernel/comprehension_test.exs b/lib/elixir/test/elixir/kernel/comprehension_test.exs
@@ -496,7 +496,11 @@ defmodule Kernel.ComprehensionTest do
   test "binary for comprehensions with variable size" do
     s = 16
     bin = <<1, 2, 3, 4, 5, 6>>
-    assert for(<<x::size(s) <- bin>>, into: "", do: to_bin(div(x, 2))) == <<129, 130, 131>>
+    assert for(<<x::size(^s) <- bin>>, into: "", do: to_bin(div(x, 2))) == <<129, 130, 131>>
+
+    s = 8
+    bin = <<1, 2, 3, 4, 5, 6>>
+    assert for(<<s, x::size(^s) <- bin>>, into: "", do: <<x * s>>) == <<2, 12, 30>>
 
     # Aligned
     bin = <<8, 1, 16, 2, 3>>
diff --git a/lib/elixir/test/elixir/kernel/errors_test.exs b/lib/elixir/test/elixir/kernel/errors_test.exs
@@ -842,13 +842,6 @@ defmodule Kernel.ErrorsTest do
     )
   end
 
-  test "invalid size in bitstrings" do
-    assert_compile_error(
-      ["nofile:1", "misplaced operator ^x"],
-      ~c"x = 8; <<a, b::size(^x)>> = <<?a, ?b>>"
-    )
-  end
-
   test "function head with guard" do
     assert_compile_error(["nofile:2", "missing :do option in \"def\""], ~c"""
     defmodule Kernel.ErrorsTest.BodyessFunctionWithGuard do
diff --git a/lib/elixir/test/elixir/kernel/expansion_test.exs b/lib/elixir/test/elixir/kernel/expansion_test.exs
@@ -2545,7 +2545,7 @@ defmodule Kernel.ExpansionTest do
     defmacro offset(size, binary) do
       quote do
         offset = unquote(size)
-        <<_::size(offset)>> = unquote(binary)
+        <<_::size(^offset)>> = unquote(binary)
       end
     end
 
diff --git a/lib/elixir/unicode/unicode.ex b/lib/elixir/unicode/unicode.ex
diff --git a/lib/logger/lib/logger/utils.ex b/lib/logger/lib/logger/utils.ex
diff --git a/lib/mix/lib/mix/task.ex b/lib/mix/lib/mix/task.ex
