[vala] Fix lock statement

[Jürg Billeter <juergbi src gnome org>]

commit 40c1dbfbfedb6c4a6b88df045eb1c2e7bdd38d93
Author: JiÅ?í Zárevúcky <zarevucky jiri gmail com>
Date:   Wed Mar 24 08:36:37 2010 +0100

    Fix lock statement
    
    This patch converts lock statements into try finally statements to
    ensure that unlock is always called.
    
    Fixes bug 582553.

 codegen/valaccodebasemodule.vala |   40 +++++++++++++---------
 codegen/valaccodegenerator.vala  |    4 ++
 codegen/valaccodemodule.vala     |    4 ++
 vala/Makefile.am                 |    1 +
 vala/valacodevisitor.vala        |    8 ++++
 vala/valaflowanalyzer.vala       |    6 +++-
 vala/valalockstatement.vala      |   39 +++++++++++++++++----
 vala/valaunlockstatement.vala    |   70 ++++++++++++++++++++++++++++++++++++++
 8 files changed, 147 insertions(+), 25 deletions(-)
---
diff --git a/codegen/valaccodebasemodule.vala b/codegen/valaccodebasemodule.vala
index 89ef3d5..223d49a 100644
--- a/codegen/valaccodebasemodule.vala
+++ b/codegen/valaccodebasemodule.vala
@@ -3400,28 +3400,26 @@ internal class Vala.CCodeBaseModule : CCodeModule {
 		return "__lock_%s".printf (symname);
 	}
 
