Detect mismatched CONTINUE and EXIT statements at plpgsql compile time.

With a bit of tweaking of the compile namestack data structure, we can
verify at compile time whether a CONTINUE or EXIT is legal.  This is
surely better than leaving it to runtime, both because earlier is better
and because we can issue a proper error pointer.  Also, we can get rid
of the ad-hoc old way of detecting the problem, which only took care of
CONTINUE not EXIT.

Jim Nasby, adjusted a bit by me

5 files changed, 107 insertions, 55 deletions

diff --git a/src/pl/plpgsql/src/pl_comp.c b/src/pl/plpgsql/src/pl_comp.c
index 05268e3d340..1ae4bb7e7cf 100644
--- a/src/pl/plpgsql/src/pl_comp.c
+++ b/src/pl/plpgsql/src/pl_comp.c
@@ -371,7 +371,7 @@ do_compile(FunctionCallInfo fcinfo,
 	 * variables (such as FOUND), and is named after the function itself.
 	 */
 	plpgsql_ns_init();
-	plpgsql_ns_push(NameStr(procStruct->proname));
+	plpgsql_ns_push(NameStr(procStruct->proname), PLPGSQL_LABEL_BLOCK);
 	plpgsql_DumpExecTree = false;
 	plpgsql_start_datums();
 
@@ -851,7 +851,7 @@ plpgsql_compile_inline(char *proc_source)
 	function->extra_errors = 0;
 
 	plpgsql_ns_init();
-	plpgsql_ns_push(func_name);
+	plpgsql_ns_push(func_name, PLPGSQL_LABEL_BLOCK);
 	plpgsql_DumpExecTree = false;
 	plpgsql_start_datums();
 
diff --git a/src/pl/plpgsql/src/pl_exec.c b/src/pl/plpgsql/src/pl_exec.c
index fb9333606d4..935fa628511 100644
--- a/src/pl/plpgsql/src/pl_exec.c
+++ b/src/pl/plpgsql/src/pl_exec.c
@@ -437,19 +437,9 @@ plpgsql_exec_function(PLpgSQL_function *func, FunctionCallInfo fcinfo,
 	{
 		estate.err_stmt = NULL;
 		estate.err_text = NULL;
-
-		/*
-		 * Provide a more helpful message if a CONTINUE has been used outside
-		 * the context it can work in.
-		 */
-		if (rc == PLPGSQL_RC_CONTINUE)
-			ereport(ERROR,
-					(errcode(ERRCODE_SYNTAX_ERROR),
-					 errmsg("CONTINUE cannot be used outside a loop")));
-		else
-			ereport(ERROR,
-			   (errcode(ERRCODE_S_R_E_FUNCTION_EXECUTED_NO_RETURN_STATEMENT),
-				errmsg("control reached end of function without RETURN")));
+		ereport(ERROR,
+				(errcode(ERRCODE_S_R_E_FUNCTION_EXECUTED_NO_RETURN_STATEMENT),
+				 errmsg("control reached end of function without RETURN")));
 	}
 
 	/*
@@ -791,19 +781,9 @@ plpgsql_exec_trigger(PLpgSQL_function *func,
 	{
 		estate.err_stmt = NULL;
 		estate.err_text = NULL;
-
-		/*
-		 * Provide a more helpful message if a CONTINUE has been used outside
-		 * the context it can work in.
-		 */
-		if (rc == PLPGSQL_RC_CONTINUE)
-			ereport(ERROR,
-					(errcode(ERRCODE_SYNTAX_ERROR),
-					 errmsg("CONTINUE cannot be used outside a loop")));
-		else
-			ereport(ERROR,
-			   (errcode(ERRCODE_S_R_E_FUNCTION_EXECUTED_NO_RETURN_STATEMENT),
-				errmsg("control reached end of trigger procedure without RETURN")));
+		ereport(ERROR,
+				(errcode(ERRCODE_S_R_E_FUNCTION_EXECUTED_NO_RETURN_STATEMENT),
+		 errmsg("control reached end of trigger procedure without RETURN")));
 	}
 
 	estate.err_stmt = NULL;
@@ -919,19 +899,9 @@ plpgsql_exec_event_trigger(PLpgSQL_function *func, EventTriggerData *trigdata)
 	{
 		estate.err_stmt = NULL;
 		estate.err_text = NULL;
-
-		/*
-		 * Provide a more helpful message if a CONTINUE has been used outside
-		 * the context it can work in.
-		 */
-		if (rc == PLPGSQL_RC_CONTINUE)
-			ereport(ERROR,
-					(errcode(ERRCODE_SYNTAX_ERROR),
-					 errmsg("CONTINUE cannot be used outside a loop")));
-		else
-			ereport(ERROR,
-			   (errcode(ERRCODE_S_R_E_FUNCTION_EXECUTED_NO_RETURN_STATEMENT),
-				errmsg("control reached end of trigger procedure without RETURN")));
+		ereport(ERROR,
+				(errcode(ERRCODE_S_R_E_FUNCTION_EXECUTED_NO_RETURN_STATEMENT),
+		 errmsg("control reached end of trigger procedure without RETURN")));
 	}
 
 	estate.err_stmt = NULL;
diff --git a/src/pl/plpgsql/src/pl_funcs.c b/src/pl/plpgsql/src/pl_funcs.c
index 900ba3c13b6..1b29024832e 100644
--- a/src/pl/plpgsql/src/pl_funcs.c
+++ b/src/pl/plpgsql/src/pl_funcs.c
@@ -51,11 +51,11 @@ plpgsql_ns_init(void)
  * ----------
  */
 void
-plpgsql_ns_push(const char *label)
+plpgsql_ns_push(const char *label, enum PLpgSQL_label_types label_type)
 {
 	if (label == NULL)
 		label = "";
-	plpgsql_ns_additem(PLPGSQL_NSTYPE_LABEL, 0, label);
+	plpgsql_ns_additem(PLPGSQL_NSTYPE_LABEL, (int) label_type, label);
 }
 
 
@@ -206,6 +206,25 @@ plpgsql_ns_lookup_label(PLpgSQL_nsitem *ns_cur, const char *name)
 }
 
 
