[erlang-patches] [erlang-bugs] syntax_tools anonymous function error

Discussion:

unknown

2013-05-19 10:33:12 UTC

Hello Michael,

This patch fixes support of implicit funs with variables in igor.

git fetch https://github.com/nox/otp.git igor-funs

https://github.com/nox/otp/compare/erlang:maint...igor-funs
https://github.com/nox/otp/compare/erlang:maint...igor-funs.patch

Regards,
--
Anthony Ramine

Hi,
in function erl_syntax_lib:analyze_function_name/1 (erl_syntax_lib.erl, line 1500)
in call from igor:transform_implicit_fun/3 (igor.erl, line 1807)
in call from igor:transform_list/3 (igor.erl, line 1748)
in call from igor:transform_1/3 (igor.erl, line 1741)
in call from igor:default_transform/3 (igor.erl, line 1733)
in call from igor:transform_list/3 (igor.erl, line 1748)
in call from igor:transform_1/3 (igor.erl, line 1741)
in call from igor:transform_1/3 (igor.erl, line 1742)
Thanks,
Michael
_______________________________________________
erlang-bugs mailing list
erlang-bugs
http://erlang.org/mailman/listinfo/erlang-bugs

unknown

2013-05-20 08:12:31 UTC

Permalink

Post by unknown
Hello Michael,
This patch fixes support of implicit funs with variables in igor.
git fetch https://github.com/nox/otp.git igor-funs
https://github.com/nox/otp/compare/erlang:maint...igor-funs
https://github.com/nox/otp/compare/erlang:maint...igor-funs.patch
Regards,

Hello Anthony,
I've fetched your patch and it should be visible in the 'pu' branch shortly.
Thanks,

--
BR Fredrik Gustafsson
Erlang OTP Team

unknown

2013-10-01 11:27:17 UTC

Permalink

Hello,

I see on the development page that an action is required from me for this patch [1].

And what exactly is required from me..?

Regards,

[1] http://www.erlang.org/development/

Post by unknown

Hello Anthony,
I've fetched your patch and it should be visible in the 'pu' branch shortly.
Thanks,
--
BR Fredrik Gustafsson
Erlang OTP Team

--
Anthony Ramine

unknown

2013-10-01 11:58:00 UTC

Permalink

Hi
I was awaiting an answer to this:
@richcarl <https://github.com/richcarl>Should I amend this commit?
And the possible amend to the commit.

If that is not going to happen I can put it through the test-merge precedure

/Henrik

Post by unknown
Hello,
I see on the development page that an action is required from me for this patch [1].
And what exactly is required from me..?
Regards,
[1] http://www.erlang.org/development/

Post by unknown

Hello Anthony,
I've fetched your patch and it should be visible in the 'pu' branch shortly.
Thanks,
--
BR Fredrik Gustafsson
Erlang OTP Team

--
/Henrik Nord Erlang/OTP

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://erlang.org/pipermail/erlang-patches/attachments/20131001/540ad180/attachment.html>

unknown

2013-10-01 12:25:38 UTC

Permalink

Sorry, drowning in stuff as usual. Anthony, I trust that your analysis
of why that clause could be deleted, so that should be ok. But I thought
there was something weird about the format changes, and now I think I
see what it is: calls like arity_qualifier_body(Name) should never
return a naked atom or integer - always a syntax tree. So the fix for
the revert "fun F/A" case should be done in implicit_fun_name/1 instead,
just as it's handled for "fun M:F/A". And in the reverting of "fun
M:F/A", you shouldn't have to change anything at all, since it's already
handled.

/Richard

Post by unknown
Hi
@richcarl <https://github.com/richcarl>Should I amend this commit?
And the possible amend to the commit.
If that is not going to happen I can put it through the test-merge precedure
/Henrik

Post by unknown
Hello,
I see on the development page that an action is required from me for this patch [1].
And what exactly is required from me..?
Regards,
[1]http://www.erlang.org/development/

Post by unknown

Post by unknown
Hello Michael,
This patch fixes support of implicit funs with variables in igor.
git fetchhttps://github.com/nox/otp.git igor-funs
https://github.com/nox/otp/compare/erlang:maint...igor-funs
https://github.com/nox/otp/compare/erlang:maint...igor-funs.patch
Regards,

