Lift restrictions for matching of binaries and maps #6415
Conversation
CT Test Results 3 files 361 suites 44m 36s ⏱️ Results for commit fe049b3. ♻️ This comment has been updated with latest results. To speed up review, make sure that you have read Contributing to Erlang/OTP and that all checks pass. See the TESTING and DEVELOPMENT HowTo guides for details about how to run test locally. Artifacts// Erlang/OTP Github Action Bot |
|
Thank you for this change! It makes the language a bit simpler, and I like that it clarifies the semantics of assignment. |
bcf9221 to
2983a25
Compare
|
This PR seems to have introduced #6445 |
2983a25 to
517da04
Compare
|
@RobinMorisset Thanks for catching this problem. Fixed. |
1435879 to
0dcf977
Compare
908223c to
705a255
Compare
|
This PR introduces an erlc crash on the following code: -module(validator).
-compile([export_all]).
f(#{[] := <<_>>, [] := <<_>>}) ->
ok.It results in the following error message: I could not reproduce this bug on master, only with this PR. @bjorng I'm not sure whether such PR-specific bug reports should go here or in their own issue, please tell me if you have any preference (or more generally if I should report the bugs I find in a different way). |
|
As long as a PR is not merged, I prefer bug reports to go into that PR as comments. |
6e2be21 to
f138b60
Compare
|
I have pushed a bug fix. Thanks for noticing the problem. |
|
Here is another testcase that only crashes erlc with this PR: -module(kernel_to_ssa5).
-compile([export_all]).
f1() ->
ok.
f2() ->
#{Y := _} = (Y = ((_ = X) = f1())).error message: |
f138b60 to
5e3769e
Compare
|
Thanks! Fixed. |
|
Here is a testcase that breaks with the latest version of this PR and not on master: f(X, <<Y>>) when ((Y > X) xor X); (not X); X ->
ok.Error message: |
|
And here is another with sensibly the same error message, I'm putting it here in case it helps with debugging: f(<<X>>, Y) when {(X / 1), (Y andalso true)}; Y ->
ok. |
|
Thanks! Fixed. |
|
It seems to still be possible to trigger the same bug (or at least one with the same error message) with different testcases, such as: f(#{0 := <<_>>, 0 := <<X>>}) ->
X.or f(#{0 := <<_>>, 0 := <<_, 0>>}) ->
ok.or f(#{0 := <<_>>, 0 := <<_, _:(0 div 0)>>}) ->
0.or f(#{0 := <<X:0>>, 0 := <<X>>}) ->
ok.or f() ->
case (catch <<0 || true>>) of
#{[] := <<_>>, [] := <<X>>} ->
X
end. |
39eb24b to
7f0bd6d
Compare
|
Yes, it is the same bug. I have now pushed a fix that cares of it in a different way than my previous attempts. |
|
Thank you for your reactivity. The fuzzer can no longer trigger that particular bug, but it now found some very similar testcases that trigger a different error message: f(#{0 := <<_, _:(true andalso 0)>>, 0 := <<_>>}) ->
ok.and f() ->
case catch ({}+{}) of
#{[] := <<X:0, _:X>>, [] := <<_>>} ->
ok
end.Both cause a crash with the following error message (that is for the second case, the first one is the same except for some numbers): |
|
Your latest example triggered a bug in the https://github.com/bjorng/otp/tree/bjorn/compiler/parallel-match-WIP |
1b57a7f to
b5fee1a
Compare
|
I have now fixed the other bugs and updated this pull request. |
c75d19f to
1a0b7c7
Compare
|
@rvirding Do you have any opinions about this pull request and the In particular, what do you think about that in bodies, multiple I am asking because we have had some discussions in the OTP team as a preparation for an OTP Technical Board meeting we will have later this week. |
2b450c8 to
4b1b796
Compare
|
The OTP Technical Board has now approved this pull request. I have updated the documentation to clarify the semantics of the match operator and the compound pattern operator (a new name invented by @RaimoNiskanen in order to more easily distinguish the two flavors of the After the updates of the documentation have been reviewed, I hope to merge this pull request next week. |
| 2> <input>F = fun({A, B} = E) -> {E, A + B} end, F({1,2}).</input> | ||
| {{1,2},3} | ||
| 3> <input>G = fun(<<A:8,B:8>> = <<C:16>>) -> {A, B, C} end, G(<<42,43>>).</input> | ||
| {42,42,10795}</pre> |
There was a problem hiding this comment.
Shouldn't the second element be 43 here?
There was a problem hiding this comment.
Yes, it should. I changed example to make it clearer and changed it manually instead of copy-pasting the whole thing from the output of the shell.
| <p>If the matching succeeds, any unbound variable in the pattern | ||
| becomes bound and the value of <c>Expr2</c> is returned.</p> | ||
| becomes bound and the value of <c>Expr</c> is returned.</p> | ||
| <p>If multiple match operators are applied in sequence, they will be |
There was a problem hiding this comment.
I'd maybe add that even if there is a single match operator, its right hand side will be executed before the pattern on its left hand side is checked. So for example the following is valid:
<<X:Y>> = begin Y = 8, <<42:8>> endAnd it correctly binds X to 42, as Y is bound to 8 before the validity of the pattern on the left hand side is checked.
There was a problem hiding this comment.
Yes, I will add that clarification and an example.
4b1b796 to
fe049b3
Compare
There has always been an implementation limitation for matching
of binaries (for technical reasons). For example:
foo(Bin) ->
<<A:8>> = <<X:4,Y:4>> = Bin,
{A,X,Y}.
This would fail to compile with the following message:
t.erl:5:5: binary patterns cannot be matched in parallel using '='
% 5| <<A:8>> = <<X:4,Y:4>> = Bin,
% | ^
This commit lifts this restriction, making the example legal.
A restriction for map matching is also lifted, but before we can
describe that, we'll need a digression to talk about the `=` operator.
The `=` operator can be used for two similar but slightly differently
purposes.
When used in a pattern in a clause, for example in a function head,
both the left-hand and right-hand side operands must be patterns:
Pattern1 = Pattern2
For example:
bar(#{a := A} = #{b := B}) -> {A, B}.
The following example will not compile because the right-hand side
is not a pattern but an expression:
wrong(#{a := A} = #{b => B}) -> {A, B}.
t.erl:4:23: illegal pattern
% 4| wrong(#{a := A} = #{b => B}) -> {A, B}.
% | ^
Used in this context, the `=` operator does not imply that the two
patterns are matched in any particular order. Attempting to use a
variable matched out on the left-hand side on the right-hand side, or
vice versa, will fail:
also_wrong1(#{B := A} = #{b := B}) -> {A,B}.
also_wrong2(#{a := A} = #{A := B}) -> {A,B}.
t.erl:6:15: variable 'B' is unbound
% 6| also_wrong1(#{B := A} = #{b := B}) -> {A,B}.
% | ^
t.erl:7:27: variable 'A' is unbound
% 7| also_wrong2(#{a := A} = #{A := B}) -> {A,B}.
% | ^
The other way to use `=` is in a function body. Used in this way,
the right-hand side must be an expression:
Pattern = Expression
For example:
foobar(Value) ->
#{a := A} = #{a => Value},
A.
Used in this context, the right-hand side of `=` must **not** be a pattern:
illegal_foobar(Value) ->
#{a := A} = #{a := Value},
A.
t.erl:18:21: only association operators '=>' are allowed in map construction
% 18| #{a := A} = #{a := Value},
% | ^
When used in a body context, the value of the `=` operator is the
value of its right-hand side operand. When multiple `=` operators are
combined, they are evaluted from right to left. That means that any
number of patterns can be matched at once:
Pattern1 = Pattern2 = ... = PatternN = Expr
which is equivalent to:
Var = Expr
PatternN = Var
.
.
.
Pattern2 = Var
Pattern1 = Var
Given that there is a well-defined evaluation order from right to
left, one would expect that the following example would be legal:
baz(M) ->
#{K := V} = #{k := K} = M,
V.
It is not. In Erlang/OTP 25 or earlier, the compilation fails with the
following message:
t.erl:28:7: variable 'K' is unbound
% 28| #{K := V} = #{k := K} = M,
% | ^
That restriction is now lifted, making the example legal.
Closes erlang#6348
Closes erlang#6444
Closes erlang#6467
fe049b3 to
6a9ebe6
Compare
Lift restrictions for matching of binaries and maps
There has always been an implementation limitation for matching
of binaries (for technical reasons). For example:
This would fail to compile with the following message:
This commit lifts this restriction, making the example legal.
A restriction for map matching is also lifted, but before we can
describe that, we'll need a digression to talk about the
=operator.The
=operator can be used for two similar but slightly differentlypurposes.
When used in a pattern in a clause, for example in a function head,
both the left-hand and right-hand side operands must be patterns:
For example:
The following example will not compile because the right-hand side
is not a pattern but an expression:
Used in this context, the
=operator does not imply that the twopatterns are matched in any particular order. Attempting to use a
variable matched out on the left-hand side on the right-hand side, or
vice versa, will fail:
The other way to use
=is in a function body. Used in this way,the right-hand side must be an expression:
For example:
Used in this context, the right-hand side of
=must not be a pattern:When used in a body context, the value of the
=operator is thevalue of its right-hand side operand. When multiple
=operators arecombined, they are evaluted from right to left. That means that any
number of patterns can be matched at once:
which is equivalent to:
Given that there is a well-defined evaluation order from right to
left, one would expect that the following example would be legal:
It is not. In Erlang/OTP 25 or earlier, the compilation fails with the
following message:
That restriction is now lifted, making the example legal.
Closes #6348
Closes #6444
Closes #6467