uwiger / parse_trans

Parse transform utilities for Erlang
Apache License 2.0
230 stars 118 forks source link

Including ct_expand parse_transform breaks dialyzer in some cases #28

Closed djnym closed 4 years ago

djnym commented 6 years ago

Here's the minimal breaking code

-module(wct).
-compile([ {parse_transform, ct_expand} ]).
-record (rec, {cnt = #{} :: #{ none | {one, binary()} => non_neg_integer()}}).
-export ([b/0]).
b() -> #rec{}.

When I compile

% erlc +debug_info -pa _build/default/lib/parse_trans/ebin/ wct.erl

Then dialyze

% dialyzer -c wct.beam
  Checking whether the PLT /home/molinaro/.dialyzer_plt is up-to-date... yes
  Proceeding with analysis...
=ERROR REPORT==== 6-Jun-2018::08:13:41 ===
Error in process <0.64.0> with exit value:
{{case_clause,map_type_assoc},
 [{erl_types,'-from_form/5-PairsFromForm/3-0-',5,
             [{file,"erl_types.erl"},{line,4680}]},
  {erl_types,from_form,5,[{file,"erl_types.erl"},{line,4673}]},
  {erl_types,t_from_form2,5,[{file,"erl_types.erl"},{line,4542}]},
  {erl_types,t_from_form_check_remote,4,[{file,"erl_types.erl"},{line,4516}]},
  {dialyzer_utils,'-process_record_remote_types/1-fun-0-',7,
                  [{file,"dialyzer_utils.erl"},{line,333}]},
  {lists,mapfoldl,3,[{file,"lists.erl"},{line,1354}]},
  {dialyzer_utils,'-process_record_remote_types/1-fun-1-',7,
                  [{file,"dialyzer_utils.erl"},{line,332}]},
  {lists,mapfoldl,3,[{file,"lists.erl"},{line,1354}]}]}

dialyzer: Analysis failed with error:
{function_clause,
    [{dialyzer_codeserver,get_callbacks,
         [{{case_clause,map_type_assoc},
           [{erl_types,'-from_form/5-PairsFromForm/3-0-',5,
                [{file,"erl_types.erl"},{line,4680}]},
            {erl_types,from_form,5,[{file,"erl_types.erl"},{line,4673}]},
            {erl_types,t_from_form2,5,[{file,"erl_types.erl"},{line,4542}]},
            {erl_types,t_from_form_check_remote,4,
                [{file,"erl_types.erl"},{line,4516}]},
            {dialyzer_utils,'-process_record_remote_types/1-fun-0-',7,
                [{file,"dialyzer_utils.erl"},{line,333}]},
            {lists,mapfoldl,3,[{file,"lists.erl"},{line,1354}]},
            {dialyzer_utils,'-process_record_remote_types/1-fun-1-',7,
                [{file,"dialyzer_utils.erl"},{line,332}]},
            {lists,mapfoldl,3,[{file,"lists.erl"},{line,1354}]}]}],
         [{file,"dialyzer_codeserver.erl"},{line,368}]},
     {dialyzer_plt,insert_callbacks,2,[{file,"dialyzer_plt.erl"},{line,149}]},
     {dialyzer_analysis_callgraph,analysis_start,3,
         [{file,"dialyzer_analysis_callgraph.erl"},{line,143}]}]}
Last messages in the log cache:
  Reading files and computing callgraph... done in 0.07 secs
  Removing edges... done in 0.00 secs

=ERROR REPORT==== 6-Jun-2018::08:13:41 ===
Error in process <0.60.0> with exit value:
{function_clause,
    [{dialyzer_codeserver,get_callbacks,
         [{{case_clause,map_type_assoc},
           [{erl_types,'-from_form/5-PairsFromForm/3-0-',5,
                [{file,"erl_types.erl"},{line,4680}]},
            {erl_types,from_form,5,[{file,"erl_types.erl"},{line,4673}]},
            {erl_types,t_from_form2,5,[{file,"erl_types.erl"},{line,4542}]},
            {erl_types,t_from_form_check_remote,4,
                [{file,"erl_types.erl"},{line,4516}]},
            {dialyzer_utils,'-process_record_remote_types/1-fun-0-',7,
                [{file,"dialyzer_utils.erl"},{line,333}]},
            {lists,mapfoldl,3,[{file,"lists.erl"},{line,1354}]},
            {dialyzer_utils,'-process_record_remote_types/1-fun-1-',7,
                [{file,"dialyzer_utils.erl"},{line,332}]},
            {lists,mapfoldl,3,[{file,"lists.erl"},{line,1354}]}]}],
         [{file,"dialyzer_codeserver.erl"},{line,368}]},
     {dialyzer_plt,insert_callbacks,2,[{file,"dialyzer_plt.erl"},{line,149}]},
     {dialyzer_analysis_callgraph,analysis_start,3,
         [{file,"dialyzer_analysis_callgraph.erl"},{line,143}]}]}

In addition if I do the pretty printer

% alias pp='escript _build/default/lib/parse_trans/ebin/parse_trans_pp.beam'
% pp wct.beam
escript: exception error: no function clause matching
                 erl_pp:map_pair_type({type,5,map_type_assoc,
                                       [{type,5,union,
                                         [{atom,5,none},
                                          {type,5,tuple,
                                           [{atom,5,one},
                                            {type,5,binary,[]}]}]},
                                        {type,5,non_neg_integer,[]}]},
                                      0) (erl_pp.erl, line 364)
  in function  erl_pp:'-ltypes/3-lc$^0/1-0-'/3 (erl_pp.erl, line 432)
  in call from erl_pp:tuple_type/2 (erl_pp.erl, line 383)
  in call from erl_pp:map_type/1 (erl_pp.erl, line 359)
  in call from erl_pp:ltype/2 (erl_pp.erl, line 316)
  in call from erl_pp:typed/2 (erl_pp.erl, line 380)
  in call from erl_pp:record_field/2 (erl_pp.erl, line 745)
  in call from erl_pp:'-lexprs/3-lc$^0/1-0-'/3 (erl_pp.erl, line 899)

Remove the '-compile' line from the example, and everything works as expected. I tried to figure out a little bit what is happening and as far as I can tell a map_field_assoc is being converted to a map_type_assoc somewhere in ct_expand, but I've not quite figured out where, so figured I'd create the issue and maybe you'll know how to fix it, otherwise I'll keep digging tomorrow.

fenollp commented 6 years ago

Thanks for filling this in: I have been having this same exact error for a while and hadn’t investigated/thought it could be coming from this project!

Maybe the abstract_map construction in ct_expand.erl is no longer correct on newer OTP releases? (ie >=20) Or maybe ct_expand rewrites the whole AST including types and incorrectly replaces a map construction in types (map_type_assoc) with map_field_assoc?

djnym commented 6 years ago

Yeah, I was thinking the same thing, and I forgot to mention this seems to fail with erlang 19 or 20, I haven't yet tested with 21-rc2, but will probably do so today just for completeness.

djnym commented 6 years ago

Just an FYI problem still exists with 21-rc2 as well.

fenollp commented 6 years ago

parse_trans/src/issue28.erl:

-module(issue28).
-compile({parse_transform, ct_expand}).
-export([f/0]).
-record(r, {f1 :: #{k := v}}).
f() -> #r{}.

Then rebar3 dialyzer reproduces this issue.

So far:

Looking at the forms in and out of ct_expand it looks like map_field_exact gets replaced with map_type_exact and Dialyzer doesn't like it even though the compiler is fine with it.

Before:

[{attribute,1,file,
     {"/home/pete/wefwefwef/parse_trans.git/_build/default/lib/parse_trans/src/issue28.erl",
      1}},
 {attribute,1,module,issue28},
 {attribute,3,export,[{f,0}]},
 {attribute,4,record,
     {r,[{typed_record_field,
             {record_field,4,{atom,4,f1}},
             {type,4,map,
                 [{type,4,map_field_exact,[{atom,4,k},{atom,4,v}]}]}}]}},
 {function,5,f,0,[{clause,5,[],[],[{record,5,r,[]}]}]},
 {eof,6}] 

After:

[{attribute,1,file,
     {"/home/pete/wefwefwef/parse_trans.git/_build/default/lib/parse_trans/src/issue28.erl",
      1}},
 {attribute,1,module,issue28},
 {attribute,3,export,[{f,0}]},
 {attribute,4,record,
     {r,[{typed_record_field,
             {record_field,4,{atom,4,f1}},
             {type,4,map,
                 [{type,4,map_type_exact,[{atom,4,k},{atom,4,v}]}]}}]}},
 {function,5,f,0,[{clause,5,[],[],[{record,5,r,[]}]}]},
 {eof,6}] 
fenollp commented 6 years ago

So the minimal example applies the ct_expand parse_transform without actually calling ct_expand:term/1 so it is basically just calling erl_syntax functions followed by erl_syntax:revert/1.

Looking at this file there is an explicit mapping to/from map_{field,type}_{exact,assoc}: https://github.com/erlang/otp/blob/3b3e2f46841e3e86c991be92d62cbb0360ca80e3/lib/syntax_tools/src/erl_syntax.erl#L720-L721

But there is a typo when reverting this mapping: https://github.com/erlang/otp/blob/3b3e2f46841e3e86c991be92d62cbb0360ca80e3/lib/syntax_tools/src/erl_syntax.erl#L5331-L5389

Fortunately, this was discovered and patched 9 days ago! https://github.com/erlang/otp/commit/d129131ee8ffda4713f807e6148b601c16f1b0bb

fenollp commented 6 years ago

Now if it's not in 21 rc2, maybe it'll be in the final 21? How do we make sure of this?

fenollp commented 6 years ago

OTP fix is now part of 21 as well as 20.3.8.1: https://github.com/erlang/otp/releases/tag/OTP-20.3.8.1 This can be closed.

djnym commented 6 years ago

It can be closed if this library is restricted to 20.3.8.1 or greater. Not sure if that is wanted or whether it should’ve hacked to correct it’s use in 19. Not sure if there’s a way to black hole a version, then it can be marked as requiring less than 19 or greater than or equal to 20.3.8.1.

fenollp commented 6 years ago

You're right. AFAI can see an update to the README is the best that can be done here...