fogfish/datum

Backport Pattern from release 3.x due matching causes error

KrzysiekJ opened this issue · 5 comments

The following example fails to compile:

-module(monad_test).

-compile({parse_transform, category}).

-export(
   [foo/0]).

foo() ->
   [m_identity ||
      {A, B} <- unit({100, 101}),
      unit(A)
   ].

The error is:

error in parse transform 'category': {function_clause,
                                      [{datum_cat_kleisli,'.',
                                        [m_identity,
                                         {monad,'_Vx56',
                                          {call,11,
                                           {remote,11,
                                            {atom,11,m_identity},
                                            {atom,11,'>>='}},
                                           [{call,11,
                                             {remote,11,
                                              {atom,11,m_identity},
                                              {atom,11,unit}},
                                             [{var,11,'A'}]},
                                            {'fun',11,
                                             {clauses,
                                              [{clause,11,
                                                [{var,11,'_Vx55'}],
                                                [],
                                                [{call,11,
                                                  {remote,11,
                                                   {atom,11,m_identity},
                                                   {atom,11,unit}},
                                                  [{var,11,'_Vx55'}]}]}]}}]}},
                                         {generate,10,
                                          {tuple,10,
                                           [{var,10,'A'},{var,10,'B'}]},
                                          {call,10,
                                           {remote,10,
                                            {atom,10,m_identity},
                                            {atom,10,unit}},
                                           [{tuple,10,
                                             [{integer,10,100},
                                              {integer,10,101}]}]}}],
                                        [{file,
                                          "src/category/datum_cat_kleisli.erl"},
                                         {line,21}]},
                                       {datum_cat,join,3,
                                        [{file,"src/category/datum_cat.erl"}, 
                                         {line,274}]},
                                       {datum_cat,category,3,
                                        [{file,"src/category/datum_cat.erl"},
                                         {line,71}]},
                                       {category,exprs,1,
                                        [{file,"src/category/category.erl"},
                                         {line,381}]},
                                       {category,clause,1,
                                        [{file,"src/category/category.erl"},
                                         {line,139}]},
                                       {category,clauses,1,
                                        [{file,"src/category/category.erl"},
                                         {line,130}]},
                                       {category,function,3,
                                        [{file,"src/category/category.erl"},
                                         {line,124}]},
                                       {category,form,1,
                                        [{file,"src/category/category.erl"},
                                         {line,81}]}]}

Tested on 4.3.3. If the line containing pattern matching is replaced with A <- unit({100, 101}), there is no longer a compilation error.

Indeed there is an example.

f() ->
   [m_identity ||
      [X, Y] <- unit([1, 2]),
      {A, B} <- unit({100, 101}),
      unit([X, Y, A, B])
   ].

the pattern matching feature has been supported by 3.x version of library via monad parse transform. However, version 4.x re-implements monads through kleisli category (category parse transform). The pattern matching is not supported yet at category parse transform. Essentially, this is a bug at documentation.

I was able to use the composition very well without a pattern match:

foo() ->
   [m_id ||
      unit({100, 101}),
      bar(_)
   ].

bar({A, _}) ->
   A.
foo() ->
   [m_id ||
      unit({100, 101}),
      lens:get(lens:t1(), _)
   ].

I just saw a few use-case where pattern matching is need. It is mainly inline functions. Personally, I would not advise to use composition for inline function due readability, maintainability.

If you are curious about internal then the problem is here:
https://github.com/fogfish/datum/blob/master/src/category/datum_cat_kleisli.erl#L26

Composition assumes only a single variable but {tuple,10,[{var,10,'A'},{var,10,'B'}]} is passed.

All-in-all, sorry for buggy doc, please check category.md it should be more up-to-date.

What is your vision about pattern match within composition? Do you need it, it is extremely vital for your experiments. I might consider to back port a feature from 3.x to 4.x.

In my case I want to use a maybe/option monad to avoid staircased indentation when parsing a binary with a non-trivial specification. Here is the relevant specification fragment:

  • string “Ercoin ” — 7 bytes.
  • address — 32 bytes.
  • length of the “locked until” field — 0 to 3, 1 byte. Use 0 unless the account is going to be locked.
  • locked until — 0 to 3 bytes, depending on the previous field.
  • intended validator address — 32 bytes, present if the “locked until” field is greater than 0.
  • signature of all the fields starting from “Ercoin ”.

