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

Fix crashes on ExDoc #79

Merged
merged 4 commits into from
May 27, 2022
Merged

Fix crashes on ExDoc #79

merged 4 commits into from
May 27, 2022

Conversation

erszcz
Copy link
Member

@erszcz erszcz commented May 24, 2022

This partially addresses #78. Fixing the initial IO.ANSI.format() issue uncovers more crashes which also have to be taken care of.

@erszcz
Copy link
Member Author

erszcz commented May 24, 2022

With #79 and josefs/Gradualizer#415 the current error is:

17:23:58.147 [error] Unsupported guards format [{:op, [generated: true, location: 327], :"=:=", {:var, 327, :_arity@1}, {:var, [generated: true, location: 327], :_@4}}, {:op, [generated: true, location: 327], :"=:=", {:var, 327, :_name@1}, {:var, [generated: true, location: 327], :_@3}}]
** (FunctionClauseError) no function clause matching in :lists.map/2

    The following arguments were given to :lists.map/2:

        # 1
        #Function<61.46472160/1 in :typechecker.infer_clause/2>

        # 2
        {:op, [generated: true, location: 327], :"=:=", {:var, 327, :_arity@1}, {:var, [generated: true, location: 327], :_@4}}

    (stdlib 3.15.2) lists.erl:1242: :lists.map/2
    (stdlib 3.15.2) lists.erl:1243: :lists.map/2
    (gradualizer 0.1.3+build.1082.refa72e7f0) /Users/erszcz/work/erszcz/gradualizer/src/typechecker.erl:3368: :typechecker.infer_clause/2
    (stdlib 3.15.2) lists.erl:1243: :lists.map/2
    (gradualizer 0.1.3+build.1082.refa72e7f0) /Users/erszcz/work/erszcz/gradualizer/src/typechecker.erl:3354: :typechecker.infer_clauses/2
    (gradualizer 0.1.3+build.1082.refa72e7f0) /Users/erszcz/work/erszcz/gradualizer/src/typechecker.erl:1868: :typechecker.type_check_fun/2
    (gradualizer 0.1.3+build.1082.refa72e7f0) /Users/erszcz/work/erszcz/gradualizer/src/typechecker.erl:1490: :typechecker.type_check_expr/2
    (gradualizer 0.1.3+build.1082.refa72e7f0) /Users/erszcz/work/erszcz/gradualizer/src/typechecker.erl:2258: :typechecker.do_type_check_expr_in/3

@erszcz
Copy link
Member Author

erszcz commented May 25, 2022

With this Gradualizer diff:

diff --git a/src/typechecker.erl b/src/typechecker.erl
index 204e146..c574d2b 100644
--- a/src/typechecker.erl
+++ b/src/typechecker.erl
@@ -3365,7 +3365,9 @@ infer_clause(Env, {clause, _, Args, Guards, Block}) ->
     % TODO: Can there be variable bindings in a guard? Right now we just
     % discard them.
     % TODO: Should we check that guards return boolean()?