Hello Anthony,
I've fetched your patch and it should be visible in the 'pu' branch shortly.
Thanks,
--
BR Fredrik Gustafsson
Erlang OTP Team

--
/Henrik Nord Erlang/OTP
_______________________________________________
erlang-patches mailing list
erlang-patches
http://erlang.org/mailman/listinfo/erlang-patches

unknown

2013-10-01 12:30:07 UTC

Permalink

Thanks for the reply, will amend this tonight.

Sorry, drowning in stuff as usual. Anthony, I trust that your analysis of why that clause could be deleted, so that should be ok. But I thought there was something weird about the format changes, and now I think I see what it is: calls like arity_qualifier_body(Name) should never return a naked atom or integer - always a syntax tree. So the fix for the revert "fun F/A" case should be done in implicit_fun_name/1 instead, just as it's handled for "fun M:F/A". And in the reverting of "fun M:F/A", you shouldn't have to change anything at all, since it's already handled.
/Richard

Post by unknown
Hello,
I see on the development page that an action is required from me for this patch [1].
And what exactly is required from me..?
Regards,
[1]http://www.erlang.org/development/

Post by unknown

Post by unknown
Hello Michael,
This patch fixes support of implicit funs with variables in igor.
git fetchhttps://github.com/nox/otp.git igor-funs
https://github.com/nox/otp/compare/erlang:maint...igor-funs
https://github.com/nox/otp/compare/erlang:maint...igor-funs.patch
Regards,

Hello Anthony,
I've fetched your patch and it should be visible in the 'pu' branch shortly.
Thanks,
--
BR Fredrik Gustafsson
Erlang OTP Team

--
/Henrik Nord Erlang/OTP
_______________________________________________
erlang-patches mailing list
erlang-patches
http://erlang.org/mailman/listinfo/erlang-patches

--
Anthony Ramine

unknown

2013-10-02 08:55:26 UTC

Permalink

I guess we can merge it now then.

Maybe I got confused - was the format change in the other direction, i.e., used to be atoms but is now trees? In that case, I guess you're right. Just make sure that the code does the right thing in all combinations of cases, and you're done. :-)
/Richard

In fact I really don't see what you mean. The problem is revert_implicit_fun/1 calling concrete/1 on the individual parts when it shouldn't.
I don't see how changing another function would fix anything.

Post by unknown
Thanks for the reply, will amend this tonight.

Sorry, drowning in stuff as usual. Anthony, I trust that your analysis of why that clause could be deleted, so that should be ok. But I thought there was something weird about the format changes, and now I think I see what it is: calls like arity_qualifier_body(Name) should never return a naked atom or integer - always a syntax tree. So the fix for the revert "fun F/A" case should be done in implicit_fun_name/1 instead, just as it's handled for "fun M:F/A". And in the reverting of "fun M:F/A", you shouldn't have to change anything at all, since it's already handled.
/Richard

Post by unknown
Hello,
I see on the development page that an action is required from me for this patch [1].
And what exactly is required from me..?
Regards,
[1]http://www.erlang.org/development/

Post by unknown

Post by unknown
Hello Michael,
This patch fixes support of implicit funs with variables in igor.
git fetchhttps://github.com/nox/otp.git igor-funs
https://github.com/nox/otp/compare/erlang:maint...igor-funs
https://github.com/nox/otp/compare/erlang:maint...igor-funs.patch
Regards,

Hello Anthony,
I've fetched your patch and it should be visible in the 'pu' branch shortly.
Thanks,
--
BR Fredrik Gustafsson
Erlang OTP Team

--
/Henrik Nord Erlang/OTP
_______________________________________________
erlang-patches mailing list
erlang-patches
http://erlang.org/mailman/listinfo/erlang-patches

--
Anthony Ramine

unknown

2013-10-03 12:30:35 UTC

Permalink

I will move it into the que
Thank you for your contribution!

Post by unknown
I guess we can merge it now then.

Post by unknown
Thanks for the reply, will amend this tonight.