Here is the most relevant code fragment (adjusted to not use pattern matching):

-spec payload_amount_to_score(binary(), non_neg_integer()) -> datum:option(score()).
payload_amount_to_score(
  <<"Ercoin "/utf8,
    Address:32/binary,
    LockedUntilLength,
    LockedUntil:LockedUntilLength/unit:8,
    Rest/binary>>=Payload,
  Amount)
  when LockedUntilLength =< 3 ->
    MaybeScoreMsgLength =
        case LockedUntil > 0 of
            true ->
                case Rest of
                    <<ValidatorAddress:32/binary, _/binary>> ->
                        unit(
                          {{Amount,
                            {Address, LockedUntil, ValidatorAddress}},
                           7 + 32 + 1 + LockedUntilLength + 32});
                    _ ->
                        fail("Validator address not present.")
                end;
            false ->
                unit(
                  {{Amount, {Address, none, none}},
                   7 + 32 + 1 + LockedUntilLength})
        end,
    [option ||
        ScoreMsgLength <- (fun () -> MaybeScoreMsgLength end)(),
        Score =< element(1, ScoreMsgLength),
        MsgLength =< element(2, ScoreMsgLength),
        Msg =< binary:part(Payload, 0, MsgLength),
        Sig =< binary:part(Payload, MsgLength, byte_size(Payload) - MsgLength),
        cats:require(
          ercoin_sig:verify_detached(Sig, Msg, Address),
          Score,
          "Invalid signature.")];
payload_amount_to_score(_, _) ->
    fail("Unrecognized format.").

-spec line_to_score(string()) -> datum:option(score()).
line_to_score(Line) ->
    [_, _, AmountStr, ScriptHex|_] = string:split(Line, " ", all),
    [option ||
        Payload <- payload_from_script_hex(ScriptHex),
        Amount =< amount_str_to_satoshis(AmountStr),
        payload_amount_to_score(Payload, Amount)].

The full commit is available (on a branch which is not merged yet).

Allowing pattern matching in this case would allow to remove 2 lines of code, allowing case expression in composition (refs #30) could also make it a little simpler. While MaybeScoreMsgLength could be created using a separate function, it doesn’t sound like a natural code division.

For now however I’m not advocating any specific changes, as I don’t have much experience in such style of programming.

Right, Thank you for clarification of your use-case. I'll study the ability to back-port pattern match feature from 3.x.

In your particular example, I would recommend to use Erlang's native pattern match as much as possible. I would re-write your packet parser in following manner. The usage of pattern match emphasis your rules into head of function, which is good for maintainability.

-spec payload_amount_to_score(binary(), non_neg_integer()) -> datum:option(score()).

payload_amount_to_score(
  <<"Ercoin "/utf8,
    Address:32/binary,
    LockedUntilLength,
    0:LockedUntilLength/unit:8,
    _/binary>>=Payload,
  Amount)
  when LockedUntilLength =< 3 ->
   payload_amount_to_score({Amount, {Address, none, none}}, 7 + 32 + 1 + LockedUntilLength, Address, Payload);

payload_amount_to_score(
  <<"Ercoin "/utf8,
    Address:32/binary,
    LockedUntilLength,
    LockedUntil:LockedUntilLength/unit:8,
    ValidatorAddress:32/binary,
    _/binary>>=Payload,
  Amount)
  when LockedUntilLength =< 3 ->
   payload_amount_to_score({Amount, {Address, LockedUntil, ValidatorAddress}}, 7 + 32 + 1 + LockedUntilLength + 32, Address, Payload);

payload_amount_to_score(_) ->
   fail("Unrecognized format.").


payload_amount_to_score(Score, MsgLength, Address, Payload) ->
    [option ||
        Msg =< binary:part(Payload, 0, MsgLength),
        Sig =< binary:part(Payload, MsgLength, byte_size(Payload) - MsgLength),
        cats:require(
          ercoin_sig:verify_detached(Sig, Msg, Address),
          Score,
          "Invalid signature.")].

Thank you. The name of the helper function would actually need to be like score_msglength_address_payload_to_maybe_score, which emphasizes that it would be an artificial division of code. Because of that and of some redundancy in the proposed pattern matching, I’ll stay with the current form of payload_amount_to_score for now.

via #70

Finally, I've managed to back port a pattern matching for do-notation. I still do not have a good proposal for identity and undefined category. Fill free to validate the feature against a master branch.