slepher / astranaut

traverse erlang ast and elixir macro in erlang.
MIT License
15 stars 0 forks source link

Contributing to syntax tools? #11

Open richcarl opened 3 years ago

richcarl commented 3 years ago

Just opening an issue here to have a little chat. You seem to have some interesting features here, and I'm wondering if you have at all considered submitting some PRs to make some of those features directly available as part of syntax_tools?

slepher commented 3 years ago

Sure. Which part of those futures do you need.

richcarl commented 3 years ago

I haven't had time to look closer, so I'm more interested in the reverse question: while working on this, which things did you think should definitely have been part of syntax tools to begin with? You could extract those one by one and turn them into PRs, if you have the time for it.

slepher commented 3 years ago

Theses are mainly four step features of this application

These features make code look cleaner.

Other features are

I will make a plan

slepher commented 3 years ago

Here is my plan:

maxno-kivra commented 3 years ago

Hi, I'm a bit late to the party but want to chime in. As it happens I've also written a parse transform library, but with a different design. I thought we could compare notes and see which parts best fit in OTP.

Mine builds on merl, hence the name merlin, but like yours it uses a phase parameter for entering/exiting nodes. I used node instead of leaf, but I got so inspired I actually change it to leaf as well 😄 .

I did some cleanup and pushed what I got, but it might still have a few rough edges. I've also written a couple of everyday use transforms using it to get a feel for what you need in the real world™️ . As far as I can see you use attributes and magical functions, where as I use macros. I do that so that the reader knows something is happening compile time, even though it does way more then a vanilla. macro can.

Speaking of which, we both implemented hygienic/procedural variants. But again, I prefer to start as a vanilla macro while you use plain old function calls. Here's eval:

%% The repetition of the macro name is for error reporting purposes
%% since there's no other way to figure out the calling macros name
-define(eval(Expression), ?procedural(eval(Expression),
    erl_parse:abstract(Expression, ?LINE)
)).

%% If I understood it correctly this is somewhat equivalent
-use_macro({eval/1, []}).

eval(Term) ->
    quote(Term).

This became a wall of text, but I hope it's ok. Love to hear your feedback 😃


Edit: My biggest worry is that introducing monads into OTP might be a bit much for the average developer. On the other hand it has a proven track record in other functional languages. Also in Rust, the Result type comes to mind, and JavaScripts Promises.

slepher commented 3 years ago

Edit: My biggest worry is that introducing monads into OTP might be a bit much for the average developer. On the other hand it has a proven track record in other functional languages. Also in Rust, the Result type comes to mind, and JavaScripts Promises.

monads it's hidden by this function: astranaut_traverse:mapfold/4 people could use it without knowning anything of monad. I just wrote it for myself, because some transforms is really complicated: rebinding for example.

maxno-kivra commented 3 years ago

monads it's hidden by this function: astranaut_traverse:mapfold/4

I read the docs from the README, and this seems mostly true. Except for some of the return types, although I assume those could be removed.

 traverse_fun_return(SA) :: SA | {error, error()} | {error, SA, error()} | 
                           {warning, SA, error()} | {warning, error()} |
                           continue | {continue, SA} |
                           astranaut_walk_return:astranaut_walk_return(A) |
                           astranaut_traverse_m:astranaut_traverse_m(S, A) |
                           astranaut_return_m:astranaut_return_m(A) |
                           astranaut_base_m:astranaut_base_m(A).

SA is same return type in traverse_fun(), but A is always node(), and S is always state().

If you look into astranaut_traverse it tells a different story. I noticed this line:

https://github.com/slepher/astranaut/blob/2fb9ec8ac0185e74cee1b86872f860b5c84adf0d/src/traverse/astranaut_traverse.erl#L218

Which indicates that you could Bring Your Own Monad, but looking at the traverse_opts() type, it is not mentioned:

https://github.com/slepher/astranaut/blob/2fb9ec8ac0185e74cee1b86872f860b5c84adf0d/src/traverse/astranaut_traverse.erl#L28-L35

Is it just some leftover code, or can you actually BYOM?

people could use it without knowning anything of monad.

But it's not just about the end users, it's also (maybe more so) about the maintainers. Does the OTP maintainers know monads? Are they comfortable writing monadic code?

This is especially true when using the do parse transform. I'd say it's pretty much an requirement for using monads, compare:

[ maybe ||
  User <- users:get(123),
  #{ id := UserId } = User,
  Comments <- comments:get(#{ user_id => UserId),
  {User, Comments}
]

with

maybe:'>>='(users:get(123), fun(User) ->
    #{ id := UserId } = User,
    maybe:'>>='(comments:get(#{ user_id => UserId), fun(Comments) ->
        {User, Comments}
    end)
end)

But the do transform changes the semantics of list comprehensions, which makes it really confusing to read for someone new/doesn't know about the transform.

I did also noticed you haven't been using the do transform in all files. Am I correct in assuming it's because the do transform depends on these modules, and you can't use it? Or do you have a different reason?

I just wrote it for myself, because some transforms is really complicated: rebinding for example.

This I totally understand. Both that the code can get quite gnarly, and that you write code you yourself understand/feel comfortable with. When Write merlin I had a lot of trouble getting "returning multiple nodes" to work. I ended up copying erl_syntax_lib:mapfold_subtrees/3 and tweaking it to flatten the resulting list of nodes.

I just finished my own rebinder. Much simpler then yours, and mine probably have some subtle bugs. But it might be an interesting comparison. You use an attribute, I require special variable naming (e.g. Foo@ -> Foo0 etc), take a look at the example if you wish.


The point I'm trying to make is that code written for a large, long term, project must be optimized for maintainability. Speed is also something you must think about, but in the end there will be bugs and someone has to take care of them. Thus it's best if as many developers as possible can chip in. Erlang just isn't ready for monads IMO. If you look at the discussion about @richcarl addition of the pinning operator, you can get a sense how much resistance there seems to be in the Erlang community.

I don't argue that we shouldn't try to introduce new concepts. On the contrary, if there's anything constant in our line of work is the constant change. But they need to be introduced one small step at a time. Perhaps in the future Erlang gets first class support for monads, but right now I'm not so sure.

slepher commented 3 years ago

This is a simple reply from phone,I will modify it later.

about moand,You are right, Im trying to spilt it with two level, one with monad and one without. ps:code is modified here, slepher/otp branch syntax_tools/traverse, sorry for the commit nums,I have meet some doc issue.

maxno-kivra commented 3 years ago

Looking forward to see what you come up with. And again, I hope we can join forces to get the best in OTP. Have you looked at merlin?

slepher commented 3 years ago

Looking forward to see what you come up with. And again, I hope we can join forces to get the best in OTP. Have you looked at merlin?

I remove this attributes because it's already processed in map_m, no need to do the duplicated thing, It's more proper to move it to map_m function

https://github.com/slepher/astranaut/blob/2fb9ec8ac0185e74cee1b86872f860b5c84adf0d/src/traverse/astranaut_traverse.erl#L218

and monad and monad_class is depricated, I forget to remove it.

astranaut_traverse_m work the same way as fun(A0, State0) -> {A, State1} end. the detail is which State you choose.

So I have seen no base difference between your traverse and my traverse. Some Extra works is for:

for macro, because parse_transform could do everything I prefer this way:

for rebinder, variables in case and variables in functions works different I have not seen any special treat in your code. you could see my test cases https://github.com/slepher/astranaut/blob/master/test/rebinding/astranaut_rebinding_test.erl and see what https://github.com/slepher/astranaut/blob/master/src/rebinding/astranaut_rebinding_scope.erl is written for.