Re: [Vala] Improved foreach

[Jamie McCracken <jamie mccrack googlemail com>]

On Sun, 2008-05-25 at 23:51 +0200, Jürg Billeter wrote:
> On Sun, 2008-05-25 at 00:51 -0400, Jamie McCracken wrote:
> > Var to be used in foreach
> 
> We should certainly allow this, also in Vala. The logic in
> ForeachStatement.accept_children is slightly broken as you're missing
> visit_end_full_expression in the "var" case. It also seems that you
> could simplify some changes in the semantic analyzer by reordering but
> difficult to see as you've mixed two change sets, please keep patches
> separated.
> 

updated patch attached with just the VAR in foreach

in the var case, accept_collection is called first then element type is
calculated and type reference is set then accept_children is called
which will now hit visit_end_full_expression as type-reference is no
longer null

if above is not right then please explain...

jamie

Index: vala/valaforeachstatement.vala
===================================================================
--- vala/valaforeachstatement.vala	(revision 1419)
+++ vala/valaforeachstatement.vala	(working copy)
@@ -30,11 +30,13 @@
 	/**
 	 * Specifies the element type.
 	 */
-	public DataType type_reference {
+	public DataType? type_reference {
 		get { return _data_type; }
 		set {
 			_data_type = value;
-			_data_type.parent_node = this;
+			if (_data_type != null) {
+				_data_type.parent_node = this;
+			}
 		}
 	}
 	
@@ -98,7 +100,7 @@
 	 * @param source reference to source code
 	 * @return       newly created foreach statement
 	 */
-	public ForeachStatement (DataType type_reference, string variable_name, Expression collection, Block body, SourceReference source_reference) {
+	public ForeachStatement (DataType? type_reference, string variable_name, Expression collection, Block body, SourceReference source_reference) {
 		this.variable_name = variable_name;
 		this.collection = collection;
 		this.body = body;
@@ -111,13 +113,22 @@
 	}
 
 	public override void accept_children (CodeVisitor visitor) {
-		type_reference.accept (visitor);
-
-		collection.accept (visitor);
-		visitor.visit_end_full_expression (collection);
-
+	
+		if (type_reference == null) {
+			collection.accept (visitor);
+		} else {
+		
+			type_reference.accept (visitor);
+			collection.accept (visitor);
+			visitor.visit_end_full_expression (collection);
+		}
 		body.accept (visitor);
+		
 	}
+	
+	public void accept_collection (CodeVisitor visitor) {
+		collection.accept (visitor);
+	}
 
 	public override void replace_expression (Expression old_node, Expression new_node) {
 		if (collection == old_node) {
Index: vala/valasemanticanalyzer.vala
===================================================================
--- vala/valasemanticanalyzer.vala	(revision 1419)
+++ vala/valasemanticanalyzer.vala	(working copy)
@@ -1016,7 +1016,7 @@
 			stmt.error = true;
 			Report.error (stmt.condition.source_reference, "Condition must be boolean");
 			return;
-		}
+		} 
 	}
 
 	public override void visit_for_statement (ForStatement stmt) {
@@ -1036,20 +1036,13 @@
 	}
 
 	public override void visit_foreach_statement (ForeachStatement stmt) {
-		current_source_file.add_type_dependency (stmt.type_reference, SourceFileDependencyType.SOURCE);
 
-		stmt.element_variable = new LocalVariable (stmt.type_reference, stmt.variable_name);
-
-		stmt.body.scope.add (stmt.variable_name, stmt.element_variable);
-
-		stmt.body.add_local_variable (stmt.element_variable);
-		stmt.element_variable.active = true;
-
 		stmt.owner = current_symbol.scope;
 		current_symbol = stmt;
 
-		stmt.accept_children (this);
-
+		/* handle collection first in case we need to infer the data type of the type reference */
+		stmt.accept_collection (this);
+		
 		foreach (LocalVariable local in stmt.get_local_variables ()) {
 			local.active = false;
 		}
@@ -1067,11 +1060,7 @@
 		}
 
 		var collection_type = stmt.collection.value_type.copy ();
-		stmt.collection_variable = new LocalVariable (collection_type, "%s_collection".printf (stmt.variable_name));
-
-		stmt.add_local_variable (stmt.collection_variable);
-		stmt.collection_variable.active = true;
-	
+		
 		DataType element_data_type = null;
 		bool need_type_check = false;
 	
@@ -1108,7 +1097,26 @@
 			Report.error (stmt.source_reference, "Collection not iterable");
 			return;
 		}
+		
+	
+		/* can be null if inference is needed */
+		if (stmt.type_reference == null) {
+			stmt.type_reference = element_data_type.copy ();
+		}
+		
+		current_source_file.add_type_dependency (stmt.type_reference, SourceFileDependencyType.SOURCE);
+		stmt.element_variable = new LocalVariable (stmt.type_reference, stmt.variable_name);
+		stmt.body.scope.add (stmt.variable_name, stmt.element_variable);
+		stmt.body.add_local_variable (stmt.element_variable);
+		stmt.element_variable.active = true;
+		
+		stmt.accept_children (this);
+		
+		stmt.collection_variable = new LocalVariable (collection_type, "%s_collection".printf (stmt.variable_name));
 
+		stmt.add_local_variable (stmt.collection_variable);
+		stmt.collection_variable.active = true;
+
 		if (need_type_check && element_data_type != null) {
 			/* allow implicit upcasts ? */
 			if (!element_data_type.compatible (stmt.type_reference)) {
@@ -1486,11 +1494,10 @@
 		// don't call g_object_ref_sink on inner and outer expression
 		expr.value_type.floating_reference = false;
 	}
-
+	
 	public override void visit_member_access (MemberAccess expr) {
 		Symbol base_symbol = null;
 		bool may_access_instance_members = false;
-
 		expr.symbol_reference = null;
 
 		if (expr.inner == null) {