+/* ----------
+ * plpgsql_ns_find_nearest_loop		Find innermost loop label in namespace chain
+ * ----------
+ */
+PLpgSQL_nsitem *
+plpgsql_ns_find_nearest_loop(PLpgSQL_nsitem *ns_cur)
+{
+	while (ns_cur != NULL)
+	{
+		if (ns_cur->itemtype == PLPGSQL_NSTYPE_LABEL &&
+			ns_cur->itemno == PLPGSQL_LABEL_LOOP)
+			return ns_cur;
+		ns_cur = ns_cur->prev;
+	}
+
+	return NULL;				/* no loop found */
+}
+
+
 /*
  * Statement type as a string, for use in error messages etc.
  */
diff --git a/src/pl/plpgsql/src/pl_gram.y b/src/pl/plpgsql/src/pl_gram.y
index 00978909a34..225c62ab36f 100644
--- a/src/pl/plpgsql/src/pl_gram.y
+++ b/src/pl/plpgsql/src/pl_gram.y
@@ -186,7 +186,8 @@ static	void			check_raise_parameters(PLpgSQL_stmt_raise *stmt);
 %type <forvariable>	for_variable
 %type <stmt>	for_control
 
-%type <str>		any_identifier opt_block_label opt_label option_value
+%type <str>		any_identifier opt_block_label opt_loop_label opt_label
+%type <str>		option_value
 
 %type <list>	proc_sect stmt_elsifs stmt_else
 %type <loop_body>	loop_body
