Fix handling of higher-order functions.

src/solidity-common/solidity_ast.ml

@@ -362,6 +362,18 @@ let same_mutability m1 m2 =
   | MNonPayable, MNonPayable -> true
   | _ -> false
 
+(* allowed conversions according to https://docs.soliditylang.org/en/v0.6.0/types.html#function-types *)
+let mutability_is_more_restrictive m1 m2 =
+  match m1, m2 with
+  | MPure, (MView | MNonPayable) -> true
+  | MView, MNonPayable -> true
+  | MPayable, MNonPayable -> true
+  | MPure, MPure -> true
+  | MView, MView -> true
+  | MPayable, MPayable -> true
+  | MNonPayable, MNonPayable -> true
+  | _ -> false
+
 (* for the purpose of overriding *)
 let convertible_mutability ~from ~to_ =
   match from, to_ with
@@ -384,6 +396,17 @@ let same_visibility v1 v2 =
   | VPrivate, VPrivate -> true
   | _ -> false
 
+(* visbility checks when passing functions as arguments *)
+(* v2 is the expected visibility and v1 is the actual visibility *)
+let convertible_visibility_hof v1 v2 =
+  match v1, v2 with
+  | VPublic, VInternal -> true
+  | VPrivate, VInternal -> true
+  | VInternal, VInternal -> true
+  | VExternal, VExternal -> true
+  | VPublic, VExternal -> true
+  | _ -> false
+
 (* for the purpose of overriding *)
 let convertible_visibility ~from ~to_ =
   match from, to_ with

src/solidity-common/solidity_ast.mli

@@ -399,6 +399,10 @@ val is_inheritable : visibility -> bool
 (** Checks the equality of mutabilities *)
 val same_mutability : fun_mutability -> fun_mutability -> bool
 
+(** Checks if the first mutability is more restrictive than the second mutability
+    according to https://docs.soliditylang.org/en/v0.6.0/types.html#function-types *)
+val mutability_is_more_restrictive : fun_mutability -> fun_mutability -> bool
+
 (** Tests if a function with `from` mutability can be overridden by a
     function with `to` mutability. *)
 val convertible_mutability :
@@ -407,6 +411,10 @@ val convertible_mutability :
 (** Checks the equality of visibilities *)
 val same_visibility : visibility -> visibility -> bool
 
+(** Checks if a function expecting the second visibility can accept function
+    arguments (of function type) of the first visibility *)
+val convertible_visibility_hof : visibility -> visibility -> bool
+
 (** Tests if a function with `from` visibility can be overridden by a
     function with `to` visibility. *)
 val convertible_visibility : from:visibility -> to_:visibility -> bool

src/solidity-typechecker/solidity_type.ml

@@ -33,7 +33,7 @@ let same_options o1 o2 =
   o1.gas = o2.gas &&
   o1.salt = o2.salt
 
-let rec same_type ?(ignore_loc=false) t1 t2 =
+let rec same_type ?(ignore_loc=false) ?(relax_visibility=false) t1 t2 =
   match t1, t2 with
   | TBool, TBool ->
       true
@@ -73,7 +73,8 @@ let rec same_type ?(ignore_loc=false) t1 t2 =
       same_type_pl ~ignore_loc fd1.function_params fd2.function_params &&
       same_type_pl ~ignore_loc fd1.function_returns fd2.function_returns &&
       same_mutability fd1.function_mutability fd2.function_mutability &&
-      same_visibility fd1.function_visibility fd2.function_visibility &&
+      (if relax_visibility then convertible_visibility_hof fd1.function_visibility fd2.function_visibility
+      else same_visibility fd1.function_visibility fd2.function_visibility) &&
       same_options fo1 fo2
   | TModifier (md1), TModifier (md2) ->
       same_type_pl ~ignore_loc md1.modifier_params md2.modifier_params
@@ -117,10 +118,10 @@ and same_type_ol ?(ignore_loc=false) tl1 tl2 =
       | None, None -> true
     ) tl1 tl2
 
-and same_type_pl ?(ignore_loc=false) tl1 tl2 =
+and same_type_pl ?(ignore_loc=false) ?(relax_visibility=false) tl1 tl2 =
   List.length tl1 = List.length tl2 &&
   List.for_all2 (fun (t1, _) (t2, _) ->
-      same_type ~ignore_loc t1 t2
+      same_type ~ignore_loc ~relax_visibility t1 t2
     ) tl1 tl2
 
 and same_magic_type ?(ignore_loc=false) t1 t2 =

src/solidity-typechecker/solidity_type_conv.ml

@@ -140,6 +140,11 @@ let rec implicitly_convertible ?(ignore_loc=false) ~from ~to_ () =
   | TBytes _, TString _ when !for_freeton -> true
   | TString _, TBytes _ when !for_freeton -> true
   | _, TDots when !for_freeton -> true
+  | TFunction (fd1, _), TFunction (fd2, _) ->
+      Solidity_type.same_type_pl ~ignore_loc ~relax_visibility:true fd1.function_params fd2.function_params &&
+      Solidity_type.same_type_pl ~ignore_loc fd1.function_returns fd2.function_returns &&
+      Solidity_ast.mutability_is_more_restrictive fd1.function_mutability fd2.function_mutability &&
+      Solidity_ast.convertible_visibility_hof fd1.function_visibility fd2.function_visibility
   | _ ->
       Solidity_type.same_type from to_
 

src/solidity-typechecker/solidity_typechecker.ml

@@ -1926,6 +1926,18 @@ let preprocess_contract_definitions cd =
                            payable, but is \"%s\""
                   (Solidity_printer.string_of_fun_mutability fd.fun_mutability)
             end;
+
+          (* if the function has function-typed parameters, the function cannot be
+             `public` or `external` if the parameters are `internal` *)
+          List.iter (fun param ->
+            match param with
+            | FunctionType(f), _, _ ->
+                (match f.fun_type_visibility, fd.fun_visibility with
+                | VInternal, (VPublic|VExternal) -> error pos "Internal type is not allowed for public or external functions";
+                | _ -> ())
+            | _ -> ()
+            ) fd.fun_params;
+
           let fd, method_ =
             match contract_kind with
             | Interface -> { fd with fun_virtual = true }, true

test/raw_tests/fails/hof_ko1.sol

@@ -0,0 +1,19 @@
+contract A {
+    uint256 internal a;
+
+    function fool(int256 b) external pure {
+        b = b + 1;
+    }
+
+    // somfunc is `internal`
+    function hof(function(int256) somfunc, int256 x) internal pure {
+        x = x + 1;
+    }
+
+    function useHof() public {
+        // `external` functions cannot be passed to functions expecting `internal` (even with `this`)
+        hof(fool, 10);
+        hof(this.fool, 10);
+        a = 1 + 1;
+    }
+}

test/raw_tests/fails/hof_ko2.sol

@@ -0,0 +1,17 @@
+contract A {
+    uint256 internal a;
+
+    function fool(int256 b) internal pure {
+        b = b + 1;
+    }
+
+    function hof(function(int256) external somfunc, int256 x) internal pure {
+        x = x + 1;
+    }
+
+    function useHof() public {
+        // `internal` functions cannot be passed to functions expecting `external`
+        hof(fool, 10);
+        a = 1 + 1;
+    }
+}

test/raw_tests/fails/hof_ko3.sol

@@ -0,0 +1,17 @@
+contract A {
+    uint256 internal a;
+
+    function fool(int256 b) private pure {
+        b = b + 1;
+    }
+
+    function hof(function(int256) external somfunc, int256 x) internal pure {
+        x = x + 1;
+    }
+
+    function useHof() public {
+        // `private` functions cannot be passed to functions expecting `external`
+        hof(fool, 10);
+        a = 1 + 1;
+    }
+}

test/raw_tests/fails/hof_ko4.sol

@@ -0,0 +1,12 @@
+contract A {
+    uint256 internal a;
+
+    function fool(int256 b) internal pure {
+        b = b + 1;
+    }
+
+    // `external` functions cannot have `internal` function-type arguments
+    function hof(function(int256) internal somfunc, int256 x) external pure {
+        x = x + 1;
+    }
+}

test/raw_tests/fails/hof_ko5.sol

@@ -0,0 +1,12 @@
+contract A {
+    uint256 internal a;
+
+    function fool(int256 b) internal pure {
+        b = b + 1;
+    }
+
+    // `public` functions cannot have `internal` function-type arguments
+    function hof(function(int256) internal somfunc, int256 x) public pure {
+        x = x + 1;
+    }
+}

test/raw_tests/successes/hof_ok1.sol

@@ -0,0 +1,18 @@
+contract A {
+    uint256 internal a;
+
+    function fool(int256 b) public pure {
+        b = b + 1;
+    }
+
+    // somfunc is `internal`
+    function hof(function(int256) somfunc, int256 x) internal pure {
+        x = x + 1;
+    }
+
+    function useHof() public {
+        // `public` functions can be passed to functions expecting `internal`
+        hof(fool, 10);
+        a = 1 + 1;
+    }
+}

test/raw_tests/successes/hof_ok2.sol

@@ -0,0 +1,18 @@
+contract A {
+    uint256 internal a;
+
+    function fool(int256 b) private pure {
+        b = b + 1;
+    }
+
+    // somfunc is `internal`
+    function hof(function(int256) somfunc, int256 x) internal pure {
+        x = x + 1;
+    }
+
+    function useHof() public {
+        // `private` functions can be passed to functions expecting `internal`
+        hof(fool, 10);
+        a = 1 + 1;
+    }
+}

test/raw_tests/successes/hof_ok3.sol

@@ -0,0 +1,18 @@
+contract A {
+    uint256 internal a;
+
+    function fool(int256 b) internal pure {
+        b = b + 1;
+    }
+
+    // somfunc is `internal`
+    function hof(function(int256) somfunc, int256 x) internal pure {
+        x = x + 1;
+    }
+
+    function useHof() public {
+        // `internal` functions can be passed to functions expecting `internal`
+        hof(fool, 10);
+        a = 1 + 1;
+    }
+}

test/raw_tests/successes/hof_ok4.sol

@@ -0,0 +1,17 @@
+contract A {
+    uint256 internal a;
+
+    function fool(int256 b) public pure {
+        b = b + 1;
+    }
+
+    function hof(function(int256) external somfunc, int256 x) internal pure {
+        x = x + 1;
+    }
+
+    function useHof() public {
+        // `public` functions can be passed to functions expecting `external`, using `this`
+        hof(this.fool, 10);
+        a = 1 + 1;
+    }
+}

test/raw_tests/successes/hof_ok5.sol

@@ -0,0 +1,17 @@
+contract A {
+    uint256 internal a;
+
+    function fool(int256 b) external pure {
+        b = b + 1;
+    }
+
+    function hof(function(int256) external somfunc, int256 x) internal pure {
+        x = x + 1;
+    }
+
+    function useHof() public {
+        // `external` functions can be passed to functions expecting `external`, using `this`
+        hof(this.fool, 10);
+        a = 1 + 1;
+    }
+}

test/raw_tests/successes/hof_ok6.sol

@@ -0,0 +1,40 @@
+contract A {
+    uint256 internal a;
+
+    function fool(int256 b) internal pure {
+        b = b + 1;
+    }
+
+    // `public` functions can have `external` function-type arguments
+    function hof(function(int256) external somfunc, int256 x) public pure {
+        x = x + 1;
+    }
+
+    // `private` functions can have `external` function-type arguments
+    function hof2(function(int256) external somfunc, int256 x) private pure {
+        x = x + 1;
+    }
+
+    // `private` functions can have `internal` function-type arguments
+    function hof3(function(int256) internal somfunc, int256 x) private pure {
+        x = x + 1;
+    }
+
+    // `internal` functions can have `internal` function-type arguments
+    function hof4(function(int256) internal somfunc, int256 x) internal pure {
+        x = x + 1;
+    }
+
+    // `internal` functions can have `external` function-type arguments
+    function hof5(function(int256) external somfunc, int256 x) internal pure {
+        x = x + 1;
+    }
+
+    // `external` functions can have `external` function-type arguments
+    function hof6(function(int256) external somfunc, int256 x) external pure {
+        x = x + 1;
+    }
+
+    // The other two combinations that fail (interal-external) and (internal-public)
+    //   are in `fails/hof_ko4.sol` and `fails/hof_ko5.sol` respectively
+}

test/raw_tests/successes/hof_ok7.sol

@@ -0,0 +1,22 @@
+contract A {
+    uint256 internal a;
+
+    function fool(int256 b) internal pure {
+        b = b + 1;
+    }
+
+    // somfunc is `internal`
+    function hof(function(int256) somfunc, int256 x) internal pure {
+        x = x + 1;
+    }
+
+    function hofhof(function(function(int256), int256) somotherfunc, int256 x) internal {
+        x = x + 1;
+    }
+
+    function useHof() public {
+        hof(fool, 10);
+        hofhof(hof, 10);
+        a = 1 + 1;
+    }
+}