redefining labels is a warning now

* Allowing label redefinitions might have been useful when used as comments,
  since you will want to be able to define arbitrary comments.
  However as flow control constructs, this introduced a certain ambiguity since
  gotos might jump to different locations, depending on the progression
  of the parser.
* On the other hand, making label redefinition an error might disqualify labels as
  comments when writing or porting classic TECO code.
  Therefore, it has been made a warning as a compromise.
* Added test case

5 files changed, 94 insertions, 64 deletions

diff --git a/doc/sciteco.7.template b/doc/sciteco.7.template
index f9cad80..6ca81ab 100644
--- a/doc/sciteco.7.template
+++ b/doc/sciteco.7.template
@@ -2078,7 +2078,7 @@ The most basic flow control command in \*(ST is the Go-to command.
 Since it is really an ordinary command, exceptional only in setting
 the program counter and influencing parsing, it is described in this
 document's command reference.
-\*(ST may do simple unconditional and computed gotos.
+\*(ST can perform simple unconditional and computed gotos.
 .LP
 .SCITECO_TOPIC label
 Labels are symbolic and are defined with the following syntax:
@@ -2093,9 +2093,14 @@ When a label is encountered, it is cached in a macro-invocation level
 specific goto table if it is not in there already.
 Therefore every macro invocation has its own label namespace and gotos
 to a label have constant complexity once a label has been parsed.
+\# The table lookup is not constant of course.
 Terminating a macro execution (or command line) fails if a label that
 is jumped to has not been defined.
 Labels are also historically used as comments in TECO code.
+That's why \*(ST allows several identical labels in the same scope \(em
+goto commands will always jump to the first occurrance of the label.
+In case of label redefinition, a warning will be printed, so you are
+encouraged to use \(lqtrue\(rq comments as described below.
 .
 .SS Comments
 .SCITECO_TOPIC comment
diff --git a/src/goto-commands.c b/src/goto-commands.c
index ad802e0..b87fc32 100644
--- a/src/goto-commands.c
+++ b/src/goto-commands.c
@@ -30,6 +30,7 @@
 #include "lexer.h"
 #include "core-commands.h"
 #include "undo.h"
+#include "interface.h"
 #include "goto.h"
 #include "goto-commands.h"
 
@@ -62,15 +63,20 @@ teco_state_label_input(teco_machine_main_t *ctx, gunichar chr, GError **error)
 	}
 
 	if (chr == '!') {
-		/*
-		 * NOTE: If the label already existed, its PC will be restored
-		 * on rubout.
-		 * Otherwise, the label will be removed (PC == -1).
-		 */
 		gssize existing_pc = teco_goto_table_set(&ctx->goto_table, ctx->goto_label.data,
 		                                         ctx->goto_label.len, ctx->macro_pc);
+		if (existing_pc == ctx->macro_pc)
+			/* encountered the same label again */
+			return &teco_state_start;
+		if (existing_pc >= 0) {
+			g_autofree gchar *label_printable = teco_string_echo(ctx->goto_label.data,
+			                                                     ctx->goto_label.len);
+			teco_interface_msg(TECO_MSG_WARNING, "Ignoring goto label \"%s\" redefinition",
+			                   label_printable);
+			return &teco_state_start;
+		}
 		if (ctx->parent.must_undo)
-			teco_goto_table_undo_set(&ctx->goto_table, ctx->goto_label.data, ctx->goto_label.len, existing_pc);
+			teco_goto_table_undo_remove(&ctx->goto_table, ctx->goto_label.data, ctx->goto_label.len);
 
 		if (teco_goto_skip_label.len > 0 &&
 		    !teco_string_cmp(&ctx->goto_label, teco_goto_skip_label.data, teco_goto_skip_label.len)) {
diff --git a/src/goto.c b/src/goto.c
index 1076532..855c9f9 100644
--- a/src/goto.c
+++ b/src/goto.c
@@ -78,53 +78,26 @@ teco_goto_table_dump(teco_goto_table_t *ctx)
 }
 #endif
 
-/** @memberof teco_goto_table_t */
-gssize
+/**
+ * Remove label from goto table.
+ *
+ * @param ctx Goto table
+ * @param name Label name
+ * @param len Length of label name
+ * @return TRUE if the label existed and was removed
+ *
+ * @memberof teco_goto_table_t
+ */
+gboolean
 teco_goto_table_remove(teco_goto_table_t *ctx, const gchar *name, gsize len)
 {
-	gssize existing_pc = -1;
-
 	teco_goto_label_t *label = (teco_goto_label_t *)teco_rb3str_find(&ctx->tree, TRUE, name, len);
-	if (label) {
-		existing_pc = label->pc;
-		rb3_unlink_and_rebalance(&label->head.head);
-		teco_goto_label_free(label);
-	}
+	if (!label)
+		return FALSE;
 
-	return existing_pc;
-}
-
-/** @memberof teco_goto_table_t */
-gssize
-teco_goto_table_find(teco_goto_table_t *ctx, const gchar *name, gsize len)
-{
-	teco_goto_label_t *label = (teco_goto_label_t *)teco_rb3str_find(&ctx->tree, TRUE, name, len);
-	return label ? label->pc : -1;
-}
-
-/** @memberof teco_goto_table_t */
-gssize
-teco_goto_table_set(teco_goto_table_t *ctx, const gchar *name, gsize len, gssize pc)
-{
-	if (pc < 0)
-		return teco_goto_table_remove(ctx, name, len);
-
-	gssize existing_pc = -1;
-
-	teco_goto_label_t *label = (teco_goto_label_t *)teco_rb3str_find(&ctx->tree, TRUE, name, len);
-	if (label) {
-		existing_pc = label->pc;
-		label->pc = pc;
-	} else {
-		label = teco_goto_label_new(name, len, pc);
-		teco_rb3str_insert(&ctx->tree, TRUE, &label->head);
-	}
-
-#ifdef DEBUG
-	teco_goto_table_dump(ctx);
-#endif
-
-	return existing_pc;
+	rb3_unlink_and_rebalance(&label->head.head);
+	teco_goto_label_free(label);
+	return TRUE;
 }
 
 /*
@@ -135,35 +108,35 @@ teco_goto_table_set(teco_goto_table_t *ctx, const gchar *name, gsize len, gssize
  */
 typedef struct {
 	teco_goto_table_t *table;
-	gssize pc;
 	gsize len;
 	gchar name[];
-} teco_goto_table_undo_set_t;
+} teco_goto_table_undo_remove_t;
 
 static void
-teco_goto_table_undo_set_action(teco_goto_table_undo_set_t *ctx, gboolean run)
+teco_goto_table_undo_remove_action(teco_goto_table_undo_remove_t *ctx, gboolean run)
 {
-	if (run) {
-		teco_goto_table_set(ctx->table, ctx->name, ctx->len, ctx->pc);
+	if (!run)
+		return;
+
+	G_GNUC_UNUSED gboolean removed = teco_goto_table_remove(ctx->table, ctx->name, ctx->len);
+	g_assert(removed == TRUE);
 #ifdef DEBUG
-		teco_goto_table_dump(ctx->table);
+	teco_goto_table_dump(ctx->table);
 #endif
-	}
 }
 
 /** @memberof teco_goto_table_t */
 void
-teco_goto_table_undo_set(teco_goto_table_t *ctx, const gchar *name, gsize len, gssize pc)
+teco_goto_table_undo_remove(teco_goto_table_t *ctx, const gchar *name, gsize len)
 {
 	if (!ctx->must_undo)
 		return;
 
-	teco_goto_table_undo_set_t *token;
-	token = teco_undo_push_size((teco_undo_action_t)teco_goto_table_undo_set_action,
+	teco_goto_table_undo_remove_t *token;
+	token = teco_undo_push_size((teco_undo_action_t)teco_goto_table_undo_remove_action,
 	                            sizeof(*token) + len);
 	if (token) {
 		token->table = ctx;
-		token->pc = pc;
 		token->len = len;
 		if (name)
 			memcpy(token->name, name, len);
@@ -171,6 +144,44 @@ teco_goto_table_undo_set(teco_goto_table_t *ctx, const gchar *name, gsize len, g
 }
 
 /** @memberof teco_goto_table_t */
+gssize
+teco_goto_table_find(teco_goto_table_t *ctx, const gchar *name, gsize len)
+{
+	teco_goto_label_t *label = (teco_goto_label_t *)teco_rb3str_find(&ctx->tree, TRUE, name, len);
+	return label ? label->pc : -1;
+}
+
+/**
+ * Insert label into goto table.
+ *
+ * @param ctx Goto table
+ * @param name Label name
+ * @param len Length of label name
+ * @param pc Program counter of the new label
+ * @return The program counter of any label of the same name
+ *   or -1. The label is inserted only if there is no label in the
+ *   table already.
+ *
+ * @memberof teco_goto_table_t
+ */
+gssize
+teco_goto_table_set(teco_goto_table_t *ctx, const gchar *name, gsize len, gsize pc)
+{
+	gssize existing_pc = teco_goto_table_find(ctx, name, len);
+	if (existing_pc >= 0)
+		return existing_pc;
+
+	teco_goto_label_t *label = teco_goto_label_new(name, len, pc);
+	teco_rb3str_insert(&ctx->tree, TRUE, &label->head);
+
+#ifdef DEBUG
+	teco_goto_table_dump(ctx);
+#endif
+
+	return -1;
+}
+
+/** @memberof teco_goto_table_t */
 void
 teco_goto_table_clear(teco_goto_table_t *ctx)
 {
diff --git a/src/goto.h b/src/goto.h
index 49007fb..eda823f 100644
--- a/src/goto.h
+++ b/src/goto.h
@@ -40,12 +40,12 @@ teco_goto_table_init(teco_goto_table_t *ctx, gboolean must_undo)
 	ctx->must_undo = must_undo;
 }
 
-gssize teco_goto_table_remove(teco_goto_table_t *ctx, const gchar *name, gsize len);
+gboolean teco_goto_table_remove(teco_goto_table_t *ctx, const gchar *name, gsize len);
+void teco_goto_table_undo_remove(teco_goto_table_t *ctx, const gchar *name, gsize len);
 
 gssize teco_goto_table_find(teco_goto_table_t *ctx, const gchar *name, gsize len);
 
-gssize teco_goto_table_set(teco_goto_table_t *ctx, const gchar *name, gsize len, gssize pc);
-void teco_goto_table_undo_set(teco_goto_table_t *ctx, const gchar *name, gsize len, gssize pc);
+gssize teco_goto_table_set(teco_goto_table_t *ctx, const gchar *name, gsize len, gsize pc);
 
 /** @memberof teco_goto_table_t */
 static inline gboolean
diff --git a/tests/testsuite.at b/tests/testsuite.at
index f17a711..3f0b7e5 100644
--- a/tests/testsuite.at
+++ b/tests/testsuite.at
@@ -270,6 +270,14 @@ AT_SETUP([Jump to beginning of macro])
 AT_CHECK([$SCITECO -e "%a-2\"< F< ' Qa-2\"N(0/0)'"], 0, ignore, ignore)
 AT_CLEANUP
 
+AT_SETUP([Gotos and labels])
+# Not a label redefinition, there must not even be a warning.
+AT_CHECK([$SCITECO -e '2<!foo!>'], 0, ignore, stderr)
+AT_FAIL_IF([$GREP "^Warning:" stderr])
+# Will print a warning about label redefinition, though...
+AT_CHECK([$SCITECO -e "!foo! Qa\"S^C' !foo! Qa\"S(0/0)' -Ua @O/foo/"], 0, ignore, ignore)
+AT_CLEANUP
+
 #
 # Command-line editing bugs.
 #