beam_validator: Track the unit of saved bs match positions #2744
Conversation
jhogberg
added
fix
team:VM
|
|
||
| ok. | ||
|
|
||
| bgbct_1(<<Bin/binary>> = Bin) -> |
There was a problem hiding this comment.
Shouldn't something like this be rejected by the linter in the first place, though?
There was a problem hiding this comment.
I suppose we could warn about it from OTP 24 onwards. As iffy as this seems it's still legal, and I'd rather not change it in a patch.
There was a problem hiding this comment.
I'm original reporter of this issue for Elixir (elixir-lang/elixir#10309) and I want explain why I happened to write code like that.
I need this expression to include argument name bin in documents generated by ex_doc.
If I only include <<bin::binary>>, then argument in document is named as arg.
And I prefer <<bin::binary>> pattern matching to is_binary(bin) guard.
There was a problem hiding this comment.
@jechol Would it work for the documentation if you would write the function head like this?
def encode_bin(<<_::binary>> = bin) do
The compiler will generate efficient code for that.
There was a problem hiding this comment.
@bjorng That's what I want. Thanks.
There was a problem hiding this comment.
I was specifically referring to variables on the left & right of patterns being "compared". I don't think it can ever succeed beyond this special case in binary where the pattern match is a no-op. Everywhere else a part of a data structure can't be equal to the data structure itself, e.g. {X, _} = X shouldn't even compile as it will always fail.
There was a problem hiding this comment.
@michalmuskala I think that {X, _} = X should produce a warning. That is what we do for other unmatchable patterns such as [X,Y] = {X,Y}. Our reasoning is that macros, parse transformations and naive code generators could produce nonsensical but working code.
There was a problem hiding this comment.
Introducing a warning in such cases would be a good improvement, agreed.
jhogberg
removed
the
testing
jhogberg
force-pushed
the
john/compiler/fix-bs-validation-error/ERL-1340/OTP-16838
branch
from
f092dde
to
db7ab57
Compare
jhogberg
changed the title
jhogberg
force-pushed
the
john/compiler/fix-bs-validation-error/ERL-1340/OTP-16838
branch
from
db7ab57
to
b6ba7f7
Compare
jhogberg
force-pushed
the
john/compiler/fix-bs-validation-error/ERL-1340/OTP-16838
branch
from
b6ba7f7
to
3556c9d
Compare
Prior to this commit, the unit of a match context would remain unchanged when rewinding, which would fail validation at best and let bugs slip by at worst. After fixing this it became apparent that unit updates on successful matches were broken as well, providing narrower types than they should have, so we'll fix that in this commit too to avoid breaking bisect for everyone.
jhogberg
force-pushed
the
john/compiler/fix-bs-validation-error/ERL-1340/OTP-16838
branch
from
3556c9d
to
5474277
Compare
jhogberg
added
the
testing
https://bugs.erlang.org/browse/ERL-1340