Fix warnings reporting and stdlib warnings (#367)

* Fix stdlib warnings

* Mark unused includes when used from non-included files

* Do not mark indirectly included files as unused

* Show unused include warning only for files that are never used

* Remove unused include from Option.aes

* Consider functions passed as args as used

* Return warnings as a sorted list

* Fix failing tests

* Fix dialyzer warning

* Fix warning in Func.aes

priv/stdlib/BLS12_381.aes

@@ -7,13 +7,13 @@ namespace BLS12_381 =
   record gt  = { x1 : fp, x2 : fp, x3 : fp, x4 : fp, x5 : fp, x6 : fp,
                  x7 : fp, x8 : fp, x9 : fp, x10 : fp, x11 : fp, x12 : fp }
 
-  function pairing_check(xs : list(g1), ys : list(g2)) =
+  function pairing_check(us : list(g1), vs : list(g2)) =
-    switch((xs, ys))
+    switch((us, vs))
       ([], []) => true
       (x :: xs, y :: ys) => pairing_check_(pairing(x, y), xs, ys)
 
-  function pairing_check_(acc : gt, xs : list(g1), ys : list(g2)) =
+  function pairing_check_(acc : gt, us : list(g1), vs : list(g2)) =
-    switch((xs, ys))
+    switch((us, vs))
       ([], []) => gt_is_one(acc)
       (x :: xs, y :: ys) =>
         pairing_check_(gt_mul(acc, pairing(x, y)), xs, ys)

priv/stdlib/Bitwise.aes

@@ -116,16 +116,6 @@ namespace Bitwise =
         1 => ubxor__(a / 2, b / 2, val * 2, acc + val)
         _ => ubxor__(a / 2, b / 2, val * 2, acc)
 
-  // Bitwise combined 'and' and 'not' of second argument for positive integers
-  // x 'bnand' y = x 'band' ('bnot' y)
-  // The tricky bit is that after negation the second argument has an infinite number of 1's
-  // use as many as needed!
-  //
-  // NOTE: this function is not symmetric!
-  private function ubnand(a, b) =
-    require(a >= 0 && b >= 0, "ubxor is only defined for non-negative integers")
-    ubnand__(a, b, 1, 0)
-
   private function ubnand_(a, b) = ubnand__(a, b, 1, 0)
 
   private function

priv/stdlib/Func.aes

@@ -2,7 +2,7 @@ namespace Func =
 
   function id(x : 'a) : 'a = x
 
-  function const(x : 'a) : 'b => 'a = (y) => x
+  function const(x : 'a) : 'b => 'a = (_) => x
 
   function flip(f : ('a, 'b) => 'c) : ('b, 'a) => 'c = (b, a) => f(a, b)
 

priv/stdlib/List.aes

@@ -173,7 +173,7 @@ namespace List =
    if (n == 0) l
    else switch(l)
       []   => []
-      h::t => drop_(n-1, t)
+      _::t => drop_(n-1, t)
 
 /** Get the longest prefix of a list in which every element
  * matches predicate `p`
@@ -191,7 +191,7 @@ namespace List =
 /** Splits list into two lists of elements that respectively
  * match and don't match predicate `p`
  */
-  function partition(p : 'a => bool, l : list('a)) : (list('a) * list('a)) = switch(l)
+  function partition(p : 'a => bool, lst : list('a)) : (list('a) * list('a)) = switch(lst)
     []   => ([], [])
     h::t =>
       let (l, r) = partition(p, t)

priv/stdlib/ListInternal.aes

@@ -2,8 +2,8 @@ namespace ListInternal =
 
   // -- Flatmap ----------------------------------------------------------------
 
-  function flat_map(f : 'a => list('b), xs : list('a)) : list('b) =
+  function flat_map(f : 'a => list('b), lst : list('a)) : list('b) =
-    switch(xs)
+    switch(lst)
       []      => []
       x :: xs => f(x) ++ flat_map(f, xs)
 

priv/stdlib/Option.aes

@@ -1,5 +1,3 @@
-include "List.aes"
-
 namespace Option =
 
   function is_none(o : option('a)) : bool = switch(o)

priv/stdlib/String.aes

@@ -53,21 +53,21 @@ namespace String =
 
   // Converts a decimal ("123", "-253") or a hexadecimal ("0xa2f", "-0xBBB") string
   // into an integer. If the string doesn't contain a valid number `None` is returned.
-  function to_int(s : string) : option(int) =
+  function to_int(str : string) : option(int) =
-    let s = StringInternal.to_list(s)
+    let lst = StringInternal.to_list(str)
-    switch(is_prefix(['-'], s))
+    switch(is_prefix(['-'], lst))
-      None    => to_int_pos(s)
+      None    => to_int_pos(lst)
       Some(s) => switch(to_int_pos(s))
                    None => None
                    Some(x) => Some(-x)
 
   // Private helper functions below
-  private function to_int_pos(s : list(char)) =
+  private function to_int_pos(chs : list(char)) =
-    switch(is_prefix(['0', 'x'], s))
+    switch(is_prefix(['0', 'x'], chs))
       None =>
-        to_int_(s, ch_to_int_10, 0, 10)
+        to_int_(chs, ch_to_int_10, 0, 10)
-      Some(s) =>
+      Some(str) =>
-        to_int_(s, ch_to_int_16, 0, 16)
+        to_int_(str, ch_to_int_16, 0, 16)
 
   private function
     tokens_(_, [], acc) = [StringInternal.from_list(List.reverse(acc))]
@@ -84,8 +84,8 @@ namespace String =
     contains_(ix, str, substr) =
       switch(is_prefix(substr, str))
         None =>
-          let _ :: str = str
+          let _ :: tailstr = str
-          contains_(ix + 1, str, substr)
+          contains_(ix + 1, tailstr, substr)
         Some(_) =>
           Some(ix)
 
@@ -101,15 +101,15 @@ namespace String =
     to_int_(i :: is, value, x, b) =
       switch(value(i))
         None => None
-        Some(i) => to_int_(is, value, x * b + i, b)
+        Some(n) => to_int_(is, value, x * b + n, b)
 
-  private function ch_to_int_10(c) =
+  private function ch_to_int_10(ch) =
-    let c = Char.to_int(c)
+    let c = Char.to_int(ch)
     if(c >= 48 && c =< 57) Some(c - 48)
     else None
 
-  private function ch_to_int_16(c) =
+  private function ch_to_int_16(ch) =
-    let c = Char.to_int(c)
+    let c = Char.to_int(ch)
     if(c >= 48 && c =< 57)    Some(c - 48)
     elif(c >= 65 && c =< 70)  Some(c - 55)
     elif(c >= 97 && c =< 102) Some(c - 87)

src/aeso_ast_infer_types.erl

@@ -375,7 +375,19 @@ lookup_env(Env, Kind, Ann, Name) ->
             case [ Res || QName <- Names, Res <- [lookup_env1(Env, Kind, Ann, QName)], Res /= false] of
                 []    -> false;
                 [Res = {_, {AnnR, _}}] ->
-                    when_warning(warn_unused_includes, fun() -> used_include(AnnR) end),
+                    when_warning(warn_unused_includes,
+                                 fun() ->
+                                         %% If a file is used from a different file, we
+                                         %% can then mark it as used
+                                         F1 = proplists:get_value(file, Ann, no_file),
+                                         F2 = proplists:get_value(file, AnnR, no_file),
+                                         if
+                                             F1 /= F2 ->
+                                                 used_include(AnnR);
+                                             true ->
+                                                 ok
+                                         end
+                                 end),
                     Res;
                 Many  ->
                     type_error({ambiguous_name, qid(Ann, Name), [{qid, A, Q} || {Q, {A, _}} <- Many]}),
@@ -814,7 +826,8 @@ infer(Contracts, Options) ->
         {Env1, Decls} = infer1(Env, Contracts1, [], Options),
         when_warning(warn_unused_functions, fun() -> destroy_and_report_unused_functions() end),
         when_option(warn_error, fun() -> destroy_and_report_warnings_as_type_errors() end),
-        Warnings = lists:map(fun mk_warning/1, ets_tab2list(warnings)),
+        WarningsUnsorted = lists:map(fun mk_warning/1, ets_tab2list(warnings)),
+        Warnings = aeso_warnings:sort_warnings(WarningsUnsorted),
         {Env2, DeclsFolded, DeclsUnfolded} =
             case proplists:get_value(dont_unfold, Options, false) of
                 true  -> {Env1, Decls, Decls};
@@ -851,7 +864,11 @@ infer1(Env, [{Contract, Ann, ConName, Code} | Rest], Acc, Options)
     Env3 = bind_contract(Contract1, Env2),
     infer1(Env3, Rest, [Contract1 | Acc], Options);
 infer1(Env, [{namespace, Ann, Name, Code} | Rest], Acc, Options) ->
-    when_warning(warn_unused_includes, fun() -> potential_unused_include(Ann, proplists:get_value(src_file, Options, no_file)) end),
+    when_warning(warn_unused_includes,
+                 fun() ->
+                     SrcFile = proplists:get_value(src_file, Options, no_file),
+                     potential_unused_include(Ann, SrcFile)
+                 end),
     check_scope_name_clash(Env, namespace, Name),
     {Env1, Code1} = infer_contract_top(push_scope(namespace, Name, Env), namespace, Code, Options),
     Namespace1 = {namespace, Ann, Name, Code1},
@@ -1440,13 +1457,15 @@ app_t(Ann, Name, Args) -> {app_t, Ann, Name, Args}.
 lookup_name(Env, As, Name) ->
     lookup_name(Env, As, Name, []).
 
-lookup_name(Env = #env{ namespace = NS, current_function = {id, _, Fun} }, As, Id, Options) ->
+lookup_name(Env = #env{ namespace = NS, current_function = {id, _, Fun} = CurFn }, As, Id, Options) ->
     case lookup_env(Env, term, As, qname(Id)) of
         false ->
             type_error({unbound_variable, Id}),
             {Id, fresh_uvar(As)};
         {QId, {_, Ty}} ->
             when_warning(warn_unused_variables, fun() -> used_variable(NS, Fun, QId) end),
+            when_warning(warn_unused_functions,
+                         fun() -> register_function_call(NS ++ qname(CurFn), QId) end),
             Freshen = proplists:get_value(freshen, Options, false),
             check_stateful(Env, Id, Ty),
             Ty1 = case Ty of
@@ -1629,22 +1648,15 @@ infer_expr(Env, {app, Ann, Fun, Args0} = App) ->
         prefix ->
             infer_op(Env, Ann, Fun, Args, fun infer_prefix/1);
         _ ->
-            CurrentFun = Env#env.current_function,
-            Namespace = Env#env.namespace,
             NamedArgsVar = fresh_uvar(Ann),
             NamedArgs1 = [ infer_named_arg(Env, NamedArgsVar, Arg) || Arg <- NamedArgs ],
             NewFun0 = infer_expr(Env, Fun),
             NewArgs = [infer_expr(Env, A) || A <- Args],
             ArgTypes = [T || {typed, _, _, T} <- NewArgs],
-            NewFun1 = {typed, _, Name, FunType} = infer_var_args_fun(Env, NewFun0, NamedArgs1, ArgTypes),
+            NewFun1 = {typed, _, _, FunType} = infer_var_args_fun(Env, NewFun0, NamedArgs1, ArgTypes),
             When = {infer_app, Fun, NamedArgs1, Args, FunType, ArgTypes},
             GeneralResultType = fresh_uvar(Ann),
             ResultType = fresh_uvar(Ann),
-            when_warning(warn_unused_functions,
-                         fun() -> if element(1, Name) == lam -> ok;
-                                     true -> register_function_call(Namespace ++ qname(CurrentFun), Name)
-                                  end
-                         end),
             unify(Env, FunType, {fun_t, [], NamedArgsVar, ArgTypes, GeneralResultType}, When),
             when_warning(warn_negative_spend, fun() -> warn_potential_negative_spend(Ann, NewFun1, NewArgs) end),
             add_constraint(
@@ -2838,9 +2850,8 @@ create_unused_functions() ->
     ets_new(function_calls, [bag]),
     ets_new(all_functions, [set]).
 
-register_function_call(_Caller, {proj, _, _, _}) -> ok;
 register_function_call(Caller, Callee) ->
-    ets_insert(function_calls, {Caller, qname(Callee)}).
+    ets_insert(function_calls, {Caller, Callee}).
 
 potential_unused_function(#env{ what = namespace }, Ann, FunQName, FunId) ->
     ets_insert(all_functions, {Ann, FunQName, FunId, not aeso_syntax:get_ann(private, Ann, false)});

src/aeso_parser.erl

@@ -685,8 +685,9 @@ expand_includes([{include, Ann, {string, _SAnn, File}} | AST], Included, Acc, Op
             Hashed = hash_include(File, Code),
             case sets:is_element(Hashed, Included) of
                 false ->
+                    SrcFile = proplists:get_value(src_file, Opts, no_file),
                     IncludeType = case proplists:get_value(file, Ann) of
-                                      no_file -> direct;
+                                      SrcFile -> direct;
                                       _       -> indirect
                                   end,
                     Opts1 = lists:keystore(src_file, 1, Opts, {src_file, File}),

src/aeso_warnings.erl

@@ -11,6 +11,7 @@
 -export([ new/1
         , new/2
         , warn_to_err/2
+        , sort_warnings/1
         , pp/1
         ]).
 
@@ -23,5 +24,8 @@ new(Pos, Msg) ->
 warn_to_err(Kind, #warn{ pos = Pos, message = Msg }) ->
     aeso_errors:new(Kind, Pos, lists:flatten(Msg)).
 
+sort_warnings(Warnings) ->
+    lists:sort(fun(W1, W2) -> W1#warn.pos =< W2#warn.pos end, Warnings).
+
 pp(#warn{ pos = Pos, message = Msg }) ->
     lists:flatten(io_lib:format("Warning~s:\n~s", [aeso_errors:pp_pos(Pos), Msg])).

test/aeso_compiler_tests.erl

@@ -264,7 +264,9 @@ warnings() ->
       <<?PosW(44, 3)
         "The function `called_unused_function2` is defined but never used.">>,
       <<?PosW(48, 5)
-        "Unused return value.">>
+        "Unused return value.">>,
+      <<?PosW(60, 5)
+        "The function `dec` is defined but never used.">>
      ]).
 
 failing_contracts() ->
@@ -831,7 +833,9 @@ failing_contracts() ->
                    <<?Pos(44, 3)
                      "The function `called_unused_function2` is defined but never used.">>,
                    <<?Pos(48, 5)
-                     "Unused return value.">>
+                     "Unused return value.">>,
+                   <<?Pos(60, 5)
+                     "The function `dec` is defined but never used.">>
                   ])
     ].
 

test/contracts/manual_stdlib_include.aes

@@ -1,4 +1,4 @@
-// This should include Lists.aes implicitly, since Option.aes does.
+include "List.aes"
 include "Option.aes"
 
 contract Test =

test/contracts/warnings.aes

@@ -47,3 +47,14 @@ contract Warnings =
   entrypoint unused_return_value() =
     rv()
     2
+
+namespace FunctionsAsArgs =
+    function f() = g()
+
+    private function g() = h(inc)
+    private function h(fn : (int => int)) = fn(1)
+
+    // Passed as arg to h in g
+    private function inc(n : int) : int = n + 1
+    // Never used
+    private function dec(n : int) : int = n - 1