patch 8.2.2658: :for cannot loop over a stringv8.2.2658

Problem:    :for cannot loop over a string.
Solution:   Accept a string argument and iterate over its characters.

9 files changed, 163 insertions, 32 deletions

diff --git a/runtime/doc/eval.txt b/runtime/doc/eval.txt
index 86df7b5704..428cb08e54 100644
--- a/runtime/doc/eval.txt
+++ b/runtime/doc/eval.txt
@@ -439,8 +439,8 @@ Changing the order of items in a list: >
 
 For loop ~
 
-The |:for| loop executes commands for each item in a list.  A variable is set
-to each item in the list in sequence.  Example: >
+The |:for| loop executes commands for each item in a List, String or Blob.
+A variable is set to each item in sequence.  Example with a List: >
 	:for item in mylist
 	:   call Doit(item)
 	:endfor
@@ -457,7 +457,7 @@ If all you want to do is modify each item in the list then the |map()|
 function will be a simpler method than a for loop.
 
 Just like the |:let| command, |:for| also accepts a list of variables.  This
-requires the argument to be a list of lists. >
+requires the argument to be a List of Lists. >
 	:for [lnum, col] in [[1, 3], [2, 8], [3, 0]]
 	:   call Doit(lnum, col)
 	:endfor
@@ -473,6 +473,14 @@ It is also possible to put remaining items in a List variable: >
 	:   endif
 	:endfor
 
+For a Blob one byte at a time is used.
+
+For a String one character, including any composing characters, is used as a
+String.  Example: >
+	for c in text
+	  echo 'This character is ' .. c
+	endfor
+
 
 List functions ~
 						*E714*
diff --git a/src/errors.h b/src/errors.h
index 04179a31c6..cb0f84652d 100644
--- a/src/errors.h
+++ b/src/errors.h
@@ -389,3 +389,5 @@ EXTERN char e_non_empty_string_required_for_argument_nr[]
 	INIT(= N_("E1175: Non-empty string required for argument %d"));
 EXTERN char e_misplaced_command_modifier[]
 	INIT(= N_("E1176: Misplaced command modifier"));
+EXTERN char e_for_loop_on_str_not_supported[]
+	INIT(= N_("E1177: For loop on %s not supported"));
diff --git a/src/eval.c b/src/eval.c
index 95dd5e2e6c..b4593babaa 100644
--- a/src/eval.c
+++ b/src/eval.c
@@ -41,6 +41,8 @@ typedef struct
     list_T	*fi_list;	// list being used
     int		fi_bi;		// index of blob
     blob_T	*fi_blob;	// blob being used
+    char_u	*fi_string;	// copy of string being used
+    int		fi_byte_idx;	// byte index in fi_string
 } forinfo_T;
 
 static int tv_op(typval_T *tv1, typval_T *tv2, char_u  *op);
@@ -1738,6 +1740,14 @@ eval_for_line(
 		}
 		clear_tv(&tv);
 	    }
+	    else if (tv.v_type == VAR_STRING)
+	    {
+		fi->fi_byte_idx = 0;
+		fi->fi_string = tv.vval.v_string;
+		tv.vval.v_string = NULL;
+		if (fi->fi_string == NULL)
+		    fi->fi_string = vim_strsave((char_u *)"");
+	    }
 	    else
 	    {
 		emsg(_(e_listreq));
@@ -1790,7 +1800,23 @@ next_for_item(void *fi_void, char_u *arg)
 	tv.vval.v_number = blob_get(fi->fi_blob, fi->fi_bi);
 	++fi->fi_bi;
 	return ex_let_vars(arg, &tv, TRUE, fi->fi_semicolon,
-				       fi->fi_varcount, flag, NULL) == OK;
+					    fi->fi_varcount, flag, NULL) == OK;
+    }
+
+    if (fi->fi_string != NULL)
+    {
+	typval_T	tv;
+	int		len;
+
+	len = mb_ptr2len(fi->fi_string + fi->fi_byte_idx);
+	if (len == 0)
+	    return FALSE;
+	tv.v_type = VAR_STRING;
+	tv.v_lock = VAR_FIXED;
+	tv.vval.v_string = vim_strnsave(fi->fi_string + fi->fi_byte_idx, len);
+	fi->fi_byte_idx += len;
+	return ex_let_vars(arg, &tv, TRUE, fi->fi_semicolon,
+					    fi->fi_varcount, flag, NULL) == OK;
     }
 
     item = fi->fi_lw.lw_item;
@@ -1800,7 +1826,7 @@ next_for_item(void *fi_void, char_u *arg)
     {
 	fi->fi_lw.lw_item = item->li_next;
 	result = (ex_let_vars(arg, &item->li_tv, TRUE, fi->fi_semicolon,
-				      fi->fi_varcount, flag, NULL) == OK);
+					   fi->fi_varcount, flag, NULL) == OK);
     }
     return result;
 }
@@ -1813,13 +1839,17 @@ free_for_info(void *fi_void)
 {
     forinfo_T    *fi = (forinfo_T *)fi_void;
 
-    if (fi != NULL && fi->fi_list != NULL)
+    if (fi == NULL)
+	return;
+    if (fi->fi_list != NULL)
     {
 	list_rem_watch(fi->fi_list, &fi->fi_lw);
 	list_unref(fi->fi_list);
     }
-    if (fi != NULL && fi->fi_blob != NULL)
+    else if (fi->fi_blob != NULL)
 	blob_unref(fi->fi_blob);
+    else
+	vim_free(fi->fi_string);
     vim_free(fi);
 }
 
diff --git a/src/testdir/test_vim9_disassemble.vim b/src/testdir/test_vim9_disassemble.vim
index bb74fada07..407e2611ff 100644
--- a/src/testdir/test_vim9_disassemble.vim
+++ b/src/testdir/test_vim9_disassemble.vim
@@ -1061,7 +1061,6 @@ def Test_disassemble_for_loop_eval()
         '\d STORE -1 in $1\_s*' ..
         '\d PUSHS "\["one", "two"\]"\_s*' ..
         '\d BCALL eval(argc 1)\_s*' ..
-        '\d CHECKTYPE list<any> stack\[-1\]\_s*' ..
         '\d FOR $1 -> \d\+\_s*' ..
         '\d STORE $2\_s*' ..
         'res ..= str\_s*' ..
@@ -1071,7 +1070,7 @@ def Test_disassemble_for_loop_eval()
         '\d\+ CONCAT\_s*' ..
         '\d\+ STORE $0\_s*' ..
         'endfor\_s*' ..
-        '\d\+ JUMP -> 6\_s*' ..
+        '\d\+ JUMP -> 5\_s*' ..
         '\d\+ DROP\_s*' ..
         'return res\_s*' ..
         '\d\+ LOAD $0\_s*' ..
diff --git a/src/testdir/test_vim9_script.vim b/src/testdir/test_vim9_script.vim
index b9c1c57488..1169c86924 100644
--- a/src/testdir/test_vim9_script.vim
+++ b/src/testdir/test_vim9_script.vim
@@ -2322,6 +2322,25 @@ def Test_for_loop()
     res ..= n .. s
   endfor
   assert_equal('1a2b', res)
+
+  # loop over string
+  res = ''
+  for c in 'aéc̀d'
+    res ..= c .. '-'
+  endfor
+  assert_equal('a-é-c̀-d-', res)
+
+  res = ''
+  for c in ''
+    res ..= c .. '-'
+  endfor
+  assert_equal('', res)
+
+  res = ''
+  for c in test_null_string()
+    res ..= c .. '-'
+  endfor
+  assert_equal('', res)
 enddef
 
 def Test_for_loop_fails()
@@ -2333,10 +2352,17 @@ def Test_for_loop_fails()
   CheckDefFailure(['var x = 5', 'for x in range(5)'], 'E1017:')
   CheckScriptFailure(['def Func(arg: any)', 'for arg in range(5)', 'enddef', 'defcompile'], 'E1006:')
   delfunc! g:Func
-  CheckDefFailure(['for i in "text"'], 'E1012:')
   CheckDefFailure(['for i in xxx'], 'E1001:')
   CheckDefFailure(['endfor'], 'E588:')
   CheckDefFailure(['for i in range(3)', 'echo 3'], 'E170:')
+
+  # wrong type detected at compile time
+  CheckDefFailure(['for i in {a: 1}', 'echo 3', 'endfor'], 'E1177: For loop on dict not supported')
+
+  # wrong type detected at runtime
+  g:adict = {a: 1}
+  CheckDefExecFailure(['for i in g:adict', 'echo 3', 'endfor'], 'E1177: For loop on dict not supported')
+  unlet g:adict
 enddef
 
 def Test_for_loop_script_var()
diff --git a/src/testdir/test_vimscript.vim b/src/testdir/test_vimscript.vim
index b57d86d471..f12d810e02 100644
--- a/src/testdir/test_vimscript.vim
+++ b/src/testdir/test_vimscript.vim
@@ -7484,6 +7484,26 @@ func Test_trinary_expression()
   call assert_equal(v:false, eval(string(v:false)))
 endfunction
 
+func Test_for_over_string()
+  let res = ''
+  for c in 'aéc̀d'
+    let res ..= c .. '-'
+  endfor
+  call assert_equal('a-é-c̀-d-', res)
+
+  let res = ''
+  for c in ''
+    let res ..= c .. '-'
+  endfor
+  call assert_equal('', res)
+
+  let res = ''
+  for c in test_null_string()
+    let res ..= c .. '-'
+  endfor
+  call assert_equal('', res)
+endfunc
+
 "-------------------------------------------------------------------------------
 " Modelines								    {{{1
 " vim: ts=8 sw=2 sts=2 expandtab tw=80 fdm=marker
diff --git a/src/version.c b/src/version.c
index e09153fe28..a7406d96d9 100644
--- a/src/version.c
+++ b/src/version.c
@@ -751,6 +751,8 @@ static char *(features[]) =
 static int included_patches[] =
 {   /* Add new patch number below this line */
 /**/
+    2658,
+/**/
     2657,
 /**/
     2656,
diff --git a/src/vim9compile.c b/src/vim9compile.c
index c5f0f2a0be..c1adb36c67 100644
--- a/src/vim9compile.c
+++ b/src/vim9compile.c
@@ -7264,11 +7264,15 @@ compile_for(char_u *arg_start, cctx_T *cctx)
     }
     arg_end = arg;
 
-    // Now that we know the type of "var", check that it is a list, now or at
-    // runtime.
+    // If we know the type of "var" and it is a not a list or string we can
+    // give an error now.
     vartype = ((type_T **)stack->ga_data)[stack->ga_len - 1];
-    if (need_type(vartype, &t_list_any, -1, 0, cctx, FALSE, FALSE) == FAIL)
+    if (vartype->tt_type != VAR_LIST && vartype->tt_type != VAR_STRING
+						&& vartype->tt_type != VAR_ANY)
     {
+	// TODO: support Blob
+	semsg(_(e_for_loop_on_str_not_supported),
+					       vartype_name(vartype->tt_type));
 	drop_scope(cctx);
 	return NULL;
     }
diff --git a/src/vim9execute.c b/src/vim9execute.c
index 2eb0bec13d..b80d533c91 100644
--- a/src/vim9execute.c
+++ b/src/vim9execute.c
@@ -2741,36 +2741,76 @@ call_def_function(
 	    // top of a for loop
 	    case ISN_FOR:
 		{
-		    list_T	*list = STACK_TV_BOT(-1)->vval.v_list;
+		    typval_T	*ltv = STACK_TV_BOT(-1);
 		    typval_T	*idxtv =
 				   STACK_TV_VAR(iptr->isn_arg.forloop.for_idx);
 
-		    // push the next item from the list
 		    if (GA_GROW(&ectx.ec_stack, 1) == FAIL)
 			goto failed;
-		    ++idxtv->vval.v_number;
-		    if (list == NULL || idxtv->vval.v_number >= list->lv_len)
+		    if (ltv->v_type == VAR_LIST)
 		    {
-			// past the end of the list, jump to "endfor"
-			ectx.ec_iidx = iptr->isn_arg.forloop.for_end;
-			may_restore_cmdmod(&funclocal);
+			list_T *list = ltv->vval.v_list;
+
+			// push the next item from the list
+			++idxtv->vval.v_number;
+			if (list == NULL
+				       || idxtv->vval.v_number >= list->lv_len)
+			{
+			    // past the end of the list, jump to "endfor"
+			    ectx.ec_iidx = iptr->isn_arg.forloop.for_end;
+			    may_restore_cmdmod(&funclocal);
+			}
+			else if (list->lv_first == &range_list_item)
+			{
+			    // non-materialized range() list
+			    tv = STACK_TV_BOT(0);
+			    tv->v_type = VAR_NUMBER;
+			    tv->v_lock = 0;
+			    tv->vval.v_number = list_find_nr(
+					     list, idxtv->vval.v_number, NULL);
+			    ++ectx.ec_stack.ga_len;
+			}
+			else
+			{
+			    listitem_T *li = list_find(list,
+							 idxtv->vval.v_number);
+
+			    copy_tv(&li->li_tv, STACK_TV_BOT(0));
+			    ++ectx.ec_stack.ga_len;
+			}
 		    }
-		    else if (list->lv_first == &range_list_item)
+		    else if (ltv->v_type == VAR_STRING)
 		    {
-			// non-materialized range() list
-			tv = STACK_TV_BOT(0);
-			tv->v_type = VAR_NUMBER;
-			tv->v_lock = 0;
-			tv->vval.v_number = list_find_nr(
-					     list, idxtv->vval.v_number, NULL);
-			++ectx.ec_stack.ga_len;
+			char_u	*str = ltv->vval.v_string;
+			int	len = str == NULL ? 0 : (int)STRLEN(str);
+
+			// Push the next character from the string.  The index
+			// is for the last byte of the previous character.
+			++idxtv->vval.v_number;
+			if (idxtv->vval.v_number >= len)
+			{
+			    // past the end of the string, jump to "endfor"
+			    ectx.ec_iidx = iptr->isn_arg.forloop.for_end;
+			    may_restore_cmdmod(&funclocal);
+			}
+			else
+			{
+			    int	clen = mb_ptr2len(str + idxtv->vval.v_number);
+
+			    tv = STACK_TV_BOT(0);
+			    tv->v_type = VAR_STRING;
+			    tv->vval.v_string = vim_strnsave(
+					     str + idxtv->vval.v_number, clen);
+			    ++ectx.ec_stack.ga_len;
+			    idxtv->vval.v_number += clen - 1;
+			}
 		    }
 		    else
 		    {
-			listitem_T *li = list_find(list, idxtv->vval.v_number);
-
-			copy_tv(&li->li_tv, STACK_TV_BOT(0));
-			++ectx.ec_stack.ga_len;
+			// TODO: support Blob
+			semsg(_(e_for_loop_on_str_not_supported),
+						    vartype_name(ltv->v_type));
+			goto failed;
 		    }
 		}
 		break;