Browse Source

Fix implicit return of arrow function for Void (#8539)

* handle expected `Void` return for arrow functions

* handle void return if expected type is unknown

* fix explicit return in subexpr of implicit return

* fix js test

* ignore Void for top-down unify in arrow functions

* cleanup

* fix explicit `return;` in `switch`
Aleksandr Kuzmenko 6 years ago
parent
commit
55ade7d2de

+ 7 - 0
src-json/meta.json

@@ -508,6 +508,13 @@
 		"targets": ["TExpr"],
 		"targets": ["TExpr"],
 		"internal": true
 		"internal": true
 	},
 	},
+	{
+		"name": "ImplicitReturn",
+		"metadata": ":implicitReturn",
+		"doc": "Generated automatically on the AST when an implicit return is inserted for arrow function.",
+		"targets": ["TExpr"],
+		"internal": true
+	},
 	{
 	{
 		"name": "Include",
 		"name": "Include",
 		"metadata": ":include",
 		"metadata": ":include",

+ 7 - 2
src/context/display/displayEmitter.ml

@@ -15,7 +15,12 @@ open Display
 open DisplayPosition
 open DisplayPosition
 
 
 let get_expected_name with_type = match with_type with
 let get_expected_name with_type = match with_type with
-	| WithType.Value (Some s) | WithType.WithType(_,Some s) -> Some s
+	| WithType.Value (Some src) | WithType.WithType(_,Some src) ->
+		(match src with
+		| WithType.FunctionArgument name -> Some name
+		| WithType.StructureField name -> Some name
+		| WithType.ImplicitReturn -> None
+		)
 	| _ -> None
 	| _ -> None
 
 
 let sort_fields l with_type tk =
 let sort_fields l with_type tk =
@@ -25,7 +30,7 @@ let sort_fields l with_type tk =
 	in
 	in
 	let expected_name = get_expected_name with_type in
 	let expected_name = get_expected_name with_type in
 	let l = List.map (fun ci ->
 	let l = List.map (fun ci ->
-		let i = get_sort_index tk ci (Option.default Globals.null_pos p) (Option.map fst expected_name) in
+		let i = get_sort_index tk ci (Option.default Globals.null_pos p) expected_name in
 		ci,i
 		ci,i
 	) l in
 	) l in
 	let sort l =
 	let sort l =

+ 20 - 11
src/context/display/displayException.ml

@@ -79,20 +79,29 @@ let to_json ctx de =
 	| DisplayHover None ->
 	| DisplayHover None ->
 		jnull
 		jnull
 	| DisplayHover (Some hover) ->
 	| DisplayHover (Some hover) ->
-		let name_source_kind_to_int = function
-			| WithType.FunctionArgument -> 0
-			| WithType.StructureField -> 1
+		let named_source_kind = function
+			| WithType.FunctionArgument name -> (0, name)
+			| WithType.StructureField name -> (1, name)
+			| _ -> assert false
 		in
 		in
 		let ctx = Genjson.create_context GMFull in
 		let ctx = Genjson.create_context GMFull in
-		let generate_name (name,kind) = jobject [
-			"name",jstring name;
-			"kind",jint (name_source_kind_to_int kind);
-		] in
+		let generate_name kind =
+			let i, name = named_source_kind kind in
+			jobject [
+				"name",jstring name;
+				"kind",jint i;
+			]
+		in
 		let expected = match hover.hexpected with
 		let expected = match hover.hexpected with
-			| Some(WithType.WithType(t,name)) ->
-				jobject (("type",generate_type ctx t) :: (match name with None -> [] | Some name -> ["name",generate_name name]))
-			| Some(Value(Some name)) ->
-				jobject ["name",generate_name name]
+			| Some(WithType.WithType(t,src)) ->
+				jobject (("type",generate_type ctx t)
+				:: (match src with
+					| None -> []
+					| Some ImplicitReturn -> []
+					| Some src -> ["name",generate_name src])
+				)
+			| Some(Value(Some ((FunctionArgument name | StructureField name) as src))) ->
+				jobject ["name",generate_name src]
 			| _ -> jnull
 			| _ -> jnull
 		in
 		in
 		jobject [
 		jobject [

+ 2 - 0
src/context/typecore.ml

@@ -141,6 +141,7 @@ let make_call_ref : (typer -> texpr -> texpr list -> t -> ?force_inline:bool ->
 let type_expr_ref : (?mode:access_mode -> typer -> expr -> WithType.t -> texpr) ref = ref (fun ?(mode=MGet) _ _ _ -> assert false)
 let type_expr_ref : (?mode:access_mode -> typer -> expr -> WithType.t -> texpr) ref = ref (fun ?(mode=MGet) _ _ _ -> assert false)
 let type_block_ref : (typer -> expr list -> WithType.t -> pos -> texpr) ref = ref (fun _ _ _ _ -> assert false)
 let type_block_ref : (typer -> expr list -> WithType.t -> pos -> texpr) ref = ref (fun _ _ _ _ -> assert false)
 let unify_min_ref : (typer -> texpr list -> t) ref = ref (fun _ _ -> assert false)
 let unify_min_ref : (typer -> texpr list -> t) ref = ref (fun _ _ -> assert false)
+let unify_min_for_type_source_ref : (typer -> texpr list -> WithType.with_type_source option -> t) ref = ref (fun _ _ _ -> assert false)
 let analyzer_run_on_expr_ref : (Common.context -> texpr -> texpr) ref = ref (fun _ _ -> assert false)
 let analyzer_run_on_expr_ref : (Common.context -> texpr -> texpr) ref = ref (fun _ _ -> assert false)
 
 
 let pass_name = function
 let pass_name = function
@@ -161,6 +162,7 @@ let make_call ctx e el t p = (!make_call_ref) ctx e el t p
 let type_expr ?(mode=MGet) ctx e with_type = (!type_expr_ref) ~mode ctx e with_type
 let type_expr ?(mode=MGet) ctx e with_type = (!type_expr_ref) ~mode ctx e with_type
 
 
 let unify_min ctx el = (!unify_min_ref) ctx el
 let unify_min ctx el = (!unify_min_ref) ctx el
+let unify_min_for_type_source ctx el src = (!unify_min_for_type_source_ref) ctx el src
 
 
 let make_static_this c p =
 let make_static_this c p =
 	let ta = TAnon { a_fields = c.cl_statics; a_status = ref (Statics c) } in
 	let ta = TAnon { a_fields = c.cl_statics; a_status = ref (Statics c) } in

+ 4 - 0
src/core/type.ml

@@ -2824,6 +2824,10 @@ module TExprToExpr = struct
 end
 end
 
 
 module ExtType = struct
 module ExtType = struct
+	let is_mono = function
+		| TMono { contents = None } -> true
+		| _ -> false
+
 	let is_void = function
 	let is_void = function
 		| TAbstract({a_path=[],"Void"},_) -> true
 		| TAbstract({a_path=[],"Void"},_) -> true
 		| _ -> false
 		| _ -> false

+ 15 - 15
src/core/withType.ml

@@ -1,31 +1,31 @@
 open Type
 open Type
 
 
-type expected_name_source =
-	| FunctionArgument
-	| StructureField
-
-type expected_name = string * expected_name_source
+type with_type_source =
+	| FunctionArgument of string
+	| StructureField of string
+	| ImplicitReturn
 
 
 type t =
 type t =
 	| NoValue
 	| NoValue
-	| Value of expected_name option
-	| WithType of Type.t * expected_name option
+	| Value of with_type_source option
+	| WithType of Type.t * with_type_source option
 
 
 let with_type t = WithType(t,None)
 let with_type t = WithType(t,None)
-let with_argument t name = WithType(t,Some(name,FunctionArgument))
-let with_structure_field t name = WithType(t,Some(name,StructureField))
+let of_implicit_return t = WithType(t,Some ImplicitReturn)
+let with_argument t name = WithType(t,Some(FunctionArgument name))
+let with_structure_field t name = WithType(t,Some(StructureField name))
 let value = Value None
 let value = Value None
-let named_argument name = Value (Some(name,FunctionArgument))
-let named_structure_field name = Value (Some(name,StructureField))
+let named_argument name = Value (Some(FunctionArgument name))
+let named_structure_field name = Value (Some(StructureField name))
 let no_value = NoValue
 let no_value = NoValue
 
 
 let to_string = function
 let to_string = function
 	| NoValue -> "NoValue"
 	| NoValue -> "NoValue"
-	| Value None -> "Value"
-	| Value (Some(s,_)) -> "Value " ^ s
+	| Value (None | Some ImplicitReturn) -> "Value"
+	| Value (Some(FunctionArgument s | StructureField s)) -> "Value " ^ s
 	| WithType(t,s) ->
 	| WithType(t,s) ->
 		let name = match s with
 		let name = match s with
-			| None -> "None"
-			| Some(s,_) -> s
+			| Some(FunctionArgument s | StructureField s) -> s
+			| _ -> "None"
 		in
 		in
 		Printf.sprintf "WithType(%s, %s)" (s_type (print_context()) t) name
 		Printf.sprintf "WithType(%s, %s)" (s_type (print_context()) t) name

+ 3 - 1
src/syntax/grammar.mly

@@ -1108,7 +1108,9 @@ and arrow_expr = parser
 
 
 and arrow_function p1 al er s =
 and arrow_function p1 al er s =
 	let make e =
 	let make e =
-		EFunction(None, { f_params = []; f_type = None; f_args = al; f_expr = Some (EReturn(Some e), (snd e));  }), punion p1 (pos e)
+		let p = pos e in
+		let return = (EMeta((Meta.ImplicitReturn, [], null_pos), (EReturn(Some e), p)), p) in
+		EFunction(None, { f_params = []; f_type = None; f_args = al; f_expr = Some return;  }), punion p1 p
 	in
 	in
 	List.iter (fun (_,_,ml,_,_) ->	match ml with
 	List.iter (fun (_,_,ml,_,_) ->	match ml with
 		| (_,_,p) :: _ -> syntax_error (Custom "Metadata on arrow function arguments is not allowed") ~pos:(Some p) s ()
 		| (_,_,p) :: _ -> syntax_error (Custom "Metadata on arrow function arguments is not allowed") ~pos:(Some p) s ()

+ 9 - 4
src/typing/matcher.ml

@@ -1549,9 +1549,13 @@ module Match = struct
 			| None -> cases
 			| None -> cases
 			| Some (eo,p) -> cases @ [[EConst (Ident "_"),p],None,eo,p]
 			| Some (eo,p) -> cases @ [[EConst (Ident "_"),p],None,eo,p]
 		in
 		in
-		let tmono,with_type = match with_type with
-			| WithType.WithType(t,_) -> (match follow t with TMono _ -> Some t,WithType.value | _ -> None,with_type)
-			| _ -> None,with_type
+		let tmono,with_type,allow_min_void = match with_type with
+			| WithType.WithType(t,src) ->
+				(match follow t, src with
+				| TMono _, Some ImplicitReturn -> Some t, WithType.Value src, true
+				| TMono _, _ -> Some t,WithType.value,false
+				| _ -> None,with_type,false)
+			| _ -> None,with_type,false
 		in
 		in
 		let cases = List.map (fun (el,eg,eo,p) ->
 		let cases = List.map (fun (el,eg,eo,p) ->
 			let p = match eo with Some e when p = null_pos -> pos e | _ -> p in
 			let p = match eo with Some e when p = null_pos -> pos e | _ -> p in
@@ -1580,7 +1584,8 @@ module Match = struct
 								(* If we have no block we have to use the `case pattern` position because that's all we have. *)
 								(* If we have no block we have to use the `case pattern` position because that's all we have. *)
 								mk (TBlock []) ctx.t.tvoid case.Case.case_pos
 								mk (TBlock []) ctx.t.tvoid case.Case.case_pos
 						) cases in
 						) cases in
-						unify_min ctx el
+						if allow_min_void then unify_min_for_type_source ctx el (Some WithType.ImplicitReturn)
+						else unify_min ctx el
 					end
 					end
 				| WithType.WithType(t,_) -> t
 				| WithType.WithType(t,_) -> t
 		in
 		in

+ 33 - 5
src/typing/typer.ml

@@ -283,6 +283,13 @@ let unify_min ctx el =
 		if not ctx.untyped then display_error ctx (error_msg (Unify l)) p;
 		if not ctx.untyped then display_error ctx (error_msg (Unify l)) p;
 		(List.hd el).etype
 		(List.hd el).etype
 
 
+let unify_min_for_type_source ctx el src =
+	match src with
+	| Some WithType.ImplicitReturn when List.exists (fun e -> ExtType.is_void (follow e.etype)) el ->
+		ctx.com.basic.tvoid
+	| _ ->
+		unify_min ctx el
+
 let rec type_ident_raise ctx i p mode =
 let rec type_ident_raise ctx i p mode =
 	match i with
 	match i with
 	| "true" ->
 	| "true" ->
@@ -1890,7 +1897,8 @@ and type_try ctx e1 catches with_type p =
 	let e1,catches,t = match with_type with
 	let e1,catches,t = match with_type with
 		| WithType.NoValue -> e1,catches,ctx.t.tvoid
 		| WithType.NoValue -> e1,catches,ctx.t.tvoid
 		| WithType.Value _ -> e1,catches,unify_min ctx el
 		| WithType.Value _ -> e1,catches,unify_min ctx el
-		| WithType.WithType(t,_) when (match follow t with TMono _ -> true | _ -> false) -> e1,catches,unify_min ctx el
+		| WithType.WithType(t,src) when (match follow t with TMono _ -> true | t -> ExtType.is_void t) ->
+			e1,catches,unify_min_for_type_source ctx el src
 		| WithType.WithType(t,_) ->
 		| WithType.WithType(t,_) ->
 			let e1 = AbstractCast.cast_or_unify ctx t e1 e1.epos in
 			let e1 = AbstractCast.cast_or_unify ctx t e1 e1.epos in
 			let catches = List.map (fun (v,e) ->
 			let catches = List.map (fun (v,e) ->
@@ -2166,19 +2174,24 @@ and type_array_comprehension ctx e with_type p =
 		mk (TLocal v) v.v_type p;
 		mk (TLocal v) v.v_type p;
 	]) v.v_type p
 	]) v.v_type p
 
 
-and type_return ctx e with_type p =
+and type_return ?(implicit=false) ctx e with_type p =
 	match e with
 	match e with
 	| None ->
 	| None ->
 		let v = ctx.t.tvoid in
 		let v = ctx.t.tvoid in
 		unify ctx v ctx.ret p;
 		unify ctx v ctx.ret p;
 		let expect_void = match with_type with
 		let expect_void = match with_type with
 			| WithType.WithType(t,_) -> ExtType.is_void (follow t)
 			| WithType.WithType(t,_) -> ExtType.is_void (follow t)
+			| WithType.Value (Some ImplicitReturn) -> true
 			| _ -> false
 			| _ -> false
 		in
 		in
 		mk (TReturn None) (if expect_void then v else t_dynamic) p
 		mk (TReturn None) (if expect_void then v else t_dynamic) p
 	| Some e ->
 	| Some e ->
 		try
 		try
-			let e = type_expr ctx e (WithType.with_type ctx.ret) in
+			let with_expected_type =
+				if implicit then WithType.of_implicit_return ctx.ret
+				else WithType.with_type ctx.ret
+			in
+			let e = type_expr ctx e with_expected_type in
 			let e = AbstractCast.cast_or_unify ctx ctx.ret e p in
 			let e = AbstractCast.cast_or_unify ctx ctx.ret e p in
 			begin match follow e.etype with
 			begin match follow e.etype with
 			| TAbstract({a_path=[],"Void"},_) ->
 			| TAbstract({a_path=[],"Void"},_) ->
@@ -2240,7 +2253,8 @@ and type_if ctx e e1 e2 with_type p =
 		let e1,e2,t = match with_type with
 		let e1,e2,t = match with_type with
 			| WithType.NoValue -> e1,e2,ctx.t.tvoid
 			| WithType.NoValue -> e1,e2,ctx.t.tvoid
 			| WithType.Value _ -> e1,e2,unify_min ctx [e1; e2]
 			| WithType.Value _ -> e1,e2,unify_min ctx [e1; e2]
-			| WithType.WithType(t,_) when (match follow t with TMono _ -> true | _ -> false) -> e1,e2,unify_min ctx [e1; e2]
+			| WithType.WithType(t,src) when (match follow t with TMono _ -> true | t -> ExtType.is_void t) ->
+				e1,e2,unify_min_for_type_source ctx [e1; e2] src
 			| WithType.WithType(t,_) ->
 			| WithType.WithType(t,_) ->
 				let e1 = AbstractCast.cast_or_unify ctx t e1 e1.epos in
 				let e1 = AbstractCast.cast_or_unify ctx t e1 e1.epos in
 				let e2 = AbstractCast.cast_or_unify ctx t e2 e2.epos in
 				let e2 = AbstractCast.cast_or_unify ctx t e2 e2.epos in
@@ -2311,6 +2325,11 @@ and type_meta ?(mode=MGet) ctx m e1 with_type p =
 				display_error ctx "Call or function expected after inline keyword" p;
 				display_error ctx "Call or function expected after inline keyword" p;
 				e();
 				e();
 			end
 			end
+		| (Meta.ImplicitReturn,_,_) ->
+			begin match e1 with
+			| (EReturn e, p) -> type_return ~implicit:true ctx e with_type p
+			| _ -> e()
+			end
 		| _ -> e()
 		| _ -> e()
 	in
 	in
 	ctx.meta <- old;
 	ctx.meta <- old;
@@ -2438,7 +2457,15 @@ and type_expr ?(mode=MGet) ctx (e,p) (with_type:WithType.t) =
 		Texpr.type_constant ctx.com.basic c p
 		Texpr.type_constant ctx.com.basic c p
 	| EBinop (op,e1,e2) ->
 	| EBinop (op,e1,e2) ->
 		type_binop ctx op e1 e2 false with_type p
 		type_binop ctx op e1 e2 false with_type p
-	| EBlock [] when with_type <> WithType.NoValue ->
+	| EBlock [] when (match with_type with
+			| NoValue -> false
+			(*
+				If expected type is unknown then treat `(...) -> {}` as an empty function
+				(just like `function(...) {}`) instead of returning an object.
+			*)
+			| WithType (t, Some ImplicitReturn) -> not (ExtType.is_mono (follow t))
+			| _ -> true
+		) ->
 		type_expr ctx (EObjectDecl [],p) with_type
 		type_expr ctx (EObjectDecl [],p) with_type
 	| EBlock l ->
 	| EBlock l ->
 		let locals = save_locals ctx in
 		let locals = save_locals ctx in
@@ -2683,6 +2710,7 @@ let rec create com =
 
 
 ;;
 ;;
 unify_min_ref := unify_min;
 unify_min_ref := unify_min;
+unify_min_for_type_source_ref := unify_min_for_type_source;
 make_call_ref := make_call;
 make_call_ref := make_call;
 build_call_ref := build_call;
 build_call_ref := build_call;
 type_call_target_ref := type_call_target;
 type_call_target_ref := type_call_target;

+ 15 - 0
tests/optimization/src/issues/Issue8128.hx

@@ -0,0 +1,15 @@
+package issues;
+
+class Issue8128 {
+	static var tmp:Any;
+	@:js('
+		issues_Issue8128.tmp = function() {
+			return;
+		};
+	')
+	static function test() {
+		tmp = () -> {};
+	}
+
+	public function new() {}
+}

+ 72 - 0
tests/unit/src/unit/issues/Issue7376.hx

@@ -0,0 +1,72 @@
+package unit.issues;
+
+import utest.Assert;
+
+class Issue7376 extends unit.Test {
+	function testTryCatch() {
+		foo(bool -> {
+			try {
+				intJob();
+			} catch(e:Dynamic) {
+				voidJob();
+			}
+		});
+
+		var fn = bool -> {
+			try {
+				intJob();
+			} catch(e:Dynamic) {
+				voidJob();
+			}
+		}
+		foo(fn);
+
+		var fn = bool -> {
+			try {
+				intJob();
+			} catch(e:Dynamic) {
+				return;
+			}
+		}
+		foo(fn);
+
+		noAssert();
+	}
+
+	function testSwitch() {
+		foo(bool -> switch bool {
+			case true: intJob();
+			case false:
+		});
+
+		var fn = bool -> switch bool {
+			case true: intJob();
+			case false:
+		}
+		foo(fn);
+
+		var fn = bool -> switch bool {
+			case true: intJob();
+			case false: return;
+		}
+		foo(fn);
+
+		noAssert();
+	}
+
+	function testIfElse() {
+		foo(bool -> bool ? intJob() : voidJob());
+
+		var fn = bool -> if(bool) intJob() else {};
+		foo(fn);
+
+		var fn = bool -> if (bool) intJob() else return;
+		foo(fn);
+
+		noAssert();
+	}
+
+	@:pure(false) static function foo(f:(Bool)->Void) f(true);
+	@:pure(false) static function intJob():Int return 0;
+	@:pure(false) static function voidJob():Void {}
+}