[vala/0.42] codegen: Properly handle and catch inner-error of finally-block
- From: Rico Tzschichholz <ricotz src gnome org>
- To: commits-list gnome org
- Cc:
- Subject: [vala/0.42] codegen: Properly handle and catch inner-error of finally-block
- Date: Thu, 28 Feb 2019 07:27:01 +0000 (UTC)
commit cd391fad976275d7bfe39808394666d0bd9afc98
Author: Rico Tzschichholz <ricotz ubuntu com>
Date: Wed Jan 23 22:39:42 2019 +0100
codegen: Properly handle and catch inner-error of finally-block
If all inner-errors are caught there is no jump-out of the current finally-
block therefore the control-flow analyzer is happy. To make this work the
surrounding inner-error must not be used or changed here.
Fixes https://gitlab.gnome.org/GNOME/vala/issues/742
codegen/valaccodebasemodule.vala | 39 ++++++++++++++++-
codegen/valaccodemethodcallmodule.vala | 2 +-
codegen/valaccodemethodmodule.vala | 5 +--
codegen/valagdbusclientmodule.vala | 6 +--
codegen/valagdbusservermodule.vala | 2 +-
codegen/valagerrormodule.vala | 32 +++++++-------
codegen/valagobjectmodule.vala | 6 +--
codegen/valagtypemodule.vala | 2 +-
tests/Makefile.am | 2 +
tests/asynchronous/catch-in-finally.vala | 72 ++++++++++++++++++++++++++++++++
tests/errors/catch-in-finally.vala | 59 ++++++++++++++++++++++++++
11 files changed, 198 insertions(+), 29 deletions(-)
---
diff --git a/codegen/valaccodebasemodule.vala b/codegen/valaccodebasemodule.vala
index 519fd8589..447dee408 100644
--- a/codegen/valaccodebasemodule.vala
+++ b/codegen/valaccodebasemodule.vala
@@ -36,6 +36,7 @@ public abstract class Vala.CCodeBaseModule : CodeGenerator {
public ArrayList<CCodeFunction> ccode_stack = new ArrayList<CCodeFunction> ();
public ArrayList<TargetValue> temp_ref_values = new ArrayList<TargetValue> ();
public int next_temp_var_id;
+ public int current_inner_error_id;
public bool current_method_inner_error;
public bool current_method_return;
public int next_coroutine_state = 1;
@@ -81,6 +82,11 @@ public abstract class Vala.CCodeBaseModule : CodeGenerator {
set { emit_context.current_catch = value; }
}
+ public int current_inner_error_id {
+ get { return emit_context.current_inner_error_id; }
+ set { emit_context.current_inner_error_id = value; }
+ }
+
public TypeSymbol? current_type_symbol {
get {
var sym = current_symbol;
@@ -273,6 +279,9 @@ public abstract class Vala.CCodeBaseModule : CodeGenerator {
int next_block_id = 0;
Map<Block,int> block_map = new HashMap<Block,int> ();
+ /* count of emitted inner_error variables in methods */
+ Map<weak Method,int> method_inner_error_var_count = new HashMap<weak Method,int> ();
+
public DataType void_type = new VoidType ();
public DataType bool_type;
public DataType char_type;
@@ -1884,7 +1893,7 @@ public abstract class Vala.CCodeBaseModule : CodeGenerator {
}
if (current_method_inner_error) {
- ccode.add_declaration ("GError *", new CCodeVariableDeclarator.zero
("_inner_error_", new CCodeConstant ("NULL")));
+ ccode.add_declaration ("GError*", new CCodeVariableDeclarator.zero
("_inner_error%d_".printf (current_inner_error_id), new CCodeConstant ("NULL")));
}
cfile.add_function (function);
@@ -1962,6 +1971,24 @@ public abstract class Vala.CCodeBaseModule : CodeGenerator {
ccode.open_block ();
}
+ if (b.parent_node is TryStatement && b == ((TryStatement) b.parent_node).finally_body) {
+ // finally-block with nested error handling
+ if (((TryStatement) b.parent_node).body.tree_can_fail) {
+ if (is_in_coroutine ()) {
+ // _inner_error0_ gets added to closure_struct in
CCodeMethodModule.visit_method()
+ if (current_inner_error_id > 0
+ && (!method_inner_error_var_count.contains (current_method)
+ || method_inner_error_var_count.get (current_method) <
current_inner_error_id)) {
+ // no initialization necessary, closure struct is zeroed
+ closure_struct.add_field ("GError*", "_inner_error%d_".printf
(current_inner_error_id));
+ method_inner_error_var_count.set (current_method,
current_inner_error_id);
+ }
+ } else {
+ ccode.add_declaration ("GError*", new CCodeVariableDeclarator.zero
("_inner_error%d_".printf (current_inner_error_id), new CCodeConstant ("NULL")));
+ }
+ }
+ }
+
if (b.captured) {
var parent_block = next_closure_block (b.parent_symbol);
@@ -2337,6 +2364,14 @@ public abstract class Vala.CCodeBaseModule : CodeGenerator {
}
}
+ public CCodeExpression get_inner_error_cexpression () {
+ if (is_in_coroutine ()) {
+ return new CCodeMemberAccess.pointer (new CCodeIdentifier ("_data_"),
"_inner_error%d_".printf (current_inner_error_id));
+ } else {
+ return new CCodeIdentifier ("_inner_error%d_".printf (current_inner_error_id));
+ }
+ }
+
public string get_local_cname (LocalVariable local) {
var cname = get_variable_cname (local.name);
if (cname[0].isdigit ()) {
@@ -4900,7 +4935,7 @@ public abstract class Vala.CCodeBaseModule : CodeGenerator {
// method can fail
current_method_inner_error = true;
// add &inner_error before the ellipsis arguments
- out_arg_map.set (get_param_pos (-1), new CCodeUnaryExpression
(CCodeUnaryOperator.ADDRESS_OF, get_variable_cexpression ("_inner_error_")));
+ out_arg_map.set (get_param_pos (-1), new CCodeUnaryExpression
(CCodeUnaryOperator.ADDRESS_OF, get_inner_error_cexpression ()));
}
if (ellipsis) {
diff --git a/codegen/valaccodemethodcallmodule.vala b/codegen/valaccodemethodcallmodule.vala
index a3f6030ca..31cb0a776 100644
--- a/codegen/valaccodemethodcallmodule.vala
+++ b/codegen/valaccodemethodcallmodule.vala
@@ -644,7 +644,7 @@ public class Vala.CCodeMethodCallModule : CCodeAssignmentModule {
// method can fail
current_method_inner_error = true;
// add &inner_error before the ellipsis arguments
- out_arg_map.set (get_param_pos (-1), new CCodeUnaryExpression
(CCodeUnaryOperator.ADDRESS_OF, get_variable_cexpression ("_inner_error_")));
+ out_arg_map.set (get_param_pos (-1), new CCodeUnaryExpression
(CCodeUnaryOperator.ADDRESS_OF, get_inner_error_cexpression ()));
} else if (m != null && m.has_error_type_parameter () && async_call != ccall) {
// inferred error argument from base method
out_arg_map.set (get_param_pos (-1), new CCodeConstant ("NULL"));
diff --git a/codegen/valaccodemethodmodule.vala b/codegen/valaccodemethodmodule.vala
index 0c8dbe25a..e51ba00cf 100644
--- a/codegen/valaccodemethodmodule.vala
+++ b/codegen/valaccodemethodmodule.vala
@@ -747,11 +747,10 @@ public abstract class Vala.CCodeMethodModule : CCodeStructModule {
* as error may be set to NULL but we're always interested in inner
errors
*/
if (m.coroutine) {
- closure_struct.add_field ("GError *", "_inner_error_");
-
// no initialization necessary, closure struct is zeroed
+ closure_struct.add_field ("GError*", "_inner_error%d_".printf
(current_inner_error_id));
} else {
- ccode.add_declaration ("GError *", new
CCodeVariableDeclarator.zero ("_inner_error_", new CCodeConstant ("NULL")));
+ ccode.add_declaration ("GError*", new
CCodeVariableDeclarator.zero ("_inner_error%d_".printf (current_inner_error_id), new CCodeConstant ("NULL")));
}
}
diff --git a/codegen/valagdbusclientmodule.vala b/codegen/valagdbusclientmodule.vala
index 32ecb6666..4e4668d2e 100644
--- a/codegen/valagdbusclientmodule.vala
+++ b/codegen/valagdbusclientmodule.vala
@@ -321,7 +321,7 @@ public class Vala.GDBusClientModule : GDBusModule {
var ccall = new CCodeFunctionCall (new CCodeIdentifier
("g_async_initable_new_finish"));
ccall.add_argument (new CCodeCastExpression (source_ref, "GAsyncInitable *"));
ccall.add_argument (get_cvalue (res));
- ccall.add_argument (new CCodeUnaryExpression (CCodeUnaryOperator.ADDRESS_OF,
get_variable_cexpression ("_inner_error_")));
+ ccall.add_argument (new CCodeUnaryExpression (CCodeUnaryOperator.ADDRESS_OF,
get_inner_error_cexpression ()));
var temp_var = get_temp_variable (expr.value_type,
expr.value_type.value_owned);
var temp_ref = get_variable_cexpression (temp_var.name);
@@ -376,7 +376,7 @@ public class Vala.GDBusClientModule : GDBusModule {
ccall.add_argument (get_delegate_target (callback));
}
} else {
- ccall.add_argument (new CCodeUnaryExpression (CCodeUnaryOperator.ADDRESS_OF,
get_variable_cexpression ("_inner_error_")));
+ ccall.add_argument (new CCodeUnaryExpression (CCodeUnaryOperator.ADDRESS_OF,
get_inner_error_cexpression ()));
}
ccall.add_argument (new CCodeConstant ("\"g-flags\""));
ccall.add_argument (get_cvalue (flags));
@@ -418,7 +418,7 @@ public class Vala.GDBusClientModule : GDBusModule {
ccall.add_argument (new CCodeCastExpression (new CCodeMemberAccess.pointer
(new CCodeIdentifier ("_data_"), "_source_object_"), "GAsyncInitable *"));
// pass GAsyncResult stored in closure to finish function
ccall.add_argument (new CCodeMemberAccess.pointer (new CCodeIdentifier
("_data_"), "_res_"));
- ccall.add_argument (new CCodeUnaryExpression (CCodeUnaryOperator.ADDRESS_OF,
get_variable_cexpression ("_inner_error_")));
+ ccall.add_argument (new CCodeUnaryExpression (CCodeUnaryOperator.ADDRESS_OF,
get_inner_error_cexpression ()));
} else {
// begin
ccode.add_expression (ccall);
diff --git a/codegen/valagdbusservermodule.vala b/codegen/valagdbusservermodule.vala
index e1984e24f..b539daee4 100644
--- a/codegen/valagdbusservermodule.vala
+++ b/codegen/valagdbusservermodule.vala
@@ -1016,7 +1016,7 @@ public class Vala.GDBusServerModule : GDBusClientModule {
cregister.add_argument (get_cvalue (obj_arg));
cregister.add_argument (get_cvalue (ma.inner));
cregister.add_argument (get_cvalue (path_arg));
- cregister.add_argument (new CCodeUnaryExpression (CCodeUnaryOperator.ADDRESS_OF,
get_variable_cexpression ("_inner_error_")));
+ cregister.add_argument (new CCodeUnaryExpression (CCodeUnaryOperator.ADDRESS_OF,
get_inner_error_cexpression ()));
if (expr.parent_node is ExpressionStatement) {
ccode.add_expression (cregister);
diff --git a/codegen/valagerrormodule.vala b/codegen/valagerrormodule.vala
index ea8367fd3..eda2493ce 100644
--- a/codegen/valagerrormodule.vala
+++ b/codegen/valagerrormodule.vala
@@ -87,7 +87,7 @@ public class Vala.GErrorModule : CCodeDelegateModule {
public override void visit_throw_statement (ThrowStatement stmt) {
// method will fail
current_method_inner_error = true;
- ccode.add_assignment (get_variable_cexpression ("_inner_error_"), get_cvalue
(stmt.error_expression));
+ ccode.add_assignment (get_inner_error_cexpression (), get_cvalue (stmt.error_expression));
add_simple_check (stmt, true);
}
@@ -168,13 +168,11 @@ public class Vala.GErrorModule : CCodeDelegateModule {
public override void add_simple_check (CodeNode node, bool always_fails = false) {
current_method_inner_error = true;
- var inner_error = get_variable_cexpression ("_inner_error_");
-
if (always_fails) {
// inner_error is always set, avoid unnecessary if statement
// eliminates C warnings
} else {
- var ccond = new CCodeBinaryExpression (CCodeBinaryOperator.INEQUALITY, inner_error,
new CCodeConstant ("NULL"));
+ var ccond = new CCodeBinaryExpression (CCodeBinaryOperator.INEQUALITY,
get_inner_error_cexpression (), new CCodeConstant ("NULL"));
var unlikely = new CCodeFunctionCall (new CCodeIdentifier ("G_UNLIKELY"));
unlikely.add_argument (ccond);
ccode.open_if (unlikely);
@@ -222,7 +220,7 @@ public class Vala.GErrorModule : CCodeDelegateModule {
if (catch_type.error_code != null) {
/* catch clause specifies a specific error code */
var error_match = new CCodeFunctionCall (new
CCodeIdentifier ("g_error_matches"));
- error_match.add_argument (inner_error);
+ error_match.add_argument (get_inner_error_cexpression
());
error_match.add_argument (new CCodeIdentifier
(get_ccode_upper_case_name (catch_type.data_type)));
error_match.add_argument (new CCodeIdentifier
(get_ccode_name (catch_type.error_code)));
@@ -230,7 +228,7 @@ public class Vala.GErrorModule : CCodeDelegateModule {
} else {
/* catch clause specifies a full error domain */
var ccond = new CCodeBinaryExpression
(CCodeBinaryOperator.EQUALITY,
- new CCodeMemberAccess.pointer
(inner_error, "domain"), new CCodeIdentifier
+ new CCodeMemberAccess.pointer
(get_inner_error_cexpression (), "domain"), new CCodeIdentifier
(get_ccode_upper_case_name
(clause.error_type.data_type)));
ccode.open_if (ccond);
@@ -256,7 +254,7 @@ public class Vala.GErrorModule : CCodeDelegateModule {
// as jump out of finally block is not supported
} else {
// should never happen with correct bindings
- uncaught_error_statement (inner_error, true);
+ uncaught_error_statement (get_inner_error_cexpression (), true);
}
} else if (current_method != null && current_method.get_error_types ().size > 0) {
// current method can fail, propagate error
@@ -271,7 +269,7 @@ public class Vala.GErrorModule : CCodeDelegateModule {
// Check the allowed error domains to propagate
var domain_check = new CCodeBinaryExpression (CCodeBinaryOperator.EQUALITY,
new CCodeMemberAccess.pointer
- (inner_error, "domain"), new CCodeIdentifier
(get_ccode_upper_case_name (error_type.data_type)));
+ (get_inner_error_cexpression (), "domain"), new CCodeIdentifier
(get_ccode_upper_case_name (error_type.data_type)));
if (ccond == null) {
ccond = domain_check;
} else {
@@ -281,16 +279,16 @@ public class Vala.GErrorModule : CCodeDelegateModule {
if (ccond != null) {
ccode.open_if (ccond);
- return_with_exception (inner_error);
+ return_with_exception (get_inner_error_cexpression ());
ccode.add_else ();
- uncaught_error_statement (inner_error);
+ uncaught_error_statement (get_inner_error_cexpression ());
ccode.close ();
} else {
- return_with_exception (inner_error);
+ return_with_exception (get_inner_error_cexpression ());
}
} else {
- uncaught_error_statement (inner_error);
+ uncaught_error_statement (get_inner_error_cexpression ());
}
if (!always_fails) {
@@ -330,7 +328,11 @@ public class Vala.GErrorModule : CCodeDelegateModule {
ccode.add_label ("__finally%d".printf (this_try_id));
if (stmt.finally_body != null) {
+ // use a dedicated inner_error variable, if there
+ // is some error handling happening in finally-block
+ current_inner_error_id++;
stmt.finally_body.emit (this);
+ current_inner_error_id--;
}
// check for errors not handled by this try statement
@@ -352,14 +354,14 @@ public class Vala.GErrorModule : CCodeDelegateModule {
if (clause.error_variable != null) {
visit_local_variable (clause.error_variable);
- ccode.add_assignment (get_variable_cexpression (get_local_cname
(clause.error_variable)), get_variable_cexpression ("_inner_error_"));
+ ccode.add_assignment (get_variable_cexpression (get_local_cname
(clause.error_variable)), get_inner_error_cexpression ());
} else {
// error object is not used within catch statement, clear it
var cclear = new CCodeFunctionCall (new CCodeIdentifier ("g_clear_error"));
- cclear.add_argument (new CCodeUnaryExpression (CCodeUnaryOperator.ADDRESS_OF,
get_variable_cexpression ("_inner_error_")));
+ cclear.add_argument (new CCodeUnaryExpression (CCodeUnaryOperator.ADDRESS_OF,
get_inner_error_cexpression ()));
ccode.add_expression (cclear);
}
- ccode.add_assignment (get_variable_cexpression ("_inner_error_"), new CCodeConstant ("NULL"));
+ ccode.add_assignment (get_inner_error_cexpression (), new CCodeConstant ("NULL"));
clause.body.emit (this);
diff --git a/codegen/valagobjectmodule.vala b/codegen/valagobjectmodule.vala
index ad808a138..a432019ed 100644
--- a/codegen/valagobjectmodule.vala
+++ b/codegen/valagobjectmodule.vala
@@ -476,7 +476,7 @@ public class Vala.GObjectModule : GTypeModule {
/* always separate error parameter and inner_error local variable
* as error may be set to NULL but we're always interested in inner errors
*/
- ccode.add_declaration ("GError *", new CCodeVariableDeclarator.zero
("_inner_error_", new CCodeConstant ("NULL")));
+ ccode.add_declaration ("GError*", new CCodeVariableDeclarator.zero
("_inner_error%d_".printf (current_inner_error_id), new CCodeConstant ("NULL")));
}
ccode.add_return (new CCodeIdentifier ("obj"));
@@ -502,7 +502,7 @@ public class Vala.GObjectModule : GTypeModule {
/* always separate error parameter and inner_error local variable
* as error may be set to NULL but we're always interested in inner errors
*/
- ccode.add_declaration ("GError *", new CCodeVariableDeclarator.zero
("_inner_error_", new CCodeConstant ("NULL")));
+ ccode.add_declaration ("GError*", new CCodeVariableDeclarator.zero
("_inner_error%d_".printf (current_inner_error_id), new CCodeConstant ("NULL")));
}
pop_context ();
@@ -524,7 +524,7 @@ public class Vala.GObjectModule : GTypeModule {
/* always separate error parameter and inner_error local variable
* as error may be set to NULL but we're always interested in inner errors
*/
- ccode.add_declaration ("GError *", new CCodeVariableDeclarator.zero
("_inner_error_", new CCodeConstant ("NULL")));
+ ccode.add_declaration ("GError*", new CCodeVariableDeclarator.zero
("_inner_error%d_".printf (current_inner_error_id), new CCodeConstant ("NULL")));
}
pop_context ();
diff --git a/codegen/valagtypemodule.vala b/codegen/valagtypemodule.vala
index a654c0197..f74b5fabf 100644
--- a/codegen/valagtypemodule.vala
+++ b/codegen/valagtypemodule.vala
@@ -1802,7 +1802,7 @@ public class Vala.GTypeModule : GErrorModule {
cl.destructor.body.emit (this);
if (current_method_inner_error) {
- ccode.add_declaration ("GError *", new CCodeVariableDeclarator.zero
("_inner_error_", new CCodeConstant ("NULL")));
+ ccode.add_declaration ("GError*", new CCodeVariableDeclarator.zero
("_inner_error%d_".printf (current_inner_error_id), new CCodeConstant ("NULL")));
}
if (current_method_return) {
diff --git a/tests/Makefile.am b/tests/Makefile.am
index 144a095da..fe5aa20ae 100644
--- a/tests/Makefile.am
+++ b/tests/Makefile.am
@@ -348,6 +348,7 @@ TESTS = \
objects/bug795225-4.test \
objects/bug795521.vala \
errors/catch-error-code.vala \
+ errors/catch-in-finally.vala \
errors/errors.vala \
errors/errordomain.vala \
errors/invalid-type-check.test \
@@ -395,6 +396,7 @@ TESTS = \
asynchronous/bug792660.vala \
asynchronous/bug792942.vala \
asynchronous/bug793158.vala \
+ asynchronous/catch-in-finally.vala \
asynchronous/closures.vala \
asynchronous/generator.vala \
asynchronous/result-pos.vala \
diff --git a/tests/asynchronous/catch-in-finally.vala b/tests/asynchronous/catch-in-finally.vala
new file mode 100644
index 000000000..9f0f95226
--- /dev/null
+++ b/tests/asynchronous/catch-in-finally.vala
@@ -0,0 +1,72 @@
+errordomain FooError {
+ FAIL
+}
+
+async void fail () throws FooError {
+ throw new FooError.FAIL ("fail");
+}
+
+async void may_fail () throws FooError {
+}
+
+async void foo () throws FooError {
+ try {
+ yield fail ();
+ } finally {
+ try {
+ yield may_fail ();
+ } catch (FooError e) {
+ assert_not_reached ();
+ }
+ }
+ assert_not_reached ();
+}
+
+async void bar () throws FooError {
+ try {
+ yield may_fail ();
+ } finally {
+ try {
+ yield fail ();
+ } catch (FooError e) {
+ }
+ }
+
+ try {
+ yield fail ();
+ } finally {
+ try {
+ yield may_fail ();
+ } catch (FooError e) {
+ assert_not_reached ();
+ }
+ }
+ assert_not_reached ();
+}
+
+async void run () {
+ foo.begin ((o,r) => {
+ try {
+ foo.end (r);
+ } catch (FooError e) {
+ assert (e.message == "fail");
+ }
+ });
+
+ bar.begin ((o,r) => {
+ try {
+ bar.end (r);
+ } catch (FooError e) {
+ assert (e.message == "fail");
+ loop.quit ();
+ }
+ });
+}
+
+MainLoop loop;
+
+void main () {
+ loop = new MainLoop ();
+ run.begin ();
+ loop.run ();
+}
diff --git a/tests/errors/catch-in-finally.vala b/tests/errors/catch-in-finally.vala
new file mode 100644
index 000000000..8845e4b6b
--- /dev/null
+++ b/tests/errors/catch-in-finally.vala
@@ -0,0 +1,59 @@
+errordomain FooError {
+ FAIL
+}
+
+void fail () throws FooError {
+ throw new FooError.FAIL ("fail");
+}
+
+void may_fail () throws FooError {
+}
+
+void foo () throws FooError {
+ try {
+ fail ();
+ } finally {
+ try {
+ may_fail ();
+ } catch (FooError e) {
+ assert_not_reached ();
+ }
+ }
+ assert_not_reached ();
+}
+
+void bar () throws FooError {
+ try {
+ may_fail ();
+ } finally {
+ try {
+ fail ();
+ } catch (FooError e) {
+ }
+ }
+
+ try {
+ fail ();
+ } finally {
+ try {
+ may_fail ();
+ } catch (FooError e) {
+ assert_not_reached ();
+ }
+ }
+ assert_not_reached ();
+}
+
+void main () {
+ try {
+ foo ();
+ } catch (FooError e) {
+ assert (e.message == "fail");
+ }
+
+ try {
+ bar ();
+ } catch (FooError e) {
+ assert (e.message == "fail");
+ }
+}
[
Date Prev][
Date Next] [
Thread Prev][
Thread Next]
[
Thread Index]
[
Date Index]
[
Author Index]