Sorry, drowning in stuff as usual. Anthony, I trust that your analysis of why that clause could be deleted, so that should be ok. But I thought there was something weird about the format changes, and now I think I see what it is: calls like arity_qualifier_body(Name) should never return a naked atom or integer - always a syntax tree. So the fix for the revert "fun F/A" case should be done in implicit_fun_name/1 instead, just as it's handled for "fun M:F/A". And in the reverting of "fun M:F/A", you shouldn't have to change anything at all, since it's already handled.
/Richard

Post by unknown
Hello,
I see on the development page that an action is required from me for this patch [1].
And what exactly is required from me..?
Regards,
[1]http://www.erlang.org/development/

Post by unknown

Post by unknown
Hello Michael,
This patch fixes support of implicit funs with variables in igor.
git fetchhttps://github.com/nox/otp.git igor-funs
https://github.com/nox/otp/compare/erlang:maint...igor-funs
https://github.com/nox/otp/compare/erlang:maint...igor-funs.patch
Regards,

Hello Anthony,
I've fetched your patch and it should be visible in the 'pu' branch shortly.
Thanks,
--
BR Fredrik Gustafsson
Erlang OTP Team

--
/Henrik Nord Erlang/OTP
_______________________________________________
erlang-patches mailing list
erlang-patches
http://erlang.org/mailman/listinfo/erlang-patches

--
Anthony Ramine

--
/Henrik Nord Erlang/OTP

unknown

2013-11-01 11:29:14 UTC

Permalink

Ping? This is still ? Status: Author ? in the Development page.

--
Anthony Ramine

Post by unknown
I will move it into the que
Thank you for your contribution!

unknown

2013-11-11 14:42:37 UTC

Permalink

Its moving forward as we speak.

Thank you for your contribution!

Post by unknown
Ping? This is still ? Status: Author ? in the Development page.

--
/Henrik Nord Erlang/OTP

unknown

2013-12-12 16:35:53 UTC

Permalink

This patch breaks reverting of local implicit funs in R16B03.

We have two ways of fixing this:

- Making erl_syntax reverts local implicit funs? parts as concrete values.
- Change the AST format in a backwards-incompatible way and use abstract values for local implicit funs too.

My vote is on fixing erl_syntax right now for R16, and change the AST in R17 as we will have maps and named funs anyway in there.

Sorry for the inconvenience.

Richard, maybe we should write tests for syntax_tools?
--
Anthony Ramine

Post by unknown
Its moving forward as we speak.
Thank you for your contribution!

Post by unknown
Ping? This is still ? Status: Author ? in the Development page.

--
/Henrik Nord Erlang/OTP

unknown

2013-12-12 23:22:03 UTC

Permalink

Hello again,

I?ve pushed the fix. It is just a partial revert of the previous commit on erl_syntax. Such a thing would have been avoided by basic tests. Or just double checking the documentation about the AST, I?m to blame for that.

Should the AST be made consistent between local and remote implicit funs in R17A? I can write a patch for that. With proper tests and documentation.

git fetch https://github.com/nox/otp.git fix-OTP-11506

https://github.com/nox/otp/compare/erlang:maint...fix-OTP-11506
https://github.com/nox/otp/compare/erlang:maint...fix-OTP-11506.patch

Regards,
--
Anthony Ramine

Post by unknown
This patch breaks reverting of local implicit funs in R16B03.
- Making erl_syntax reverts local implicit funs? parts as concrete values.
- Change the AST format in a backwards-incompatible way and use abstract values for local implicit funs too.
My vote is on fixing erl_syntax right now for R16, and change the AST in R17 as we will have maps and named funs anyway in there.
Sorry for the inconvenience.
Richard, maybe we should write tests for syntax_tools?
--
Anthony Ramine

Post by unknown
Its moving forward as we speak.
Thank you for your contribution!

Post by unknown
Ping? This is still ? Status: Author ? in the Development page.

--
/Henrik Nord Erlang/OTP

unknown

2013-12-13 08:52:26 UTC

Permalink

The patch, igor funs, was merged into maint 2013-11-29

https://github.com/erlang/otp/commit/40d21f3f803a336b3d3edf338ec71a67ea1f09b1

Also, Would you mind pressing the "pull request" button on github for
your patches?