+    is_list(Guards) orelse io:format("Guards is not a list!\nEnv: ~p\n", [EnvNew]),
     lists:map(fun (GuardConj) ->
+                      is_list(GuardConj) orelse io:format("GuardConj is not a list!\nEnv: ~p\n", [EnvNew]),
                       lists:map(fun (Guard) ->
                                         type_check_expr(EnvNew, Guard)
                                 end, GuardConj)

We get this log (truncated):

18:04:49.911 [error] Unsupported guards format ...
GuardConj is not a list!
Env: {env,
         ...
         #{module => 'Elixir.ExDoc.Language.Elixir',records => #{},
           types => #{}},
         false,false,true,
         [{clauses_controls,true}],
         30,
         {type,0,any,[]}}
** (FunctionClauseError) no function clause matching in :lists.map/2

    The following arguments were given to :lists.map/2:

        # 1
        #Function<61.83922501/1 in :typechecker.infer_clause/2>

        # 2
        {:op, [generated: true, location: 327], :"=:=", {:var, 327, :_arity@1}, {:var, [generated: true, location: 327], :_@4}}

    (stdlib 3.15.2) lists.erl:1242: :lists.map/2
    (stdlib 3.15.2) lists.erl:1243: :lists.map/2
    (gradualizer 0.1.3+build.1082.refa72e7f0) /Users/erszcz/work/erszcz/gradualizer/src/typechecker.erl:3369: :typechecker.infer_clause/2
    (stdlib 3.15.2) lists.erl:1243: :lists.map/2
    (gradualizer 0.1.3+build.1082.refa72e7f0) /Users/erszcz/work/erszcz/gradualizer/src/typechecker.erl:3354: :typechecker.infer_clauses/2
    (gradualizer 0.1.3+build.1082.refa72e7f0) /Users/erszcz/work/erszcz/gradualizer/src/typechecker.erl:1868: :typechecker.type_check_fun/2
    (gradualizer 0.1.3+build.1082.refa72e7f0) /Users/erszcz/work/erszcz/gradualizer/src/typechecker.erl:1490: :typechecker.type_check_expr/2
    (gradualizer 0.1.3+build.1082.refa72e7f0) /Users/erszcz/work/erszcz/gradualizer/src/typechecker.erl:2258: :typechecker.do_type_check_expr_in/3

Which leads to location 327 - the line with the pinning operators below:

  defp find_function_line(module_data, {name, arity}) do
    Enum.find_value(module_data.private.abst_code, fn
      {:function, anno, ^name, ^arity, _} -> anno_line(anno)
      _ -> nil
    end)
  end

It seems the pinning operator compiles down to guards which are a problem for both Gradient (https://github.com/esl/gradient/blob/main/lib/gradient/ast_specifier.ex#L543-L553) and Gradualizer (https://github.com/josefs/Gradualizer/blob/master/src/typechecker.erl#L3371 - GuardConj is not a list).

@erszcz
Copy link
Member Author

erszcz commented May 25, 2022

It seems to me we miss logic to handle non-unary guard lists in https://github.com/esl/gradient/blob/main/lib/gradient/ast_specifier.ex#L543-L553. Let's consider the following example:

3> merl:quote("fun (A, B) when A =:= 1; B =:= 2, B =/= 3 -> ok end").
{'fun',1,
       {clauses,[{clause,1,
                         [{var,1,'A'},{var,1,'B'}],
                         [[{op,1,'=:=',{var,1,'A'},{integer,1,1}}],
                          [{op,1,'=:=',{var,1,'B'},{integer,1,2}},
                           {op,1,'=/=',{var,1,'B'},{integer,1,3}}]],
                         [{atom,1,ok}]}]}}

The alternative has two conjunctions (A =:= 1 and B =:= 2, B =/= 3). The first conjunction has only a single condition (A =:= 1), but the second conjunction has two conditions (B =:= 2 and B =/= 3).

@erszcz
Copy link
Member Author

erszcz commented May 25, 2022

This patch fixes the crash, but it doesn't pass the guards through mapper yet:

diff --git a/lib/gradient/ast_specifier.ex b/lib/gradient/ast_specifier.ex
index fea2a43..9a5c226 100644
--- a/lib/gradient/ast_specifier.ex
+++ b/lib/gradient/ast_specifier.ex
@@ -548,7 +548,7 @@ defmodule Gradient.AstSpecifier do
 
       gs, {ags, ts} ->
         Logger.error("Unsupported guards format #{inspect(gs)}")
-        {gs ++ ags, ts}
+        {[gs | ags], ts}
     end)
   end

Returning an improper list, as IO.ANSI.format() does,
leads to gradualizer_fmt crashing on the ++ operator.

See #78 for the original report.
@erszcz erszcz marked this pull request as ready for review May 25, 2022 20:46
@erszcz erszcz requested a review from Premwoik May 25, 2022 20:47
@erszcz
Copy link
Member Author

erszcz commented May 25, 2022

This doesn't crash on ExDoc anymore. It reports some errors, though not a lot. I've not revised them yet, so some reported errors might be false positives, but at the very least we do not crash anymore.

Copy link
Collaborator

@Premwoik Premwoik left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good :)

@erszcz erszcz merged commit 6ffbff3 into main May 27, 2022
@erszcz erszcz deleted the fix-crashes-on-exdoc branch May 27, 2022 08:27
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

Successfully merging this pull request may close these issues.

2 participants