@@ -535,7 +536,7 @@ decl_statement	: decl_varname decl_const decl_datatype decl_collate decl_notnull
 										   $4->itemno, $1.name);
 					}
 				| decl_varname opt_scrollable K_CURSOR
-					{ plpgsql_ns_push($1.name); }
+					{ plpgsql_ns_push($1.name, PLPGSQL_LABEL_OTHER); }
 				  decl_cursor_args decl_is_for decl_cursor_query
 					{
 						PLpgSQL_var *new;
@@ -1218,7 +1219,7 @@ opt_case_else	:
 					}
 				;
 
-stmt_loop		: opt_block_label K_LOOP loop_body
+stmt_loop		: opt_loop_label K_LOOP loop_body
 					{
 						PLpgSQL_stmt_loop *new;
 
@@ -1235,7 +1236,7 @@ stmt_loop		: opt_block_label K_LOOP loop_body
 					}
 				;
 
-stmt_while		: opt_block_label K_WHILE expr_until_loop loop_body
+stmt_while		: opt_loop_label K_WHILE expr_until_loop loop_body
 					{
 						PLpgSQL_stmt_while *new;
 
@@ -1253,7 +1254,7 @@ stmt_while		: opt_block_label K_WHILE expr_until_loop loop_body
 					}
 				;
 
-stmt_for		: opt_block_label K_FOR for_control loop_body
+stmt_for		: opt_loop_label K_FOR for_control loop_body
 					{
 						/* This runs after we've scanned the loop body */
 						if ($3->cmd_type == PLPGSQL_STMT_FORI)
@@ -1282,7 +1283,7 @@ stmt_for		: opt_block_label K_FOR for_control loop_body
 						}
 
 						check_labels($1, $4.end_label, $4.end_label_location);
-						/* close namespace started in opt_block_label */
+						/* close namespace started in opt_loop_label */
 						plpgsql_ns_pop();
 					}
 				;
@@ -1603,7 +1604,7 @@ for_variable	: T_DATUM
 					}
 				;
 
-stmt_foreach_a	: opt_block_label K_FOREACH for_variable foreach_slice K_IN K_ARRAY expr_until_loop loop_body
+stmt_foreach_a	: opt_loop_label K_FOREACH for_variable foreach_slice K_IN K_ARRAY expr_until_loop loop_body
 					{
 						PLpgSQL_stmt_foreach_a *new;
 
@@ -1666,6 +1667,42 @@ stmt_exit		: exit_type opt_label opt_exitcond
 						new->label	  = $2;
 						new->cond	  = $3;
 
+						if ($2)
+						{
+							/* We have a label, so verify it exists */
+							PLpgSQL_nsitem *label;
+
+							label = plpgsql_ns_lookup_label(plpgsql_ns_top(), $2);
+							if (label == NULL)
+								ereport(ERROR,
+										(errcode(ERRCODE_SYNTAX_ERROR),
+										 errmsg("label \"%s\" does not exist",
+												$2),
+										 parser_errposition(@2)));
+							/* CONTINUE only allows loop labels */
+							if (label->itemno != PLPGSQL_LABEL_LOOP && !$1)
+								ereport(ERROR,
+										(errcode(ERRCODE_SYNTAX_ERROR),
+										 errmsg("block label \"%s\" cannot be used in CONTINUE",
+												$2),
+										 parser_errposition(@2)));
+						}
+						else
+						{
+							/*
+							 * No label, so make sure there is some loop (an
+							 * unlabelled EXIT does not match a block, so this
+							 * is the same test for both EXIT and CONTINUE)
+							 */
+							if (plpgsql_ns_find_nearest_loop(plpgsql_ns_top()) == NULL)
+								ereport(ERROR,
+										(errcode(ERRCODE_SYNTAX_ERROR),
+										 /* translator: %s is EXIT or CONTINUE */
+										 errmsg("%s cannot be used outside a loop",
+												plpgsql_stmt_typename((PLpgSQL_stmt *) new)),
+										 parser_errposition(@1)));
+						}
+
 						$$ = (PLpgSQL_stmt *)new;
 					}
 				;