We are still working on how to solve this in a good way, until then
patches, discussions and reviews in email, code on github?

And making it a pull request from the getgo ensures that it will go into
our patch monitoring system saves us some manual labor

Thanks!
/Henrik

Post by unknown
Hello again,
I?ve pushed the fix. It is just a partial revert of the previous commit on erl_syntax. Such a thing would have been avoided by basic tests. Or just double checking the documentation about the AST, I?m to blame for that.
Should the AST be made consistent between local and remote implicit funs in R17A? I can write a patch for that. With proper tests and documentation.
git fetch https://github.com/nox/otp.git fix-OTP-11506
https://github.com/nox/otp/compare/erlang:maint...fix-OTP-11506
https://github.com/nox/otp/compare/erlang:maint...fix-OTP-11506.patch
Regards,

--
/Henrik Nord Erlang/OTP

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://erlang.org/pipermail/erlang-patches/attachments/20131213/2f5866fb/attachment.html>

unknown

2013-12-13 10:08:51 UTC

Permalink

I can reluctantly make a PR, but don?t expect me to notice when comments are posted, or to comment myself there. I prefer my mailbox which I can actually archive.

--
Anthony Ramine

Also, Would you mind pressing the "pull request" button on github for your patches?

unknown

2013-12-13 09:05:48 UTC

Permalink

Hi guys,

