From da66e781f50698939544098c8efc00ee5fe817dd Mon Sep 17 00:00:00 2001 From: =?utf8?q?J=C3=BCrg=20Billeter?= Date: Sat, 19 Sep 2009 11:15:27 +0200 Subject: [PATCH] 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 d3caef85..2b804c4d 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 8611b0a8..eb91e2d0 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 31d145e8..70483eff 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 417d29af..beda1c45 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 1f32a7e5..37613800 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 80e0b003..8833d808 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 dcf2e536..0cb89cee 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; -- 2.11.4.GIT