Warn on missing parentheses in bitstring modifiers#11862
Warn on missing parentheses in bitstring modifiers#11862josevalim merged 4 commits intoelixir-lang:mainfrom
Conversation
sabiwara
changed the title
| purge(Sample) | ||
| end | ||
|
|
||
| test "custom bitstring modifier is being expanded to macro call" do |
There was a problem hiding this comment.
Can you please add the tests to kernel/binary_test.exs?
josevalim
left a comment
josevalim
left a comment
There was a problem hiding this comment.
Thank you! Can you also please warn if someone writes <<foo::binary()>> instead of <<foo::binary>>?
Note though that we can only merge this after v1.14 is out, because it is too early to introduce the warning. :)
Sure, will do!
Noted :) |
sabiwara
force-pushed
the
11811-warn-expand-bitstring-modifier
branch
from
b74fb35 to
9746f67
Compare
| io_lib:format("bitstring specifier \"~ts\" does not exist and is being expanded to \"~ts()\"," | ||
| " please use parentheses to remove the ambiguity", [Name, Name]); |
There was a problem hiding this comment.
I based this message on the following for consistency:
extra parentheses on a remote function capture &~ts.~ts()/0 have been deprecated. Please remove the parentheses: &~ts.~ts/0
| Size -> | ||
| <<Bits:Size, Bytes/binary>> = BitString, | ||
| {'<<>>', [], [{'::', [], [Bits, {size, [], [Size]}]}, {'::', [], [Bytes, {binary, [], []}]}]} | ||
| {'<<>>', [], [{'::', [], [Bits, {size, [], [Size]}]}, {'::', [], [Bytes, {binary, [], nil}]}]} |
There was a problem hiding this comment.
Without this change, @bitstring <<"foo", 16::4>> was generating a warning because expanded to binary().
Technically, this is a breaking change, but I think it is needed for consistency?
| end | ||
|
|
||
| defmacrop signed_16 do | ||
| defmacro signed_16 do |
There was a problem hiding this comment.
Note: With defmacrop, I wasn't able to access the custom modifiers in Code.eval_string even by passing __ENV__.
Please let me know if there is a better way.
josevalim
left a comment
josevalim
left a comment
There was a problem hiding this comment.
Looks great!
Waiting for the main branch to move to Elixir v1.15 before merging.
|
💚 💙 💜 💛 ❤️ |
2 participants
Closes #11811