@@ -2290,12 +2327,24 @@ expr_until_loop :
 
 opt_block_label	:
 					{
-						plpgsql_ns_push(NULL);
+						plpgsql_ns_push(NULL, PLPGSQL_LABEL_BLOCK);
+						$$ = NULL;
+					}
+				| LESS_LESS any_identifier GREATER_GREATER
+					{
+						plpgsql_ns_push($2, PLPGSQL_LABEL_BLOCK);
+						$$ = $2;
+					}
+				;
+
+opt_loop_label	:
+					{
+						plpgsql_ns_push(NULL, PLPGSQL_LABEL_LOOP);
 						$$ = NULL;
 					}
 				| LESS_LESS any_identifier GREATER_GREATER
 					{
-						plpgsql_ns_push($2);
+						plpgsql_ns_push($2, PLPGSQL_LABEL_LOOP);
 						$$ = $2;
 					}
 				;
@@ -2306,8 +2355,7 @@ opt_label	:
 					}
 				| any_identifier
 					{
-						if (plpgsql_ns_lookup_label(plpgsql_ns_top(), $1) == NULL)
-							yyerror("label does not exist");
+						/* label validity will be checked by outer production */
 						$$ = $1;
 					}
 				;
diff --git a/src/pl/plpgsql/src/plpgsql.h b/src/pl/plpgsql/src/plpgsql.h
index fd5679c23ef..696fb6155d1 100644
--- a/src/pl/plpgsql/src/plpgsql.h
+++ b/src/pl/plpgsql/src/plpgsql.h
@@ -47,6 +47,17 @@ enum
 };
 
 /* ----------
+ * A PLPGSQL_NSTYPE_LABEL stack entry must be one of these types
+ * ----------
+ */
+enum PLpgSQL_label_types
+{
+	PLPGSQL_LABEL_BLOCK,		/* DECLARE/BEGIN block */
+	PLPGSQL_LABEL_LOOP,			/* looping construct */
+	PLPGSQL_LABEL_OTHER			/* anything else */
+};
+
+/* ----------
  * Datum array node types
  * ----------
  */
@@ -331,6 +342,8 @@ typedef struct PLpgSQL_nsitem
 {								/* Item in the compilers namespace tree */
 	int			itemtype;
 	int			itemno;
+	/* For labels, itemno is a value of enum PLpgSQL_label_types. */
+	/* For other itemtypes, itemno is the associated PLpgSQL_datum's dno. */
 	struct PLpgSQL_nsitem *prev;
 	char		name[FLEXIBLE_ARRAY_MEMBER];	/* nul-terminated string */
 } PLpgSQL_nsitem;
@@ -997,7 +1010,8 @@ extern void exec_get_datum_type_info(PLpgSQL_execstate *estate,
  * ----------
  */
 extern void plpgsql_ns_init(void);
-extern void plpgsql_ns_push(const char *label);
+extern void plpgsql_ns_push(const char *label,
+				enum PLpgSQL_label_types label_type);
 extern void plpgsql_ns_pop(void);
 extern PLpgSQL_nsitem *plpgsql_ns_top(void);
 extern void plpgsql_ns_additem(int itemtype, int itemno, const char *name);
@@ -1006,6 +1020,7 @@ extern PLpgSQL_nsitem *plpgsql_ns_lookup(PLpgSQL_nsitem *ns_cur, bool localmode,
 				  const char *name3, int *names_used);
 extern PLpgSQL_nsitem *plpgsql_ns_lookup_label(PLpgSQL_nsitem *ns_cur,
 						const char *name);
+extern PLpgSQL_nsitem *plpgsql_ns_find_nearest_loop(PLpgSQL_nsitem *ns_cur);
 
 /* ----------
  * Other functions in pl_funcs.c