Since this code is out in the wild, and seems to cause some problems
(I noticed the problem when trying to work with Chicago Boss with
R16B03 and brought it to Anthony's attention), can you recommend a
good way to work around it until B04 comes out?

Thank you,

--
David N. Welton

http://www.welton.it/davidw/

http://www.dedasys.com/

unknown

2013-12-13 09:16:55 UTC

Permalink

Next release is R17pre sometime in Jan
So, If this fix that Anthony sent :

git fetchhttps://github.com/nox/otp.git fix-OTP-11506

https://github.com/nox/otp/compare/erlang:maint...fix-OTP-11506
https://github.com/nox/otp/compare/erlang:maint...fix-OTP-11506.patch

Fixes the problem, you might have to apply that yourself until we merge
it into master.

Post by unknown
Hi guys,
Since this code is out in the wild, and seems to cause some problems
(I noticed the problem when trying to work with Chicago Boss with
R16B03 and brought it to Anthony's attention), can you recommend a
good way to work around it until B04 comes out?
Thank you,

--
/Henrik Nord Erlang/OTP

unknown

2013-12-13 10:09:25 UTC

Permalink

This means any project relying on erl_syntax won?t work with R16B03. In my opinion we need an R16B03-1.

--
Anthony Ramine

Fixes the problem, you might have to apply that yourself until we merge it into master.

unknown

2013-12-13 12:31:37 UTC

Permalink

This potentially breaks pretty much any project that manipulates AST,
including many parse_transforms, other BEAM languages targetting the
Erlang AST, tools that generate code, and of course everything that
requires one of the above to work. This sounds serious enough to do a
hotfix.

Also sounds like test suites are really missing there, something should
be done about it to make sure it doesn't happen again.

Post by unknown
This means any project relying on erl_syntax won?t work with R16B03. In my opinion we need an R16B03-1.

--
Lo?c Hoguin
http://ninenines.eu

unknown

2013-12-14 01:08:18 UTC

Permalink

Hello,

I?ve added a commit which smoke tests erl_syntax:revert/1.

Henrik, please refetch.

Regards,
--
Anthony Ramine

This potentially breaks pretty much any project that manipulates AST, including many parse_transforms, other BEAM languages targetting the Erlang AST, tools that generate code, and of course everything that requires one of the above to work. This sounds serious enough to do a hotfix.
Also sounds like test suites are really missing there, something should be done about it to make sure it doesn't happen again.

Post by unknown
This means any project relying on erl_syntax won?t work with R16B03. In my opinion we need an R16B03-1.

--
Lo?c Hoguin
http://ninenines.eu

unknown

2013-12-18 11:05:52 UTC

Permalink

Hi,

Post by unknown
Next release is R17pre sometime in Jan
git fetchhttps://github.com/nox/otp.git fix-OTP-11506
https://github.com/nox/otp/compare/erlang:maint...fix-OTP-11506
https://github.com/nox/otp/compare/erlang:maint...fix-OTP-11506.patch
Fixes the problem, you might have to apply that yourself until we merge it
into master.

Applying a patch does not do much for code like Chicago Boss that
people will download and try with "the latest Erlang".

What is necessary is some code that will do

case erlang:system_info(otp_release) of
"R16B03" ->
... mangle the AST in such a way to not make the parse transform barf...;
_ -> ok
end.

I've been looking at how to do this, but I'm afraid my knowledge of
this part of Erlang is not quite up to the task so far.

Thank you,

--
David N. Welton

http://www.welton.it/davidw/

http://www.dedasys.com/

unknown

2013-12-18 11:12:51 UTC

Permalink

Post by unknown
What is necessary is some code that will do
case erlang:system_info(otp_release) of
"R16B03" ->
... mangle the AST in such a way to not make the parse transform barf...;
_ -> ok
end.

Don't do that, this doesn't mean that the syntax_tools application will
actually be the one that shipped with R16B03. Check the application
version instead: application:get_key(syntax_tools, vsn). (Note that this
is only available after the application is loaded.)

--
Lo?c Hoguin
http://ninenines.eu

unknown

2013-12-18 11:28:20 UTC

Permalink

Post by unknown
Don't do that,

Looking at version numbers like this is not something I'm in the habit
of doing often...

Post by unknown
this doesn't mean that the syntax_tools application will
actually be the one that shipped with R16B03. Check the application version
instead: application:get_key(syntax_tools, vsn). (Note that this is only
available after the application is loaded.)

I don't know how those numbers are managed, but the vsn.mk file in
master still has SYNTAX_TOOLS_VSN = 1.6.12, which is the same as I get
from my Erlang shell in R16B03; presumably, since the code in master
has been patched, the version number should get bumped?

--
David N. Welton

http://www.welton.it/davidw/

http://www.dedasys.com/

unknown

2013-12-19 09:22:04 UTC

Permalink

Post by unknown

Post by unknown
Don't do that,

Looking at version numbers like this is not something I'm in the habit
of doing often...

Problem is that we usually bump the version only when we do a release,
(includes major, minor and patch releases)

/Henrik Nord Erlang/OTP

unknown

2013-12-19 10:43:52 UTC

Permalink

Hi,

Post by unknown

Post by unknown
I don't know how those numbers are managed, but the vsn.mk file in
master still has SYNTAX_TOOLS_VSN = 1.6.12, which is the same as I get
from my Erlang shell in R16B03; presumably, since the code in master
has been patched, the version number should get bumped?

Problem is that we usually bump the version only when we do a release,
(includes major, minor and patch releases)

The functionality changed, in a way that's not backwards compatible,
so by my understanding of something like http://semver.org/ it'd be a
good idea to change the major number of the syntax_tools application.

--
David N. Welton

http://www.welton.it/davidw/

http://www.dedasys.com/

unknown

2013-12-19 10:55:46 UTC

Permalink

Post by unknown
Hi,

Post by unknown

Problem is that we usually bump the version only when we do a release,
(includes major, minor and patch releases)

That only applies to releases. If there's no release there's no need to
bump anything.

--
Lo?c Hoguin
http://ninenines.eu

unknown

2013-12-19 10:36:24 UTC

Permalink

Hi

We will not merge this patch just before Christmas and release a R16B03-1.

Our suggestion is that:
1. If you are affected by this bug, apply the patch yourself until it
will be merged into Erlang/OTP. (probably 17rc1 late January)
2. If you are using chicagoboss, stay on R16B02 if possible.
3. compile "lib/syntax_tools/src/erl_syntax.erl" and put the beam up for
download on chicagoboss.org for R16B03 users.
4. add the patch to the rebar deps of chicagoboss.

We will try to make sure things like this do not sneak into releases in
the future, and apologise for the inconvenience caused.

--
/Henrik Nord Erlang/OTP

unknown

2013-12-19 10:47:13 UTC

Permalink

Hello,

With all due respect, that is completely irresponsible.

Regards,
--
Anthony Ramine

Post by unknown
Hi
We will not merge this patch just before Christmas and release a R16B03-1.
1. If you are affected by this bug, apply the patch yourself until it will be merged into Erlang/OTP. (probably 17rc1 late January)
2. If you are using chicagoboss, stay on R16B02 if possible.
3. compile "lib/syntax_tools/src/erl_syntax.erl" and put the beam up for download on chicagoboss.org for R16B03 users.
4. add the patch to the rebar deps of chicagoboss.
We will try to make sure things like this do not sneak into releases in the future, and apologise for the inconvenience caused.
--
/Henrik Nord Erlang/OTP
_______________________________________________
erlang-patches mailing list
erlang-patches
http://erlang.org/mailman/listinfo/erlang-patches

unknown

2013-12-19 10:56:52 UTC

Permalink

Hi,

Post by unknown
We will not merge this patch just before Christmas and release a R16B03-1.

Ok.

Post by unknown
1. If you are affected by this bug, apply the patch yourself until it will
be merged into Erlang/OTP. (probably 17rc1 late January)

That's not really a workable solution for most people using CB. Zach
can correct me if I'm wrong, but I think the idea is going to simply
put a warning in CB so that people trying to use it with a R16B03 -
pretty much anyone who gets the "latest and greatest" from, say,
Erlang Solutions .deb repository - will get a user friendly message
saying that that release of Erlang is incompatible with Chicago Boss,
so please downgrade.

Post by unknown
2. If you are using chicagoboss, stay on R16B02 if possible.

That probably works now that people have been warned.

Post by unknown
3. compile "lib/syntax_tools/src/erl_syntax.erl" and put the beam up for
download on chicagoboss.org for R16B03 users.

I am not an Erlang power user like you guys, but is it possible to
include that code in CB itself, and conditionally load it, overriding
the system version of that file, in the case of R16B03?

Post by unknown
4. add the patch to the rebar deps of chicagoboss.

I think something like this could work too, but i don't know the
details of how it would work out.

Post by unknown
We will try to make sure things like this do not sneak into releases in the
future, and apologise for the inconvenience caused.

--
David N. Welton

http://www.welton.it/davidw/

http://www.dedasys.com/

unknown

2013-12-19 12:56:15 UTC

Permalink

I have pushed a workaround for parse_trans.

https://github.com/uwiger/parse_trans/blob/master/doc/parse_trans.md#revert-1

parse_trans:revert/1 and parse_trans:revert_form/1 should be usable as drop-in
replacements to erl_syntax:revert(). The workaround verifies that a transform
is actually needed by generating a dummy module, reverting it and feeding it
to the linter; if the linter crashes, the abstract code needs to be fixed.

BR,
Ulf W

Post by unknown
Hi,

Post by unknown
We will not merge this patch just before Christmas and release a R16B03-1.

Ok.

Post by unknown
1. If you are affected by this bug, apply the patch yourself until it will
be merged into Erlang/OTP. (probably 17rc1 late January)

Post by unknown
2. If you are using chicagoboss, stay on R16B02 if possible.

That probably works now that people have been warned.

Post by unknown
3. compile "lib/syntax_tools/src/erl_syntax.erl" and put the beam up for
download on chicagoboss.org for R16B03 users.

I am not an Erlang power user like you guys, but is it possible to
include that code in CB itself, and conditionally load it, overriding
the system version of that file, in the case of R16B03?

Post by unknown
4. add the patch to the rebar deps of chicagoboss.

I think something like this could work too, but i don't know the
details of how it would work out.

Post by unknown
We will try to make sure things like this do not sneak into releases in the
future, and apologise for the inconvenience caused.

I'm just a Chicago Boss user trying to figure out a solution that
improves things for everyone. I realize that parse_transforms are
kind of a 'dark corner' of Erlang, so I appreciate the help.
Thank you
--
David N. Welton
http://www.welton.it/davidw/
http://www.dedasys.com/
_______________________________________________
erlang-patches mailing list
erlang-patches
http://erlang.org/mailman/listinfo/erlang-patches

Ulf Wiger, Co-founder & Developer Advocate, Feuerlabs Inc.
http://feuerlabs.com