-	public override void visit_lock_statement (LockStatement stmt) {
-		var cn = new CCodeFragment ();
+	private CCodeExpression get_lock_expression (Statement stmt, Expression resource) {
 		CCodeExpression l = null;
-		CCodeFunctionCall fc;
-		var inner_node = ((MemberAccess)stmt.resource).inner;
-		var member = (Member)stmt.resource.symbol_reference;
-		var parent = (TypeSymbol) stmt.resource.symbol_reference.parent_symbol;
+		var inner_node = ((MemberAccess)resource).inner;
+		var member = (Member)resource.symbol_reference;
+		var parent = (TypeSymbol)resource.symbol_reference.parent_symbol;
 		
 		if (member.is_instance_member ()) {
 			if (inner_node  == null) {
 				l = new CCodeIdentifier ("self");
-			} else if (stmt.resource.symbol_reference.parent_symbol != current_type_symbol) {
-				 l = generate_instance_cast ((CCodeExpression) inner_node.ccodenode, parent);
+			} else if (resource.symbol_reference.parent_symbol != current_type_symbol) {
+				l = generate_instance_cast ((CCodeExpression) inner_node.ccodenode, parent);
 			} else {
 				l = (CCodeExpression) inner_node.ccodenode;
 			}
 
-			l = new CCodeMemberAccess.pointer (new CCodeMemberAccess.pointer (l, "priv"), get_symbol_lock_name (stmt.resource.symbol_reference.name));
+			l = new CCodeMemberAccess.pointer (new CCodeMemberAccess.pointer (l, "priv"), get_symbol_lock_name (resource.symbol_reference.name));
 		} else if (member.is_class_member ()) {
 			CCodeExpression klass;
 
-		        if (current_method != null && current_method.binding == MemberBinding.INSTANCE ||
+			if (current_method != null && current_method.binding == MemberBinding.INSTANCE ||
 			    current_property_accessor != null && current_property_accessor.prop.binding == MemberBinding.INSTANCE ||
 			    (in_constructor && !in_static_or_class_context)) {
 				var k = new CCodeFunctionCall (new CCodeIdentifier ("G_OBJECT_GET_CLASS"));
@@ -3433,23 +3431,33 @@ internal class Vala.CCodeBaseModule : CCodeModule {
 
 			var get_class_private_call = new CCodeFunctionCall (new CCodeIdentifier ("%s_GET_CLASS_PRIVATE".printf(parent.get_upper_case_cname ())));
 			get_class_private_call.add_argument (klass);
-			l = new CCodeMemberAccess.pointer (get_class_private_call, get_symbol_lock_name (stmt.resource.symbol_reference.name));
+			l = new CCodeMemberAccess.pointer (get_class_private_call, get_symbol_lock_name (resource.symbol_reference.name));
 		} else {
-			string lock_name = "%s_%s".printf(parent.get_lower_case_cname (), stmt.resource.symbol_reference.name);
+			string lock_name = "%s_%s".printf(parent.get_lower_case_cname (), resource.symbol_reference.name);
 			l = new CCodeIdentifier (get_symbol_lock_name (lock_name));
 		}
+		return l;
+	}
 		
-		fc = new CCodeFunctionCall (new CCodeIdentifier (((Method) mutex_type.scope.lookup ("lock")).get_cname ()));
+	public override void visit_lock_statement (LockStatement stmt) {
+		var l = get_lock_expression (stmt, stmt.resource);
+
+		var fc = new CCodeFunctionCall (new CCodeIdentifier (((Method) mutex_type.scope.lookup ("lock")).get_cname ()));
 		fc.add_argument (new CCodeUnaryExpression (CCodeUnaryOperator.ADDRESS_OF, l));
 
+		var cn = new CCodeFragment ();
 		cn.append (new CCodeExpressionStatement (fc));
+		stmt.ccodenode = cn;
+	}
 		
-		cn.append (stmt.body.ccodenode);
+	public override void visit_unlock_statement (UnlockStatement stmt) {
+		var l = get_lock_expression (stmt, stmt.resource);
 		
-		fc = new CCodeFunctionCall (new CCodeIdentifier (((Method) mutex_type.scope.lookup ("unlock")).get_cname ()));
+		var fc = new CCodeFunctionCall (new CCodeIdentifier (((Method) mutex_type.scope.lookup ("unlock")).get_cname ()));
 		fc.add_argument (new CCodeUnaryExpression (CCodeUnaryOperator.ADDRESS_OF, l));
-		cn.append (new CCodeExpressionStatement (fc));
 		
+		var cn = new CCodeFragment ();
+		cn.append (new CCodeExpressionStatement (fc));
 		stmt.ccodenode = cn;
 	}
 
diff --git a/codegen/valaccodegenerator.vala b/codegen/valaccodegenerator.vala
index fd465d1..f8af697 100644
--- a/codegen/valaccodegenerator.vala
+++ b/codegen/valaccodegenerator.vala
@@ -228,6 +228,10 @@ public class Vala.CCodeGenerator : CodeGenerator {
 		head.visit_lock_statement (stmt);
 	}
 
+	public override void visit_unlock_statement (UnlockStatement stmt) {
+		head.visit_unlock_statement (stmt);
+	}
+
 	public override void visit_delete_statement (DeleteStatement stmt) {
 		head.visit_delete_statement (stmt);
 	}
diff --git a/codegen/valaccodemodule.vala b/codegen/valaccodemodule.vala
index d8e88e3..0355e0c 100644
--- a/codegen/valaccodemodule.vala
+++ b/codegen/valaccodemodule.vala
@@ -207,6 +207,10 @@ public abstract class Vala.CCodeModule {
 		next.visit_lock_statement (stmt);
 	}
 
+	public virtual void visit_unlock_statement (UnlockStatement stmt) {
+		next.visit_unlock_statement (stmt);
+	}
+
 	public virtual void visit_delete_statement (DeleteStatement stmt) {
 		next.visit_delete_statement (stmt);
 	}
diff --git a/vala/Makefile.am b/vala/Makefile.am
index 69f0d0a..aa0b861 100644
--- a/vala/Makefile.am
+++ b/vala/Makefile.am
@@ -151,6 +151,7 @@ libvalacore_la_VALASOURCES = \
 	valatypeparameter.vala \
 	valatypesymbol.vala \
 	valaunaryexpression.vala \
+	valaunlockstatement.vala \
 	valaunresolvedsymbol.vala \
 	valaunresolvedtype.vala \
 	valausingdirective.vala \
diff --git a/vala/valacodevisitor.vala b/vala/valacodevisitor.vala
index f11a3ef..55f87d7 100644
--- a/vala/valacodevisitor.vala
+++ b/vala/valacodevisitor.vala
@@ -405,6 +405,14 @@ public abstract class Vala.CodeVisitor {
 	}
 
 	/**
+	 * Visit operation called for unlock statements.
+	 *
+	 * @param stmt an unlock statement
+	 */
+	public virtual void visit_unlock_statement (UnlockStatement stmt) {
+	}
+
+	/**
 	 * Visit operation called for delete statements.
 	 *
 	 * @param stmt a delete statement
diff --git a/vala/valaflowanalyzer.vala b/vala/valaflowanalyzer.vala
index ff5f255..193f610 100644
--- a/vala/valaflowanalyzer.vala
+++ b/vala/valaflowanalyzer.vala
@@ -1010,8 +1010,12 @@ public class Vala.FlowAnalyzer : CodeVisitor {
 		if (unreachable (stmt)) {
 			return;
 		}
+	}
 
-		stmt.body.accept (this);
+	public override void visit_unlock_statement (UnlockStatement stmt) {
+		if (unreachable (stmt)) {
+			return;
+		}
 	}
 
 	public override void visit_expression (Expression expr) {
diff --git a/vala/valalockstatement.vala b/vala/valalockstatement.vala
index 9ad5dfb..da9a0be 100644
--- a/vala/valalockstatement.vala
+++ b/vala/valalockstatement.vala
@@ -1,5 +1,6 @@
 /* valalockstatement.vala
  *
+ * Copyright (C) 2009  JiÅ?í Zárevúcky
  * Copyright (C) 2006-2007  Raffaele Sandrini, Jürg Billeter
  *
  * This library is free software; you can redistribute it and/or
@@ -16,14 +17,19 @@
  * License along with this library; if not, write to the Free Software
  * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301  USA
  *
- * Author:
+ * Authors:
  * 	Raffaele Sandrini <raffaele sandrini ch>
+ * 	JiÅ?í Zárevúcky <zarevucky jiri gmail com>
  */
 
 using GLib;
 
 /**
- * Represents a lock statement e.g. {{{ lock (a) { f(a) } }}}.
+ * Represents a lock statement e.g. {{{ lock (a); }}} or {{{ lock (a) { f(a); } }}}.
+ *
+ * If the statement is empty, the mutex remains locked until a corresponding UnlockStatement
+ * occurs. Otherwise it's translated into a try/finally statement which unlocks the mutex
+ * after the block is finished.
  */
 public class Vala.LockStatement : CodeNode, Statement {
 	/**
@@ -34,9 +40,9 @@ public class Vala.LockStatement : CodeNode, Statement {
 	/**
 	 * The statement during its execution the resource is locked.
 	 */
-	public Block body { get; set; }
+	public Block? body { get; set; }
 	
-	public LockStatement (Expression resource, Block body, SourceReference? source_reference = null) {
+	public LockStatement (Expression resource, Block? body, SourceReference? source_reference = null) {
 		this.body = body;
 		this.source_reference = source_reference;
 		this.resource = resource;
@@ -44,11 +50,29 @@ public class Vala.LockStatement : CodeNode, Statement {
 	
 	public override void accept (CodeVisitor visitor) {
 		resource.accept (visitor);
-		body.accept (visitor);
+		if (body != null) {
+			body.accept (visitor);
+		}
 		visitor.visit_lock_statement (this);
 	}
 
 	public override bool check (SemanticAnalyzer analyzer) {
+		if (body != null) {
+			// if the statement isn't empty, it is converted into a try statement
+
+			var fin_body = new Block (source_reference);
+			fin_body.add_statement (new UnlockStatement (resource, source_reference));
+
+			var block = new Block (source_reference);
+			block.add_statement (new LockStatement (resource, null, source_reference));
+			block.add_statement (new TryStatement (body, fin_body, source_reference));
+
+			var parent_block = (Block) parent_node;
+			parent_block.replace_statement (this, block);
+
+			return block.check (analyzer);
+		}
+
 		if (checked) {
 			return !error;
 		}
@@ -56,11 +80,10 @@ public class Vala.LockStatement : CodeNode, Statement {
 		checked = true;
 
 		resource.check (analyzer);
-		body.check (analyzer);
 
 		/* resource must be a member access and denote a Lockable */
 		if (!(resource is MemberAccess && resource.symbol_reference is Lockable)) {
-		    	error = true;
+			error = true;
 			resource.error = true;
 			Report.error (resource.source_reference, "Expression is either not a member access or does not denote a lockable member");
 			return false;
@@ -68,7 +91,7 @@ public class Vala.LockStatement : CodeNode, Statement {
 
 		/* parent symbol must be the current class */
 		if (resource.symbol_reference.parent_symbol != analyzer.current_class) {
-		    	error = true;
+			error = true;
 			resource.error = true;
 			Report.error (resource.source_reference, "Only members of the current class are lockable");
 		}
diff --git a/vala/valaunlockstatement.vala b/vala/valaunlockstatement.vala
new file mode 100644
index 0000000..e0ae1a0
--- /dev/null
+++ b/vala/valaunlockstatement.vala
@@ -0,0 +1,70 @@
+/* valaunlockstatement.vala
+ *
+ * Copyright (C) 2009  JiÅ?í Zárevúcky, Jürg Billeter
+ *
+ * This library is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 2.1 of the License, or (at your option) any later version.
+
+ * This library is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * Lesser General Public License for more details.
+
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with this library; if not, write to the Free Software
+ * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301  USA
+ *
+ * Author:
+ * 	JiÅ?í Zárevúcky <zarevucky jiri gmail com>
+ */
+
+/**
+ * Represents an unlock statement e.g. {{{ unlock (a); }}}.
+ */
+public class Vala.UnlockStatement : CodeNode, Statement {
+	/**
+	 * Expression representing the resource to be unlocked.
+	 */
+	public Expression resource { get; set; }
+
+	public UnlockStatement (Expression resource, SourceReference? source_reference = null) {
+		this.source_reference = source_reference;
+		this.resource = resource;
+	}
+
+	public override void accept (CodeVisitor visitor) {
+		resource.accept (visitor);
+		visitor.visit_unlock_statement (this);
+	}
+
+	public override bool check (SemanticAnalyzer analyzer) {
+		if (checked) {
+			return !error;
+		}
+
+		checked = true;
+
+		resource.check (analyzer);
+
+		/* resource must be a member access and denote a Lockable */
+		if (!(resource is MemberAccess && resource.symbol_reference is Lockable)) {
+			error = true;
+			resource.error = true;
+			Report.error (resource.source_reference, "Expression is either not a member access or does not denote a lockable member");
+			return false;
+		}
+
+		/* parent symbol must be the current class */
+		if (resource.symbol_reference.parent_symbol != analyzer.current_class) {
+			error = true;
+			resource.error = true;
+			Report.error (resource.source_reference, "Only members of the current class are lockable");
+		}
+
+		((Lockable) resource.symbol_reference).set_lock_used (true);
+
+		return !error;
+	}
+}