GNOME.org
 

[vala] Fix handling of owned delegates

commit da66e781f50698939544098c8efc00ee5fe817dd
Author: Jürg Billeter <j bitron ch>
Date:   Sat Sep 19 11:15:27 2009 +0200

    Fix handling of owned delegates
    
    Fixes bug 595639.

 codegen/valaccodebasemodule.vala          |   37 +++++++++++++++++++++++-
 codegen/valaccodedelegatemodule.vala      |   43 +++++++++++++++++++++++-----
 codegen/valaccodemethodcallmodule.vala    |    2 +-
 codegen/valagtypemodule.vala              |    6 ++++
 vala/valadelegatetype.vala                |    4 +++
 vala/valamemberaccess.vala                |    8 +++++
 vala/valareferencetransferexpression.vala |    8 +++++
 7 files changed, 97 insertions(+), 11 deletions(-)
---
diff --git a/codegen/valaccodebasemodule.vala b/codegen/valaccodebasemodule.vala
index d3caef8..2b804c4 100644
--- a/codegen/valaccodebasemodule.vala
+++ b/codegen/valaccodebasemodule.vala
@@ -1033,6 +1033,7 @@ internal class Vala.CCodeBaseModule : CCodeModule {
 
 				var ma = new MemberAccess (this_access, f.name);
 				ma.symbol_reference = f;
+				ma.value_type = f.field_type.copy ();
 				instance_finalize_fragment.append (new CCodeExpressionStatement (get_unref_expression (lhs, f.field_type, ma)));
 			}
 		} else if (f.binding == MemberBinding.CLASS)  {
@@ -1676,6 +1677,7 @@ internal class Vala.CCodeBaseModule : CCodeModule {
 					if (requires_destroy (local.variable_type)) {
 						var ma = new MemberAccess.simple (local.name);
 						ma.symbol_reference = local;
+						ma.value_type = local.variable_type.copy ();
 						free_block.add_statement (new CCodeExpressionStatement (get_unref_expression (new CCodeMemberAccess.pointer (new CCodeIdentifier ("_data%d_".printf (block_id)), get_variable_cname (local.name)), local.variable_type, ma)));
 					}
 				}
@@ -1722,11 +1724,14 @@ internal class Vala.CCodeBaseModule : CCodeModule {
 						param_type.value_owned = true;
 						data.add_field (param_type.get_cname (), get_variable_cname (param.name));
 
+						bool is_unowned_delegate = param.parameter_type is DelegateType && !param.parameter_type.value_owned;
+
 						// create copy if necessary as captured variables may need to be kept alive
 						CCodeExpression cparam = get_variable_cexpression (param.name);
-						if (requires_copy (param_type) && !param.parameter_type.value_owned)  {
+						if (requires_copy (param_type) && !param.parameter_type.value_owned && !is_unowned_delegate)  {
 							var ma = new MemberAccess.simple (param.name);
 							ma.symbol_reference = param;
+							ma.value_type = param.parameter_type.copy ();
 							// directly access parameters in ref expressions
 							param.captured = false;
 							cparam = get_ref_cexpression (param.parameter_type, cparam, ma, param);
@@ -1750,9 +1755,10 @@ internal class Vala.CCodeBaseModule : CCodeModule {
 							}
 						}
 
-						if (requires_destroy (param_type)) {
+						if (requires_destroy (param_type) && !is_unowned_delegate) {
 							var ma = new MemberAccess.simple (param.name);
 							ma.symbol_reference = param;
+							ma.value_type = param_type.copy ();
 							free_block.add_statement (new CCodeExpressionStatement (get_unref_expression (new CCodeMemberAccess.pointer (new CCodeIdentifier ("_data%d_".printf (block_id)), get_variable_cname (param.name)), param.parameter_type, ma)));
 						}
 					}
@@ -1807,6 +1813,7 @@ internal class Vala.CCodeBaseModule : CCodeModule {
 			if (!local.floating && !local.captured && requires_destroy (local.variable_type)) {
 				var ma = new MemberAccess.simple (local.name);
 				ma.symbol_reference = local;
+				ma.value_type = local.variable_type.copy ();
 				cblock.add_statement (new CCodeExpressionStatement (get_unref_expression (get_variable_cexpression (local.name), local.variable_type, ma)));
 			}
 		}
@@ -1817,6 +1824,7 @@ internal class Vala.CCodeBaseModule : CCodeModule {
 				if (!param.captured && requires_destroy (param.parameter_type) && param.direction == ParameterDirection.IN) {
 					var ma = new MemberAccess.simple (param.name);
 					ma.symbol_reference = param;
+					ma.value_type = param.parameter_type.copy ();
 					cblock.add_statement (new CCodeExpressionStatement (get_unref_expression (get_variable_cexpression (param.name), param.parameter_type, ma)));
 				}
 			}
@@ -2425,6 +2433,24 @@ internal class Vala.CCodeBaseModule : CCodeModule {
 	}
 
 	public virtual CCodeExpression get_unref_expression (CCodeExpression cvar, DataType type, Expression expr, bool is_macro_definition = false) {
+		if (type is DelegateType) {
+			CCodeExpression delegate_target_destroy_notify;
+			var delegate_target = get_delegate_target_cexpression (expr, out delegate_target_destroy_notify);
+
+			var ccall = new CCodeFunctionCall (delegate_target_destroy_notify);
+			ccall.add_argument (delegate_target);
+
+			var cisnull = new CCodeBinaryExpression (CCodeBinaryOperator.EQUALITY, delegate_target_destroy_notify, new CCodeConstant ("NULL"));
+
+			var ccomma = new CCodeCommaExpression ();
+			ccomma.append_expression (new CCodeConditionalExpression (cisnull, new CCodeConstant ("NULL"), ccall));
+			ccomma.append_expression (new CCodeAssignment (cvar, new CCodeConstant ("NULL")));
+			ccomma.append_expression (new CCodeAssignment (delegate_target, new CCodeConstant ("NULL")));
+			ccomma.append_expression (new CCodeAssignment (delegate_target_destroy_notify, new CCodeConstant ("NULL")));
+
+			return ccomma;
+		}
+
 		var ccall = new CCodeFunctionCall (get_destroy_func_expression (type));
 
 		if (type is ValueType && !type.nullable) {
@@ -2573,6 +2599,7 @@ internal class Vala.CCodeBaseModule : CCodeModule {
 		foreach (LocalVariable local in temp_ref_vars) {
 			var ma = new MemberAccess.simple (local.name);
 			ma.symbol_reference = local;
+			ma.value_type = local.variable_type.copy ();
 			expr_list.append_expression (get_unref_expression (get_variable_cexpression (local.name), local.variable_type, ma));
 		}
 		
@@ -2702,6 +2729,7 @@ internal class Vala.CCodeBaseModule : CCodeModule {
 			if (local.active && !local.floating && !local.captured && requires_destroy (local.variable_type)) {
 				var ma = new MemberAccess.simple (local.name);
 				ma.symbol_reference = local;
+				ma.value_type = local.variable_type.copy ();
 				cfrag.append (new CCodeExpressionStatement (get_unref_expression (get_variable_cexpression (local.name), local.variable_type, ma)));
 			}
 		}
@@ -2757,6 +2785,7 @@ internal class Vala.CCodeBaseModule : CCodeModule {
 			if (requires_destroy (param.parameter_type) && param.direction == ParameterDirection.IN) {
 				var ma = new MemberAccess.simple (param.name);
 				ma.symbol_reference = param;
+				ma.value_type = param.parameter_type.copy ();
 				cfrag.append (new CCodeExpressionStatement (get_unref_expression (get_variable_cexpression (param.name), param.parameter_type, ma)));
 			}
 		}
@@ -3184,6 +3213,10 @@ internal class Vala.CCodeBaseModule : CCodeModule {
 	}
 
 	public virtual CCodeExpression? get_ref_cexpression (DataType expression_type, CCodeExpression cexpr, Expression? expr, CodeNode node) {
+		if (expression_type is DelegateType) {
+			return cexpr;
+		}
+
 		if (expression_type is ValueType && !expression_type.nullable) {
 			// normal value type, no null check
 			// (copy (&expr, &temp), temp)
diff --git a/codegen/valaccodedelegatemodule.vala b/codegen/valaccodedelegatemodule.vala
index 8611b0a..eb91e2d 100644
--- a/codegen/valaccodedelegatemodule.vala
+++ b/codegen/valaccodedelegatemodule.vala
@@ -135,6 +135,13 @@ internal class Vala.CCodeDelegateModule : CCodeArrayModule {
 				is_out = true;
 			}
 		}
+
+		bool expr_owned = delegate_expr.value_type.value_owned;
+
+		if (delegate_expr is ReferenceTransferExpression) {
+			var reftransfer_expr = (ReferenceTransferExpression) delegate_expr;
+			delegate_expr = reftransfer_expr.inner;
+		}
 		
 		if (delegate_expr is MethodCall) {
 			var invocation_expr = (MethodCall) delegate_expr;
@@ -147,7 +154,7 @@ internal class Vala.CCodeDelegateModule : CCodeArrayModule {
 			if (closure_block != null) {
 				int block_id = get_block_id (closure_block);
 				var delegate_target = get_variable_cexpression ("_data%d_".printf (block_id));
-				if (delegate_expr.value_type.value_owned) {
+				if (expr_owned) {
 					var ref_call = new CCodeFunctionCall (new CCodeIdentifier ("block%d_data_ref".printf (block_id)));
 					ref_call.add_argument (delegate_target);
 					delegate_target = ref_call;
@@ -156,7 +163,7 @@ internal class Vala.CCodeDelegateModule : CCodeArrayModule {
 				return delegate_target;
 			} else if (get_this_type () != null || in_constructor) {
 				CCodeExpression delegate_target = new CCodeIdentifier ("self");
-				if (delegate_expr.value_type.value_owned) {
+				if (expr_owned) {
 					if (get_this_type () != null) {
 						var ref_call = new CCodeFunctionCall (get_dup_func_expression (get_this_type (), delegate_expr.source_reference));
 						ref_call.add_argument (delegate_target);
@@ -187,15 +194,21 @@ internal class Vala.CCodeDelegateModule : CCodeArrayModule {
 					return new CCodeMemberAccess.pointer (new CCodeIdentifier ("data"), get_delegate_target_cname (get_variable_cname (param.name)));
 				} else {
 					CCodeExpression target_expr = new CCodeIdentifier (get_delegate_target_cname (get_variable_cname (param.name)));
-					delegate_target_destroy_notify = new CCodeIdentifier (get_delegate_target_destroy_notify_cname (get_variable_cname (param.name)));
+					if (expr_owned) {
+						delegate_target_destroy_notify = new CCodeIdentifier (get_delegate_target_destroy_notify_cname (get_variable_cname (param.name)));
+					}
 					if (param.direction != ParameterDirection.IN) {
 						// accessing argument of out/ref param
 						target_expr = new CCodeUnaryExpression (CCodeUnaryOperator.POINTER_INDIRECTION, target_expr);
-						delegate_target_destroy_notify = new CCodeUnaryExpression (CCodeUnaryOperator.POINTER_INDIRECTION, delegate_target_destroy_notify);
+						if (expr_owned) {
+							delegate_target_destroy_notify = new CCodeUnaryExpression (CCodeUnaryOperator.POINTER_INDIRECTION, delegate_target_destroy_notify);
+						}
 					}
 					if (is_out) {
 						// passing delegate as out/ref
-						delegate_target_destroy_notify = new CCodeUnaryExpression (CCodeUnaryOperator.ADDRESS_OF, delegate_target_destroy_notify);
+						if (expr_owned) {
+							delegate_target_destroy_notify = new CCodeUnaryExpression (CCodeUnaryOperator.ADDRESS_OF, delegate_target_destroy_notify);
+						}
 						return new CCodeUnaryExpression (CCodeUnaryOperator.ADDRESS_OF, target_expr);
 					} else {
 						return target_expr;
@@ -213,9 +226,13 @@ internal class Vala.CCodeDelegateModule : CCodeArrayModule {
 					return new CCodeMemberAccess.pointer (new CCodeIdentifier ("data"), get_delegate_target_cname (get_variable_cname (local.name)));
 				} else {
 					var target_expr = new CCodeIdentifier (get_delegate_target_cname (get_variable_cname (local.name)));
-					delegate_target_destroy_notify = new CCodeIdentifier (get_delegate_target_destroy_notify_cname (get_variable_cname (local.name)));
+					if (expr_owned) {
+						delegate_target_destroy_notify = new CCodeIdentifier (get_delegate_target_destroy_notify_cname (get_variable_cname (local.name)));
+					}
 					if (is_out) {
-						delegate_target_destroy_notify = new CCodeUnaryExpression (CCodeUnaryOperator.ADDRESS_OF, delegate_target_destroy_notify);
+						if (expr_owned) {
+							delegate_target_destroy_notify = new CCodeUnaryExpression (CCodeUnaryOperator.ADDRESS_OF, delegate_target_destroy_notify);
+						}
 						return new CCodeUnaryExpression (CCodeUnaryOperator.ADDRESS_OF, target_expr);
 					} else {
 						return target_expr;
@@ -223,7 +240,8 @@ internal class Vala.CCodeDelegateModule : CCodeArrayModule {
 				}
 			} else if (delegate_expr.symbol_reference is Field) {
 				var field = (Field) delegate_expr.symbol_reference;
-				var target_cname = get_delegate_target_cname (field.get_cname ());
+				string target_cname = get_delegate_target_cname (field.get_cname ());
+				string target_destroy_notify_cname = get_delegate_target_destroy_notify_cname (field.get_cname ());
 
 				var ma = (MemberAccess) delegate_expr;
 
@@ -244,11 +262,20 @@ internal class Vala.CCodeDelegateModule : CCodeArrayModule {
 					}
 					if (((TypeSymbol) field.parent_symbol).is_reference_type ()) {
 						target_expr = new CCodeMemberAccess.pointer (inst, target_cname);
+						if (expr_owned) {
+							delegate_target_destroy_notify = new CCodeMemberAccess.pointer (inst, target_destroy_notify_cname);
+						}
 					} else {
 						target_expr = new CCodeMemberAccess (inst, target_cname);
+						if (expr_owned) {
+							delegate_target_destroy_notify = new CCodeMemberAccess (inst, target_destroy_notify_cname);
+						}
 					}
 				} else {
 					target_expr = new CCodeIdentifier (target_cname);
+					if (expr_owned) {
+						delegate_target_destroy_notify = new CCodeIdentifier (target_destroy_notify_cname);
+					}
 				}
 
 				if (is_out) {
diff --git a/codegen/valaccodemethodcallmodule.vala b/codegen/valaccodemethodcallmodule.vala
index 31d145e..70483ef 100644
--- a/codegen/valaccodemethodcallmodule.vala
+++ b/codegen/valaccodemethodcallmodule.vala
@@ -330,7 +330,7 @@ internal class Vala.CCodeMethodCallModule : CCodeAssignmentModule {
 					// (ret_tmp = call (&tmp), var1 = (assign_tmp = dup (tmp), free (var1), assign_tmp), ret_tmp)
 					if (param.direction != ParameterDirection.IN && requires_destroy (arg.value_type)
 					    && (param.direction == ParameterDirection.OUT || !param.parameter_type.value_owned)
-					    && !(param.parameter_type is ArrayType)) {
+					    && !(param.parameter_type is ArrayType) && !(param.parameter_type is DelegateType)) {
 						var unary = (UnaryExpression) arg;
 
 						var ccomma = new CCodeCommaExpression ();
diff --git a/codegen/valagtypemodule.vala b/codegen/valagtypemodule.vala
index 417d29a..beda1c4 100644
--- a/codegen/valagtypemodule.vala
+++ b/codegen/valagtypemodule.vala
@@ -301,6 +301,9 @@ internal class Vala.GTypeModule : GErrorModule {
 						if (delegate_type.delegate_symbol.has_target) {
 							// create field to store delegate target
 							instance_struct.add_field ("gpointer", get_delegate_target_cname (f.name));
+							if (delegate_type.value_owned) {
+								instance_struct.add_field ("GDestroyNotify", get_delegate_target_destroy_notify_cname (f.name));
+							}
 						}
 					}
 				} else if (f.binding == MemberBinding.CLASS) {
@@ -391,6 +394,9 @@ internal class Vala.GTypeModule : GErrorModule {
 						if (delegate_type.delegate_symbol.has_target) {
 							// create field to store delegate target
 							instance_priv_struct.add_field ("gpointer", get_delegate_target_cname (f.name));
+							if (delegate_type.value_owned) {
+								instance_priv_struct.add_field ("GDestroyNotify", get_delegate_target_destroy_notify_cname (f.name));
+							}
 						}
 					}
 				}
diff --git a/vala/valadelegatetype.vala b/vala/valadelegatetype.vala
index 1f32a7e..3761380 100644
--- a/vala/valadelegatetype.vala
+++ b/vala/valadelegatetype.vala
@@ -83,4 +83,8 @@ public class Vala.DelegateType : DataType {
 	public override bool check (SemanticAnalyzer analyzer) {
 		return delegate_symbol.check (analyzer);
 	}
+
+	public override bool is_disposable () {
+		return delegate_symbol.has_target && value_owned;
+	}
 }
diff --git a/vala/valamemberaccess.vala b/vala/valamemberaccess.vala
index 80e0b00..8833d80 100644
--- a/vala/valamemberaccess.vala
+++ b/vala/valamemberaccess.vala
@@ -575,6 +575,10 @@ public class Vala.MemberAccess : Expression {
 			} else {
 				value_type = new InvalidType ();
 			}
+
+			if (target_type != null) {
+				value_type.value_owned = target_type.value_owned;
+			}
 		} else {
 			// implicit this access
 			if (instance && inner == null) {
@@ -593,6 +597,10 @@ public class Vala.MemberAccess : Expression {
 			if (symbol_reference is Method) {
 				var m = (Method) symbol_reference;
 
+				if (target_type != null) {
+					value_type.value_owned = target_type.value_owned;
+				}
+
 				Method base_method;
 				if (m.base_method != null) {
 					base_method = m.base_method;
diff --git a/vala/valareferencetransferexpression.vala b/vala/valareferencetransferexpression.vala
index dcf2e53..0cb89ce 100644
--- a/vala/valareferencetransferexpression.vala
+++ b/vala/valareferencetransferexpression.vala
@@ -102,6 +102,14 @@ public class Vala.ReferenceTransferExpression : Expression {
 			return false;
 		}
 
+		if (inner.value_type is DelegateType) {
+			// would require fixing code generator to ensure _target_destroy_notify
+			// is set to NULL as well after reference transfer, leads to double free otherwise
+			error = true;
+			Report.error (source_reference, "Reference transfer not supported for delegates");
+			return false;
+		}
+
 		value_type = inner.value_type.copy ();
 		value_type.value_owned = true;
[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]