patch 8.0.1669: :vimgrep may add entries to the wrong quickfix list

Problem:    :vimgrep may add entries to the wrong quickfix list.
Solution:   Use the list identifier. (Yegappan Lakshmanan)

src/quickfix.c

@@ -4159,6 +4159,21 @@ ex_cfile(exarg_T *eap)
 	qf_jump(qi, 0, 0, eap->forceit);	/* display first error */
 }
 
+/*
+ * Return the quickfix/location list number with the given identifier.
+ * Returns -1 if list is not found.
+ */
+    static int
+qf_id2nr(qf_info_T *qi, int_u qfid)
+{
+    int		qf_idx;
+
+    for (qf_idx = 0; qf_idx < qi->qf_listcount; qf_idx++)
+	if (qi->qf_lists[qf_idx].qf_id == qfid)
+	    return qf_idx;
+    return -1;
+}
+
 /*
  * Return the vimgrep autocmd name.
  */
@@ -4272,40 +4287,32 @@ vgr_load_dummy_buf(
  */
     static int
 vgr_qflist_valid(
+	win_T	    *wp,
 	qf_info_T   *qi,
-	int_u	    save_qfid,
+	int_u	    qfid,
-	qfline_T    *cur_qf_start,
-	int	    loclist_cmd,
 	char_u	    *title)
 {
-    if (loclist_cmd)
+    /* Verify that the quickfix/location list was not freed by an autocmd */
+    if (!qflist_valid(wp, qfid))
     {
-	/*
+	if (wp != NULL)
-	 * Verify that the location list is still valid. An autocmd might
-	 * have freed the location list.
-	 */
-	if (!qflist_valid(curwin, save_qfid))
 	{
+	    /* An autocmd has freed the location list. */
 	    EMSG(_(e_loc_list_changed));
 	    return FALSE;
 	}
-    }
+	else
-    if (cur_qf_start != qi->qf_lists[qi->qf_curlist].qf_start)
 	{
-	int idx;
+	    /* Quickfix list is not found, create a new one. */
+	    qf_new_list(qi, title);
+	    return TRUE;
+	}
+    }
 
+    if (qi->qf_lists[qi->qf_curlist].qf_id != qfid)
 	/* Autocommands changed the quickfix list.  Find the one we were
 	 * using and restore it. */
-	for (idx = 0; idx < LISTCOUNT; ++idx)
+	qi->qf_curlist = qf_id2nr(qi, qfid);
-	    if (cur_qf_start == qi->qf_lists[idx].qf_start)
-	    {
-		qi->qf_curlist = idx;
-		break;
-	    }
-	if (idx == LISTCOUNT)
-	    /* List cannot be found, create a new one. */
-	    qf_new_list(qi, title);
-    }
 
     return TRUE;
 }
@@ -4424,10 +4431,8 @@ ex_vimgrep(exarg_T *eap)
     char_u	*p;
     int		fi;
     qf_info_T	*qi = &ql_info;
-    int		loclist_cmd = FALSE;
     int_u	save_qfid;
-    qfline_T	*cur_qf_start;
+    win_T	*wp = NULL;
-    win_T	*wp;
     buf_T	*buf;
     int		duplicate_name = FALSE;
     int		using_dummy;
@@ -4461,7 +4466,7 @@ ex_vimgrep(exarg_T *eap)
 	qi = ll_get_or_alloc_list(curwin);
 	if (qi == NULL)
 	    return;
-	loclist_cmd = TRUE;
+	wp = curwin;
     }
 
     if (eap->addr_count > 0)
@@ -4518,10 +4523,9 @@ ex_vimgrep(exarg_T *eap)
      * ":lcd %:p:h" changes the meaning of short path names. */
     mch_dirname(dirname_start, MAXPATHL);
 
-     /* Remember the current values of the quickfix list and qf_start, so that
+     /* Remember the current quickfix list identifier, so that we can check for
-      * we can check for autocommands changing the current quickfix list. */
+      * autocommands changing the current quickfix list. */
     save_qfid = qi->qf_lists[qi->qf_curlist].qf_id;
-    cur_qf_start = qi->qf_lists[qi->qf_curlist].qf_start;
 
     seconds = (time_t)0;
     for (fi = 0; fi < fcount && !got_int && tomatch > 0; ++fi)
@@ -4549,11 +4553,11 @@ ex_vimgrep(exarg_T *eap)
 	    /* Use existing, loaded buffer. */
 	    using_dummy = FALSE;
 
-	/* Check whether the quickfix list is still valid */
+	/* Check whether the quickfix list is still valid. When loading a
-	if (!vgr_qflist_valid(qi, save_qfid, cur_qf_start, loclist_cmd,
+	 * buffer above, autocommands might have changed the quickfix list. */
-		    *eap->cmdlinep))
+	if (!vgr_qflist_valid(wp, qi, save_qfid, *eap->cmdlinep))
 	    goto theend;
-	cur_qf_start = qi->qf_lists[qi->qf_curlist].qf_start;
+	save_qfid = qi->qf_lists[qi->qf_curlist].qf_id;
 
 	if (buf == NULL)
 	{
@@ -4567,8 +4571,6 @@ ex_vimgrep(exarg_T *eap)
 	    found_match = vgr_match_buflines(qi, fname, buf, &regmatch,
 		    tomatch, duplicate_name, flags);
 
-	    cur_qf_start = qi->qf_lists[qi->qf_curlist].qf_start;
-
 	    if (using_dummy)
 	    {
 		if (found_match && first_match_buf == NULL)
@@ -4649,7 +4651,6 @@ ex_vimgrep(exarg_T *eap)
      * The QuickFixCmdPost autocmd may free the quickfix list. Check the list
      * is still valid.
      */
-    wp = loclist_cmd ? curwin : NULL;
     if (!qflist_valid(wp, save_qfid))
 	goto theend;
 
@@ -4994,21 +4995,6 @@ qf_get_list_from_lines(dict_T *what, dictitem_T *di, dict_T *retdict)
     return status;
 }
 
-/*
- * Return the quickfix/location list number with the given identifier.
- * Returns -1 if list is not found.
- */
-    static int
-qf_id2nr(qf_info_T *qi, int_u qfid)
-{
-    int		qf_idx;
-
-    for (qf_idx = 0; qf_idx < qi->qf_listcount; qf_idx++)
-	if (qi->qf_lists[qf_idx].qf_id == qfid)
-	    return qf_idx;
-    return -1;
-}
-
 /*
  * Return the quickfix/location list window identifier in the current tabpage.
  */

src/testdir/test_quickfix.vim

@@ -3111,3 +3111,44 @@ func Test_qfwin_pos()
   call assert_equal(3, winnr())
   close
 endfunc
+
+" Tests for quickfix/location lists changed by autocommands when
+" :vimgrep/:lvimgrep commands are running.
+func Test_vimgrep_autocmd()
+  call setqflist([], 'f')
+  call writefile(['stars'], 'Xtest1.txt')
+  call writefile(['stars'], 'Xtest2.txt')
+
+  " Test 1:
+  " When searching for a pattern using :vimgrep, if the quickfix list is
+  " changed by an autocmd, the results should be added to the correct quickfix
+  " list.
+  autocmd BufRead Xtest2.txt cexpr '' | cexpr ''
+  silent vimgrep stars Xtest*.txt
+  call assert_equal(1, getqflist({'nr' : 0}).nr)
+  call assert_equal(3, getqflist({'nr' : '$'}).nr)
+  call assert_equal('Xtest2.txt', bufname(getqflist()[1].bufnr))
+  au! BufRead Xtest2.txt
+
+  " Test 2:
+  " When searching for a pattern using :vimgrep, if the quickfix list is
+  " freed, then a error should be given.
+  silent! %bwipe!
+  call setqflist([], 'f')
+  autocmd BufRead Xtest2.txt for i in range(10) | cexpr '' | endfor
+  call assert_fails('vimgrep stars Xtest*.txt', 'E925:')
+  au! BufRead Xtest2.txt
+
+  " Test 3:
+  " When searching for a pattern using :lvimgrep, if the location list is
+  " freed, then the command should error out.
+  silent! %bwipe!
+  let g:save_winid = win_getid()
+  autocmd BufRead Xtest2.txt call setloclist(g:save_winid, [], 'f')
+  call assert_fails('lvimgrep stars Xtest*.txt', 'E926:')
+  au! BufRead Xtest2.txt
+
+  call delete('Xtest1.txt')
+  call delete('Xtest2.txt')
+  call setqflist([], 'f')
+endfunc

src/version.c

@@ -762,6 +762,8 @@ static char *(features[]) =
 
 static int included_patches[] =
 {   /* Add new patch number below this line */
+/**/
+    1669,
 /**/
     1668,
 /**/