Browse Source

Check for abstract `this` modification in code inlined into non-inline methods (#8454)

* Check for `this` modification in code inlined into non-inline methods

* check for `this` modification after inlined expr is evaluated for sure
Aleksandr Kuzmenko 6 years ago
parent
commit
4125e4b1f5

+ 1 - 0
src/core/type.ml

@@ -384,6 +384,7 @@ type flag_tclass_field =
 	| CfExtern (* This is only set if the field itself is extern, not just the class. *)
 	| CfFinal
 	| CfOverridden
+	| CfModifiesThis (* This is set for methods which reassign `this`. E.g. `this = value` *)
 
 (* Flags *)
 

+ 10 - 0
src/typing/calls.ml

@@ -57,6 +57,16 @@ let make_call ctx e params t ?(force_inline=false) p =
 				None
 		in
 		ignore(follow f.cf_type); (* force evaluation *)
+		(match cl, ctx.curclass.cl_kind, params with
+			| Some c, KAbstractImpl _, { eexpr = TLocal { v_meta = v_meta } } :: _ when c == ctx.curclass ->
+				if
+					has_meta Meta.This v_meta
+					&& not (assign_to_this_is_allowed ctx)
+					&& has_class_field_flag f CfModifiesThis
+				then
+					error ("Abstract 'this' value can only be modified inside an inline function. '" ^ f.cf_name ^ "' modifies 'this'") p;
+			| _ -> ()
+		);
 		let params = List.map (ctx.g.do_optimize ctx) params in
 		let force_inline = is_forced_inline cl f in
 		(match f.cf_expr_unoptimized,f.cf_expr with

+ 3 - 4
src/typing/typer.ml

@@ -296,12 +296,11 @@ let rec type_ident_raise ctx i p mode =
 		else
 			AKNo i
 	| "this" ->
+		if mode = MSet then add_class_field_flag ctx.curfield CfModifiesThis;
 		(match mode, ctx.curclass.cl_kind with
 		| MSet, KAbstractImpl _ ->
-			(match ctx.curfield.cf_kind with
-			| Method MethInline -> ()
-			| Method _ when ctx.curfield.cf_name = "_new" -> ()
-			| _ -> error "Abstract 'this' value can only be modified inside an inline function" p);
+			if not (assign_to_this_is_allowed ctx) then
+				error "Abstract 'this' value can only be modified inside an inline function" p;
 			AKExpr (get_this ctx p)
 		| (MCall, KAbstractImpl _) | (MGet, _)-> AKExpr(get_this ctx p)
 		| _ -> AKNo i)

+ 10 - 0
src/typing/typerBase.ml

@@ -93,6 +93,16 @@ let get_this ctx p =
 	| FunConstructor | FunMember ->
 		mk (TConst TThis) ctx.tthis p
 
+let assign_to_this_is_allowed ctx =
+	match ctx.curclass.cl_kind with
+		| KAbstractImpl _ ->
+			(match ctx.curfield.cf_kind with
+				| Method MethInline -> true
+				| Method _ when ctx.curfield.cf_name = "_new" -> true
+				| _ -> false
+			)
+		| _ -> false
+
 let rec type_module_type ctx t tparams p =
 	match t with
 	| TClassDecl {cl_kind = KGenericBuild _} ->

+ 13 - 0
tests/misc/projects/Issue6340/Main.hx

@@ -0,0 +1,13 @@
+class Main {
+	static function main() {}
+}
+
+abstract Color(Int) from Int to Int  {
+	var argb(never,set):Int;
+
+	inline function set_argb(v) return this = v;
+
+	public function copyFrom(c:Color) {
+		argb = c;
+	}
+}

+ 1 - 0
tests/misc/projects/Issue6340/compile-fail.hxml

@@ -0,0 +1 @@
+-main Main

+ 1 - 0
tests/misc/projects/Issue6340/compile-fail.hxml.stderr

@@ -0,0 +1 @@
+Main.hx:11: characters 3-11 : Abstract 'this' value can only be modified inside an inline function. 'set_argb' modifies 'this'