From: NeilBrown <neil@brown.name>
Date: Mon, 8 Nov 2021 09:19:29 +0000 (+1100)
Subject: oceani: add comments to var_block_close() and remove dead code.
X-Git-Url: https://ocean-lang.org/code/?p=ocean;a=commitdiff_plain;h=fdc93978981976920d369c57b80da002374090a5

oceani: add comments to var_block_close() and remove dead code.

This function is subtle and deserves good documentation for various
cases.
Some of the cases were dead, so remove them.

Signed-off-by: NeilBrown <neil@brown.name>
---

diff --git a/csrc/oceani-tests.mdc b/csrc/oceani-tests.mdc
index 794d918..fc9f0bd 100644
--- a/csrc/oceani-tests.mdc
+++ b/csrc/oceani-tests.mdc
@@ -80,7 +80,7 @@ arguments separated from the name by commas.  For each test, there is a section
 		@mv *.gcov coverage ; [ -f .gcov ] && mv .gcov coverage || true
 		@ awk '/NOTEST/ { next } /^ *[1-9]/ {ran+=1} /^ *###/ {skip+=1} \
 		    END {printf "coverage: %6.2f%%\n", ran * 100 / (ran + skip); \
-		         if (ran < (ran + skip) *0.956) exit(1) }' \
+		         if (ran < (ran + skip) *0.958) exit(1) }' \
 		        coverage/oceani.mdc.gcov
 		@rm -f .tmp*
 
diff --git a/csrc/oceani.mdc b/csrc/oceani.mdc
index 04cae63..7e1f2ac 100644
--- a/csrc/oceani.mdc
+++ b/csrc/oceani.mdc
@@ -1141,7 +1141,14 @@ all pending-scope variables become conditionally scoped.
 
 	static void var_block_close(struct parse_context *c, enum closetype ct)
 	{
-		/* Close off all variables that are in_scope */
+		/* Close off all variables that are in_scope.
+		 * Some variables in c->scope may already be not-in-scope,
+		 * such as when a PendingScope variable is hidden by a new
+		 * variable with the same name.
+		 * So we check for v->name->var != v and drop them.
+		 * If we choose to make a variable OutScope, we drop it
+		 * immediately too.
+		 */
 		struct variable *v, **vp, *v2;
 
 		scope_pop(c);
@@ -1151,12 +1158,11 @@ all pending-scope variables become conditionally scoped.
 		     ? (*vp =  v->in_scope, 0)
 		     : ( vp = &v->in_scope, 0)) {
 			v->min_depth = c->scope_depth;
-			if (v->name->var != v) {
+			if (v->name->var != v)
 				/* This is still in scope, but we haven't just
 				 * closed the scope.
 				 */
 				continue;
-			}
 			switch (ct) {
 			case CloseElse:
 			case CloseParallel: /* handle PendingScope */
@@ -1164,14 +1170,15 @@ all pending-scope variables become conditionally scoped.
 				case InScope:
 				case CondScope:
 					if (c->scope_stack->child_count == 1)
+						/* first among parallel branches */
 						v->scope = PendingScope;
 					else if (v->previous &&
 					         v->previous->scope == PendingScope)
+						/* all previous branches used name */
 						v->scope = PendingScope;
 					else if (v->type == Tlabel)	// UNTESTED
+						/* Labels remain pending even when not used */
 						v->scope = PendingScope;	// UNTESTED
-					else if (v->name->var == v)	// UNTESTED
-						v->scope = OutScope;	// UNTESTED
 					if (ct == CloseElse) {
 						/* All Pending variables with this name
 						 * are now Conditional */
@@ -1182,13 +1189,16 @@ all pending-scope variables become conditionally scoped.
 					}
 					break;
 				case PendingScope:
-					for (v2 = v;
-					     v2 && v2->scope == PendingScope;
-					     v2 = v2->previous)
-						if (v2->type != Tlabel)
-							v2->scope = OutScope;
-					break;
-				case OutScope: break;	// UNTESTED
+					/* Not possible as it would require
+					 * parallel scope to be nested immediately
+					 * in a parallel scope, and that never
+					 * happens.
+					 */
+				case OutScope:
+					/* Not possible as we already tested for
+					 * OutScope
+					 */
+					abort();		// NOTEST
 				}
 				break;
 			case CloseSequential: