Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Contributing to syntax tools? #11

Open
richcarl opened this issue Jan 20, 2021 · 10 comments
Open

Contributing to syntax tools? #11

richcarl opened this issue Jan 20, 2021 · 10 comments

Comments

@richcarl
Copy link

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
Copy link
Owner

slepher commented Jan 20, 2021

Sure.
Which part of those futures do you need.

@richcarl
Copy link
Author

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
Copy link
Owner

slepher commented Jan 20, 2021

Theses are mainly four step features of this application

  • Monad related modules (from erlando, but erlando deps on this application, so it's a minimal implementation)
  • AST traverse function
  • Quote: Elixir style quote
  • Marcro: Elixir style macro

These features make code look cleaner.

  • Compile Meta: get compiled data like errors, warnings, forms at runtime, developed for test this application.
  • Do: do comprehension syntax from erlando

Other features are

  • Rebinding: Elixir style variable binding.
  • Struct: Elixir style Struct
  • Disable TCO: disable Tail Call Optimization to get more information in exceptions.

I will make a plan

@slepher
Copy link
Owner

slepher commented Jan 21, 2021

Here is my plan:

  • commit ast_traverse, quote, macro with one or three pr
  • commit rebinding and struct with two pr.
  • complete edocs
  • I'm not a native english speaker, some of function and options name maybe improperly, which should be renamed
  • for not change the old codes, rename module begins with astranaut_ to erl_syntax_

@maxno-kivra
Copy link

maxno-kivra commented Jan 26, 2021

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
Copy link
Owner

slepher commented Jan 27, 2021

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
Copy link

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:

Attr = maps:without([traverse, parse_transform, monad, monad_class, formatter], Opts),

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

-type traverse_opts() :: #{traverse => traverse_style(), parse_transform => boolean(),
simplify_return => boolean(), parent => atom(),
match_right_first => boolean(),
children => boolean(),
sequence_children => fun((any()) -> any()),
transform => fun((any()) -> any()),
syntax_lib => module(),
node => node_type(), formatter => module() }.

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
Copy link
Owner

slepher commented Jan 30, 2021

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
Copy link

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
Copy link
Owner

slepher commented Feb 1, 2021

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

Attr = maps:without([traverse, parse_transform, monad, monad_class, formatter], Opts),

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:

  • convert between normal return and monad return
  • erl_syntax:subtrees returns syntaxTree(), not erl_parse(), I have to convert it back, especially for -type and -spec attributes
  • user defined subtree traverse order
  • three levels errors to treat: error, formatted_error, formatted_error_with_file.

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

  • design a legal erl_syntax format
  • use -attribute to provide options
  • transform erl_syntax format to your want.

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants